qemu: Move qemuNetworkIfaceConnect to qemu_interface.c and rename

Move the misplaced function from qemu_command.c to qemu_interface.c
since it's closer in functionality there and had less to do with building
the command line.

Rename function to qemuInterfaceBridgeConnect and modify callers.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

po/POTFILES.in

@@ -128,6 +128,7 @@ src/qemu/qemu_domain.c
 src/qemu/qemu_driver.c
 src/qemu/qemu_hostdev.c
 src/qemu/qemu_hotplug.c
+src/qemu/qemu_interface.c
 src/qemu/qemu_migration.c
 src/qemu/qemu_monitor.c
 src/qemu/qemu_monitor_json.c

src/qemu/qemu_command.c

@@ -29,7 +29,6 @@
 #include "qemu_interface.h"
 #include "cpu/cpu.h"
 #include "dirname.h"
-#include "passfd.h"
 #include "viralloc.h"
 #include "virlog.h"
 #include "virarch.h"
@@ -154,214 +153,6 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST,
               "preferred",
               "interleave");
 
-/**
- * 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(virQEMUDriverConfigPtr cfg,
-                                            const char *brname,
-                                            char **ifname,
-                                            int *tapfd,
-                                            unsigned int flags)
-{
-    virCommandPtr cmd;
-    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;
-    }
-
-    if (!virFileIsExecutable(cfg->bridgeHelperName)) {
-        virReportSystemError(errno, _("'%s' is not a suitable bridge helper"),
-                             cfg->bridgeHelperName);
-        return -1;
-    }
-
-    cmd = virCommandNew(cfg->bridgeHelperName);
-    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 = recvfd(pair[0], 0);
-    } while (*tapfd < 0 && errno == EINTR);
-
-    if (*tapfd < 0) {
-        char ebuf[1024];
-        char *errstr = NULL;
-
-        if (!(cmdstr = virCommandToString(cmd)))
-            goto cleanup;
-        virCommandAbort(cmd);
-
-        if (errbuf && *errbuf &&
-            virAsprintf(&errstr, "\nstderr=%s", errbuf) < 0)
-            goto cleanup;
-
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-            _("%s: failed to communicate with bridge helper: %s%s"),
-            cmdstr, virStrerror(errno, ebuf, sizeof(ebuf)),
-            errstr ? 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);
-    virCommandFree(cmd);
-    VIR_FORCE_CLOSE(pair[0]);
-    return *tapfd < 0 ? -1 : 0;
-}
-
-/* qemuNetworkIfaceConnect - *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
-qemuNetworkIfaceConnect(virDomainDefPtr def,
-                        virQEMUDriverPtr driver,
-                        virDomainNetDefPtr 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;
-    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-    const char *tunpath = "/dev/net/tun";
-
-    if (net->backend.tap) {
-        tunpath = net->backend.tap;
-        if (!(virQEMUDriverIsPrivileged(driver))) {
-            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 ||
-        STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) ||
-        strchr(net->ifname, '%')) {
-        VIR_FREE(net->ifname);
-        if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_PREFIX "%d") < 0)
-            goto cleanup;
-        /* avoid exposing vnet%d in getXMLDesc or error outputs */
-        template_ifname = true;
-    }
-
-    if (net->model && STREQ(net->model, "virtio"))
-        tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
-
-    if (virQEMUDriverIsPrivileged(driver)) {
-        if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
-                                           def->uuid, tunpath, tapfd, *tapfdSize,
-                                           virDomainNetGetActualVirtPortProfile(net),
-                                           virDomainNetGetActualVlan(net),
-                                           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 add an fdb
-             * entry ourselves (during qemuInterfaceStartDevices(),
-             * 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->uuid, net) < 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);
-    }
-    virObjectUnref(cfg);
-
-    return ret;
-}
-
 static bool
 qemuDomainSupportsNicdev(virDomainDefPtr def,
                          virQEMUCapsPtr qemuCaps,
@@ -8425,8 +8216,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
 
         memset(tapfd, -1, tapfdSize * sizeof(tapfd[0]));
 
-        if (qemuNetworkIfaceConnect(def, driver, net,
-                                    tapfd, &tapfdSize) < 0)
+        if (qemuInterfaceBridgeConnect(def, driver, net,
+                                       tapfd, &tapfdSize) < 0)
             goto cleanup;
     } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
         tapfdSize = net->driver.virtio.queues;

src/qemu/qemu_command.h

