Re: [PATCH v2] qapi: Allow getting flat output from 'query-named-block-nodes'

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2759 bytes --]

On 20.01.20 09:50, Peter Krempa wrote:
> When a management application manages node names there's no reason to
> recurse into backing images in the output of query-named-block-nodes.
> 
> Add a parameter to the command which will return just the top level
> structs.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> 
> Diff to v1:
>  - rewrote setting of 'return_flat' in qmp_query_named_block_nodes
>  - tried to clarify the QMP schema docs for the new field
> 
> This patch does not aim to fix the rather suboptimal original
> documentation of the command as that is going to end up in a bunch of
> bikeshedding.
> 
> While I know that there are plans for a new command that should fix
> this, the plans were already there for quite some time without much
> happening. This is a quick fix to a real problem, because if you have
> (maybe unpractically) deep backing chains, the returned JSON is getting
> huge. (140 nesting levels exceeds 10MiB of JSON)

The main reason nothing is happening is because nobody is pressing for
it, I think.  We talk about it from time to time but then our result is
“As long as nobody seriously complains and tells us what we need, we’re
going to assume what we have is good enough.”

For example:
https://lists.nongnu.org/archive/html/qemu-block/2020-01/msg00049.html
(Under “Query function situation”)

>  block.c               |  5 +++--
>  block/qapi.c          | 10 ++++++++--
>  blockdev.c            |  8 ++++++--
>  include/block/block.h |  2 +-
>  include/block/qapi.h  |  4 +++-
>  monitor/hmp-cmds.c    |  2 +-
>  qapi/block-core.json  |  7 ++++++-
>  7 files changed, 28 insertions(+), 10 deletions(-)

[...]

> diff --git a/block/qapi.c b/block/qapi.c
> index 9a5d0c9b27..84048e1a57 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -42,7 +42,9 @@
>  #include "qemu/cutils.h"
> 
>  BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
> -                                        BlockDriverState *bs, Error **errp)
> +                                        BlockDriverState *bs,
> +                                        bool flat,
> +                                        Error **errp)
>  {
>      ImageInfo **p_image_info;
>      BlockDriverState *bs0;
> @@ -156,6 +158,10 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
>              return NULL;
>          }
> 
> +        /* stop gathering data for flat output */
> +        if (flat)
> +            break;

This should be enclosed in curly brackets (qemu coding style).

Shall I fix that up?

Max

> +
>          if (bs0->drv && bs0->backing) {
>              info->backing_file_depth++;
>              bs0 = bs0->backing->bs;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]