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

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

[Peter Krempa]

v3:
 - Doc wording tweak (Eric)

v2:
 - More descriptive docs text for the new QAPI option (Markus; new
   wording not approved)
 - use new options in hmp_nbd_server_start to avoid fetching unused
   backing image info, impossible in 'info block' worker (Markus)

Peter Krempa (2):
  block: Wire up 'flat' mode also for 'query-block'
  hmp_nbd_server_start: Don't ask for backing image data

 block/monitor/block-hmp-cmds.c | 4 ++--
 block/qapi.c                   | 9 +++++----
 qapi/block-core.json           | 5 +++++
 ui/cocoa.m                     | 2 +-
 4 files changed, 13 insertions(+), 7 deletions(-)

-- 
2.52.0

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

[Peter Krempa]

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>
---
 block/monitor/block-hmp-cmds.c | 4 ++--
 block/qapi.c                   | 9 +++++----
 qapi/block-core.json           | 5 +++++
 ui/cocoa.m                     | 2 +-
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 3391cee4d2..bde25bb588 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -422,7 +422,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
     /* Then try adding all block devices.  If one fails, close all and
      * exit.
      */
-    block_list = qmp_query_block(NULL);
+    block_list = qmp_query_block(false, false, NULL);

     for (info = block_list; info; info = info->next) {
         if (!info->value->inserted) {
@@ -741,7 +741,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)

     /* Print BlockBackend information */
     if (!nodes) {
-        block_list = qmp_query_block(NULL);
+        block_list = qmp_query_block(false, false, NULL);
     } else {
         block_list = NULL;
     }
diff --git a/block/qapi.c b/block/qapi.c
index 27e0ac6a32..3688d0e713 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -456,7 +456,7 @@ fail:

 /* @p_info will be set only on success. */
 static void GRAPH_RDLOCK
-bdrv_query_info(BlockBackend *blk, BlockInfo **p_info, Error **errp)
+bdrv_query_info(BlockBackend *blk, bool flat, BlockInfo **p_info, Error **errp)
 {
     BlockInfo *info = g_malloc0(sizeof(*info));
     BlockDriverState *bs = blk_bs(blk);
@@ -488,7 +488,7 @@ bdrv_query_info(BlockBackend *blk, BlockInfo **p_info, Error **errp)
     }

     if (bs && bs->drv) {
-        info->inserted = bdrv_block_device_info(blk, bs, false, errp);
+        info->inserted = bdrv_block_device_info(blk, bs, flat, errp);
         if (info->inserted == NULL) {
             goto err;
         }
@@ -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;

     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)
+#
 # Returns: a list describing each virtual block device.  Filter nodes
 #     that were created implicitly are skipped over.
 #
@@ -945,6 +949,7 @@
 #        }
 ##
 { 'command': 'query-block', 'returns': ['BlockInfo'],
+  'data': { '*flat': 'bool' },
   'allow-preconfig': true }

 ##
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 23b7a736d7..5b21fe3aea 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1849,7 +1849,7 @@ static void addRemovableDevicesMenuItems(void)
     BlockInfoList *currentDevice, *pointerToFree;
     NSString *deviceName;

-    currentDevice = qmp_query_block(NULL);
+    currentDevice = qmp_query_block(false, false, NULL);
     pointerToFree = currentDevice;

     menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
-- 
2.52.0

[PATCH v3 2/2] hmp_nbd_server_start: Don't ask for backing image data

[Peter Krempa]

From: Peter Krempa <pkrempa@redhat.com>

'hmp_nbd_server_start' uses only the device name from the data returned
from 'qmp_query_block', thus no backing file information. Use the new
options to suppress asking for the unused parts to save on resources.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/monitor/block-hmp-cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index bde25bb588..1fd28d59eb 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -422,7 +422,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
     /* Then try adding all block devices.  If one fails, close all and
      * exit.
      */
-    block_list = qmp_query_block(false, false, NULL);
+    block_list = qmp_query_block(true, true, NULL);

     for (info = block_list; info; info = info->next) {
         if (!info->value->inserted) {
-- 
2.52.0

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

[Markus Armbruster]

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.

>
>      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?

>  # Returns: a list describing each virtual block device.  Filter nodes
>  #     that were created implicitly are skipped over.
>  #
> @@ -945,6 +949,7 @@
>  #        }
>  ##
>  { 'command': 'query-block', 'returns': ['BlockInfo'],
> +  'data': { '*flat': 'bool' },
>    'allow-preconfig': true }
>
>  ##

[...]

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

[Peter Krempa]

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.

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.

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.

> 
> >
> >      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.

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

[Markus Armbruster]

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)

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

[Peter Krempa]

On Wed, Feb 04, 2026 at 07:11:44 +0100, Markus Armbruster wrote:
> 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.

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