@@ -223,12 +223,6 @@ char *qemuBuildHubDevStr(virDomainDefPtr def,
 char *qemuBuildRedirdevDevStr(virDomainDefPtr def,
                               virDomainRedirdevDefPtr dev,
                               virQEMUCapsPtr qemuCaps);
-int qemuNetworkIfaceConnect(virDomainDefPtr def,
-                            virQEMUDriverPtr driver,
-                            virDomainNetDefPtr net,
-                            int *tapfd,
-                            size_t *tapfdSize)
-    ATTRIBUTE_NONNULL(2);
 
 int qemuOpenVhostNet(virDomainDefPtr def,
                      virDomainNetDefPtr net,

src/qemu/qemu_hotplug.c

@@ -922,8 +922,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
         if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0)
             goto cleanup;
         memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize);
-        if (qemuNetworkIfaceConnect(vm->def, driver, net,
-                                    tapfd, &tapfdSize) < 0)
+        if (qemuInterfaceBridgeConnect(vm->def, driver, net,
+                                       tapfd, &tapfdSize) < 0)
             goto cleanup;
         iface_connected = true;
         if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd, &vhostfdSize) < 0)

src/qemu/qemu_interface.c

@@ -26,14 +26,22 @@
 
 #include "network_conf.h"
 #include "domain_audit.h"
+#include "domain_nwfilter.h"
 #include "qemu_interface.h"
+#include "passfd.h"
 #include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
 #include "virnetdev.h"
 #include "virnetdevtap.h"
 #include "virnetdevmacvlan.h"
 #include "virnetdevbridge.h"
 #include "virnetdevvportprofile.h"
 
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+VIR_LOG_INIT("qemu.qemu_interface");
+
 /**
  * qemuInterfaceStartDevice:
  * @net: net device to start
@@ -276,3 +284,219 @@ qemuInterfaceDirectConnect(virDomainDefPtr def,
     virObjectUnref(cfg);
     return ret;
 }
+
+
+/**
+ * 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(virQEMUDriverConfigPtr cfg,
+                                 const char *brname,
+                                 char **ifname,
+                                 int *tapfd,
+                                 unsigned int flags)
+{
+    virCommandPtr cmd;
+    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;
+    }
+
+    if (!virFileIsExecutable(cfg->bridgeHelperName)) {
+        virReportSystemError(errno, _("'%s' is not a suitable bridge helper"),
+                             cfg->bridgeHelperName);
+        return -1;
+    }
+
+    cmd = virCommandNew(cfg->bridgeHelperName);
+    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 = recvfd(pair[0], 0);
+    } while (*tapfd < 0 && errno == EINTR);
+
+    if (*tapfd < 0) {
+        char ebuf[1024];
+        char *errstr = NULL;
+
+        if (!(cmdstr = virCommandToString(cmd)))
+            goto cleanup;
+        virCommandAbort(cmd);
+
+        if (errbuf && *errbuf &&
+            virAsprintf(&errstr, "\nstderr=%s", errbuf) < 0)
+            goto cleanup;
+
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+            _("%s: failed to communicate with bridge helper: %s%s"),
+            cmdstr, virStrerror(errno, ebuf, sizeof(ebuf)),
+            errstr ? 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);
+    virCommandFree(cmd);
+    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(virDomainDefPtr def,
+                           virQEMUDriverPtr driver,
+                           virDomainNetDefPtr 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;
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    const char *tunpath = "/dev/net/tun";
+
+    if (net->backend.tap) {
+        tunpath = net->backend.tap;
+        if (!(virQEMUDriverIsPrivileged(driver))) {
+            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 ||
+        STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) ||
+        strchr(net->ifname, '%')) {
+        VIR_FREE(net->ifname);
+        if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_PREFIX "%d") < 0)
+            goto cleanup;
+        /* avoid exposing vnet%d in getXMLDesc or error outputs */
+        template_ifname = true;
+    }
+
+    if (net->model && STREQ(net->model, "virtio"))
+        tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
+
+    if (virQEMUDriverIsPrivileged(driver)) {
+        if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
+                                           def->uuid, tunpath, tapfd, *tapfdSize,
+                                           virDomainNetGetActualVirtPortProfile(net),
+                                           virDomainNetGetActualVlan(net),
+                                           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 add an fdb
+             * entry ourselves (during qemuInterfaceStartDevices(),
+             * 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->uuid, net) < 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);
+    }
+    virObjectUnref(cfg);
+
+    return ret;
+}

src/qemu/qemu_interface.h

@@ -40,4 +40,10 @@ int qemuInterfaceDirectConnect(virDomainDefPtr def,
                                 size_t tapfdSize,
                                 virNetDevVPortProfileOp vmop);
 
+int qemuInterfaceBridgeConnect(virDomainDefPtr def,
+                               virQEMUDriverPtr driver,
+                               virDomainNetDefPtr net,
+                               int *tapfd,
+                               size_t *tapfdSize)
+    ATTRIBUTE_NONNULL(2);
 #endif /* __QEMU_INTERFACE_H__ */