macvtap teardown rework

Rework and simplification of teardown of the macvtap device.

Basically all devices with the same MAC address and link device are kept
alive and not attempted to be torn down. If a macvtap device linked to a
physical interface with a certain MAC address 'M' is to be created it
will automatically fail if the interface is 'up'ed and another macvtap
with the same properties (MAC addr 'M', link dev) happens to be 'up'.
This will prevent the VM from starting or the device from being attached
to a running VM. Stale interfaces are assumed to be there for some
reason and not stem from libvirt.

In the VM shutdown path, it's assuming that an interface name is always
available so that if the device type is DIRECT it can be torn down
using its name.

* src/util/macvtap.h src/libvirt_macvtap.syms: change of deleting routine
* src/util/macvtap.c: cleanups and change of deleting routine
* src/qemu/qemu_driver.c: change cleanup on shutdown
* src/qemu/qemu_conf.c: don't delete Macvtap in qemudPhysIfaceConnect()

diff --git a/src/libvirt_macvtap.syms b/src/libvirt_macvtap.syms
index d02213dbec44f3d9378813aebebcfb6d4e167a18..ae229a0af0d743ca1b3dffee73893cdf7a84b871 100644 (file)
--- a/src/libvirt_macvtap.syms
+++ b/src/libvirt_macvtap.syms
@@ -2,4 +2,4 @@
 
 # macvtap.h
 openMacvtapTap;
-delMacvtapByMACAddress;
+delMacvtap;

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index c9fe55bebd976b3db62e1d37f9b3b45d4cdd9aec..646b3ed4e9de8d1f1c792e8486d78643f425052f 100644 (file)
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1439,15 +1439,6 @@ qemudPhysIfaceConnect(virConnectPtr conn,
     int rc;
 #if WITH_MACVTAP
     char *res_ifname = NULL;
-    int hasBusyDev = 0;
-
-    delMacvtapByMACAddress(net->mac, &hasBusyDev);
-
-    if (hasBusyDev) {
-        virReportSystemError(errno, "%s",
-                             _("A macvtap with the same MAC address is in use"));
-        return -1;
-    }
 
     rc = openMacvtapTap(conn, net->ifname, net->mac, linkdev, brmode,
                         &res_ifname);
@@ -1460,7 +1451,6 @@ qemudPhysIfaceConnect(virConnectPtr conn,
     (void)net;
     (void)linkdev;
     (void)brmode;
-    (void)conn;
     qemuReportError(VIR_ERR_INTERNAL_ERROR,
                     "%s", _("No support for macvtap device"));
     rc = -1;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8766ca2427186a77f71a2ac0fce8b668c9e55514..aaeb7f615b7004dcb316fe07938a35fed598eeb0 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2942,17 +2942,6 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
         }
     }
 
-#if WITH_MACVTAP
-    def = vm->def;
-    for (i = 0; i < def->nnets; i++) {
-        virDomainNetDefPtr net = def->nets[i];
-        if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
-            int dummy;
-            delMacvtapByMACAddress(net->mac, &dummy);
-        }
-    }
-#endif
-
     if (virKillProcess(vm->pid, 0) == 0 &&
         virKillProcess(vm->pid, SIGTERM) < 0)
         virReportSystemError(errno,
@@ -2999,6 +2988,17 @@ static void qemudShutdownVMDaemon(struct qemud_driver *driver,
 
     qemuDomainReAttachHostDevices(driver, vm->def);
 
+#if WITH_MACVTAP
+    def = vm->def;
+    for (i = 0; i < def->nnets; i++) {
+        virDomainNetDefPtr net = def->nets[i];
+        if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
+            if (net->ifname)
+                delMacvtap(net->ifname);
+        }
+    }
+#endif
+
 retry:
     if ((ret = qemuRemoveCgroup(driver, vm, 0)) < 0) {
         if (ret == -EBUSY && (retries++ < 5)) {
@@ -6347,8 +6347,8 @@ qemudDomainDetachNetDevice(struct qemud_driver *driver,
 
 #if WITH_MACVTAP
     if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
-        int dummy;
-        delMacvtapByMACAddress(detach->mac, &dummy);
+        if (detach->ifname)
+            delMacvtap(detach->ifname);
     }
 #endif
 

diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 194ad366c7cfb5ad8402a941381f05e61a414622..88a64ceb883371ad41b63345cccdec8b85c5ad89 100644 (file)
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -40,7 +40,6 @@
 #include <linux/rtnetlink.h>
 #include <linux/if_tun.h>
 
-#include "c-ctype.h"
 #include "util.h"
 #include "memory.h"
 #include "macvtap.h"
@@ -447,15 +446,14 @@ buffer_too_small:
 
 
 static int
-link_del(const char *type,
-         const char *name)
+link_del(const char *name)
 {
     int rc = 0;
     char nlmsgbuf[256];
     struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp;
     struct nlmsgerr *err;
     char rtattbuf[64];
-    struct rtattr *rta, *rta1;
+    struct rtattr *rta;
     struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
     char *recvbuf = NULL;
     int recvbuflen;
@@ -467,23 +465,6 @@ link_del(const char *type,
     if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo)))
         goto buffer_too_small;
 
-    rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_LINKINFO, NULL, 0);
-    if (!rta)
-        goto buffer_too_small;
-
-    if (!(rta1 = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len)))
-        goto buffer_too_small;
-
-    rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_INFO_KIND,
-                       type, strlen(type));
-    if (!rta)
-        goto buffer_too_small;
-
-    if (!nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
-        goto buffer_too_small;
-
-    rta1->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)rta1;
-
     rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME,
                        name, strlen(name)+1);
     if (!rta)
