public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [libvirt PATCH 0/3] Support for restarting passt backend
@ 2023-02-22  0:35 Laine Stump
  2023-02-22  0:35 ` [libvirt PATCH 1/3] qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT Laine Stump
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Laine Stump @ 2023-02-22  0:35 UTC (permalink / raw)
  To: libvir-list; +Cc: passt-dev

If the passt process that implements the backend of a QEMU emulated
network device terminates, QEMU itself is incapable of restarting a
new process so that the networking can begin... working again.

However, what QEMU can and does do is:

1) send the NETDEV_STREAM_DISCONNECTED event to libvirt when it sees
   that the socket has been closed (this event has been in QEMU for as
   long as support for "-netdev stream", which is used for connecting
   to passt)

2) as of QEMU 8.0.0, the qemu commandline for -netdev stream accepts
   the "reconnect" option, which tells QEMU to attempt reconnecting to
   the same socket it previously used, repeating the attempt every "n"
   seconds (the only argument to reconnect) until it is successful.

If libvirt adds the reconnect option to the qemu commandline, and then
responds to a NETDEV_STREAM_DISCONNECTED event by re-running the same
passt command that it ran when the device was originally connected,
then a guest will be able to recover in the (very unlikely, according
to Stefano!) event that the original passt process unexpectedly exits,
or is killed by some external entity.

Patch 2/3 handles (2) above, while patch 3/3 handles (1). (patch 1/3
is a short guest appearance by pkrempa. Thanks pkrempa!).

This resolves https://bugzilla.redhat.com/2172098

Along with Stefano's series fixing up selinux issues related to
running the passt process, they make the passt backend very usable.

Laine Stump (2):
  qemu: add reconnect=5 to passt qemu commandline options when available
  qemu: respond to NETDEV_STREAM_DISCONNECTED event

Peter Krempa (1):
  qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT

 src/qemu/qemu_capabilities.c                  |  4 +
 src/qemu/qemu_capabilities.h                  |  3 +
 src/qemu/qemu_domain.c                        |  1 +
 src/qemu/qemu_domain.h                        |  1 +
 src/qemu/qemu_driver.c                        | 82 +++++++++++++++++++
 src/qemu/qemu_monitor.c                       | 11 +++
 src/qemu/qemu_monitor.h                       |  6 ++
 src/qemu/qemu_monitor_json.c                  | 16 ++++
 src/qemu/qemu_passt.c                         | 11 +++
 src/qemu/qemu_process.c                       | 18 ++++
 .../caps_8.0.0.x86_64.xml                     |  1 +
 .../net-user-passt.x86_64-7.2.0.args          | 37 +++++++++
 .../net-user-passt.x86_64-latest.args         |  2 +-
 tests/qemuxml2argvtest.c                      |  1 +
 14 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args

-- 
2.39.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [libvirt PATCH 1/3] qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT
  2023-02-22  0:35 [libvirt PATCH 0/3] Support for restarting passt backend Laine Stump
@ 2023-02-22  0:35 ` Laine Stump
  2023-02-22  0:35 ` [libvirt PATCH 2/3] qemu: add reconnect=5 to passt qemu commandline options when available Laine Stump
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Laine Stump @ 2023-02-22  0:35 UTC (permalink / raw)
  To: libvir-list; +Cc: passt-dev

From: Peter Krempa <pkrempa@redhat.com>

Detect that the 'stream' netdev backend supports reconnecting.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/qemu/qemu_capabilities.c                     | 4 ++++
 src/qemu/qemu_capabilities.h                     | 3 +++
 tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml | 1 +
 3 files changed, 8 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d92d5a62dd..3cb5785baa 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -687,6 +687,9 @@ VIR_ENUM_IMPL(virQEMUCaps,
               "virtio-crypto", /* QEMU_CAPS_DEVICE_VIRTIO_CRYPTO */
               "cryptodev-backend-lkcf", /* QEMU_CAPS_OBJECT_CRYPTO_LKCF */
               "pvpanic-pci", /* QEMU_CAPS_DEVICE_PANIC_PCI */
+
+              /* 445 */
+              "netdev.stream.reconnect", /* QEMU_CAPS_NETDEV_STREAM_RECONNECT */
     );
 
 
@@ -1559,6 +1562,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = {
     { "nbd-server-start/arg-type/tls-creds", QEMU_CAPS_NBD_TLS },
     { "nbd-server-add/arg-type/bitmap", QEMU_CAPS_NBD_BITMAP },
     { "netdev_add/arg-type/+stream", QEMU_CAPS_NETDEV_STREAM },
+    { "netdev_add/arg-type/+stream/reconnect", QEMU_CAPS_NETDEV_STREAM_RECONNECT },
     { "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA },
     /* JSON support for -netdev was introduced for the 'dgram' netdev type */
     { "netdev_add/arg-type/type/^dgram", QEMU_CAPS_NETDEV_JSON },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index b72348cf88..d049f79dd9 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -667,6 +667,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
     QEMU_CAPS_OBJECT_CRYPTO_LKCF, /* -object cryptodev-backend-lkcf */
     QEMU_CAPS_DEVICE_PANIC_PCI, /* -device pvpanic-pci */
 
+    /* 445 */
+    QEMU_CAPS_NETDEV_STREAM_RECONNECT, /* -netdev stream supports reconnect */
+
     QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
 
diff --git a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
index bcad3db057..ce051d3f1c 100644
--- a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
@@ -205,6 +205,7 @@
   <flag name='virtio-crypto'/>
   <flag name='cryptodev-backend-lkcf'/>
   <flag name='pvpanic-pci'/>
+  <flag name='netdev.stream.reconnect'/>
   <version>7002050</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>43100244</microcodeVersion>
-- 
@@ -205,6 +205,7 @@
   <flag name='virtio-crypto'/>
   <flag name='cryptodev-backend-lkcf'/>
   <flag name='pvpanic-pci'/>
+  <flag name='netdev.stream.reconnect'/>
   <version>7002050</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>43100244</microcodeVersion>
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [libvirt PATCH 2/3] qemu: add reconnect=5 to passt qemu commandline options when available
  2023-02-22  0:35 [libvirt PATCH 0/3] Support for restarting passt backend Laine Stump
  2023-02-22  0:35 ` [libvirt PATCH 1/3] qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT Laine Stump
