Re: [PATCH v2 06/10] block: Convert bdrv_block_device_info into co_wrapper

[Hanna Czenczek <hreitz@redhat.com>]

On 09.06.23 22:19, Fabiano Rosas wrote:
> We're converting callers of bdrv_get_allocated_file_size() to run in
> coroutines because that function will be made asynchronous when called
> (indirectly) from the QMP dispatcher.
>
> This function is a candidate because it calls bdrv_query_image_info()
> -> bdrv_do_query_node_info() -> bdrv_get_allocated_file_size().
>
> It is safe to turn this is a coroutine because the code it calls is
> made up of either simple accessors and string manipulation functions
> [1] or it has already been determined to be safe [2].
>
> 1) bdrv_refresh_filename(), bdrv_is_read_only(),
>     blk_enable_write_cache(), bdrv_cow_bs(), blk_get_public(),
>     throttle_group_get_name(), bdrv_write_threshold_get(),
>     bdrv_query_dirty_bitmaps(), throttle_group_get_config(),
>     bdrv_filter_or_cow_bs(), bdrv_skip_implicit_filters()
>
> 2) bdrv_do_query_node_info() (see previous commit);
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   block/qapi.c         |  8 ++++----
>   include/block/qapi.h | 12 ++++++++----
>   2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index a2e71edaff..20660e15d6 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -41,10 +41,10 @@
>   #include "qemu/qemu-print.h"
>   #include "sysemu/block-backend.h"
>   
> -BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
> -                                        BlockDriverState *bs,
> -                                        bool flat,
> -                                        Error **errp)
> +BlockDeviceInfo *coroutine_fn bdrv_co_block_device_info(BlockBackend *blk,
> +                                                        BlockDriverState *bs,
> +                                                        bool flat,
> +                                                        Error **errp)
>   {
>       ImageInfo **p_image_info;
>       ImageInfo *backing_info;
> diff --git a/include/block/qapi.h b/include/block/qapi.h
> index 7035bcd1ae..5cb0202791 100644
> --- a/include/block/qapi.h
> +++ b/include/block/qapi.h
> @@ -30,10 +30,14 @@
>   #include "block/snapshot.h"
>   #include "qapi/qapi-types-block-core.h"
>   
> -BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
> -                                        BlockDriverState *bs,
> -                                        bool flat,
> -                                        Error **errp);
> +BlockDeviceInfo *coroutine_fn bdrv_co_block_device_info(BlockBackend *blk,
> +                                                        BlockDriverState *bs,
> +                                                        bool flat,
> +                                                        Error **errp);
> +BlockDeviceInfo *co_wrapper bdrv_block_device_info(BlockBackend *blk,
> +                                                   BlockDriverState *bs,
> +                                                   bool flat,
> +                                                   Error **errp);

bdrv_co_block_device_info() is now marked as GRAPH_RDLOCK, so should 
this use a co_wrapper_bdrv_rdlock instead?

Hanna

>   int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>                                     SnapshotInfoList **p_list,
>                                     Error **errp);