Il 14/06/2013 13:43, Stefan Hajnoczi ha scritto:
> If a block driver has no file descriptors to monitor but there are still
> active requests, it can return 1 from .io_flush(). This is used to spin
> during synchronous I/O.
>
> Stop relying on .io_flush() and instead check
> QLIST_EMPTY(&bs->tracked_requests) to decide whether there are active
> requests.
>
> This is the first step in removing .io_flush() so that event loops no
> longer need to have the concept of synchronous I/O. Eventually we may
> be able to kill synchronous I/O completely by running everything in a
> coroutine, but that is future work.
>
> Note this patch moves bs->throttled_reqs initialization to bdrv_new() so
> that bdrv_requests_pending(bs) can safely access it. In practice bs is
> g_malloc0() so the memory is already zeroed but it's safer to initialize
> the queue properly.
>
> In bdrv_delete() make sure to call bdrv_make_anon() *after* bdrv_close()
> so that the device is still seen by bdrv_drain_all() when iterating
> bdrv_states.
I wonder if this last change should be separated out and CCed to
qemu-stable. It seems like a bug if you close a device that has pending
throttled operations.
Paolo
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> block.c | 50 +++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/block.c b/block.c
> index 79ad33d..04821d8 100644
> --- a/block.c
> +++ b/block.c
> @@ -148,7 +148,6 @@ static void bdrv_block_timer(void *opaque)
>
> void bdrv_io_limits_enable(BlockDriverState *bs)
> {
> - qemu_co_queue_init(&bs->throttled_reqs);
> bs->block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
> bs->io_limits_enabled = true;
> }
> @@ -305,6 +304,7 @@ BlockDriverState *bdrv_new(const char *device_name)
> }
> bdrv_iostatus_disable(bs);
> notifier_list_init(&bs->close_notifiers);
> + qemu_co_queue_init(&bs->throttled_reqs);
>
> return bs;
> }
> @@ -1412,6 +1412,35 @@ void bdrv_close_all(void)
> }
> }
>
> +/* Check if any requests are in-flight (including throttled requests) */
> +static bool bdrv_requests_pending(BlockDriverState *bs)
> +{
> + if (!QLIST_EMPTY(&bs->tracked_requests)) {
> + return true;
> + }
> + if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
> + return true;
> + }
> + if (bs->file && bdrv_requests_pending(bs->file)) {
> + return true;
> + }
> + if (bs->backing_hd && bdrv_requests_pending(bs->backing_hd)) {
> + return true;
> + }
> + return false;
> +}
> +
> +static bool bdrv_requests_pending_all(void)
> +{
> + BlockDriverState *bs;
> + QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + if (bdrv_requests_pending(bs)) {
> + return true;
> + }
> + }
> + return false;
> +}
> +
> /*
> * Wait for pending requests to complete across all BlockDriverStates
> *
> @@ -1426,27 +1455,22 @@ void bdrv_close_all(void)
> */
> void bdrv_drain_all(void)
> {
> + /* Always run first iteration so any pending completion BHs run */
> + bool busy = true;
> BlockDriverState *bs;
> - bool busy;
> -
> - do {
> - busy = qemu_aio_wait();
>
> + while (busy) {
> /* FIXME: We do not have timer support here, so this is effectively
> * a busy wait.
> */
> QTAILQ_FOREACH(bs, &bdrv_states, list) {
> if (!qemu_co_queue_empty(&bs->throttled_reqs)) {
> qemu_co_queue_restart_all(&bs->throttled_reqs);
> - busy = true;
> }
> }
> - } while (busy);
>
> - /* If requests are still pending there is a bug somewhere */
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> - assert(QLIST_EMPTY(&bs->tracked_requests));
> - assert(qemu_co_queue_empty(&bs->throttled_reqs));
> + busy = bdrv_requests_pending_all();
> + busy |= aio_poll(qemu_get_aio_context(), busy);
> }
> }
>
> @@ -1591,11 +1615,11 @@ void bdrv_delete(BlockDriverState *bs)
> assert(!bs->job);
> assert(!bs->in_use);
>
> + bdrv_close(bs);
> +
> /* remove from list, if necessary */
> bdrv_make_anon(bs);
>
> - bdrv_close(bs);
> -
> g_free(bs);
> }
>
>
