Re: [PATCH v2 1/4] nbd: Add multi-conn option

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On Tue, Apr 29, 2025 at 01:31:44PM +0200, Markus Armbruster wrote:
>> > In the context of qemu that might suggest having separate
>> > multi_conn_requested and multi_conn fields, where the latter can be
>> > queried over QMP to find out what is actually going on.  Could even
>> > add multi_conn_max to allow MAX_MULTI_CONN constant to be read out.
>>
>> You decide what to do with my feedback :)
>
> I've got a local patch that adds the ability for
> query-named-block-nodes (and query-block) to output image-specific
> information for NBD that includes how many connections are actually in
> use.  But now I've got a QMP question:
>
> My patch, as written, makes the output look like this:
>
> "format-specific": {"mode": "extended", "type": "nbd", "connections": 1}},
>
> by changing block-core.json like this (partial patch shown):
>
> @@ -208,10 +223,12 @@
>  #
>  # @file: Since 8.0
>  #
> +# @nbd: Since 10.1
> +#
>  # Since: 1.7
>  ##
>  { 'enum': 'ImageInfoSpecificKind',
> -  'data': [ 'qcow2', 'vmdk', 'luks', 'rbd', 'file' ] }
> +  'data': [ 'qcow2', 'vmdk', 'luks', 'rbd', 'file', 'nbd' ] }
>
>  ##
>  # @ImageInfoSpecificQCow2Wrapper:
> @@ -284,7 +301,8 @@
>        'luks': 'ImageInfoSpecificLUKSWrapper',
>        'rbd': 'ImageInfoSpecificRbdWrapper',
> -      'file': 'ImageInfoSpecificFileWrapper'
> +      'file': 'ImageInfoSpecificFileWrapper',
> +      'nbd': 'ImageInfoSpecificNbd'
>    } }
>
> But that is different from all of the other image modes, where the
> output looks more like:
>
> "format-specific": {"type": "rbd", "data": {"encryption-format":"..."}}},
>
> note the extra layer of nesting, due to historical reasons of being
> added in a time when the QMP generator was not as nice on supporting
> flatter union-style coding.

Correct, this is an artifact of development history, specifically
"simple" unions and their elimination.

> Must I create an ImageInfoSpecificNbdWrapper type, with the sole
> purpose of having the same (pointless, IMO) "data":{} wrapper as all
> the other branches of the union type, or am I okay with my addition
> using the flatter style?

I dislike these wrappers, possibly more than is reasonable.  Still, I
think local consistency is more important.

Precedence: commit 7f36a50ab4e (block/file: Add file-specific image
info) added a new branch with a wrapper well after we eliminated
"simple" unions.

I think you should add the silly wrapper.