[libvirt] [RFC][PATCH 1/2] Separate backend setup from NIC device add/remove

Ed Swierk eswierk at aristanetworks.com
Fri Feb 19 23:35:17 UTC 2010


This patch moves backend setup code from qemudDomainAttachNetDevice and
qemudDomainDetachNetDevice into separate functions.  While the intent is
to allow the new functions to be called separately without affecting VM
NIC devices, this is arguably a worthwhile refactoring on its own.

Currently it's based on 0.7.6 with some macvtap patches.  I can rebase
it against git if there's interest.

Signed-off-by: Ed Swierk <eswierk at aristanetworks.com>

---
Move the code that creates and destroys the network backend and
connects it to qemu from the code that creates the virtual NIC.

Index: libvirt-0.7.6/src/qemu/qemu_driver.c
===================================================================
--- libvirt-0.7.6.orig/src/qemu/qemu_driver.c
+++ libvirt-0.7.6/src/qemu/qemu_driver.c
@@ -132,6 +132,17 @@ static int qemuDetectVcpuPIDs(virConnect
 static int qemuUpdateActivePciHostdevs(struct qemud_driver *driver,
                                        virDomainDefPtr def);
 
+static int qemudDomainConnectNetBackend(virConnectPtr conn,
+                                        struct qemud_driver *driver,
+                                        virDomainObjPtr vm,
+                                        virDomainNetDefPtr net,
+                                        unsigned int qemuCmdFlags);
+
+static int qemudDomainDisconnectNetBackend(virConnectPtr conn,
+                                           struct qemud_driver *driver,
+                                           virDomainObjPtr vm,
+                                           virDomainNetDefPtr net);
+
 static struct qemud_driver *qemu_driver = NULL;
 
 
@@ -5609,19 +5620,17 @@ error:
 }
 
 
-static int qemudDomainAttachNetDevice(virConnectPtr conn,
-                                      struct qemud_driver *driver,
-                                      virDomainObjPtr vm,
-                                      virDomainNetDefPtr net,
-                                      unsigned int qemuCmdFlags)
+static int qemudDomainConnectNetBackend(virConnectPtr conn,
+                                        struct qemud_driver *driver,
+                                        virDomainObjPtr vm,
+                                        virDomainNetDefPtr net,
+                                        unsigned int qemuCmdFlags)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
     char *tapfd_name = NULL;
     int tapfd = -1;
-    char *nicstr = NULL;
     char *netstr = NULL;
     int ret = -1;
-    virDomainDevicePCIAddress guestAddr;
     int vlan;
 
     if (!(qemuCmdFlags & QEMUD_CMD_FLAG_HOST_NET_ADD)) {
@@ -5630,142 +5639,132 @@ static int qemudDomainAttachNetDevice(vi
         return -1;
     }
 
+    if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
+        qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
+                         _("network device type '%s' cannot be attached: "
+                           "qemu is not using a unix socket monitor"),
+                         virDomainNetTypeToString(net->type));
+        return -1;
+    }
+
+    if ((vlan = qemuDomainNetVLAN(net)) < 0) {
+        qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s",
+                         _("Unable to connect network backend without vlan"));
+        goto out;
+    }
+
     if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
         net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
-        if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
-            qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
-                             _("network device type '%s' cannot be attached: "
-                               "qemu is not using a unix socket monitor"),
-                             virDomainNetTypeToString(net->type));
-            return -1;
-        }
-
-        if ((tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags)) < 0)
-            return -1;
+        tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags);
     } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
-        if (priv->monConfig->type != VIR_DOMAIN_CHR_TYPE_UNIX) {
-            qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
-                             _("direct device type '%s' cannot be attached: "
-                               "qemu is not using a unix socket monitor"),
-                             virDomainNetTypeToString(net->type));
-            return -1;
-        }
+        tapfd = qemudPhysIfaceConnect(conn, net,
+                                      net->data.direct.linkdev,
+                                      net->data.direct.mode);
+    }
+    if (tapfd < 0)
+        return -1;
 
-        if ((tapfd = qemudPhysIfaceConnect(conn, net,
-                                           net->data.direct.linkdev,
-                                           net->data.direct.mode)) < 0)
-            return -1;
+    if (virAsprintf(&tapfd_name, "fd-%s", net->info.alias) < 0) {
+        virReportOOMError(conn);
+        close(tapfd);
+        return -1;
     }
 
