Re: [RFC PATCH 3/6] Convert query-block/info_block to coroutine

[Claudio Fontana <cfontana@suse.de>]

On 5/23/23 23:39, Fabiano Rosas wrote:
> From: Lin Ma <lma@suse.com>
> 
> Sometimes the query-block performs time-consuming I/O(say waiting for
> the fstat of NFS complete), So let's make this QMP handler runs in a
> coroutine.
> 
> The following patch moves the fstat() into a thread pool.


Hi, this message talks about QMP and query-block only and the subject talks about info_block as well.
We need to clarify this.

Also, lets make it clear that one is a QMP command, and the other is an HMP command.

In any case I would say:

"Convert QMP command query-block to use a coroutine", and in case we also need info_block, an additional patch could take care of the HMP command with a subject like:

"Convert HMP command info_block to use a coroutine".

Ciao,

Claudio


> 
> Signed-off-by: Lin Ma <lma@suse.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  block/monitor/block-hmp-cmds.c | 2 +-
>  block/qapi.c                   | 2 +-
>  hmp-commands-info.hx           | 1 +
>  include/block/block-hmp-cmds.h | 2 +-
>  qapi/block-core.json           | 2 +-
>  5 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> index ca2599de44..4b704010bc 100644
> --- a/block/monitor/block-hmp-cmds.c
> +++ b/block/monitor/block-hmp-cmds.c
> @@ -738,7 +738,7 @@ static void print_block_info(Monitor *mon, BlockInfo *info,
>      }
>  }
>  
> -void hmp_info_block(Monitor *mon, const QDict *qdict)
> +void coroutine_fn hmp_info_block(Monitor *mon, const QDict *qdict)
>  {
>      BlockInfoList *block_list, *info;
>      BlockDeviceInfoList *blockdev_list, *blockdev;
> diff --git a/block/qapi.c b/block/qapi.c
> index 79bf80c503..ae6cd1c2ff 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -667,7 +667,7 @@ bdrv_query_bds_stats(BlockDriverState *bs, bool blk_level)
>      return s;
>  }
>  
> -BlockInfoList *qmp_query_block(Error **errp)
> +BlockInfoList *coroutine_fn qmp_query_block(Error **errp)
>  {
>      BlockInfoList *head = NULL, **p_next = &head;
>      BlockBackend *blk;
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 47d63d26db..996895f417 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -65,6 +65,7 @@ ERST
>          .help       = "show info of one block device or all block devices "
>                        "(-n: show named nodes; -v: show details)",
>          .cmd        = hmp_info_block,
> +        .coroutine  = true,
>      },
>  
>  SRST
> diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
> index 71113cd7ef..6d9152318f 100644
> --- a/include/block/block-hmp-cmds.h
> +++ b/include/block/block-hmp-cmds.h
> @@ -48,7 +48,7 @@ void hmp_eject(Monitor *mon, const QDict *qdict);
>  
>  void hmp_qemu_io(Monitor *mon, const QDict *qdict);
>  
> -void hmp_info_block(Monitor *mon, const QDict *qdict);
> +void coroutine_fn hmp_info_block(Monitor *mon, const QDict *qdict);
>  void hmp_info_blockstats(Monitor *mon, const QDict *qdict);
>  void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
>  void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 98d9116dae..da95fe456c 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -838,7 +838,7 @@
>  #    }
>  ##
>  { 'command': 'query-block', 'returns': ['BlockInfo'],
> -  'allow-preconfig': true }
> +  'allow-preconfig': true, 'coroutine': true }
>  
>  ##
>  # @BlockDeviceTimedStats: