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

Friday, 19 February 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(a)aristanetworks.com&gt;

---
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,



    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

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