Re: [Qemu-devel] [PATCH v9 02/11] block: use the block job list in bdrv_drain_all()

[Max Reitz]

On 04.04.2016 15:43, Alberto Garcia wrote:
> bdrv_drain_all() pauses all block jobs by using bdrv_next() to iterate
> over all top-level BlockDriverStates. Therefore the code is unable to
> find block jobs in other nodes.
> 
> This patch uses block_job_next() to iterate over all block jobs.
> 
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>  block/io.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index c4869b9..884ed1e 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -288,15 +288,21 @@ void bdrv_drain_all(void)
>      /* Always run first iteration so any pending completion BHs run */
>      bool busy = true;
>      BlockDriverState *bs = NULL;
> +    BlockJob *job = NULL;
>      GSList *aio_ctxs = NULL, *ctx;
>  
> +    while ((job = block_job_next(job))) {
> +        AioContext *aio_context = bdrv_get_aio_context(job->bs);
> +
> +        aio_context_acquire(aio_context);
> +        block_job_pause(job);
> +        aio_context_release(aio_context);
> +    }
> +
>      while ((bs = bdrv_next(bs))) {
>          AioContext *aio_context = bdrv_get_aio_context(bs);
>  
>          aio_context_acquire(aio_context);
> -        if (bs->job) {
> -            block_job_pause(bs->job);
> -        }
>          bdrv_drain_recurse(bs);
>          aio_context_release(aio_context);
>  
> @@ -333,14 +339,11 @@ void bdrv_drain_all(void)
>          }
>      }
>  
> -    bs = NULL;
> -    while ((bs = bdrv_next(bs))) {
> -        AioContext *aio_context = bdrv_get_aio_context(bs);
> +    while ((job = block_job_next(job))) {
> +        AioContext *aio_context = bdrv_get_aio_context(job->bs);

Technically, the "bs = NULL;" before didn't do anything either. But in
my opinion, it made the code more readable, therefore I'd really like a
"job = NULL;" before this loop, too.

But it's correct as it is, so:

Reviewed-by: Max Reitz <address@hidden>

>  
>          aio_context_acquire(aio_context);
> -        if (bs->job) {
> -            block_job_resume(bs->job);
> -        }
> +        block_job_resume(job);
>          aio_context_release(aio_context);
>      }
>      g_slist_free(aio_ctxs);
>

signature.asc
Description: OpenPGP digital signature