hypervisor: Move domain interface mgmt methods

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>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

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,231 @@ 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;
+}

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;

src/libvirt_private.syms

@@ -1647,6 +1647,7 @@ virDomainDriverSetupPersistentDefBlkioParams;
 
 # hypervisor/domain_interface.h
 virDomainClearNetBandwidth;
+virDomainInterfaceBridgeConnect;
 virDomainInterfaceClearQoS;
 virDomainInterfaceDeleteDevice;
 virDomainInterfaceEthernetConnect;

src/qemu/qemu_command.c

@@ -8642,6 +8642,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));
 
@@ -8652,8 +8653,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,
+        if (virDomainInterfaceBridgeConnect(vm->def, net,
-                                       tapfd, &tapfdSize) < 0)
+                                            tapfd, &tapfdSize,
+                                            priv->driver->privileged,
+                                            priv->driver->ebtables,
+                                            priv->driver->config->macFilter,
+                                            cfg->bridgeHelperName) < 0)
             return -1;
         break;
 

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.

src/qemu/qemu_interface.h

@@ -32,13 +32,6 @@ int qemuInterfaceDirectConnect(virDomainDef *def,
                                size_t tapfdSize,
                                virNetDevVPortProfileOp vmop);
 
-int qemuInterfaceBridgeConnect(virDomainDef *def,
-                               virQEMUDriver *driver,
-                               virDomainNetDef *net,
-                               int *tapfd,
-                               size_t *tapfdSize)
-    ATTRIBUTE_NONNULL(2) G_NO_INLINE;
-
 int qemuInterfaceOpenVhostNet(virDomainObj *def,
                               virDomainNetDef *net) G_NO_INLINE;
 

tests/qemuxml2argvmock.c

@@ -35,6 +35,7 @@
 #include "virutil.h"
 #include "qemu/qemu_interface.h"
 #include "qemu/qemu_command.h"
+#include "domain_interface.h"
 #include <unistd.h>
 #include <fcntl.h>
 
@@ -115,11 +116,14 @@ virNetDevTapCreate(char **ifname,
 
 
 int
-qemuInterfaceBridgeConnect(virDomainDef *def G_GNUC_UNUSED,
+virDomainInterfaceBridgeConnect(virDomainDef *def G_GNUC_UNUSED,
-                           virQEMUDriver *driver G_GNUC_UNUSED,
+                                virDomainNetDef *net G_GNUC_UNUSED,
-                           virDomainNetDef *net G_GNUC_UNUSED,
+                                int *tapfd,
-                           int *tapfd,
+                                size_t *tapfdSize,
-                           size_t *tapfdSize)
+                                bool privileged G_GNUC_UNUSED,
+                                ebtablesContext *ebtables G_GNUC_UNUSED,
+                                bool macFilter G_GNUC_UNUSED,
+                                const char *bridgeHelperName G_GNUC_UNUSED)
 {
     size_t i;