Re: [PATCH v3 1/2] block: Wire up 'flat' mode also for 'query-block'

[Markus Armbruster <armbru@redhat.com>]

Peter Krempa <pkrempa@redhat.com> writes:

> On Tue, Feb 03, 2026 at 13:01:01 +0100, Markus Armbruster wrote:
>> Peter Krempa <pkrempa@redhat.com> writes:
>> 
>> > From: Peter Krempa <pkrempa@redhat.com>
>> >
>> > Some time ago (commit facda5443f5a8) I've added 'flat' mode (which
>> > omits 'backing-image' key in reply) to 'query-named-block-nodes' to
>> > minimize the size of the returned JSON for deeper backing chains.
>> >
>> > While 'query-block' behaved slightly better it turns out that in libvirt
>> > we do call 'query-block' to figure out some information about the
>> > block device (e.g. throttling info) but we don't look at the backing
>> > chain itself.
>> >
>> > Wire up 'flat' for 'query-block' so that libvirt can ask for an
>> > abbreviated output. The implementation is much simpler as the internals
>> > are shared with 'query-named-block-nodes'.
>> >
>> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> > Acked-by: Markus Armbruster <armbru@redhat.com>
>> 
>> [...]
>> 
>> > diff --git a/block/qapi.c b/block/qapi.c
>> > index 27e0ac6a32..3688d0e713 100644
>> > --- a/block/qapi.c
>> > +++ b/block/qapi.c
>> 
>> [...]
>> 
>> > @@ -698,11 +698,12 @@ bdrv_query_bds_stats(BlockDriverState *bs, bool blk_level)
>> >      return s;
>> >  }
>> >
>> > -BlockInfoList *qmp_query_block(Error **errp)
>> > +BlockInfoList *qmp_query_block(bool has_flat, bool flat, Error **errp)
>> >  {
>> >      BlockInfoList *head = NULL, **p_next = &head;
>> >      BlockBackend *blk;
>> >      Error *local_err = NULL;
>> > +    bool return_flat = has_flat && flat;
>> 
>> This is fine.  Just flat would also be fine: flat implies has_flat.

Verbose version: This is a QMP command handler.  The generated
unmarshaler passes has_flat=false, flat=false when @flat is absent, and
has_flat=true, flat=F when @flat is present with value F.  Therefore,
flat can only be true when has_flat is also true.  Therefore, flat
implies has_flat.

Other code calling QMP command handlers (e.g. HMP commands) must satisfy
this precondition, too.

In fact, any code passing around QAPI-style has_FOO, FOO pairs should
stick to the "when has_FOO is false, FOO is zero bits" convention to
keep things consistent and predictable.

> I've borrowed this from my old patch that added it to
> query-named-block-nodes.
>
> Otherwise I'd have to mark 'has_flat' as unused. Nice side effect would
> be that 'flat' can then be used directly in the call to bdrv_query_info.

We don't -Wunused-variable, so no need to mark anything.

> Alternatively I can move the 'has_flat && flat' expression into the
> arguments of the call to bdrv_query_info to avoid the extra local
> variable.
>
> Whichever you prefer.

Matter of taste, up to you.  I just wanted to make sure you know.

>> >
>> >      GRAPH_RDLOCK_GUARD_MAINLOOP();
>> >
>> > @@ -714,7 +715,7 @@ BlockInfoList *qmp_query_block(Error **errp)
>> >          }
>> >
>> >          info = g_malloc0(sizeof(*info));
>> > -        bdrv_query_info(blk, &info->value, &local_err);
>> > +        bdrv_query_info(blk, return_flat, &info->value, &local_err);
>> >          if (local_err) {
>> >              error_propagate(errp, local_err);
>> >              g_free(info);
>> > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> > index b82af74256..a701f6bbee 100644
>> > --- a/qapi/block-core.json
>> > +++ b/qapi/block-core.json
>> > @@ -855,6 +855,10 @@
>> >  #
>> >  # Get a list of `BlockInfo` for all virtual block devices.
>> >  #
>> > +# @flat: Omit nested data about the backing image (ie. the contents at
>> > +#        'inserted.image.backing-image' are trimmed).
>> > +#        Default is false (Since 11.0)
>> > +#
>> 
>> s/ie./i.e./
>> 
>> Remind me: true suppresses member inserted.image.backing-image, i.e. it
>> is absent then.  Correct?
>
> Yes the 'backing-image' key is completely missing when @flat is true.

Okay.  What about this:

# @flat: Omit nested data about the backing image, i.e. `BlockInfo`
#     member 'inserted.image.backing-image' will be absent.  Default is
#     false.  (Since 11.0)