-    if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0)
-        goto no_memory;
+    if (!(netstr = qemuBuildHostNetStr(conn, net, ' ',
+                                       vlan, tapfd_name))) {
+        VIR_FREE(tapfd_name);
+        close(tapfd);
+        return -1;
+    }
+
+    qemuDomainObjEnterMonitorWithDriver(driver, vm);
+
+    if (qemuMonitorSendFileHandle(priv->mon, tapfd_name, tapfd) < 0)
+        goto out;
+
+    if (qemuMonitorAddHostNetwork(priv->mon, netstr) < 0) {
+        if (qemuMonitorCloseFileHandle(priv->mon, tapfd_name) < 0)
+            VIR_WARN(_("Failed to close tapfd with '%s'"), tapfd_name);
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    qemuDomainObjExitMonitorWithDriver(driver, vm);
+    VIR_FREE(netstr);
+    VIR_FREE(tapfd_name);
+    close(tapfd);
+    return ret;
+}
+
+static int qemudDomainAttachNetDevice(virConnectPtr conn,
+                                      struct qemud_driver *driver,
+                                      virDomainObjPtr vm,
+                                      virDomainNetDefPtr net,
+                                      unsigned int qemuCmdFlags)
+{
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    char *nicstr = NULL;
+    int ret = -1;
+    virDomainDevicePCIAddress guestAddr;
+    int vlan;
+
+    if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets+1) < 0) {
+        virReportOOMError(conn);
+        return -1;
+    }
 
     if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) ||
         (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) {
         if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0)
-            goto cleanup;
+            goto out;
     }
 
     if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) &&
         qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0)
-        goto cleanup;
-
-    vlan = qemuDomainNetVLAN(net);
+        goto out;
 
-    if (vlan < 0) {
+    if ((vlan = qemuDomainNetVLAN(net)) < 0) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s",
                          _("Unable to attach network devices without vlan"));
-        goto cleanup;
-    }
-
-    if (tapfd != -1) {
-        if (virAsprintf(&tapfd_name, "fd-%s", net->info.alias) < 0)
-            goto no_memory;
-
-        qemuDomainObjEnterMonitorWithDriver(driver, vm);
-        if (qemuMonitorSendFileHandle(priv->mon, tapfd_name, tapfd) < 0) {
-            qemuDomainObjExitMonitorWithDriver(driver, vm);
-            goto cleanup;
-        }
-        qemuDomainObjExitMonitorWithDriver(driver, vm);
+        goto out;
     }
 
-    if (!(netstr = qemuBuildHostNetStr(conn, net, ' ',
-                                       vlan, tapfd_name)))
-        goto try_tapfd_close;
-
-    qemuDomainObjEnterMonitorWithDriver(driver, vm);
-    if (qemuMonitorAddHostNetwork(priv->mon, netstr) < 0) {
-        qemuDomainObjExitMonitorWithDriver(driver, vm);
-        goto try_tapfd_close;
+    if (qemudDomainConnectNetBackend(conn, driver, vm, net, qemuCmdFlags) < 0) {
+        VIR_WARN0(_("Failed to connect network backend"));
+        goto out;
     }
-    qemuDomainObjExitMonitorWithDriver(driver, vm);
-
-    if (tapfd != -1)
-        close(tapfd);
-    tapfd = -1;
 
     if (!(nicstr = qemuBuildNicStr(conn, net, NULL, vlan)))
-        goto try_remove;
+        goto out_remove;
 
     qemuDomainObjEnterMonitorWithDriver(driver, vm);
     if (qemuMonitorAddPCINetwork(priv->mon, nicstr,
                                  &guestAddr) < 0) {
         qemuDomainObjExitMonitorWithDriver(driver, vm);
-        goto try_remove;
+        goto out_remove;
     }
     net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
     memcpy(&net->info.addr.pci, &guestAddr, sizeof(guestAddr));
     qemuDomainObjExitMonitorWithDriver(driver, vm);
 
+    vm->def->nets[vm->def->nnets++] = net;
     ret = 0;
+    goto out;
 
-    vm->def->nets[vm->def->nnets++] = net;
+out_remove:
+    if (qemudDomainDisconnectNetBackend(conn, driver, vm, net) < 0)
+       VIR_WARN0("Unable to disconnect network backend");
 
-cleanup:
+out:
     if ((ret != 0) &&
         (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) &&
         (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
         qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &net->info) < 0)
         VIR_WARN0("Unable to release PCI address on NIC");
-
     VIR_FREE(nicstr);
-    VIR_FREE(netstr);
-    VIR_FREE(tapfd_name);
-    if (tapfd != -1)
-        close(tapfd);
-
     return ret;
-
-try_remove:
-    if (vlan < 0) {
-        VIR_WARN0(_("Unable to remove network backend"));
-    } else {
-        char *hostnet_name;
-        if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0)
-            goto no_memory;
-        qemuDomainObjEnterMonitorWithDriver(driver, vm);
-        if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0)
-            VIR_WARN(_("Failed to remove network backend for vlan %d, net %s"),
-                     vlan, hostnet_name);
-        qemuDomainObjExitMonitorWithDriver(driver, vm);
-        VIR_FREE(hostnet_name);
-    }
-    goto cleanup;
-
-try_tapfd_close:
-    if (tapfd_name) {
-        qemuDomainObjEnterMonitorWithDriver(driver, vm);
-        if (qemuMonitorCloseFileHandle(priv->mon, tapfd_name) < 0)
-            VIR_WARN(_("Failed to close tapfd with '%s'"), tapfd_name);
-        qemuDomainObjExitMonitorWithDriver(driver, vm);
-    }
-
-    goto cleanup;
-
-no_memory:
-    virReportOOMError(conn);
-    goto cleanup;
 }
 
 