@@ -633,7 +614,8 @@ macvtapModeFromInt(enum virDomainNetdevMacvtapType mode)
 }
 
 
-/* openMacvtapTap:
+/**
+ * openMacvtapTap:
  * Create an instance of a macvtap device and open its tap character
  * device.
  * @conn: Pointer to virConnect object
@@ -707,14 +689,17 @@ create_name:
     rc = ifUp(cr_ifname, 1);
     if (rc != 0) {
         virReportSystemError(errno,
-                             _("cannot 'up' interface %s"), cr_ifname);
+                             _("cannot 'up' interface %s -- another "
+                             "macvtap device may be 'up' and have the same "
+                             "MAC address"),
+                             cr_ifname);
         rc = -1;
         goto link_del_exit;
     }
 
     rc = openTap(cr_ifname, 10);
 
-    if (rc > 0)
+    if (rc >= 0)
         *res_ifname = strdup(cr_ifname);
     else
         goto link_del_exit;
@@ -722,79 +707,22 @@ create_name:
     return rc;
 
 link_del_exit:
-    link_del(type, ifname);
+    link_del(cr_ifname);
 
     return rc;
 }
 
 
-/* Delete a macvtap type of interface given the MAC address. This
- * function will delete all macvtap type of interfaces that have the
- * given MAC address.
- * @macaddress : Pointer to 6 bytes holding the MAC address of the
- *    macvtap device(s) to destroy
+/**
+ * delMacvtapByName:
+ * @ifname : The name of the macvtap interface
  *
- * Returns 0 in case of success, negative value in case of error.
+ * Delete an interface given its name.
  */
-int
-delMacvtapByMACAddress(const unsigned char *macaddress,
-                       int *hasBusyDev)
+void
+delMacvtap(const char *ifname)
 {
-    struct ifreq ifr;
-    FILE *file;
-    char *ifname, *pos;
-    char buffer[1024];
-    off_t oldpos = 0;
-    int tapfd;
-
-    *hasBusyDev = 0;
-
-    file = fopen("/proc/net/dev", "r");
-
-    if (!file) {
-        virReportSystemError(errno, "%s",
-                             _("cannot open file to read network interfaces "
-                             "from"));
-        return -1;
-    }
-
-    int sock = socket(AF_INET, SOCK_DGRAM, 0);
-    if (sock < 0) {
-        virReportSystemError(errno, "%s",
-                             _("cannot open socket"));
-        goto sock_err;
-    }
-
-    while (NULL != (ifname = fgets(buffer, sizeof(buffer), file))) {
-        if (c_isspace(ifname[0]))
-            ifname++;
-        if ((pos = strchr(ifname, ':')) != NULL) {
-            pos[0] = 0;
-            if (virStrncpy(ifr.ifr_name, ifname, strlen(ifname),
-                           sizeof(ifr.ifr_name)) == NULL)
-                continue;
-            if (ioctl(sock, SIOCGIFHWADDR, (char *)&ifr) >= 0) {
-                if (memcmp(&ifr.ifr_hwaddr.sa_data[0], macaddress, 6) == 0) {
-                    tapfd = openTap(ifname, 0);
-                    if (tapfd > 0) {
-                        close(tapfd);
-                        ifUp(ifname, 0);
-                        if (link_del("macvtap", ifname) == 0)
-                            fseeko(file, oldpos, SEEK_SET);
-                    } else {
-                        *hasBusyDev = 1;
-                    }
-                }
-            }
-        }
-        oldpos = ftello(file);
-    }
-
-    close(sock);
-sock_err:
-    fclose(file);
-
-    return 0;
+    link_del(ifname);
 }
 
 #endif

diff --git a/src/util/macvtap.h b/src/util/macvtap.h
index aa961dd22aec5e7a6f8a87d65b8233194e0acd2c..a1a0a7a8afed25b4da88b4c06847a0d716dfef88 100644 (file)
--- a/src/util/macvtap.h
+++ b/src/util/macvtap.h
@@ -35,8 +35,7 @@ int openMacvtapTap(virConnectPtr conn,
                    int mode,
                    char **res_ifname);
 
-int delMacvtapByMACAddress(const unsigned char *macaddress,
-                           int *hasBusyDev);
+void delMacvtap(const char *name);
 
 #endif /* WITH_MACVTAP */