When "original passt" support was added, we decided that we always
wanted to reconnect (i.e. restart the passt process) if it was somehow
terminated. Generic vhost-user only turns on reconnect if specified in
the config, but there is no reason to require this if the other end of
the vhost-user socket is a passt process - we know what has happened
and what we want to do; no reason to make the default configuration
"do the *wrong* thing".
Resolves: https://issues.redhat.com/browse/RHEL-80169
Signed-off-by: Laine Stump <laine@redhat.com>
---
src/qemu/qemu_passt.c | 6 ++++++
.../qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args | 6 +++---
.../schema-reorder-domain-subelements.x86_64-latest.args | 2 +-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index bc495eca1e..018630a5de 100644
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -182,6 +182,12 @@ qemuPasstPrepareVhostUser(virDomainObj *vm,
*/
g_free(net->data.vhostuser->data.nix.path);
net->data.vhostuser->data.nix.path = qemuPasstCreateSocketPath(vm, net);
+
+ /* reconnect is always enabled, with timeout always at 5 seconds, when
+ * using passt
+ */
+ net->data.vhostuser->data.nix.reconnect.enabled = VIR_TRISTATE_BOOL_YES;
+ net->data.vhostuser->data.nix.reconnect.timeout = 5;
}
int
diff --git a/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args b/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args
index 7c030d7067..afbbe188cf 100644
--- a/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args
+++ b/tests/qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args
@@ -28,13 +28,13 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
-boot strict=on \
-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-1-storage","read-only":false}' \
-device '{"driver":"ide-hd","bus":"ide.0","unit":0,"drive":"libvirt-1-storage","id":"ide0-0-0","bootindex":1}' \
--chardev socket,id=charnet0,path=/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net0.socket \
+-chardev socket,id=charnet0,path=/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net0.socket,reconnect-ms=5000 \
-netdev '{"type":"vhost-user","chardev":"charnet0","id":"hostnet0"}' \
-device '{"driver":"virtio-net-pci","netdev":"hostnet0","id":"net0","mac":"00:11:22:33:44:55","bus":"pci.0","addr":"0x2"}' \
--chardev socket,id=charnet1,path=/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net1.socket \
+-chardev socket,id=charnet1,path=/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net1.socket,reconnect-ms=5000 \
-netdev '{"type":"vhost-user","chardev":"charnet1","id":"hostnet1"}' \
-device '{"driver":"virtio-net-pci","netdev":"hostnet1","id":"net1","mac":"00:11:22:33:44:11","bus":"pci.0","addr":"0x3"}' \
--chardev socket,id=charnet2,path=/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net2.socket \
+-chardev socket,id=charnet2,path=/var/run/libvirt/qemu/passt/-1-QEMUGuest1-net2.socket,reconnect-ms=5000 \
-netdev '{"type":"vhost-user","chardev":"charnet2","id":"hostnet2"}' \
-device '{"driver":"virtio-net-pci","netdev":"hostnet2","id":"net2","mac":"00:11:22:33:44:11","bus":"pci.0","addr":"0x4"}' \
-audiodev '{"id":"audio1","driver":"none"}' \
diff --git a/tests/qemuxmlconfdata/schema-reorder-domain-subelements.x86_64-latest.args b/tests/qemuxmlconfdata/schema-reorder-domain-subelements.x86_64-latest.args
index d27dd77d9f..76df9c30b0 100644
--- a/tests/qemuxmlconfdata/schema-reorder-domain-subelements.x86_64-latest.args
+++ b/tests/qemuxmlconfdata/schema-reorder-domain-subelements.x86_64-latest.args
@@ -29,7 +29,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-passtvhostuu/.config \
-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
-blockdev '{"driver":"file","filename":"/home/laine/libvirt/images/fedora-34.img","node-name":"libvirt-1-storage","read-only":false}' \
-device '{"driver":"virtio-blk-pci","bus":"pci.0","multifunction":true,"addr":"0x3","drive":"libvirt-1-storage","id":"virtio-disk0","bootindex":1}' \
--chardev socket,id=charnet0,path=/var/run/libvirt/qemu/passt/-1-passtvhostuu-net0.socket \
+-chardev socket,id=charnet0,path=/var/run/libvirt/qemu/passt/-1-passtvhostuu-net0.socket,reconnect-ms=5000 \
-netdev '{"type":"vhost-user","chardev":"charnet0","id":"hostnet0"}' \
-device '{"driver":"virtio-net-pci","netdev":"hostnet0","id":"net0","mac":"52:54:00:21:de:6c","bus":"pci.0","addr":"0x2"}' \
-chardev pty,id=charserial0 \
--
2.49.0On Thu, Apr 10, 2025 at 02:58:30 -0400, Laine Stump wrote: > When "original passt" support was added, we decided that we always > wanted to reconnect (i.e. restart the passt process) if it was somehow > terminated. Generic vhost-user only turns on reconnect if specified in > the config, but there is no reason to require this if the other end of > the vhost-user socket is a passt process - we know what has happened > and what we want to do; no reason to make the default configuration > "do the *wrong* thing". > > Resolves: https://issues.redhat.com/browse/RHEL-80169 > > Signed-off-by: Laine Stump <laine@redhat.com> > --- > src/qemu/qemu_passt.c | 6 ++++++ > .../qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args | 6 +++--- > .../schema-reorder-domain-subelements.x86_64-latest.args | 2 +- > 3 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c > index bc495eca1e..018630a5de 100644 > --- a/src/qemu/qemu_passt.c > +++ b/src/qemu/qemu_passt.c > @@ -182,6 +182,12 @@ qemuPasstPrepareVhostUser(virDomainObj *vm, > */ > g_free(net->data.vhostuser->data.nix.path); > net->data.vhostuser->data.nix.path = qemuPasstCreateSocketPath(vm, net); > + > + /* reconnect is always enabled, with timeout always at 5 seconds, when > + * using passt > + */ > + net->data.vhostuser->data.nix.reconnect.enabled = VIR_TRISTATE_BOOL_YES; > + net->data.vhostuser->data.nix.reconnect.timeout = 5; Hmm, shouldn't this be reflected in the XML? That is, I would expect a post-parse function to set these defaults. And the current code overrides user specified settings (either disabling or setting a different timeout), which doesn't is not right. Actually, looking at our documentation the <reconnect> it seems we don't even support setting reconnect for vhost-user with passt backend. If that's the case, the code is fine as is except for the following nit, which I'd like to see addressed anyway... And I suggest relacing the 5 seconds timeout with a macro that would also be used in qemuPasstAddNetProps. Jirka
On 4/10/25 2:55 AM, Jiri Denemark wrote: > On Thu, Apr 10, 2025 at 02:58:30 -0400, Laine Stump wrote: >> When "original passt" support was added, we decided that we always >> wanted to reconnect (i.e. restart the passt process) if it was somehow >> terminated. Generic vhost-user only turns on reconnect if specified in >> the config, but there is no reason to require this if the other end of >> the vhost-user socket is a passt process - we know what has happened >> and what we want to do; no reason to make the default configuration >> "do the *wrong* thing". >> >> Resolves: https://issues.redhat.com/browse/RHEL-80169 >> >> Signed-off-by: Laine Stump <laine@redhat.com> >> --- >> src/qemu/qemu_passt.c | 6 ++++++ >> .../qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args | 6 +++--- >> .../schema-reorder-domain-subelements.x86_64-latest.args | 2 +- >> 3 files changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c >> index bc495eca1e..018630a5de 100644 >> --- a/src/qemu/qemu_passt.c >> +++ b/src/qemu/qemu_passt.c >> @@ -182,6 +182,12 @@ qemuPasstPrepareVhostUser(virDomainObj *vm, >> */ >> g_free(net->data.vhostuser->data.nix.path); >> net->data.vhostuser->data.nix.path = qemuPasstCreateSocketPath(vm, net); >> + >> + /* reconnect is always enabled, with timeout always at 5 seconds, when >> + * using passt >> + */ >> + net->data.vhostuser->data.nix.reconnect.enabled = VIR_TRISTATE_BOOL_YES; >> + net->data.vhostuser->data.nix.reconnect.timeout = 5; > > Hmm, shouldn't this be reflected in the XML? Nope. Actually I thought about this when giving the commit messages a final look before I sent last night/morning - just as with the socket path for the associated character device is hardcoded when vhostuser is being used for passt, the reconnect timeout for the character device is hardcoded, so it's not just a "default", it is the only possible value. I'll reflect that in the commit log. The entire net->data.vhostuser part of the object isn't configurable when vhostuser is being used for passt, the corresponding attributes in <source> aren't allowed in the input XML, and won't be output when the object is formatted. > That is, I would expect a > post-parse function to set these defaults. > And the current code > overrides user specified settings (either disabling or setting a > different timeout), which doesn't is not right. Except that the user can't set it :-) > Actually, looking at our > documentation the <reconnect> it seems we don't even support setting > reconnect for vhost-user with passt backend. Correct. This is the same behavior as for non-vhostuser passt, where we decided that there was no downside to having reconnect always enabled and no reasonable way to explain how or why a user should modify the timeout, so made it always on and always 5 second timeout. > If that's the case, the > code is fine as is except for the following nit, which I'd like to see > addressed anyway... > > And I suggest relacing the 5 seconds timeout with a macro that would > also be used in qemuPasstAddNetProps. Sure, I can do that. Thanks for the quick review!
On Thu, Apr 10, 2025 at 11:45:15 -0400, Laine Stump wrote: > On 4/10/25 2:55 AM, Jiri Denemark wrote: > > On Thu, Apr 10, 2025 at 02:58:30 -0400, Laine Stump wrote: > >> When "original passt" support was added, we decided that we always > >> wanted to reconnect (i.e. restart the passt process) if it was somehow > >> terminated. Generic vhost-user only turns on reconnect if specified in > >> the config, but there is no reason to require this if the other end of > >> the vhost-user socket is a passt process - we know what has happened > >> and what we want to do; no reason to make the default configuration > >> "do the *wrong* thing". > >> > >> Resolves: https://issues.redhat.com/browse/RHEL-80169 > >> > >> Signed-off-by: Laine Stump <laine@redhat.com> > >> --- > >> src/qemu/qemu_passt.c | 6 ++++++ > >> .../qemuxmlconfdata/net-vhostuser-passt.x86_64-latest.args | 6 +++--- > >> .../schema-reorder-domain-subelements.x86_64-latest.args | 2 +- > >> 3 files changed, 10 insertions(+), 4 deletions(-) > >> > >> diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c > >> index bc495eca1e..018630a5de 100644 > >> --- a/src/qemu/qemu_passt.c > >> +++ b/src/qemu/qemu_passt.c > >> @@ -182,6 +182,12 @@ qemuPasstPrepareVhostUser(virDomainObj *vm, > >> */ > >> g_free(net->data.vhostuser->data.nix.path); > >> net->data.vhostuser->data.nix.path = qemuPasstCreateSocketPath(vm, net); > >> + > >> + /* reconnect is always enabled, with timeout always at 5 seconds, when > >> + * using passt > >> + */ > >> + net->data.vhostuser->data.nix.reconnect.enabled = VIR_TRISTATE_BOOL_YES; > >> + net->data.vhostuser->data.nix.reconnect.timeout = 5; ... > > Actually, looking at our > > documentation the <reconnect> it seems we don't even support setting > > reconnect for vhost-user with passt backend. > > Correct. This is the same behavior as for non-vhostuser passt, where we > decided that there was no downside to having reconnect always enabled > and no reasonable way to explain how or why a user should modify the > timeout, so made it always on and always 5 second timeout. Great. > > > If that's the case, the > > code is fine as is except for the following nit, which I'd like to see > > addressed anyway... > > > > And I suggest relacing the 5 seconds timeout with a macro that would > > also be used in qemuPasstAddNetProps. > > Sure, I can do that. > > Thanks for the quick review! With this small change Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
© 2016 - 2025 Red Hat, Inc.