Re: [PATCH] qemu: fix multiple missing setup/teardown of passt process for interface type='vhostuser'

[Martin Kletzander via Devel]

On Sun, Aug 24, 2025 at 11:49:10PM -0400, Laine Stump via Devel wrote:

passt networking support was originally added only for <interface
type='user'>, and all of the codepaths leading to qemuPasst*()
functions were protected with

  if (net->type == VIR_DOMAIN_NET_TYPE_USER &&
      net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST)

When support was later added to use a vhost-user socket to connect
between the passt process and qemu process, *some* of the conditionals
similar to the above were changed to be

  if ((net->type == VIR_DOMAIN_NET_TYPE_USER ||
       net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER) &&
      net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST)

As a matter of fact, enough of these places were changed to make
passt+vhostuser work. However I missed a few places that resulted in
the passt process not being properly shutdown/cleaned up when the
interface type was vhostuser, and also as far as I can see from
examining the code, the passt process wasn't being added to the cgroup
for the domain.

We could fix these problems by adding the extra condition to all the
missing places (checking for either 'user' or 'vhostuser' as well as
for backend type of 'passt'), but since validation already guarantees
that if backend type='passt' then the interface type MUST be either
'user' or 'vhostuser', it's really just adding extra code for no good
purpose (and would leave open the possibility of the same problem
recurring in the future if a different interface type begins using
passt as well). So the better solution is to not bother checking
net->type at all in those locations - if backend type is 'passt' then
we call the passt-related code.

Resolves: https://issues.redhat.com/browse/RHEL-80285
Resolves: https://issues.redhat.com/browse/RHEL-92842
Fixes: 1e9054b9c79d721a55f413c2983c5370044f8f60
Signed-off-by: Laine Stump <la...@redhat.com>

Great, so our new list works ;)

With one nitpick in the code below,

Reviewed-by: Martin Kletzander <mklet...@redhat.com>

---
src/qemu/qemu_extdevice.c | 15 +++------------
src/qemu/qemu_hotplug.c   | 23 ++++++++---------------
2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e9568af125..866dbace90 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1356,6 +1349,10 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
        goto cleanup;
    }

+    /* both NET_TYPE_USER and NET_TYPE_VHOSTUSER might use passt */
+    if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST && qemuPasstStart(vm, 
net) < 0)

If it's not that much of a bother, I'd read it way faster if the
condition was split into two lines like this:

    if (net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST &&
        qemuPasstStart(vm, net) < 0)

Thanks

+        goto cleanup;
+
    /* Set device online immediately */
    if (virDomainInterfaceStartDevice(net) < 0)
        goto cleanup;

signature.asc
Description: PGP signature