openvswitch: Check if OVS_VSCTL exists when getting interface name

[PATCH] openvswitch: Check if OVS_VSCTL exists when getting interface name

Posted by Michal Privoznik 3 years, 3 months ago

So far we assumed that any vhostuser interface is plugged into an
OVS bridge and thus 'ovs-vsctl' exists. But this is not always
true. In testing scenarios it is possible to create a vhostuser
interface with this tool dpdk-testpmd (part of dpdk RPM) which
creates/connects to UNIX socket needed for vhostuser. Of course,
since there is no OVS then there is no interface name in which
case virNetDevOpenvswitchGetVhostuserIfname() should return 0.

The rest of APIs that assume OVS are not 'fixed' because we still
want them to fail (e.g. getting statistics, plugging interface
into an OVS bridge, unplugging it from an OVS bridge, ...).

The only API that is fixed is
virNetDevOpenvswitchGetVhostuserIfname() because it is called
explicitly when starting a guest (and callers are okay if no name
was found).

The other way to fix this bug seems to be to simply require
'ovs-vsctl' on spec file level, but that is too heavy gun given
that vhostuser is used by a small set of our users (assumption
made on requirements for vhostuser). Also, this way would drag in
yet another dependency for all users (even those who want minimal
libvirt).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1913156
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virnetdevopenvswitch.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 896c1bd917..f9b3369b2a 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -29,6 +29,7 @@
 #include "virstring.h"
 #include "virlog.h"
 #include "virjson.h"
+#include "virfile.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -529,9 +530,19 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
                                        bool server,
                                        char **ifname)
 {
-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
+    g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *absoluteOvsVsctlPath = NULL;
     int status;
 
+    if (!(absoluteOvsVsctlPath = virFindFileInPath(OVS_VSCTL))) {
+        /* If there is no 'ovs-vsctl' then the interface is
+         * probably not an OpenVSwitch interface and the @path to
+         * socket was created by some DPDK testing script (e.g.
+         * dpdk-testpmd). */
+        return 0;
+    }
+
+    cmd = virNetDevOpenvswitchCreateCmd();
 
     if (server) {
         virCommandAddArgList(cmd, "--no-headings", "--columns=name", "find",
-- 
2.26.2

Re: [PATCH] openvswitch: Check if OVS_VSCTL exists when getting interface name

Posted by Ján Tomko 3 years, 3 months ago

On a Monday in 2021, Michal Privoznik wrote:
>So far we assumed that any vhostuser interface is plugged into an
>OVS bridge and thus 'ovs-vsctl' exists. But this is not always
>true. In testing scenarios it is possible to create a vhostuser
>interface with this tool dpdk-testpmd (part of dpdk RPM) which
>creates/connects to UNIX socket needed for vhostuser. Of course,
>since there is no OVS then there is no interface name in which
>case virNetDevOpenvswitchGetVhostuserIfname() should return 0.
>
>The rest of APIs that assume OVS are not 'fixed' because we still
>want them to fail (e.g. getting statistics, plugging interface
>into an OVS bridge, unplugging it from an OVS bridge, ...).
>
>The only API that is fixed is
>virNetDevOpenvswitchGetVhostuserIfname() because it is called
>explicitly when starting a guest (and callers are okay if no name
>was found).
>
>The other way to fix this bug seems to be to simply require
>'ovs-vsctl' on spec file level, but that is too heavy gun given
>that vhostuser is used by a small set of our users (assumption
>made on requirements for vhostuser). Also, this way would drag in
>yet another dependency for all users (even those who want minimal
>libvirt).
>
>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1913156
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/util/virnetdevopenvswitch.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano