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

[Zhi Yong Wu <zwu.kernel@gmail.com>]

On Fri, Feb 24, 2012 at 4:49 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Feb 20, 2012 at 12:50:30PM +0800, zwu.kernel@gmail.com wrote:
>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>
>> If one guest has multiple disks with enabling I/O throttling function separately, when draining activities are done, some requests maybe are in the throttled queue; So we need to restart them at first.
>>
>> Moreover, when only one disk need to be drained such as hotplug out, if another disk still has some requests in its throttled queue, these request should not be effected.
>>
>> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>> ---
>>  block.c     |   29 ++++++++++++++++++++++-------
>>  block_int.h |    1 +
>>  2 files changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index ae297bb..f78df78 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -853,25 +853,40 @@ void bdrv_close_all(void)
>>      }
>>  }
>>
>> -/*
>> - * Wait for pending requests to complete across all BlockDriverStates
>> - *
>> - * This function does not flush data to disk, use bdrv_flush_all() for that
>> - * after calling this function.
>> - */
>> -void bdrv_drain_all(void)
>> +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.
>
> If the semantics of bdrv_drain_request() are that no requests are
> pending when it returns then we need a loop here.
>
> BTW bdrv_drain() would be a shorter name for this function.
For this function's semantics, i have some concerns.
Is it used to drain all requests of one single disk or all disks for one guest?
which is more suitable?

>
> Stefan



-- 
Regards,

Zhi Yong Wu