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

[Markus Armbruster <armbru@redhat.com>]

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 chardev id.
>
> 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": 1739538634, "microseconds": 920450 },
>   "event": "NETDEV_VHOST_USER_DISCONNECTED",
>   "data": { "netdev-id": "netdev0" } }
>
> [automatic reconnection with reconnect-ms]
>
> { "timestamp": { "seconds": 1739538638, "microseconds": 354181 },
>   "event": "NETDEV_VHOST_USER_CONNECTED",
>   "data": { "netdev-id": "netdev0", "chardev-id": "chr0" } }
>
> Tested-by: Stefano Brivio <sbrivio@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>
> Notes:
>     v4:
>       - as ChardevInfo is not needed, move events definition from
>         qapi/char.json to qapi/net.json
>     
>     v3:
>       - remove ChardevInfo, provides only the chardev id
>     
>     v2:
>       - remove duplicate line info.frontend_open
>
>  qapi/net.json    | 40 ++++++++++++++++++++++++++++++++++++++++
>  net/vhost-user.c |  3 +++
>  2 files changed, 43 insertions(+)
>
> diff --git a/qapi/net.json b/qapi/net.json
> index 2739a2f42332..310cc4fd1907 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -1031,3 +1031,43 @@
>  ##
>  { 'event': 'NETDEV_STREAM_DISCONNECTED',
>    'data': { 'netdev-id': 'str' } }
> +
> +##
> +# @NETDEV_VHOST_USER_CONNECTED:
> +#
> +# Emitted when the vhost-user chardev is connected
> +#
> +# @netdev-id: QEMU netdev id that is connected
> +#
> +# @chardev-id: The character device id used by the QEMU netdev
> +#
> +# Since: 10.0
> +#
> +# .. qmp-example::
> +#
> +#     <- { "timestamp": {"seconds": 1739538638, "microseconds": 354181 },
> +#          "event": "NETDEV_VHOST_USER_CONNECTED",
> +#          "data": { "netdev-id": "netdev0", "chardev-id": "chr0" } }
> +#
> +##
> +{ 'event': 'NETDEV_VHOST_USER_CONNECTED',
> +  'data': { 'netdev-id': 'str', 'chardev-id': 'str' } }
> +
> +##
> +# @NETDEV_VHOST_USER_DISCONNECTED:
> +#
> +# Emitted when the vhost-user chardev is disconnected
> +#
> +# @netdev-id: QEMU netdev id that is disconnected
> +#
> +# Since: 10.0
> +#
> +# .. qmp-example::
> +#
> +#     <- { "timestamp": { "seconds": 1739538634, "microseconds": 920450 },
> +#          "event": "NETDEV_VHOST_USER_DISCONNECTED",
> +#          "data": { "netdev-id": "netdev0" } }
> +#
> +##
> +{ 'event': 'NETDEV_VHOST_USER_DISCONNECTED',
> +  'data': { 'netdev-id': 'str' } }
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 12555518e838..0b235e50c650 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -16,6 +16,7 @@
>  #include "chardev/char-fe.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-net.h"
> +#include "qapi/qapi-events-net.h"
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>  #include "qemu/option.h"
> @@ -271,6 +272,7 @@ static void chr_closed_bh(void *opaque)
>      if (err) {
>          error_report_err(err);
>      }
> +    qapi_event_send_netdev_vhost_user_disconnected(name);
>  }
>  
>  static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
> @@ -300,6 +302,7 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
>                                           net_vhost_user_watch, s);
>          qmp_set_link(name, true, &err);
>          s->started = true;
> +        qapi_event_send_netdev_vhost_user_connected(name, chr->label);

We seem to use "label" and "id" interchangeably.  Unfortunate.


>          break;
>      case CHR_EVENT_CLOSED:
>          /* a close event may happen during a read/write, but vhost

Like Daniel, I wonder whether provding events for chardevs instead would
be more broadly useful.

That said, there's nothing wrong with the patch itself, so
Acked-by: Markus Armbruster <armbru@redhat.com>