Re: [Qemu-devel] [PATCH 3/6] block: Introduce query-block-node

[Markus Armbruster <armbru@redhat.com>]

Kevin Wolf <kwolf@redhat.com> writes:

> This new command is a renamed version of query-named-block-nodes
> with an additional argument that allows querying only one specific
> BlockDriverState instead of getting a list for all of them.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c               | 15 +++++++++++++--
>  blockdev.c            |  8 +++++++-
>  include/block/block.h |  2 +-
>  qapi/block-core.json  | 19 ++++++++++++++++---
>  4 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/block.c b/block.c
> index bcd952a..6c538d3 100644
> --- a/block.c
> +++ b/block.c
> @@ -3823,13 +3823,24 @@ BlockDriverState *bdrv_find_node(const char *node_name)
>  }
>  
>  /* Put this QMP function here so it can access the static graph_bdrv_states. */
> -BlockDeviceInfoList *bdrv_named_nodes_list(void)
> +BlockDeviceInfoList *bdrv_named_nodes_list(const char *device, Error **errp)
>  {
>      BlockDeviceInfoList *list, *entry;
>      BlockDriverState *bs;
> +    Error *local_err = NULL;
> +
> +    if (device) {
> +        bs = bdrv_lookup_bs(device, device, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return NULL;
> +        }
> +    } else {
> +        bs = QTAILQ_FIRST(&graph_bdrv_states);
> +    }
>  
>      list = NULL;
> -    QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {
> +    for (; bs; bs = device ? NULL : QTAILQ_NEXT(bs, node_list)) {
>          entry = g_malloc0(sizeof(*entry));
>          entry->value = bdrv_block_device_info(bs);
>          entry->next = list;

Same loop as in PATCH 2/6, same suggestion to simplify it.

> diff --git a/blockdev.c b/blockdev.c
> index b361fbb..a194d04 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2127,9 +2127,15 @@ void qmp_drive_backup(const char *device, const char *target,
>      }
>  }
>  
> +BlockDeviceInfoList *qmp_query_block_node(bool has_device, const char *device,
> +                                          Error **errp)
> +{
> +    return bdrv_named_nodes_list(has_device ? device : NULL, errp);
> +}
> +
>  BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
>  {
> -    return bdrv_named_nodes_list();
> +    return qmp_query_block_node(false, NULL, errp);
>  }
>  
>  #define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
> diff --git a/include/block/block.h b/include/block/block.h
> index 07d6d8e..0caab0d 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -403,7 +403,7 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag);
>  const char *bdrv_get_format_name(BlockDriverState *bs);
>  BlockDriverState *bdrv_find(const char *name);
>  BlockDriverState *bdrv_find_node(const char *node_name);
> -BlockDeviceInfoList *bdrv_named_nodes_list(void);
> +BlockDeviceInfoList *bdrv_named_nodes_list(const char *device, Error **errp);
>  BlockDriverState *bdrv_lookup_bs(const char *device,
>                                   const char *node_name,
>                                   Error **errp);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index ddc3fe0..f170c7e 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -797,13 +797,26 @@
>  ##
>  # @query-named-block-nodes
>  #
> -# Get the named block driver list
> +# Deprecated, may be removed in future versions. Use query-block-node instead.
> +#
> +# Since 2.0, until 2.1

Suggest

 ##
 # @query-named-block-nodes
 #
 # Get the named block driver list
 #
 # Since 2.0
+# Deprecated since 2.1, use query-block-node instead

and a general understanding that "deprecated" implies "should not be
used anymore" and "may be removed in a future release".

> +##
> +{ 'command': 'query-named-block-nodes', 'returns': [ 'BlockDeviceInfo' ] }
> +
> +##
> +# @query-block-node
> +#
> +# Gets information about one or all block device nodes.
> +#
> +# @device: #optional The id or node-name of the block device to inspect.
>  #
>  # Returns: the list of BlockDeviceInfo
>  #
> -# Since 2.0
> +# Since 2.2
>  ##
> -{ 'command': 'query-named-block-nodes', 'returns': [ 'BlockDeviceInfo' ] }
> +{ 'command': 'query-block-node',
> +  'data': { '*device': 'str' },
> +  'returns': [ 'BlockDeviceInfo' ] }
>  
>  ##
>  # @drive-mirror