@@ -6207,82 +6206,95 @@ cleanup:
     return ret;
 }
 
-static int
-qemudDomainDetachNetDevice(virConnectPtr conn,
-                           struct qemud_driver *driver,
-                           virDomainObjPtr vm,
-                           virDomainDeviceDefPtr dev)
+static int qemudDomainDisconnectNetBackend(virConnectPtr conn,
+                                           struct qemud_driver *driver,
+                                           virDomainObjPtr vm,
+                                           virDomainNetDefPtr net)
 {
-    int i, ret = -1;
-    virDomainNetDefPtr detach = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     int vlan;
     char *hostnet_name = NULL;
 
+    if ((vlan = qemuDomainNetVLAN(net)) < 0) {
+        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+                         "%s", _("unable to determine original VLAN"));
+        return -1;
+    }
+
+    if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0) {
+        virReportOOMError(NULL);
+        return -1;
+    }
+
+    qemuDomainObjEnterMonitorWithDriver(driver, vm);
+    if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0)
+        VIR_WARN0("Failed to remove host network");
+    qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+#if WITH_MACVTAP
+    if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
+        if (net->ifname)
+            delMacvtap(conn, net->ifname);
+    }
+#endif
+
+    VIR_FREE(hostnet_name);
+    return 0;
+}
+
+static int qemudDomainDetachNetDevice(virConnectPtr conn,
+                                      struct qemud_driver *driver,
+                                      virDomainObjPtr vm,
+                                      virDomainDeviceDefPtr dev)
+{
+    int i = -1;
+    virDomainNetDefPtr net = NULL;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+
     for (i = 0 ; i < vm->def->nnets ; i++) {
-        virDomainNetDefPtr net = vm->def->nets[i];
+        virDomainNetDefPtr n = vm->def->nets[i];
 
-        if (!memcmp(net->mac, dev->data.net->mac,  sizeof(net->mac))) {
-            detach = net;
+        if (!memcmp(n->mac, dev->data.net->mac,  sizeof(n->mac))) {
+            net = n;
             break;
         }
     }
 
-    if (!detach) {
+    if (!net) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
                          _("network device %02x:%02x:%02x:%02x:%02x:%02x not found"),
                          dev->data.net->mac[0], dev->data.net->mac[1],
                          dev->data.net->mac[2], dev->data.net->mac[3],
                          dev->data.net->mac[4], dev->data.net->mac[5]);
-        goto cleanup;
+        return -1;
     }
 
-    if (!virDomainDeviceAddressIsValid(&detach->info,
+    if (!virDomainDeviceAddressIsValid(&net->info,
                                        VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
                          "%s", _("device cannot be detached without a PCI address"));
-        goto cleanup;
-    }
-
-    if ((vlan = qemuDomainNetVLAN(detach)) < 0) {
-        qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
-                         "%s", _("unable to determine original VLAN"));
-        goto cleanup;
-    }
-
-    if (virAsprintf(&hostnet_name, "host%s", detach->info.alias) < 0) {
-        virReportOOMError(NULL);
-        goto cleanup;
+        return -1;
     }
 
     qemuDomainObjEnterMonitorWithDriver(driver, vm);
     if (qemuMonitorRemovePCIDevice(priv->mon,
-                                   &detach->info.addr.pci) < 0) {
-        qemuDomainObjExitMonitorWithDriver(driver, vm);
-        goto cleanup;
-    }
-
-    if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0) {
+                                   &net->info.addr.pci) < 0) {
         qemuDomainObjExitMonitorWithDriver(driver, vm);
-        goto cleanup;
+        return -1;
     }
     qemuDomainObjExitMonitorWithDriver(driver, vm);
 
-#if WITH_MACVTAP
-    if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
-        if (detach->ifname)
-            delMacvtap(conn, detach->ifname);
-    }
-#endif
+    if (qemudDomainDisconnectNetBackend(conn, driver, vm, net) < 0)
+        VIR_WARN0("Failed to disconnect network backend");
 
-    if ((driver->macFilter) && (detach->ifname != NULL)) {
+    if ((driver->macFilter) && (net->ifname != NULL)) {
         if ((errno = networkDisallowMacOnPort(conn,
                                               driver,
-                                              detach->ifname,
-                                              detach->mac))) {
+                                              net->ifname,
+                                              net->mac))) {
             virReportSystemError(conn, errno,
              _("failed to remove ebtables rule on  '%s'"),
-                                 detach->ifname);
+                                 net->ifname);
         }
     }
 
@@ -6299,13 +6311,9 @@ qemudDomainDetachNetDevice(virConnectPtr
         VIR_FREE(vm->def->nets);
         vm->def->nnets = 0;
     }
-    virDomainNetDefFree(detach);
+    virDomainNetDefFree(net);
 
-    ret = 0;
-
-cleanup:
-    VIR_FREE(hostnet_name);
-    return ret;
+    return 0;
 }
 
 static int qemudDomainDetachHostPciDevice(virConnectPtr conn,





More information about the libvir-list mailing list