[PATCH 1/2] hypervisor: Move domain interface mgmt methods

Praveen K Paladugu posted 2 patches 3 months, 3 weeks ago
[PATCH 1/2] hypervisor: Move domain interface mgmt methods
Posted by Praveen K Paladugu 3 months, 3 weeks ago
From: Praveen K Paladugu <praveenkpaladugu@gmail.com>

From: Praveen K Paladugu <prapal@linux.microsoft.com>

Move methods to connect domain interfaces to host bridges to hypervisor.
This is to allow reuse between qemu and ch drivers.

Signed-off-by: Praveen K Paladugu <praveenkpaladugu@gmail.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
---
 src/hypervisor/domain_interface.c | 228 ++++++++++++++++++++++++++++++
 src/hypervisor/domain_interface.h |  10 ++
 src/libvirt_private.syms          |   1 +
 src/qemu/qemu_command.c           |   9 +-
 src/qemu/qemu_interface.c         | 219 +---------------------------
 5 files changed, 247 insertions(+), 220 deletions(-)

diff --git a/src/hypervisor/domain_interface.c b/src/hypervisor/domain_interface.c
index 756abb08e9..0c19ff859a 100644
--- a/src/hypervisor/domain_interface.c
+++ b/src/hypervisor/domain_interface.c
@@ -39,6 +39,7 @@
 #include "virnetdevmidonet.h"
 #include "virnetdevopenvswitch.h"
 #include "virnetdevtap.h"
+#include "vircommand.h"
 
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -514,3 +515,230 @@ virDomainClearNetBandwidth(virDomainDef *def)
         virDomainInterfaceClearQoS(def, def->nets[i]);
     }
 }
