Re: [PATCH v3 1/4] qapi: net: Add query-netdevs command

[Markus Armbruster]

Alexey Kirillov <lekiravi@yandex-team.ru> writes:

> Hi!
>
> 02.09.2020, 14:23, "Markus Armbruster" <armbru@redhat.com>:
>> Alexey Kirillov <lekiravi@yandex-team.ru> writes:
>>
>>>  Add a qmp command that provides information about currently attached
>>>  backend network devices and their configuration.
>>>
>>>  Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>>
>> [...]
>>>  diff --git a/qapi/net.json b/qapi/net.json
>>>  index ddb113e5e5..c09322bb42 100644
>>>  --- a/qapi/net.json
>>>  +++ b/qapi/net.json
>>>  @@ -714,3 +714,71 @@
>>>   ##
>>>   { 'event': 'FAILOVER_NEGOTIATED',
>>>     'data': {'device-id': 'str'} }
>>>  +
>>>  +##
>>>  +# @NetdevInfo:
>>>  +#
>>>  +# Configuration of a network backend device (netdev).
>>>  +#
>>>  +# @id: Device identifier.
>>>  +#
>>>  +# @type: Specify the driver used for interpreting remaining arguments.
>>>  +#
>>>  +# @peer-id: Connected frontend network device identifier.
>>>  +#
>>>  +# Since: 5.2
>>>  +##
>>>  +{ 'union': 'NetdevInfo',
>>>  + 'base': { 'id': 'str',
>>>  + 'type': 'NetClientDriver',
>>>  + '*peer-id': 'str' },
>>>  + 'discriminator': 'type',
>>>  + 'data': {
>>>  + 'user': 'NetdevUserOptions',
>>>  + 'tap': 'NetdevTapOptions',
>>>  + 'l2tpv3': 'NetdevL2TPv3Options',
>>>  + 'socket': 'NetdevSocketOptions',
>>>  + 'vde': 'NetdevVdeOptions',
>>>  + 'bridge': 'NetdevBridgeOptions',
>>>  + 'netmap': 'NetdevNetmapOptions',
>>>  + 'vhost-user': 'NetdevVhostUserOptions',
>>>  + 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
>>
>> This is union Netdev plus a common member @peer-id, less the variant
>> members for NetClientDriver values 'nic' and 'hubport'.
>>
>> Can 'type: 'nic' and 'type': 'hubport' occur?
>
> No, it can't. We don't support NIC/hubport in query-netdevs, so we neither 
> create this
> structure for them, nor store config.

Same for 'none', I guess.

As defined, NetdevInfo allows types 'none', 'nic', and 'hubport', it
just has no variant members for them.  The fact that they can't occur is
not coded into the type, and therefore not visible in introspection.

To make introspection more precise, we'd have to define a new enum type.
How much that would complicate the C code is unclear.

Do we need it to be more precise?  Eric, got an opinion?

Existing type Netdev has the same issue for 'none' and 'nic'.  I guess
this is so we can reuse the type for -net.  Not sure that's a good idea,
but not this patch's problem.

>> What's the use case for @peer-id?
>
> Main reason is to provide information "is this backend connected to frontend 
> device".
> User also may want to know which backend used for frontend device.
>
>> Assuming we want @peer-id: its documentation is too vague. Cases:
>>
>> * Not connected to a frontend: I guess @peer-id is absent then.
>
> Absolutely correct.
>
>> * Connected to a frontend
>>
>>   - that has a qdev ID (the thing you set with -device id=...): I guess
>>     it's the qdev ID then.
>
> Correct.
>
>>   - that doesn't have a qdev ID: anyone's guess.
>
> We use field "name" of structure NetClientState, so if there is no direct ID 
> setting,
> there must be generated name (in format "model.id").

Perhaps:

      # @peer-id: the connected network backend's name (absent if no
      #           backend is connected)

>>>  +
>>>  +##
>>>  +# @query-netdevs:
>>>  +#
>>>  +# Get a list of @NetdevInfo for all virtual network backend devices 
>>> (netdevs).
>>>  +#
>>>  +# Returns: a list of @NetdevInfo describing each netdev.
>>>  +#
>>>  +# Since: 5.2
>>>  +#
>>>  +# Example:
>>>  +#
>>>  +# -> { "execute": "query-netdevs" }
>>>  +# <- { "return": [
>>>  +# {
>>>  +# "ipv6": true,
>>>  +# "ipv4": true,
>>>  +# "host": "10.0.2.2",
>>>  +# "ipv6-dns": "fec0::3",
>>>  +# "ipv6-prefix": "fec0::",
>>>  +# "net": "10.0.2.0/255.255.255.0",
>>>  +# "ipv6-host": "fec0::2",
>>>  +# "type": "user",
>>>  +# "peer-id": "net0",
>>>  +# "dns": "10.0.2.3",
>>>  +# "hostfwd": [
>>>  +# {
>>>  +# "str": "tcp::20004-:22"
>>>  +# }
>>>  +# ],
>>>  +# "ipv6-prefixlen": 64,
>>>  +# "id": "netdev0",
>>>  +# "restrict": false
>>>  +# }
>>>  +# ]
>>>  +# }
>>>  +#
>>>  +##
>>>  +{ 'command': 'query-netdevs', 'returns': ['NetdevInfo'] }
>
>
> -- 
> Alexey Kirillov
> Yandex.Cloud