Re: [PATCH] net: vhost-user: add QAPI events to report connection state

[Laurent Vivier <lvivier@redhat.com>]

On 14/02/2025 15:19, Daniel P. Berrangé wrote:
> On Fri, Feb 14, 2025 at 01:59:20PM +0000, Daniel P. Berrangé wrote:
>> On Fri, Feb 14, 2025 at 11:18:55AM +0100, Laurent Vivier wrote:
>>> On 14/02/2025 11:06, Markus Armbruster wrote:
>>>> Laurent Vivier <lvivier@redhat.com> writes:
>>>>
>>>>> The netdev reports NETDEV_VHOST_USER_CONNECTED event when
>>>>> the chardev is connected, and NETDEV_VHOST_USER_DISCONNECTED
>>>>> when it is disconnected.
>>>>>
>>>>> The NETDEV_VHOST_USER_CONNECTED event includes the ChardevInfo
>>>>> (label, filename and frontend_open).
>>>>>
>>>>> This allows a system manager like libvirt to detect when the server
>>>>> fails.
>>>>>
>>>>> For instance with passt:
>>>>>
>>>>> { 'execute': 'qmp_capabilities' }
>>>>> { "return": { } }
>>>>>
>>>>> [killing passt here]
>>>>>
>>>>> { "timestamp": { "seconds": 1739517243, "microseconds": 115081 },
>>>>>     "event": "NETDEV_VHOST_USER_DISCONNECTED",
>>>>>     "data": { "netdev-id": "netdev0" } }
>>>>>
>>>>> [automatic reconnection with reconnect-ms]
>>>>>
>>>>> { "timestamp": { "seconds": 1739517290, "microseconds": 343777 },
>>>>>     "event": "NETDEV_VHOST_USER_CONNECTED",
>>>>>     "data": { "netdev-id": "netdev0",
>>>>>               "info": { "frontend-open": true,
>>>>>                         "filename": "unix:",
>>>>>                         "label": "chr0" } } }
>>>>>
>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>
>>>> Standard question for events: if a management application misses an
>>>> event, say because it restarts and reconnects, is there a way to obtain
>>>> the missed information with a query command?
>>>>
>>>
>>> query-chardev could help but it doesn't provide the netdev id.
>>
>> It doesn't have to IMHO. The application that created the NIC should know
>> what ID it assigned to both the netdev and chardev, and thus should be
>> able to use query-chardev to identify the chardev it previously
>> associated with the netdev.
> 
> That said I kind of wonder whether we should be adding events against
> the chardev directly, instead of against the netdev use of chardev.

I don't know that is the best from a libvirt point of view.

> 
> ie CHARDEV_OPENED / CHARDEV_CLOSED events. For OPENED it is trivially
> equivalent. For CLOSED it is a little trickier, since this patch does
> delayed event dispatch from a bottom-half. If the chardev code was
> able to emit this event from a bottom half itself, that could be OK
> though.

Do we really need to delay the event?
libvirt could check the chardev user is really down before restarting it?

Without bh it's very simple:

@@ -68,9 +69,11 @@ void qemu_chr_be_event(Chardev *s, QEMUChrEvent event)
      switch (event) {
          case CHR_EVENT_OPENED:
              s->be_open = 1;
+            qapi_event_send_chardev_connected(s->label);
              break;
          case CHR_EVENT_CLOSED:
              s->be_open = 0;
+            qapi_event_send_chardev_disconnected(s->label);
              break;
      case CHR_EVENT_BREAK:
      case CHR_EVENT_MUX_IN:

Thanks,
Laurent

> With regards,
> Daniel