Re: [PATCH 1/8] block: Call .bdrv_co_create(_opts) unlocked

[Eric Blake]

On Wed, May 10, 2023 at 10:35:54PM +0200, Kevin Wolf wrote:
> 
> These are functions that modify the graph, so they must be able to take
> a writer lock. This is impossible if they already hold the reader lock.
> If they need a reader lock for some of their operations, they should
> take it internally.
> 
> Many of them go through blk_*(), which will always take the lock itself.
> Direct calls of bdrv_*() need to take the reader lock. Note that while
> locking for bdrv_co_*() calls is checked by TSA, this is not the case
> for the mixed_coroutine_fns bdrv_*(). Holding the lock is still required
> when they are called from coroutine context like here!
> 
> This effectively reverts 4ec8df0183, but adds some internal locking
> instead.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---

> +++ b/block/qcow2.c

> -static int coroutine_fn
> +static int coroutine_fn GRAPH_UNLOCKED
>  qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
>  {
>      BlockdevCreateOptionsQcow2 *qcow2_opts;
> @@ -3724,8 +3726,10 @@ qcow2_co_create(BlockdevCreateOptions *create_options, 
> Error **errp)
>          goto out;
>      }
>  
> +    bdrv_graph_co_rdlock();
>      ret = qcow2_alloc_clusters(blk_bs(blk), 3 * cluster_size);
>      if (ret < 0) {
> +        bdrv_graph_co_rdunlock();
>          error_setg_errno(errp, -ret, "Could not allocate clusters for qcow2 "
>                           "header and refcount table");
>          goto out;
> @@ -3743,6 +3747,8 @@ qcow2_co_create(BlockdevCreateOptions *create_options, 
> Error **errp)
>  
>      /* Create a full header (including things like feature table) */
>      ret = qcow2_update_header(blk_bs(blk));
> +    bdrv_graph_co_rdunlock();
> +

If we ever inject any 'goto out' in the elided lines, we're in
trouble.  Would this be any safer by wrapping the intervening
statements in a scope-guarded lock?

But what you have is correct, despite my worries about potential
maintenance concerns.

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

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