Re: [Qemu-devel] [PATCH 1/2] block: add the support for draining the throttled request queue

[Paolo Bonzini <pbonzini@redhat.com>]

On 02/24/2012 09:49 AM, Stefan Hajnoczi wrote:
>> > +void bdrv_drain_request(BlockDriverState *throttled_bs)
>> >  {
>> >      BlockDriverState *bs;
>> >  
>> > +    QTAILQ_FOREACH(bs, &bdrv_states, list) {
>> > +        if (throttled_bs && throttled_bs != bs) {
>> > +            continue;
>> > +        }
>> > +        qemu_co_queue_restart_all(&bs->throttled_reqs);
>> > +    }
>> > +
>> >      qemu_aio_flush();
> Since I/O throttling is still enabled, the restarted requests could
> enqueue again if they exceed the limit.  We could still hit the assert.

Yes, and qemu_aio_flush() rightly doesn't know that there are pending
requests, because these are "not there" until the timer fires.

Perhaps the bug is simply that the assert is bogus.  If there are
throttled requests, we need to invoke qemu_aio_flush() again.  The
problem with this is that timers don't run inside qemu_aio_wait() so we
have to restart them all and we're effectively busy waiting.  Not a huge
problem since qemu_aio_flush() is rare, but not too nice either.

Paolo