Re: [Qemu-devel] [PATCH v2 2/9] block: don't keep AioContext acquired after external_snapshot_prepare()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1717 bytes --]

On 12/06/2017 08:45 AM, Stefan Hajnoczi wrote:
> It is not necessary to hold AioContext across transactions anymore since
> bdrv_drained_begin/end() is used to keep the nodes quiesced.  In fact,
> using the AioContext lock for this purpose was always buggy.
> 
> This patch reduces the scope of AioContext locked regions.  This is not
> just a cleanup but also fixes hangs that occur in BDRV_POLL_WHILE()
> because it is unware of recursive locking and does not release the

s/unware/unaware/

> AioContext the necessary number of times to allow progress to be made.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  blockdev.c | 71 ++++++++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 48 insertions(+), 23 deletions(-)
> 

> @@ -1662,31 +1662,32 @@ static void external_snapshot_prepare(BlkActionState *common,
>          return;
>      }
>  
> -    /* Acquire AioContext now so any threads operating on old_bs stop */
> -    state->aio_context = bdrv_get_aio_context(state->old_bs);
> -    aio_context_acquire(state->aio_context);
> +    aio_context = bdrv_get_aio_context(state->old_bs);
> +    aio_context_acquire(aio_context);
> +
> +    /* Paired with .clean() */
>      bdrv_drained_begin(state->old_bs);
>  
>      if (!bdrv_is_inserted(state->old_bs)) {
>          error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> -        return;
> +        goto out;
>      }

Using gcc/clang's __attribute__((cleanup)) would make this so much nicer ;)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]