+
+/**
+ * virDomainCreateInBridgePortWithHelper:
+ * @bridgeHelperName: name of the bridge helper program
+ * @brname: the bridge name
+ * @ifname: the returned interface name
+ * @tapfd: file descriptor return value for the new tap device
+ * @flags: OR of virNetDevTapCreateFlags:
+
+ *   VIR_NETDEV_TAP_CREATE_VNET_HDR
+ *     - Enable IFF_VNET_HDR on the tap device
+ *
+ * This function creates a new tap device on a bridge using an external
+ * helper.  The final name for the bridge will be stored in @ifname.
+ *
+ * Returns 0 in case of success or -1 on failure
+ */
+static int
+virDomainCreateInBridgePortWithHelper(const char *bridgeHelperName,
+                                      const char *brname,
+                                      char **ifname,
+                                      int *tapfd,
+                                      unsigned int flags)
+{
+    const char *const bridgeHelperDirs[] = {
+        "/usr/libexec",
+        "/usr/lib/qemu",
+        "/usr/lib",
+        NULL,
+    };
+    g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *bridgeHelperPath = NULL;
+    char *errbuf = NULL, *cmdstr = NULL;
+    int pair[2] = { -1, -1 };
+
+    if (!bridgeHelperName) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge helper name"));
+        return -1;
+    }
+
+    if ((flags & ~VIR_NETDEV_TAP_CREATE_VNET_HDR) != VIR_NETDEV_TAP_CREATE_IFUP)
+        return -1;
+
+    if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) {
+        virReportSystemError(errno, "%s", _("failed to create socket"));
+        return -1;
+    }
+
+    bridgeHelperPath = virFindFileInPathFull(bridgeHelperName, bridgeHelperDirs);
+
+    if (!bridgeHelperPath) {
+        virReportSystemError(errno, _("'%1$s' is not a suitable bridge helper"),
+                                      bridgeHelperName);
+        return -1;
+    }
+
+    VIR_DEBUG("Using qemu-bridge-helper: %s", bridgeHelperPath);
+    cmd = virCommandNew(bridgeHelperPath);
+    if (flags & VIR_NETDEV_TAP_CREATE_VNET_HDR)
+        virCommandAddArgFormat(cmd, "--use-vnet");
+    virCommandAddArgFormat(cmd, "--br=%s", brname);
+    virCommandAddArgFormat(cmd, "--fd=%d", pair[1]);
+    virCommandSetErrorBuffer(cmd, &errbuf);
+    virCommandDoAsyncIO(cmd);
+    virCommandPassFD(cmd, pair[1],
+                     VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+    virCommandClearCaps(cmd);
+#ifdef CAP_NET_ADMIN
+    virCommandAllowCap(cmd, CAP_NET_ADMIN);
+#endif
+    if (virCommandRunAsync(cmd, NULL) < 0) {
+        *tapfd = -1;
+        goto cleanup;
+    }
+
+    do {
+        *tapfd = virSocketRecvFD(pair[0], 0);
+    } while (*tapfd < 0 && errno == EINTR);
+
+    if (*tapfd < 0) {
+        char *errstr = NULL;
+
+        if (!(cmdstr = virCommandToString(cmd, false)))
+            goto cleanup;
+        virCommandAbort(cmd);
+
+        if (errbuf && *errbuf)
+            errstr = g_strdup_printf("stderr=%s", errbuf);
+
+        virReportSystemError(errno,
+                             _("%1$s: failed to communicate with bridge helper: %2$s"),
+                             cmdstr,
+                             NULLSTR_EMPTY(errstr));
+        VIR_FREE(errstr);
+        goto cleanup;
+    }
+
+    if (virNetDevTapGetName(*tapfd, ifname) < 0 ||
+        virCommandWait(cmd, NULL) < 0) {
+        VIR_FORCE_CLOSE(*tapfd);
+        *tapfd = -1;
+    }
+
+ cleanup:
+    VIR_FREE(cmdstr);
+    VIR_FREE(errbuf);
+    VIR_FORCE_CLOSE(pair[0]);
+    return *tapfd < 0 ? -1 : 0;
+}
+
+
+/* virDomainInterfaceBridgeConnect:
+ * @def: the definition of the VM
+ * @net: pointer to the VM's interface description
+ * @tapfd: array of file descriptor return value for the new device
+ * @tapfdsize: number of file descriptors in @tapfd
+ * @privileged: whether running as privileged user
+ * @ebtables: ebtales context
+ * @macFilter: whether driver support mac Filtering
+ * @bridgeHelperName:name of the bridge helper program to run in non-privileged mode
+ *
+ * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_NETWORK or
+ * VIR_DOMAIN_NET_TYPE_BRIDGE (i.e. if the connection is made with a tap
+ * device connecting to a bridge device)
+ */
+int
+virDomainInterfaceBridgeConnect(virDomainDef *def,
+                                virDomainNetDef *net,
+                                int *tapfd,
+                                size_t tapfdSize,
+                                bool privileged,
+                                ebtablesContext *ebtables,
+                                bool macFilter,
+                                const char *bridgeHelperName)
+{
+    const char *brname;
+    int ret = -1;
+    unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
+    bool template_ifname = false;
+    const char *tunpath = "/dev/net/tun";
+
+    if (net->backend.tap) {
+        tunpath = net->backend.tap;
+        if (!privileged) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("cannot use custom tap device in session mode"));
+            goto cleanup;
+        }
+    }
+
+    if (!(brname = virDomainNetGetActualBridgeName(net))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge name"));
+        goto cleanup;
+    }
+
+    if (!net->ifname)
+        template_ifname = true;
+
+    if (virDomainInterfaceIsVnetCompatModel(net))
+        tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
+
+    if (privileged) {
+        if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
+                                           def->uuid, tunpath, tapfd, tapfdSize,
+                                           virDomainNetGetActualVirtPortProfile(net),
+                                           virDomainNetGetActualVlan(net),
+                                           virDomainNetGetActualPortOptionsIsolated(net),
+                                           net->coalesce, 0, NULL,
+                                           tap_create_flags) < 0) {
+            virDomainAuditNetDevice(def, net, tunpath, false);
+            goto cleanup;
+        }
+        if (virDomainNetGetActualBridgeMACTableManager(net)
+            == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
+            /* libvirt is managing the FDB of the bridge this device
+             * is attaching to, so we need to turn off learning and
+             * unicast_flood on the device to prevent the kernel from
+             * adding any FDB entries for it. We will add an fdb
+             * entry ourselves (during virDomainInterfaceStartDevices(),
+             * using the MAC address from the interface config.
+             */
+            if (virNetDevBridgePortSetLearning(brname, net->ifname, false) < 0)
+                goto cleanup;
+            if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, false) < 0)
+                goto cleanup;
+        }
+    } else {
+        if (virDomainCreateInBridgePortWithHelper(bridgeHelperName, brname,
+                                                  &net->ifname,
+                                                  tapfd,
+                                                  tap_create_flags) < 0) {
+            virDomainAuditNetDevice(def, net, tunpath, false);
+            goto cleanup;
+        }
+        /* virDomainCreateInBridgePortWithHelper can only create a single FD */
+        if (tapfdSize > 1) {
+            VIR_WARN("Ignoring multiqueue network request");
+            tapfdSize = 1;
+        }
+    }
+
+    virDomainAuditNetDevice(def, net, tunpath, true);
+
+    if (macFilter &&
+        ebtablesAddForwardAllowIn(ebtables,
+                                  net->ifname,
+                                  &net->mac) < 0)
+        goto cleanup;
+
+    if (net->filter &&
+        virDomainConfNWFilterInstantiate(def->name, def->uuid, net, false) < 0) {
+        goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    if (ret < 0) {
+        size_t i;
+        for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++)
+            VIR_FORCE_CLOSE(tapfd[i]);
+        if (template_ifname)
+            VIR_FREE(net->ifname);
+    }
+
+    return ret;
+}
diff --git a/src/hypervisor/domain_interface.h b/src/hypervisor/domain_interface.h
index 572b4dd8c5..6d06fe2499 100644
--- a/src/hypervisor/domain_interface.h
+++ b/src/hypervisor/domain_interface.h
@@ -48,3 +48,13 @@ int virDomainInterfaceClearQoS(virDomainDef *def,
                                virDomainNetDef *net);
 void virDomainClearNetBandwidth(virDomainDef *def)
     ATTRIBUTE_NONNULL(1);
