Re: [PATCH v11 17/17] net: stream: add QAPI events to report connection state

[Markus Armbruster]

Sorry for the slow replay, too many distractions...

Laurent Vivier <lvivier@redhat.com> writes:

> On 10/17/22 15:23, Markus Armbruster wrote:
>> Laurent Vivier <lvivier@redhat.com> writes:
>> 
>>> The netdev reports NETDEV_STREAM_CONNECTED event when the backend
>>> is connected, and NETDEV_STREAM_EOC when it is disconnected.
>>>
>>> The NETDEV_STREAM_CONNECTED event includes the URI of the destination
>>> address.
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> Acked-by: Michael S. Tsirkin <mst@redhat.com>

[...]

>>> diff --git a/qapi/net.json b/qapi/net.json
>>> index 6a1a49749294..69f83bceff3f 100644
>>> --- a/qapi/net.json
>>> +++ b/qapi/net.json
>>> @@ -895,3 +895,49 @@
>>>   ##
>>>   { 'event': 'FAILOVER_NEGOTIATED',
>>>     'data': {'device-id': 'str'} }
>>> +
>>> +##
>>> +# @NETDEV_STREAM_CONNECTED:
>>> +#
>>> +# Emitted when the netdev stream backend is connected
>>> +#
>>> +# @netdev-id: QEMU netdev id that is connected
>>> +# @uri: The Uniform Resource Identifier identifying the destination address
>>
>> Is an URI the appropriate representation here?  It's not how we specify
>> such addresses elsewhere in QAPI/QMP...
>
> I put in the event the same information we have in info_str and displayed by 
> the HMP command 'info network'. What would be a more appropriate reprensation?

SocketAddress?

>>> +#
>>> +# Since: 7.2
>>> +#
>>> +# Example:
>>> +#
>>> +# <- { 'event': 'NETDEV_STREAM_CONNECTED',
>>> +#      'data': {'uri': 'tcp:::1:1234', 'netdev-id': 'netdev0'},
>>> +#      'timestamp': {'seconds': 1663330564, 'microseconds': 804317} }
>>> +#
>>> +# or
>>> +#
>>> +# <- { 'event': 'NETDEV_STREAM_CONNECTED',
>>> +#      'data': {'uri': ''unix:/tmp/qemu0', 'netdev-id': 'netdev0'},
>>> +#      'timestamp': {'seconds': 1663330564, 'microseconds': 804317} }
>>> +#
>>> +##
>>> +{ 'event': 'NETDEV_STREAM_CONNECTED',
>>> +  'data': { 'netdev-id': 'str',
>>> +            'uri': 'str' } }
>>> +
>>> +##
>>> +# @NETDEV_STREAM_EOC:
>>
>> What does "EOC" mean?
>
> End-Of-Connection, this is the nomenclature used in the code when the socket 
> is disconnected.
>
>> Could this be named NETDEV_STREAM_DISCONNECTED, for symmetry with
>> NETDEV_STREAM_CONNECTED?
>
> Yes, it can. EOC is shorter, it's why I used it, but if you prefer 
> "disconnected"...

For better or worse, we've always preferred longhand to abbreviations in
QAPI schema names.  Exceptions have crept in, of course.

[...]