@ 2023-02-22  0:35 ` Laine Stump
  2023-02-22  0:35 ` [libvirt PATCH 3/3] qemu: respond to NETDEV_STREAM_DISCONNECTED event Laine Stump
  2023-02-22 10:21 ` [libvirt PATCH 0/3] Support for restarting passt backend Michal Prívozník
  3 siblings, 0 replies; 7+ messages in thread
From: Laine Stump @ 2023-02-22  0:35 UTC (permalink / raw)
  To: libvir-list; +Cc: passt-dev

QEMU's "reconnect" option of "-netdev stream" tells QEMU to
periodically (period is given in seconds as an argument to the option)
attempt to reconnect to the same passt socket to which it had
originally connected to. This is useful in cases where the passt
process terminates, and libvirtd starts a new passt process in its
place (which doesn't happen yet, but will happen automatically after
an upcoming patch in this series).

Since there is no real hueristic for determining the "best" value of
the reconnect interval, rather than clutter up config with a knob that
nobody knows how to properly twiddle, we just set the reconnect timer
to 5 seconds.

"-netdev stream" first appeared in QEMU 7.2.0, but the reconnect
option won't be available until QEMU 8.0.0, so we need to check QEMU
capabilities just in case someone is using QEMU 7.2.0 (and thus can
support passt backend, but not reconnect)

Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/qemu/qemu_passt.c                         | 11 ++++++
 .../net-user-passt.x86_64-7.2.0.args          | 37 +++++++++++++++++++
 .../net-user-passt.x86_64-latest.args         |  2 +-
 tests/qemuxml2argvtest.c                      |  1 +
 4 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args

diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index 1217a6a087..55199c9d36 100644
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -83,6 +83,8 @@ qemuPasstAddNetProps(virDomainObj *vm,
 {
     g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net);
     g_autoptr(virJSONValue) addrprops = NULL;
+    qemuDomainObjPrivate *priv = vm->privateData;
+    virQEMUCaps *qemuCaps = priv->qemuCaps;
 
     if (virJSONValueObjectAdd(&addrprops,
                               "s:type", "unix",
@@ -98,6 +100,15 @@ qemuPasstAddNetProps(virDomainObj *vm,
                               NULL) < 0) {
         return -1;
     }
+
+    /* a narrow range of QEMU releases support -netdev stream, but
+     * don't support its "reconnect" option
+     */
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_STREAM_RECONNECT) &&
+        virJSONValueObjectAdd(netprops, "u:reconnect", 5, NULL) < 0) {
+        return -1;
+    }
+
     return 0;
 }
 
diff --git a/tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args b/tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args
new file mode 100644
index 0000000000..037dabb87d
--- /dev/null
+++ b/tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args
@@ -0,0 +1,37 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' \
+-machine pc-i440fx-7.2,usb=off,dump-guest-core=off,memory-backend=pc.ram \
+-accel tcg \
+-cpu qemu64 \
+-m 214 \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \
+-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \
+-netdev '{"type":"stream","addr":{"type":"unix","path":"/bad-test-used-env-xdg-runtime-dir/libvirt/qemu/run/passt/-1-QEMUGuest1-net0.socket"},"server":false,"id":"hostnet0"}' \
+-device '{"driver":"rtl8139","netdev":"hostnet0","id":"net0","mac":"00:11:22:33:44:55","bus":"pci.0","addr":"0x2"}' \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args b/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args
index 48e3e8ca8b..f84bec2ec1 100644
--- a/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/net-user-passt.x86_64-latest.args
@@ -30,7 +30,7 @@ XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
 -blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
 -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw","file":"libvirt-1-storage"}' \
 -device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-format","id":"ide0-0-0","bootindex":1}' \
--netdev '{"type":"stream","addr":{"type":"unix","path":"/bad-test-used-env-xdg-runtime-dir/libvirt/qemu/run/passt/-1-QEMUGuest1-net0.socket"},"server":false,"id":"hostnet0"}' \
+-netdev '{"type":"stream","addr":{"type":"unix","path":"/bad-test-used-env-xdg-runtime-dir/libvirt/qemu/run/passt/-1-QEMUGuest1-net0.socket"},"server":false,"reconnect":5,"id":"hostnet0"}' \
 -device '{"driver":"rtl8139","netdev":"hostnet0","id":"net0","mac":"00:11:22:33:44:55","bus":"pci.0","addr":"0x2"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b897814a5d..5a33c336c8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1471,6 +1471,7 @@ mymain(void)
     DO_TEST_CAPS_ARCH_LATEST_FULL("net-user", "x86_64", ARG_FLAGS, FLAG_SLIRP_HELPER);
     DO_TEST_NOCAPS("net-user-addr");
     DO_TEST_CAPS_LATEST("net-user-passt");
+    DO_TEST_CAPS_VER("net-user-passt", "7.2.0");
     DO_TEST_NOCAPS("net-virtio");
     DO_TEST_NOCAPS("net-virtio-device");
     DO_TEST_NOCAPS("net-virtio-disable-offloads");
-- 
@@ -1471,6 +1471,7 @@ mymain(void)
     DO_TEST_CAPS_ARCH_LATEST_FULL("net-user", "x86_64", ARG_FLAGS, FLAG_SLIRP_HELPER);
     DO_TEST_NOCAPS("net-user-addr");
     DO_TEST_CAPS_LATEST("net-user-passt");
+    DO_TEST_CAPS_VER("net-user-passt", "7.2.0");
     DO_TEST_NOCAPS("net-virtio");
     DO_TEST_NOCAPS("net-virtio-device");
     DO_TEST_NOCAPS("net-virtio-disable-offloads");
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [libvirt PATCH 3/3] qemu: respond to NETDEV_STREAM_DISCONNECTED event
  2023-02-22  0:35 [libvirt PATCH 0/3] Support for restarting passt backend Laine Stump
  2023-02-22  0:35 ` [libvirt PATCH 1/3] qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT Laine Stump
  2023-02-22  0:35 ` [libvirt PATCH 2/3] qemu: add reconnect=5 to passt qemu commandline options when available Laine Stump
@ 2023-02-22  0:35 ` Laine Stump
  2023-02-22 10:21   ` Michal Prívozník
  2023-02-22 10:21 ` [libvirt PATCH 0/3] Support for restarting passt backend Michal Prívozník
  3 siblings, 1 reply; 7+ messages in thread