+
+int virDomainInterfaceBridgeConnect(virDomainDef *def,
+                                    virDomainNetDef *net,
+                                    int *tapfd,
+                                    size_t tapfdSize,
+                                    bool privileged,
+                                    ebtablesContext *ebtables,
+                                    bool macFilter,
+                                    const char *bridgeHelperName)
+    ATTRIBUTE_NONNULL(2) G_NO_INLINE;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d15d6a6a9d..8132a17c28 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1646,6 +1646,7 @@ virDomainDriverSetupPersistentDefBlkioParams;
 
 # hypervisor/domain_interface.h
 virDomainClearNetBandwidth;
+virDomainInterfaceBridgeConnect;
 virDomainInterfaceClearQoS;
 virDomainInterfaceDeleteDevice;
 virDomainInterfaceEthernetConnect;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f15e6bda1e..0d22bebe89 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8626,6 +8626,7 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
     bool vhostfd = false; /* also used to signal processing of tapfds */
     size_t tapfdSize = net->driver.virtio.queues;
     g_autofree int *tapfd = g_new0(int, tapfdSize + 1);
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
 
     memset(tapfd, -1, (tapfdSize + 1) * sizeof(*tapfd));
 
@@ -8636,8 +8637,12 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
     case VIR_DOMAIN_NET_TYPE_NETWORK:
     case VIR_DOMAIN_NET_TYPE_BRIDGE:
         vhostfd = true;
-        if (qemuInterfaceBridgeConnect(vm->def, priv->driver, net,
-                                       tapfd, &tapfdSize) < 0)
+        if (virDomainInterfaceBridgeConnect(vm->def, net,
+                                       tapfd, tapfdSize,
+                                       priv->driver->privileged,
+                                       priv->driver->ebtables,
+                                       priv->driver->config->macFilter,
+                                       cfg->bridgeHelperName) < 0)
             return -1;
         break;
 
diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c
index c2007c7043..23a23d201a 100644
--- a/src/qemu/qemu_interface.c
+++ b/src/qemu/qemu_interface.c
@@ -34,6 +34,7 @@
 #include "virnetdevbridge.h"
 #include "virnetdevvportprofile.h"
 #include "virsocket.h"
+#include "vircommand.h"
 
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -99,224 +100,6 @@ qemuInterfaceDirectConnect(virDomainDef *def,
 }
 
 
