Re: [Qemu-devel] [PATCH 10/12] block: Drain throttling queue with BdrvChild callback

[Paolo Bonzini <pbonzini@redhat.com>]

On 22/03/2016 16:33, Kevin Wolf wrote:
> This removes the last part of I/O throttling from block/io.c and moves
> it to the BlockBackend.
> 
> When draining the queue of a BlockDriverState, we must make sure that no
> new requests can come in for it. Request sources from outside the block
> layer are disabled with aio_disable_external(), but the throttling queue
> must be handled separately.

I have looked at the strategy we talked about today to implement request
cancellation (so that e.g. system reset doesn't take ages because of
throttled requests).  While that may be a worthwhile addition anyway, I
think throttling bdrv_drain() may impose an excessive cost for cases
such as live migration.  The risk of the guest using bdrv_drain() to
game throttling is low enough that we can keep on disabling throttling
during bdrv_drain().

So for now I think we can merge the two series just fine.  The strategy
I used in my patch, adding bdrv_no_throttling_begin and
bdrv_no_throttling_end around the bdrv_drain loop, can be adapted just
as use BdrvChildRole callbacks ->drained_begin and ->drained_end.

I will post v3 of my series tomorrow, adopting your patch 1/12 of this
series and removing the recursion on bdrv_no_throttling_begin and
bdrv_no_throttling_end, which is unnecessary.

Paolo