From: Laine Stump @ 2023-02-22  0:35 UTC (permalink / raw)
  To: libvir-list; +Cc: passt-dev

When a QEMU netdev is of type "stream", if the socket it uses for
connectivity to the host network gets closed, then QEMU will send a
NETDEV_STREAM_DISCONNECTED event. We know that any stream netdev we've
created is backed by a passt process, and if the socket was closed,
that means the passt process has disappeared.

When we receive this event, we can respond by starting a new passt
process with the same options (including socket path) we originally
used. If we have previously created the stream netdev device with a
"reconnect" option, then QEMU will automatically reconnect to this new
passt process. (If we hadn't used "reconnect", then QEMU will never
try to reconnect to the new passt process, so there's no point in
starting it.)

Note that NETDEV_STREAM_DISCONNECTED is an event sent for the netdev
(ie "host side") of the network device, and so it sends the
"netdev-id" to specify which device was disconnected. But libvirt's
virDomainNetDef (the object used to keep track of network devices) is
the internal representation of both the host-side "netdev", and the
guest side device, and virDomainNetDef doesn't directly keep track of
the netdev-id, only of the device's "alias" (which is the "id"
parameter of the *guest* side of the device). Fortunately, by convention
libvirt always names the host-side of devices as "host" + alias, so in
order to search for the affected NetDef, all we need to do is trim the
1st 4 characters from the netdev-id and look for the NetDef having
that resulting trimmed string as its alias. (Contrast this to
NIC_RX_FILTER_CHANGED, which is an event received for the guest side
of the device, and so directly contains the device alias.)

Resolves: https://bugzilla.redhat.com/2172098
Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/qemu/qemu_domain.c       |  1 +
 src/qemu/qemu_domain.h       |  1 +
 src/qemu/qemu_driver.c       | 82 ++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_monitor.c      | 11 +++++
 src/qemu/qemu_monitor.h      |  6 +++
 src/qemu/qemu_monitor_json.c | 16 +++++++
 src/qemu/qemu_process.c      | 18 ++++++++
 7 files changed, 135 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e9bc0f375d..4cf9a259ea 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11238,6 +11238,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
         break;
     case QEMU_PROCESS_EVENT_WATCHDOG:
     case QEMU_PROCESS_EVENT_DEVICE_DELETED:
+    case QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED:
     case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED:
     case QEMU_PROCESS_EVENT_SERIAL_CHANGED:
     case QEMU_PROCESS_EVENT_MONITOR_EOF:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 1053d1d4cb..6adc067681 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -447,6 +447,7 @@ typedef enum {
     QEMU_PROCESS_EVENT_WATCHDOG = 0,
     QEMU_PROCESS_EVENT_GUESTPANIC,
     QEMU_PROCESS_EVENT_DEVICE_DELETED,
+    QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED,
     QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED,
     QEMU_PROCESS_EVENT_SERIAL_CHANGED,
     QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6154fe9bfe..47d6a0dd95 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -40,6 +40,7 @@
 #include "qemu_hostdev.h"
 #include "qemu_hotplug.h"
 #include "qemu_monitor.h"
+#include "qemu_passt.h"
 #include "qemu_process.h"
 #include "qemu_migration.h"
 #include "qemu_migration_params.h"
@@ -3622,6 +3623,84 @@ processDeviceDeletedEvent(virQEMUDriver *driver,
 }
 
 
+static void
+processNetdevStreamDisconnectedEvent(virDomainObj *vm,
+                                     const char *netdevId)
+{
+    virDomainDeviceDef dev;
+    virDomainNetDef *def;
+    qemuDomainObjPrivate *priv;
+    virQEMUCaps *qemuCaps;
+    const char *devAlias = NULL;
+
+    /* The event sends us the "netdev-id", but we don't store the
+     * netdev-id in the NetDef and thus can't use it to find the
+     * correct NetDef. We *do* keep the device alias in the NetDef.
+     * By definition, the netdev-id is "host" + devAlias, so we just
+     * need to remove "host" from the front of netdev-id to get
+     * something we can use to find the proper NetDef.
+     */
+    if (STREQLEN(netdevId, "host", 4))
+        devAlias = &netdevId[4];
+
+    if (!devAlias) {
+        VIR_WARN("Received NETDEV_STREAM_DISCONNECTED event for unrecognized netdev %s from domain %p %s",
+                  netdevId, vm, vm->def->name);
+        return;
+    }
+
+    VIR_DEBUG("Received NETDEV_STREAM_DISCONNECTED event for device %s from domain %p %s",
+              devAlias, vm, vm->def->name);
+
+    if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0)
+        return;
+
+    if (!virDomainObjIsActive(vm)) {
+        VIR_DEBUG("Domain is not running");
+        goto endjob;
+    }
+
+    if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0) {
+        VIR_WARN("NETDEV_STREAM_DISCONNECTED event received for non-existent device %s in domain %s",
+                 devAlias, vm->def->name);
+        goto endjob;
+    }
+    if (dev.type != VIR_DOMAIN_DEVICE_NET) {
+        VIR_WARN("NETDEV_STREAM_DISCONNECTED event received for non-network device %s in domain %s",
+                 devAlias, vm->def->name);
+        goto endjob;
+    }
+    def = dev.data.net;
+
+    if (def->backend.type != VIR_DOMAIN_NET_BACKEND_PASST) {
+        VIR_DEBUG("ignore NETDEV_STREAM_DISCONNECTED event for non-passt network device %s in domain %s",
+                  def->info.alias, vm->def->name);
+        goto endjob;
+    }
+
+    priv = vm->privateData;
+    qemuCaps = priv->qemuCaps;
+
+    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_STREAM_RECONNECT)) {
+        VIR_WARN("ignore NETDEV_STREAM_DISCONNECTED event for passt network device %s in domain %s - QEMU binary does not support reconnect",
+                  def->info.alias, vm->def->name);
+        goto endjob;
+    }
+
+    /* handle the event - restart the passt process with its original
+     * parameters
+     */
+    VIR_DEBUG("process NETDEV_STREAM_DISCONNECTED event for network device %s in domain %s",
+              def->info.alias, vm->def->name);
+
+    if (qemuPasstStart(vm, def) < 0)
+        goto endjob;
+
+ endjob:
+    virDomainObjEndJob(vm);
+}
+
+
 static void
 processNicRxFilterChangedEvent(virDomainObj *vm,
                                const char *devAlias)
