On Fri, Feb 24, 2012 at 7:18 PM, Stefan Hajnoczi
<stefa...@linux.vnet.ibm.com> wrote:
> On Fri, Feb 24, 2012 at 05:25:08PM +0800, Zhi Yong Wu wrote:
>> On Fri, Feb 24, 2012 at 4:49 PM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
>> > On Mon, Feb 20, 2012 at 12:50:30PM +0800, zwu.ker...@gmail.com wrote:
>> >> From: Zhi Yong Wu <wu...@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 <wu...@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?
>
> Both are useful:
>
> /**
> * Complete all pending requests for a block device
> */
> void bdrv_drain(BlockDriverState *bs);
>
> /**
> * Complete pending requests for all block devices
> */
> void bdrv_drain_all(void);
Great, thanks.
>
> Stefan
>
--
Regards,
Zhi Yong Wu
