Re: [Qemu-devel] [PATCH 3/5] block: convert bdrv_invalidate_cache callback to coroutine_fn

[Kevin Wolf]

Am 10.07.2017 um 18:58 hat Paolo Bonzini geschrieben:
> QED's bdrv_invalidate_cache implementation would like to reuse functions
> that acquire/release the metadata locks.  Call it from coroutine context
> to simplify the logic.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>

> diff --git a/block/qcow2.c b/block/qcow2.c
> index b5de67d113..4ff7e89009 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1854,7 +1854,8 @@ static void qcow2_close(BlockDriverState *bs)
>      qcow2_free_snapshots(bs);
>  }
>  
> -static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
> +static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
> +                                                   Error **errp)
>  {
>      BDRVQcow2State *s = bs->opaque;
>      int flags = s->flags;
> @@ -1877,7 +1878,9 @@ static void qcow2_invalidate_cache(BlockDriverState 
> *bs, Error **errp)
>      options = qdict_clone_shallow(bs->options);
>  
>      flags &= ~BDRV_O_INACTIVE;
> +    qemu_co_mutex_lock(&s->lock);
>      ret = qcow2_do_open(bs, options, flags, &local_err);
> +    qemu_co_mutex_unlock(&s->lock);
>      QDECREF(options);
>      if (local_err) {
>          error_propagate(errp, local_err);

How useful is this lock really? If we expect any requests while this
function is running (we don't, it would break horribly), it should
probably cover qcow2_close(), too.

Or maybe the cleaner way would be bdrv_drained_begin/end() around
everything directly in bdrv_co_invalidate_cache().

Kevin