@@ -3971,6 +4050,9 @@ static void qemuProcessEventHandler(void *data, void *opaque)
     case QEMU_PROCESS_EVENT_DEVICE_DELETED:
         processDeviceDeletedEvent(driver, vm, processEvent->data);
         break;
+    case QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED:
+        processNetdevStreamDisconnectedEvent(vm, processEvent->data);
+        break;
     case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED:
         processNicRxFilterChangedEvent(vm, processEvent->data);
         break;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 38f89167e0..1fa35f03cc 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1265,6 +1265,17 @@ qemuMonitorEmitNicRxFilterChanged(qemuMonitor *mon,
 }
 
 
+void
+qemuMonitorEmitNetdevStreamDisconnected(qemuMonitor *mon,
+                                        const char *devAlias)
+{
+    VIR_DEBUG("mon=%p", mon);
+
+    QEMU_MONITOR_CALLBACK(mon, domainNetdevStreamDisconnected,
+                          mon->vm, devAlias);
+}
+
+
 void
 qemuMonitorEmitSerialChange(qemuMonitor *mon,
                             const char *devAlias,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2d16214ba2..2fa06b99a3 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -250,6 +250,9 @@ typedef void (*qemuMonitorDomainDeviceUnplugErrCallback)(qemuMonitor *mon,
                                                          virDomainObj *vm,
                                                          const char *devPath,
                                                          const char *devAlias);
+typedef void (*qemuMonitorDomainNetdevStreamDisconnectedCallback)(qemuMonitor *mon,
+                                                                  virDomainObj *vm,
+                                                                  const char *devAlias);
 typedef void (*qemuMonitorDomainNicRxFilterChangedCallback)(qemuMonitor *mon,
                                                             virDomainObj *vm,
                                                             const char *devAlias);
@@ -397,6 +400,7 @@ struct _qemuMonitorCallbacks {
     qemuMonitorDomainMemoryFailureCallback domainMemoryFailure;
     qemuMonitorDomainMemoryDeviceSizeChange domainMemoryDeviceSizeChange;
     qemuMonitorDomainDeviceUnplugErrCallback domainDeviceUnplugError;
+    qemuMonitorDomainNetdevStreamDisconnectedCallback domainNetdevStreamDisconnected;
 };
 
 qemuMonitor *qemuMonitorOpen(virDomainObj *vm,
@@ -480,6 +484,8 @@ void qemuMonitorEmitDeviceDeleted(qemuMonitor *mon,
 void qemuMonitorEmitDeviceUnplugErr(qemuMonitor *mon,
                                     const char *devPath,
                                     const char *devAlias);
+void qemuMonitorEmitNetdevStreamDisconnected(qemuMonitor *mon,
+                                             const char *devAlias);
 void qemuMonitorEmitNicRxFilterChanged(qemuMonitor *mon,
                                        const char *devAlias);
 void qemuMonitorEmitSerialChange(qemuMonitor *mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index ba6276ec8e..e81b464eea 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -84,6 +84,7 @@ static void qemuMonitorJSONHandleRdmaGidStatusChanged(qemuMonitor *mon, virJSONV
 static void qemuMonitorJSONHandleMemoryFailure(qemuMonitor *mon, virJSONValue *data);
 static void qemuMonitorJSONHandleMemoryDeviceSizeChange(qemuMonitor *mon, virJSONValue *data);
 static void qemuMonitorJSONHandleDeviceUnplugErr(qemuMonitor *mon, virJSONValue *data);
+static void qemuMonitorJSONHandleNetdevStreamDisconnected(qemuMonitor *mon, virJSONValue *data);
 
 typedef struct {
     const char *type;
@@ -106,6 +107,7 @@ static qemuEventHandler eventHandlers[] = {
     { "MEMORY_FAILURE", qemuMonitorJSONHandleMemoryFailure, },
     { "MIGRATION", qemuMonitorJSONHandleMigrationStatus, },
     { "MIGRATION_PASS", qemuMonitorJSONHandleMigrationPass, },
+    { "NETDEV_STREAM_DISCONNECTED", qemuMonitorJSONHandleNetdevStreamDisconnected, },
     { "NIC_RX_FILTER_CHANGED", qemuMonitorJSONHandleNicRxFilterChanged, },
     { "PR_MANAGER_STATUS_CHANGED", qemuMonitorJSONHandlePRManagerStatusChanged, },
     { "RDMA_GID_STATUS_CHANGED", qemuMonitorJSONHandleRdmaGidStatusChanged, },
@@ -1021,6 +1023,20 @@ qemuMonitorJSONHandleDeviceUnplugErr(qemuMonitor *mon, virJSONValue *data)
 }
 
 
+static void
+qemuMonitorJSONHandleNetdevStreamDisconnected(qemuMonitor *mon, virJSONValue *data)
+{
+    const char *name;
+
+    if (!(name = virJSONValueObjectGetString(data, "netdev-id"))) {
+        VIR_WARN("missing device in NETDEV_STREAM_DISCONNECTED event");
+        return;
+    }
+
+    qemuMonitorEmitNetdevStreamDisconnected(mon, name);
+}
+
+
 static void
 qemuMonitorJSONHandleNicRxFilterChanged(qemuMonitor *mon, virJSONValue *data)
 {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d4499c6f84..63d7e1138d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1361,6 +1361,23 @@ qemuProcessHandleBlockThreshold(qemuMonitor *mon G_GNUC_UNUSED,
 }
 
 
+static void
+qemuProcessHandleNetdevStreamDisconnected(qemuMonitor *mon G_GNUC_UNUSED,
+                                          virDomainObj *vm,
+                                          const char *devAlias)
+{
+    virObjectLock(vm);
+
+    VIR_DEBUG("Device %s Netdev Stream Disconnected in domain %p %s",
+              devAlias, vm, vm->def->name);
+
+    qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED,
+                           0, 0, g_strdup(devAlias));
+
+    virObjectUnlock(vm);
+}
+
+
 static void
 qemuProcessHandleNicRxFilterChanged(qemuMonitor *mon G_GNUC_UNUSED,
                                     virDomainObj *vm,
@@ -1802,6 +1819,7 @@ static qemuMonitorCallbacks monitorCallbacks = {
     .domainMemoryFailure = qemuProcessHandleMemoryFailure,
     .domainMemoryDeviceSizeChange = qemuProcessHandleMemoryDeviceSizeChange,
     .domainDeviceUnplugError = qemuProcessHandleDeviceUnplugErr,
+    .domainNetdevStreamDisconnected = qemuProcessHandleNetdevStreamDisconnected,
 };
 
 static void
-- 
@@ -1361,6 +1361,23 @@ qemuProcessHandleBlockThreshold(qemuMonitor *mon G_GNUC_UNUSED,
 }
 
 
+static void
+qemuProcessHandleNetdevStreamDisconnected(qemuMonitor *mon G_GNUC_UNUSED,
+                                          virDomainObj *vm,
+                                          const char *devAlias)
+{
+    virObjectLock(vm);
+
+    VIR_DEBUG("Device %s Netdev Stream Disconnected in domain %p %s",
+              devAlias, vm, vm->def->name);
+
+    qemuProcessEventSubmit(vm, QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED,
+                           0, 0, g_strdup(devAlias));
+
+    virObjectUnlock(vm);
+}
+
+
 static void
 qemuProcessHandleNicRxFilterChanged(qemuMonitor *mon G_GNUC_UNUSED,
                                     virDomainObj *vm,
@@ -1802,6 +1819,7 @@ static qemuMonitorCallbacks monitorCallbacks = {
     .domainMemoryFailure = qemuProcessHandleMemoryFailure,
     .domainMemoryDeviceSizeChange = qemuProcessHandleMemoryDeviceSizeChange,
     .domainDeviceUnplugError = qemuProcessHandleDeviceUnplugErr,
+    .domainNetdevStreamDisconnected = qemuProcessHandleNetdevStreamDisconnected,
 };
 
 static void
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [libvirt PATCH 0/3] Support for restarting passt backend
  2023-02-22  0:35 [libvirt PATCH 0/3] Support for restarting passt backend Laine Stump
                   ` (2 preceding siblings ...)
  2023-02-22  0:35 ` [libvirt PATCH 3/3] qemu: respond to NETDEV_STREAM_DISCONNECTED event Laine Stump
@ 2023-02-22 10:21 ` Michal Prívozník
  3 siblings, 0 replies; 7+ messages in thread
From: Michal Prívozník @ 2023-02-22 10:21 UTC (permalink / raw)
  To: Laine Stump, libvir-list; +Cc: passt-dev

On 2/22/23 01:35, Laine Stump wrote:
> If the passt process that implements the backend of a QEMU emulated
> network device terminates, QEMU itself is incapable of restarting a
> new process so that the networking can begin... working again.
> 
> However, what QEMU can and does do is:
> 
> 1) send the NETDEV_STREAM_DISCONNECTED event to libvirt when it sees
>    that the socket has been closed (this event has been in QEMU for as
>    long as support for "-netdev stream", which is used for connecting
>    to passt)
> 
> 2) as of QEMU 8.0.0, the qemu commandline for -netdev stream accepts
>    the "reconnect" option, which tells QEMU to attempt reconnecting to
>    the same socket it previously used, repeating the attempt every "n"
>    seconds (the only argument to reconnect) until it is successful.
> 
> If libvirt adds the reconnect option to the qemu commandline, and then
> responds to a NETDEV_STREAM_DISCONNECTED event by re-running the same
> passt command that it ran when the device was originally connected,
> then a guest will be able to recover in the (very unlikely, according
> to Stefano!) event that the original passt process unexpectedly exits,
> or is killed by some external entity.
> 
> Patch 2/3 handles (2) above, while patch 3/3 handles (1). (patch 1/3
> is a short guest appearance by pkrempa. Thanks pkrempa!).
> 
> This resolves https://bugzilla.redhat.com/2172098
> 
> Along with Stefano's series fixing up selinux issues related to
> running the passt process, they make the passt backend very usable.
> 
> Laine Stump (2):
>   qemu: add reconnect=5 to passt qemu commandline options when available
>   qemu: respond to NETDEV_STREAM_DISCONNECTED event
> 
> Peter Krempa (1):
>   qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT
> 
>  src/qemu/qemu_capabilities.c                  |  4 +
>  src/qemu/qemu_capabilities.h                  |  3 +
>  src/qemu/qemu_domain.c                        |  1 +
>  src/qemu/qemu_domain.h                        |  1 +
>  src/qemu/qemu_driver.c                        | 82 +++++++++++++++++++
>  src/qemu/qemu_monitor.c                       | 11 +++
>  src/qemu/qemu_monitor.h                       |  6 ++
>  src/qemu/qemu_monitor_json.c                  | 16 ++++
>  src/qemu/qemu_passt.c                         | 11 +++
>  src/qemu/qemu_process.c                       | 18 ++++
>  .../caps_8.0.0.x86_64.xml                     |  1 +
>  .../net-user-passt.x86_64-7.2.0.args          | 37 +++++++++
>  .../net-user-passt.x86_64-latest.args         |  2 +-
>  tests/qemuxml2argvtest.c                      |  1 +
>  14 files changed, 193 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [libvirt PATCH 3/3] qemu: respond to NETDEV_STREAM_DISCONNECTED event
  2023-02-22  0:35 ` [libvirt PATCH 3/3] qemu: respond to NETDEV_STREAM_DISCONNECTED event Laine Stump
@ 2023-02-22 10:21   ` Michal Prívozník
  2023-02-22 13:30     ` Laine Stump
  0 siblings, 1 reply; 7+ messages in thread
From: Michal Prívozník @ 2023-02-22 10:21 UTC (permalink / raw)
  To: Laine Stump, libvir-list; +Cc: passt-dev

On 2/22/23 01:35, Laine Stump wrote:
> When a QEMU netdev is of type "stream", if the socket it uses for
> connectivity to the host network gets closed, then QEMU will send a
> NETDEV_STREAM_DISCONNECTED event. We know that any stream netdev we've
> created is backed by a passt process, and if the socket was closed,
> that means the passt process has disappeared.
> 
> When we receive this event, we can respond by starting a new passt
> process with the same options (including socket path) we originally
> used. If we have previously created the stream netdev device with a
> "reconnect" option, then QEMU will automatically reconnect to this new
> passt process. (If we hadn't used "reconnect", then QEMU will never
> try to reconnect to the new passt process, so there's no point in
> starting it.)
> 
> Note that NETDEV_STREAM_DISCONNECTED is an event sent for the netdev
> (ie "host side") of the network device, and so it sends the
> "netdev-id" to specify which device was disconnected. But libvirt's
> virDomainNetDef (the object used to keep track of network devices) is
> the internal representation of both the host-side "netdev", and the
> guest side device, and virDomainNetDef doesn't directly keep track of
> the netdev-id, only of the device's "alias" (which is the "id"
> parameter of the *guest* side of the device). Fortunately, by convention
> libvirt always names the host-side of devices as "host" + alias, so in
> order to search for the affected NetDef, all we need to do is trim the
> 1st 4 characters from the netdev-id and look for the NetDef having
> that resulting trimmed string as its alias. (Contrast this to
> NIC_RX_FILTER_CHANGED, which is an event received for the guest side
> of the device, and so directly contains the device alias.)
> 
> Resolves: https://bugzilla.redhat.com/2172098
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>  src/qemu/qemu_domain.c       |  1 +
>  src/qemu/qemu_domain.h       |  1 +
>  src/qemu/qemu_driver.c       | 82 ++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor.c      | 11 +++++
>  src/qemu/qemu_monitor.h      |  6 +++
>  src/qemu/qemu_monitor_json.c | 16 +++++++
>  src/qemu/qemu_process.c      | 18 ++++++++
>  7 files changed, 135 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e9bc0f375d..4cf9a259ea 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -11238,6 +11238,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
>          break;
>      case QEMU_PROCESS_EVENT_WATCHDOG:
>      case QEMU_PROCESS_EVENT_DEVICE_DELETED:
> +    case QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED:
>      case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED:
>      case QEMU_PROCESS_EVENT_SERIAL_CHANGED:
>      case QEMU_PROCESS_EVENT_MONITOR_EOF:
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 1053d1d4cb..6adc067681 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -447,6 +447,7 @@ typedef enum {
>      QEMU_PROCESS_EVENT_WATCHDOG = 0,
>      QEMU_PROCESS_EVENT_GUESTPANIC,
>      QEMU_PROCESS_EVENT_DEVICE_DELETED,
> +    QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED,
>      QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED,
>      QEMU_PROCESS_EVENT_SERIAL_CHANGED,
>      QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE,
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6154fe9bfe..47d6a0dd95 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -40,6 +40,7 @@
>  #include "qemu_hostdev.h"
>  #include "qemu_hotplug.h"
>  #include "qemu_monitor.h"
> +#include "qemu_passt.h"
>  #include "qemu_process.h"
>  #include "qemu_migration.h"
>  #include "qemu_migration_params.h"
> @@ -3622,6 +3623,84 @@ processDeviceDeletedEvent(virQEMUDriver *driver,
>  }
>  
>  
> +static void
> +processNetdevStreamDisconnectedEvent(virDomainObj *vm,
> +                                     const char *netdevId)
> +{
> +    virDomainDeviceDef dev;
> +    virDomainNetDef *def;
> +    qemuDomainObjPrivate *priv;
> +    virQEMUCaps *qemuCaps;
> +    const char *devAlias = NULL;
> +
> +    /* The event sends us the "netdev-id", but we don't store the
> +     * netdev-id in the NetDef and thus can't use it to find the
> +     * correct NetDef. We *do* keep the device alias in the NetDef.
> +     * By definition, the netdev-id is "host" + devAlias, so we just
> +     * need to remove "host" from the front of netdev-id to get
> +     * something we can use to find the proper NetDef.
> +     */
> +    if (STREQLEN(netdevId, "host", 4))
> +        devAlias = &netdevId[4];

This is open coding STRSKIP():

  devAlias = STRSKIP(netdevId, "host");

> +
> +    if (!devAlias) {
> +        VIR_WARN("Received NETDEV_STREAM_DISCONNECTED event for unrecognized netdev %s from domain %p %s",
> +                  netdevId, vm, vm->def->name);
> +        return;
> +    }
> +
> +    VIR_DEBUG("Received NETDEV_STREAM_DISCONNECTED event for device %s from domain %p %s",
> +              devAlias, vm, vm->def->name);
> +
> +    if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0)
> +        return;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        VIR_DEBUG("Domain is not running");
> +        goto endjob;
> +    }
> +
> +    if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0) {
> +        VIR_WARN("NETDEV_STREAM_DISCONNECTED event received for non-existent device %s in domain %s",
> +                 devAlias, vm->def->name);
> +        goto endjob;
> +    }
> +    if (dev.type != VIR_DOMAIN_DEVICE_NET) {
> +        VIR_WARN("NETDEV_STREAM_DISCONNECTED event received for non-network device %s in domain %s",
> +                 devAlias, vm->def->name);
> +        goto endjob;
> +    }
> +    def = dev.data.net;
> +
> +    if (def->backend.type != VIR_DOMAIN_NET_BACKEND_PASST) {
> +        VIR_DEBUG("ignore NETDEV_STREAM_DISCONNECTED event for non-passt network device %s in domain %s",
> +                  def->info.alias, vm->def->name);
> +        goto endjob;
> +    }
> +
> +    priv = vm->privateData;
> +    qemuCaps = priv->qemuCaps;
> +

These two can be done in variable declaration. Alternatively, if you
want to lose @priv variable completely you can go with:

  virQEMUCaps *caps = QEMU_DOMAIN_PRIVATE(vm)->qemuCaps;

> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_STREAM_RECONNECT)) {
> +        VIR_WARN("ignore NETDEV_STREAM_DISCONNECTED event for passt network device %s in domain %s - QEMU binary does not support reconnect",
> +                  def->info.alias, vm->def->name);
> +        goto endjob;
> +    }
> +
> +    /* handle the event - restart the passt process with its original
> +     * parameters
> +     */
> +    VIR_DEBUG("process NETDEV_STREAM_DISCONNECTED event for network device %s in domain %s",
> +              def->info.alias, vm->def->name);
> +
> +    if (qemuPasstStart(vm, def) < 0)
> +        goto endjob;
> +
> + endjob:
> +    virDomainObjEndJob(vm);
> +}
> +

Michal


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [libvirt PATCH 3/3] qemu: respond to NETDEV_STREAM_DISCONNECTED event
  2023-02-22 10:21   ` Michal Prívozník
@ 2023-02-22 13:30     ` Laine Stump
  0 siblings, 0 replies; 7+ messages in thread
From: Laine Stump @ 2023-02-22 13:30 UTC (permalink / raw)
  To: libvir-list; +Cc: passt-dev, Michal Prívozník

On 2/22/23 5:21 AM, Michal Prívozník wrote:
> On 2/22/23 01:35, Laine Stump wrote:
>> When a QEMU netdev is of type "stream", if the socket it uses for
>> connectivity to the host network gets closed, then QEMU will send a
>> NETDEV_STREAM_DISCONNECTED event. We know that any stream netdev we've
>> created is backed by a passt process, and if the socket was closed,
>> that means the passt process has disappeared.
>>
>> When we receive this event, we can respond by starting a new passt
>> process with the same options (including socket path) we originally
>> used. If we have previously created the stream netdev device with a
>> "reconnect" option, then QEMU will automatically reconnect to this new
>> passt process. (If we hadn't used "reconnect", then QEMU will never
>> try to reconnect to the new passt process, so there's no point in
>> starting it.)
>>
>> Note that NETDEV_STREAM_DISCONNECTED is an event sent for the netdev
>> (ie "host side") of the network device, and so it sends the
>> "netdev-id" to specify which device was disconnected. But libvirt's
>> virDomainNetDef (the object used to keep track of network devices) is
>> the internal representation of both the host-side "netdev", and the
>> guest side device, and virDomainNetDef doesn't directly keep track of
>> the netdev-id, only of the device's "alias" (which is the "id"
>> parameter of the *guest* side of the device). Fortunately, by convention
>> libvirt always names the host-side of devices as "host" + alias, so in
>> order to search for the affected NetDef, all we need to do is trim the
>> 1st 4 characters from the netdev-id and look for the NetDef having
>> that resulting trimmed string as its alias. (Contrast this to
>> NIC_RX_FILTER_CHANGED, which is an event received for the guest side
>> of the device, and so directly contains the device alias.)
>>
>> Resolves: https://bugzilla.redhat.com/2172098
>> Signed-off-by: Laine Stump <laine@redhat.com>
>> ---
>>   src/qemu/qemu_domain.c       |  1 +
>>   src/qemu/qemu_domain.h       |  1 +
>>   src/qemu/qemu_driver.c       | 82 ++++++++++++++++++++++++++++++++++++
>>   src/qemu/qemu_monitor.c      | 11 +++++
>>   src/qemu/qemu_monitor.h      |  6 +++
>>   src/qemu/qemu_monitor_json.c | 16 +++++++
>>   src/qemu/qemu_process.c      | 18 ++++++++
>>   7 files changed, 135 insertions(+)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index e9bc0f375d..4cf9a259ea 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -11238,6 +11238,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
>>           break;
>>       case QEMU_PROCESS_EVENT_WATCHDOG:
>>       case QEMU_PROCESS_EVENT_DEVICE_DELETED:
>> +    case QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED:
>>       case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED:
>>       case QEMU_PROCESS_EVENT_SERIAL_CHANGED:
>>       case QEMU_PROCESS_EVENT_MONITOR_EOF:
>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>> index 1053d1d4cb..6adc067681 100644
>> --- a/src/qemu/qemu_domain.h
>> +++ b/src/qemu/qemu_domain.h
>> @@ -447,6 +447,7 @@ typedef enum {
>>       QEMU_PROCESS_EVENT_WATCHDOG = 0,
>>       QEMU_PROCESS_EVENT_GUESTPANIC,
>>       QEMU_PROCESS_EVENT_DEVICE_DELETED,
>> +    QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED,
>>       QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED,
>>       QEMU_PROCESS_EVENT_SERIAL_CHANGED,
>>       QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE,
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 6154fe9bfe..47d6a0dd95 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -40,6 +40,7 @@
>>   #include "qemu_hostdev.h"
>>   #include "qemu_hotplug.h"
>>   #include "qemu_monitor.h"
>> +#include "qemu_passt.h"
>>   #include "qemu_process.h"
>>   #include "qemu_migration.h"
>>   #include "qemu_migration_params.h"
>> @@ -3622,6 +3623,84 @@ processDeviceDeletedEvent(virQEMUDriver *driver,
>>   }
>>   
>>   
>> +static void
>> +processNetdevStreamDisconnectedEvent(virDomainObj *vm,
>> +                                     const char *netdevId)
>> +{
>> +    virDomainDeviceDef dev;
>> +    virDomainNetDef *def;
>> +    qemuDomainObjPrivate *priv;
>> +    virQEMUCaps *qemuCaps;
>> +    const char *devAlias = NULL;
>> +
>> +    /* The event sends us the "netdev-id", but we don't store the
>> +     * netdev-id in the NetDef and thus can't use it to find the
>> +     * correct NetDef. We *do* keep the device alias in the NetDef.
>> +     * By definition, the netdev-id is "host" + devAlias, so we just
>> +     * need to remove "host" from the front of netdev-id to get
>> +     * something we can use to find the proper NetDef.
>> +     */
>> +    if (STREQLEN(netdevId, "host", 4))
>> +        devAlias = &netdevId[4];
> 
> This is open coding STRSKIP():
> 
>    devAlias = STRSKIP(netdevId, "host");

Ah yes, right you are! Changed.

> 
>> +
>> +    if (!devAlias) {
>> +        VIR_WARN("Received NETDEV_STREAM_DISCONNECTED event for unrecognized netdev %s from domain %p %s",
>> +                  netdevId, vm, vm->def->name);
>> +        return;
>> +    }
>> +
>> +    VIR_DEBUG("Received NETDEV_STREAM_DISCONNECTED event for device %s from domain %p %s",
>> +              devAlias, vm, vm->def->name);
>> +
>> +    if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0)
>> +        return;
>> +
>> +    if (!virDomainObjIsActive(vm)) {
>> +        VIR_DEBUG("Domain is not running");
>> +        goto endjob;
>> +    }
>> +
>> +    if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0) {
>> +        VIR_WARN("NETDEV_STREAM_DISCONNECTED event received for non-existent device %s in domain %s",
>> +                 devAlias, vm->def->name);
>> +        goto endjob;
>> +    }
>> +    if (dev.type != VIR_DOMAIN_DEVICE_NET) {
>> +        VIR_WARN("NETDEV_STREAM_DISCONNECTED event received for non-network device %s in domain %s",
>> +                 devAlias, vm->def->name);
>> +        goto endjob;
>> +    }
>> +    def = dev.data.net;
>> +
>> +    if (def->backend.type != VIR_DOMAIN_NET_BACKEND_PASST) {
>> +        VIR_DEBUG("ignore NETDEV_STREAM_DISCONNECTED event for non-passt network device %s in domain %s",
>> +                  def->info.alias, vm->def->name);
>> +        goto endjob;
>> +    }
>> +
>> +    priv = vm->privateData;
>> +    qemuCaps = priv->qemuCaps;
>> +
> 
> These two can be done in variable declaration. Alternatively, if you
> want to lose @priv variable completely you can go with:
> 
>    virQEMUCaps *caps = QEMU_DOMAIN_PRIVATE(vm)->qemuCaps;

I had originally had them initialized, but moved the assignment down 
until it was actually needed because of some warped idea about saving 
cycles, or avoiding a double de-reference in case one of them might be 
NULL or something. But in retrospect, just delaying wouldn't have solved 
that problem anyway. I'm changing it to use QEMU_DOMAIN_PRIVATE before 
pushing.

Thanks for the review!

> 
>> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_STREAM_RECONNECT)) {
>> +        VIR_WARN("ignore NETDEV_STREAM_DISCONNECTED event for passt network device %s in domain %s - QEMU binary does not support reconnect",
>> +                  def->info.alias, vm->def->name);
>> +        goto endjob;
>> +    }
>> +
>> +    /* handle the event - restart the passt process with its original
>> +     * parameters
>> +     */
>> +    VIR_DEBUG("process NETDEV_STREAM_DISCONNECTED event for network device %s in domain %s",
>> +              def->info.alias, vm->def->name);
>> +
>> +    if (qemuPasstStart(vm, def) < 0)
>> +        goto endjob;
>> +
>> + endjob:
>> +    virDomainObjEndJob(vm);
>> +}
>> +
> 
> Michal
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-02-22 13:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-22  0:35 [libvirt PATCH 0/3] Support for restarting passt backend Laine Stump
2023-02-22  0:35 ` [libvirt PATCH 1/3] qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT Laine Stump
2023-02-22  0:35 ` [libvirt PATCH 2/3] qemu: add reconnect=5 to passt qemu commandline options when available Laine Stump
2023-02-22  0:35 ` [libvirt PATCH 3/3] qemu: respond to NETDEV_STREAM_DISCONNECTED event Laine Stump
2023-02-22 10:21   ` Michal Prívozník
2023-02-22 13:30     ` Laine Stump
2023-02-22 10:21 ` [libvirt PATCH 0/3] Support for restarting passt backend Michal Prívozník

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).