-/**
- * qemuCreateInBridgePortWithHelper:
- * @cfg: the configuration object in which the helper name is looked up
- * @brname: the bridge name
- * @ifname: the returned interface name
- * @macaddr: the returned MAC address
- * @tapfd: file descriptor return value for the new tap device
- * @flags: OR of virNetDevTapCreateFlags:
-
- *   VIR_NETDEV_TAP_CREATE_VNET_HDR
- *     - Enable IFF_VNET_HDR on the tap device
- *
- * This function creates a new tap device on a bridge using an external
- * helper.  The final name for the bridge will be stored in @ifname.
- *
- * Returns 0 in case of success or -1 on failure
- */
-static int
-qemuCreateInBridgePortWithHelper(virQEMUDriverConfig *cfg,
-                                 const char *brname,
-                                 char **ifname,
-                                 int *tapfd,
-                                 unsigned int flags)
-{
-    const char *const bridgeHelperDirs[] = {
-        "/usr/libexec",
-        "/usr/lib/qemu",
-        "/usr/lib",
-        NULL,
-    };
-    g_autoptr(virCommand) cmd = NULL;
-    g_autofree char *bridgeHelperPath = NULL;
-    char *errbuf = NULL, *cmdstr = NULL;
-    int pair[2] = { -1, -1 };
-
-    if ((flags & ~VIR_NETDEV_TAP_CREATE_VNET_HDR) != VIR_NETDEV_TAP_CREATE_IFUP)
-        return -1;
-
-    if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) {
-        virReportSystemError(errno, "%s", _("failed to create socket"));
-        return -1;
-    }
-
-    bridgeHelperPath = virFindFileInPathFull(cfg->bridgeHelperName, bridgeHelperDirs);
-
-    if (!bridgeHelperPath) {
-        virReportSystemError(errno, _("'%1$s' is not a suitable bridge helper"),
-                             cfg->bridgeHelperName);
-        return -1;
-    }
-
-    VIR_DEBUG("Using qemu-bridge-helper: %s", bridgeHelperPath);
-
-    cmd = virCommandNew(bridgeHelperPath);
-    if (flags & VIR_NETDEV_TAP_CREATE_VNET_HDR)
-        virCommandAddArgFormat(cmd, "--use-vnet");
-    virCommandAddArgFormat(cmd, "--br=%s", brname);
-    virCommandAddArgFormat(cmd, "--fd=%d", pair[1]);
-    virCommandSetErrorBuffer(cmd, &errbuf);
-    virCommandDoAsyncIO(cmd);
-    virCommandPassFD(cmd, pair[1],
-                     VIR_COMMAND_PASS_FD_CLOSE_PARENT);
-    virCommandClearCaps(cmd);
-#ifdef CAP_NET_ADMIN
-    virCommandAllowCap(cmd, CAP_NET_ADMIN);
-#endif
-    if (virCommandRunAsync(cmd, NULL) < 0) {
-        *tapfd = -1;
-        goto cleanup;
-    }
-
-    do {
-        *tapfd = virSocketRecvFD(pair[0], 0);
-    } while (*tapfd < 0 && errno == EINTR);
-
-    if (*tapfd < 0) {
-        char *errstr = NULL;
-
-        if (!(cmdstr = virCommandToString(cmd, false)))
-            goto cleanup;
-        virCommandAbort(cmd);
-
-        if (errbuf && *errbuf)
-            errstr = g_strdup_printf("stderr=%s", errbuf);
-
-        virReportSystemError(errno,
-                             _("%1$s: failed to communicate with bridge helper: %2$s"),
-                             cmdstr,
-                             NULLSTR_EMPTY(errstr));
-        VIR_FREE(errstr);
-        goto cleanup;
-    }
-
-    if (virNetDevTapGetName(*tapfd, ifname) < 0 ||
-        virCommandWait(cmd, NULL) < 0) {
-        VIR_FORCE_CLOSE(*tapfd);
-        *tapfd = -1;
-    }
-
- cleanup:
-    VIR_FREE(cmdstr);
-    VIR_FREE(errbuf);
-    VIR_FORCE_CLOSE(pair[0]);
-    return *tapfd < 0 ? -1 : 0;
-}
-
-/* qemuInterfaceBridgeConnect:
- * @def: the definition of the VM
- * @driver: qemu driver data
- * @net: pointer to the VM's interface description
- * @tapfd: array of file descriptor return value for the new device
- * @tapfdsize: number of file descriptors in @tapfd
- *
- * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_NETWORK or
- * VIR_DOMAIN_NET_TYPE_BRIDGE (i.e. if the connection is made with a tap
- * device connecting to a bridge device)
- */
-int
-qemuInterfaceBridgeConnect(virDomainDef *def,
-                           virQEMUDriver *driver,
-                           virDomainNetDef *net,
-                           int *tapfd,
-                           size_t *tapfdSize)
-{
-    const char *brname;
-    int ret = -1;
-    unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
-    bool template_ifname = false;
-    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-    const char *tunpath = "/dev/net/tun";
-
-    if (net->backend.tap) {
-        tunpath = net->backend.tap;
-        if (!driver->privileged) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("cannot use custom tap device in session mode"));
-            goto cleanup;
-        }
-    }
-
-    if (!(brname = virDomainNetGetActualBridgeName(net))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge name"));
-        goto cleanup;
-    }
-
-    if (!net->ifname)
-        template_ifname = true;
-
-    if (virDomainInterfaceIsVnetCompatModel(net))
-        tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
-
-    if (driver->privileged) {
-        if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
-                                           def->uuid, tunpath, tapfd, *tapfdSize,
-                                           virDomainNetGetActualVirtPortProfile(net),
-                                           virDomainNetGetActualVlan(net),
-                                           virDomainNetGetActualPortOptionsIsolated(net),
-                                           net->coalesce, 0, NULL,
-                                           tap_create_flags) < 0) {
-            virDomainAuditNetDevice(def, net, tunpath, false);
-            goto cleanup;
-        }
-        if (virDomainNetGetActualBridgeMACTableManager(net)
-            == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
-            /* libvirt is managing the FDB of the bridge this device
-             * is attaching to, so we need to turn off learning and
-             * unicast_flood on the device to prevent the kernel from
-             * adding any FDB entries for it. We will add an fdb
-             * entry ourselves (during virDomainInterfaceStartDevices(),
-             * using the MAC address from the interface config.
-             */
-            if (virNetDevBridgePortSetLearning(brname, net->ifname, false) < 0)
-                goto cleanup;
-            if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, false) < 0)
-                goto cleanup;
-        }
-    } else {
-        if (qemuCreateInBridgePortWithHelper(cfg, brname,
-                                             &net->ifname,
-                                             tapfd, tap_create_flags) < 0) {
-            virDomainAuditNetDevice(def, net, tunpath, false);
-            goto cleanup;
-        }
-        /* qemuCreateInBridgePortWithHelper can only create a single FD */
-        if (*tapfdSize > 1) {
-            VIR_WARN("Ignoring multiqueue network request");
-            *tapfdSize = 1;
-        }
-    }
-
-    virDomainAuditNetDevice(def, net, tunpath, true);
-
-    if (cfg->macFilter &&
-        ebtablesAddForwardAllowIn(driver->ebtables,
-                                  net->ifname,
-                                  &net->mac) < 0)
-        goto cleanup;
-
-    if (net->filter &&
-        virDomainConfNWFilterInstantiate(def->name, def->uuid, net, false) < 0) {
-        goto cleanup;
-    }
-
-    ret = 0;
-
- cleanup:
-    if (ret < 0) {
-        size_t i;
-        for (i = 0; i < *tapfdSize && tapfd[i] >= 0; i++)
-            VIR_FORCE_CLOSE(tapfd[i]);
-        if (template_ifname)
-            VIR_FREE(net->ifname);
-    }
-
-    return ret;
-}
-
-
 /*
  * Returns: -1 on error, 0 on success. Populates net->privateData->slirp if
  * the slirp helper is needed.
-- 
2.44.0
Re: [PATCH 1/2] hypervisor: Move domain interface mgmt methods
Posted by Michal Prívozník 2 months, 4 weeks ago
On 8/2/24 00:25, Praveen K Paladugu wrote:
> From: Praveen K Paladugu <praveenkpaladugu@gmail.com>
> 
> From: Praveen K Paladugu <prapal@linux.microsoft.com>
> 
> Move methods to connect domain interfaces to host bridges to hypervisor.
> This is to allow reuse between qemu and ch drivers.
> 
> Signed-off-by: Praveen K Paladugu <praveenkpaladugu@gmail.com>
> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> ---
>  src/hypervisor/domain_interface.c | 228 ++++++++++++++++++++++++++++++
>  src/hypervisor/domain_interface.h |  10 ++
>  src/libvirt_private.syms          |   1 +
>  src/qemu/qemu_command.c           |   9 +-
>  src/qemu/qemu_interface.c         | 219 +---------------------------
>  5 files changed, 247 insertions(+), 220 deletions(-)
> 
> diff --git a/src/hypervisor/domain_interface.c b/src/hypervisor/domain_interface.c
> index 756abb08e9..0c19ff859a 100644
> --- a/src/hypervisor/domain_interface.c
> +++ b/src/hypervisor/domain_interface.c
> @@ -39,6 +39,7 @@
>  #include "virnetdevmidonet.h"
>  #include "virnetdevopenvswitch.h"
>  #include "virnetdevtap.h"
> +#include "vircommand.h"
>  
>  
>  #define VIR_FROM_THIS VIR_FROM_DOMAIN
> @@ -514,3 +515,230 @@ virDomainClearNetBandwidth(virDomainDef *def)
>          virDomainInterfaceClearQoS(def, def->nets[i]);
>      }
>  }
> +
> +/**
> + * virDomainCreateInBridgePortWithHelper:
> + * @bridgeHelperName: name of the bridge helper program
> + * @brname: the bridge name
> + * @ifname: the returned interface name
> + * @tapfd: file descriptor return value for the new tap device
> + * @flags: OR of virNetDevTapCreateFlags:
> +
> + *   VIR_NETDEV_TAP_CREATE_VNET_HDR
> + *     - Enable IFF_VNET_HDR on the tap device
> + *
> + * This function creates a new tap device on a bridge using an external
> + * helper.  The final name for the bridge will be stored in @ifname.
> + *
> + * Returns 0 in case of success or -1 on failure
> + */
> +static int
> +virDomainCreateInBridgePortWithHelper(const char *bridgeHelperName,
> +                                      const char *brname,
> +                                      char **ifname,
> +                                      int *tapfd,
> +                                      unsigned int flags)
> +{
> +    const char *const bridgeHelperDirs[] = {
> +        "/usr/libexec",
> +        "/usr/lib/qemu",
> +        "/usr/lib",
> +        NULL,
> +    };
> +    g_autoptr(virCommand) cmd = NULL;
> +    g_autofree char *bridgeHelperPath = NULL;
> +    char *errbuf = NULL, *cmdstr = NULL;
> +    int pair[2] = { -1, -1 };
> +
> +    if (!bridgeHelperName) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge helper name"));
> +        return -1;
> +    }
> +
> +    if ((flags & ~VIR_NETDEV_TAP_CREATE_VNET_HDR) != VIR_NETDEV_TAP_CREATE_IFUP)
> +        return -1;
> +
> +    if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) {
> +        virReportSystemError(errno, "%s", _("failed to create socket"));
> +        return -1;
> +    }
> +
> +    bridgeHelperPath = virFindFileInPathFull(bridgeHelperName, bridgeHelperDirs);
> +
> +    if (!bridgeHelperPath) {
> +        virReportSystemError(errno, _("'%1$s' is not a suitable bridge helper"),
> +                                      bridgeHelperName);
> +        return -1;
> +    }
> +
> +    VIR_DEBUG("Using qemu-bridge-helper: %s", bridgeHelperPath);
> +    cmd = virCommandNew(bridgeHelperPath);
> +    if (flags & VIR_NETDEV_TAP_CREATE_VNET_HDR)
> +        virCommandAddArgFormat(cmd, "--use-vnet");
> +    virCommandAddArgFormat(cmd, "--br=%s", brname);
> +    virCommandAddArgFormat(cmd, "--fd=%d", pair[1]);
> +    virCommandSetErrorBuffer(cmd, &errbuf);
> +    virCommandDoAsyncIO(cmd);
> +    virCommandPassFD(cmd, pair[1],
> +                     VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +    virCommandClearCaps(cmd);
> +#ifdef CAP_NET_ADMIN
> +    virCommandAllowCap(cmd, CAP_NET_ADMIN);
> +#endif
> +    if (virCommandRunAsync(cmd, NULL) < 0) {
> +        *tapfd = -1;
> +        goto cleanup;
> +    }
> +
> +    do {
> +        *tapfd = virSocketRecvFD(pair[0], 0);
> +    } while (*tapfd < 0 && errno == EINTR);
> +
> +    if (*tapfd < 0) {
> +        char *errstr = NULL;
> +
> +        if (!(cmdstr = virCommandToString(cmd, false)))
> +            goto cleanup;
> +        virCommandAbort(cmd);
> +
> +        if (errbuf && *errbuf)
> +            errstr = g_strdup_printf("stderr=%s", errbuf);
> +
> +        virReportSystemError(errno,
> +                             _("%1$s: failed to communicate with bridge helper: %2$s"),
> +                             cmdstr,
> +                             NULLSTR_EMPTY(errstr));
> +        VIR_FREE(errstr);
> +        goto cleanup;
> +    }
> +
> +    if (virNetDevTapGetName(*tapfd, ifname) < 0 ||
> +        virCommandWait(cmd, NULL) < 0) {
> +        VIR_FORCE_CLOSE(*tapfd);
> +        *tapfd = -1;
> +    }
> +
> + cleanup:
> +    VIR_FREE(cmdstr);
> +    VIR_FREE(errbuf);
> +    VIR_FORCE_CLOSE(pair[0]);
> +    return *tapfd < 0 ? -1 : 0;
> +}
> +
> +
> +/* virDomainInterfaceBridgeConnect:
> + * @def: the definition of the VM
> + * @net: pointer to the VM's interface description
> + * @tapfd: array of file descriptor return value for the new device
> + * @tapfdsize: number of file descriptors in @tapfd
> + * @privileged: whether running as privileged user
> + * @ebtables: ebtales context
> + * @macFilter: whether driver support mac Filtering
> + * @bridgeHelperName:name of the bridge helper program to run in non-privileged mode
> + *
> + * Called *only* called if actualType is VIR_DOMAIN_NET_TYPE_NETWORK or
> + * VIR_DOMAIN_NET_TYPE_BRIDGE (i.e. if the connection is made with a tap
> + * device connecting to a bridge device)
> + */
> +int
> +virDomainInterfaceBridgeConnect(virDomainDef *def,
> +                                virDomainNetDef *net,
> +                                int *tapfd,
> +                                size_t tapfdSize,

Since this ^^ is not passed as a pointer, then ...

> +                                bool privileged,
> +                                ebtablesContext *ebtables,
> +                                bool macFilter,
> +                                const char *bridgeHelperName)
> +{
> +    const char *brname;
> +    int ret = -1;
> +    unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
> +    bool template_ifname = false;
> +    const char *tunpath = "/dev/net/tun";
> +
> +    if (net->backend.tap) {
> +        tunpath = net->backend.tap;
> +        if (!privileged) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("cannot use custom tap device in session mode"));
> +            goto cleanup;
> +        }
> +    }
> +
> +    if (!(brname = virDomainNetGetActualBridgeName(net))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing bridge name"));
> +        goto cleanup;
> +    }
> +
> +    if (!net->ifname)
> +        template_ifname = true;
> +
> +    if (virDomainInterfaceIsVnetCompatModel(net))
> +        tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
> +
> +    if (privileged) {
> +        if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
> +                                           def->uuid, tunpath, tapfd, tapfdSize,
> +                                           virDomainNetGetActualVirtPortProfile(net),
> +                                           virDomainNetGetActualVlan(net),
> +                                           virDomainNetGetActualPortOptionsIsolated(net),
> +                                           net->coalesce, 0, NULL,
> +                                           tap_create_flags) < 0) {
> +            virDomainAuditNetDevice(def, net, tunpath, false);
> +            goto cleanup;
> +        }
> +        if (virDomainNetGetActualBridgeMACTableManager(net)
> +            == VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
> +            /* libvirt is managing the FDB of the bridge this device
> +             * is attaching to, so we need to turn off learning and
> +             * unicast_flood on the device to prevent the kernel from
> +             * adding any FDB entries for it. We will add an fdb
> +             * entry ourselves (during virDomainInterfaceStartDevices(),
> +             * using the MAC address from the interface config.
> +             */
> +            if (virNetDevBridgePortSetLearning(brname, net->ifname, false) < 0)
> +                goto cleanup;
> +            if (virNetDevBridgePortSetUnicastFlood(brname, net->ifname, false) < 0)
> +                goto cleanup;
> +        }
> +    } else {
> +        if (virDomainCreateInBridgePortWithHelper(bridgeHelperName, brname,
> +                                                  &net->ifname,
> +                                                  tapfd,
> +                                                  tap_create_flags) < 0) {
> +            virDomainAuditNetDevice(def, net, tunpath, false);
> +            goto cleanup;
> +        }
> +        /* virDomainCreateInBridgePortWithHelper can only create a single FD */
> +        if (tapfdSize > 1) {
> +            VIR_WARN("Ignoring multiqueue network request");
> +            tapfdSize = 1;

... this has not the desired effect.

> +        }
> +    }
> +
> +    virDomainAuditNetDevice(def, net, tunpath, true);
> +
> +    if (macFilter &&
> +        ebtablesAddForwardAllowIn(ebtables,
> +                                  net->ifname,
> +                                  &net->mac) < 0)
> +        goto cleanup;
> +
> +    if (net->filter &&
> +        virDomainConfNWFilterInstantiate(def->name, def->uuid, net, false) < 0) {
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    if (ret < 0) {
> +        size_t i;
> +        for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++)
> +            VIR_FORCE_CLOSE(tapfd[i]);
> +        if (template_ifname)
> +            VIR_FREE(net->ifname);
> +    }
> +
> +    return ret;
> +}
> diff --git a/src/hypervisor/domain_interface.h b/src/hypervisor/domain_interface.h
> index 572b4dd8c5..6d06fe2499 100644
> --- a/src/hypervisor/domain_interface.h
> +++ b/src/hypervisor/domain_interface.h
> @@ -48,3 +48,13 @@ int virDomainInterfaceClearQoS(virDomainDef *def,
>                                 virDomainNetDef *net);
>  void virDomainClearNetBandwidth(virDomainDef *def)
>      ATTRIBUTE_NONNULL(1);
> +
> +int virDomainInterfaceBridgeConnect(virDomainDef *def,
> +                                    virDomainNetDef *net,
> +                                    int *tapfd,
> +                                    size_t tapfdSize,
> +                                    bool privileged,
> +                                    ebtablesContext *ebtables,
> +                                    bool macFilter,
> +                                    const char *bridgeHelperName)
> +    ATTRIBUTE_NONNULL(2) G_NO_INLINE;
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index d15d6a6a9d..8132a17c28 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1646,6 +1646,7 @@ virDomainDriverSetupPersistentDefBlkioParams;
>  
>  # hypervisor/domain_interface.h
>  virDomainClearNetBandwidth;
> +virDomainInterfaceBridgeConnect;
>  virDomainInterfaceClearQoS;
>  virDomainInterfaceDeleteDevice;
>  virDomainInterfaceEthernetConnect;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f15e6bda1e..0d22bebe89 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8626,6 +8626,7 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
>      bool vhostfd = false; /* also used to signal processing of tapfds */
>      size_t tapfdSize = net->driver.virtio.queues;
>      g_autofree int *tapfd = g_new0(int, tapfdSize + 1);
> +    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
>  
>      memset(tapfd, -1, (tapfdSize + 1) * sizeof(*tapfd));
>  
> @@ -8636,8 +8637,12 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
>      case VIR_DOMAIN_NET_TYPE_NETWORK:
>      case VIR_DOMAIN_NET_TYPE_BRIDGE:
>          vhostfd = true;
> -        if (qemuInterfaceBridgeConnect(vm->def, priv->driver, net,
> -                                       tapfd, &tapfdSize) < 0)

There are two more occurances of qemuInterfaceBridgeConnect(): one in
qemu_interface.h (which needs to be removed) and more crucially the
other in qemuxml2argvmock.c because otherwise qemuxmlconftest fails.

> +        if (virDomainInterfaceBridgeConnect(vm->def, net,
> +                                       tapfd, tapfdSize,
> +                                       priv->driver->privileged,
> +                                       priv->driver->ebtables,
> +                                       priv->driver->config->macFilter,
> +                                       cfg->bridgeHelperName) < 0)
>              return -1;
>          break;
>  
Michal