On Wed, Feb 04, 2026 at 07:11:44 +0100, Markus Armbruster wrote:
> Peter Krempa <[email protected]> writes:
>
> > On Tue, Feb 03, 2026 at 13:01:01 +0100, Markus Armbruster wrote:
> >> Peter Krempa <[email protected]> writes:
> >>
> >> > From: Peter Krempa <[email protected]>
> >> >
> >> > 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 <[email protected]>
> >> > Acked-by: Markus Armbruster <[email protected]>
> >>
> >> [...]
> >>
> >> > 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.
That very much makes sense, thanks for the explanation.
>
> > 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.
Aaah! I'm too used to libvirt where we do use that warning. In such case
this (obviously without marking 'has_flat' as unused) is the simplest
then.
> > 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)
I had to re-wrap this slightly as sphinx moaned about > 70 column line
