Re: [PATCH 01/20] qcow2: Don't call bdrv_getlength() in coroutine_fns

[Eric Blake]

On Tue, Apr 25, 2023 at 07:31:39PM +0200, Kevin Wolf wrote:
> There is a bdrv_co_getlength() now, which should be used in coroutine
> context.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.h          |  4 +++-
>  block/qcow2-refcount.c |  2 +-
>  block/qcow2.c          | 19 +++++++++----------
>  3 files changed, 13 insertions(+), 12 deletions(-)

> 
> diff --git a/block/qcow2.h b/block/qcow2.h
> index c75decc38a..4f67eb912a 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -895,7 +895,9 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int 
> refcount_order,
>                                  void *cb_opaque, Error **errp);
>  int coroutine_fn GRAPH_RDLOCK qcow2_shrink_reftable(BlockDriverState *bs);
>  int64_t coroutine_fn qcow2_get_last_cluster(BlockDriverState *bs, int64_t 
> size);
> -int coroutine_fn qcow2_detect_metadata_preallocation(BlockDriverState *bs);
> +
> +int coroutine_fn GRAPH_RDLOCK
> +qcow2_detect_metadata_preallocation(BlockDriverState *bs);

> +++ b/block/qcow2.c
> @@ -2089,11 +2089,10 @@ static void qcow2_join_options(QDict *options, QDict 
> *old_options)
>      }
>  }
>  
> -static int coroutine_fn qcow2_co_block_status(BlockDriverState *bs,
> -                                              bool want_zero,
> -                                              int64_t offset, int64_t count,
> -                                              int64_t *pnum, int64_t *map,
> -                                              BlockDriverState **file)
> +static int coroutine_fn GRAPH_RDLOCK
> +qcow2_co_block_status(BlockDriverState *bs, bool want_zero, int64_t offset,
> +                      int64_t count, int64_t *pnum, int64_t *map,
> +                      BlockDriverState **file)
>  {

Should the commit message also call out that you are using this
opportunity to add GRAPH_RDLOCK annotations on affected functions?

Overall, all changes in this patch make sense, but I'm reluctant to
add R-b unless you confirm whether this was a rebase mistake (where
the annotations were intended to be added in a later patch).

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