[PATCHv3 3/5] netdevmacvlan: Use helper function to create unique macvlan/macvtap name

Shi Lei shi_lei at massclouds.com
Mon Dec 14 01:50:34 UTC 2020


Simplify ReserveName/GenerateName for macvlan and macvtap by using
common functions.

Signed-off-by: Shi Lei <shi_lei at massclouds.com>
---
 src/libvirt_private.syms    |   1 -
 src/libxl/libxl_driver.c    |   2 +-
 src/lxc/lxc_process.c       |   2 +-
 src/qemu/qemu_process.c     |   2 +-
 src/util/virnetdevmacvlan.c | 177 +++++-------------------------------
 src/util/virnetdevmacvlan.h |  14 +--
 6 files changed, 27 insertions(+), 171 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 64ef01e1..4d6ae84b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2655,7 +2655,6 @@ virNetDevMacVLanDelete;
 virNetDevMacVLanDeleteWithVPortProfile;
 virNetDevMacVLanIsMacvtap;
 virNetDevMacVLanModeTypeFromString;
-virNetDevMacVLanReserveName;
 virNetDevMacVLanRestartWithVPortProfile;
 virNetDevMacVLanTapOpen;
 virNetDevMacVLanTapSetup;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 6af274cb..3488d7ec 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -364,7 +364,7 @@ libxlReconnectNotifyNets(virDomainDefPtr def)
          * impolite.
          */
         if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)
-            virNetDevMacVLanReserveName(net->ifname);
+            virNetDevReserveName(net->ifname);
 
         if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
             if (!conn && !(conn = virGetConnectNetwork()))
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 0f818e2e..eb29431e 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1641,7 +1641,7 @@ virLXCProcessReconnectNotifyNets(virDomainDefPtr def)
          * impolite.
          */
         if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)
-            virNetDevMacVLanReserveName(net->ifname);
+            virNetDevReserveName(net->ifname);
 
         if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
             if (!conn && !(conn = virGetConnectNetwork()))
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1d54f201..6244ade4 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3390,7 +3390,7 @@ qemuProcessNotifyNets(virDomainDefPtr def)
          */
         switch (virDomainNetGetActualType(net)) {
         case VIR_DOMAIN_NET_TYPE_DIRECT:
-            virNetDevMacVLanReserveName(net->ifname);
+            virNetDevReserveName(net->ifname);
             break;
         case VIR_DOMAIN_NET_TYPE_BRIDGE:
         case VIR_DOMAIN_NET_TYPE_NETWORK:
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 72f0d670..36b13133 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -45,7 +45,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
 
 # include <net/if.h>
 # include <linux/if_tun.h>
-# include <math.h>
 
 # include "viralloc.h"
 # include "virlog.h"
@@ -59,129 +58,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,
 
 VIR_LOG_INIT("util.netdevmacvlan");
 
-# define VIR_NET_GENERATED_MACVTAP_PATTERN VIR_NET_GENERATED_MACVTAP_PREFIX "%d"
-# define VIR_NET_GENERATED_MACVLAN_PATTERN VIR_NET_GENERATED_MACVLAN_PREFIX "%d"
-# define VIR_NET_GENERATED_PREFIX \
-    ((flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ? \
-     VIR_NET_GENERATED_MACVTAP_PREFIX : VIR_NET_GENERATED_MACVLAN_PREFIX)
-
-
-virMutex virNetDevMacVLanCreateMutex = VIR_MUTEX_INITIALIZER;
-static int virNetDevMacVTapLastID = -1;
-static int virNetDevMacVLanLastID = -1;
-
-
-static void
-virNetDevMacVLanReserveNameInternal(const char *name)
-{
-    unsigned int id;
-    const char *idstr = NULL;
-    int *lastID = NULL;
-    int len;
-
-    if (STRPREFIX(name, VIR_NET_GENERATED_MACVTAP_PREFIX)) {
-        lastID = &virNetDevMacVTapLastID;
-        len = strlen(VIR_NET_GENERATED_MACVTAP_PREFIX);
-    } else if (STRPREFIX(name, VIR_NET_GENERATED_MACVLAN_PREFIX)) {
-        lastID = &virNetDevMacVTapLastID;
-        len = strlen(VIR_NET_GENERATED_MACVLAN_PREFIX);
-    } else {
-        return;
-    }
-
-    VIR_INFO("marking device in use: '%s'", name);
-
-    idstr = name + len;
-
-    if (virStrToLong_ui(idstr, NULL, 10, &id) >= 0) {
-        if (*lastID < (int)id)
-            *lastID = id;
-    }
-}
-
-
-/**
- * virNetDevMacVLanReserveName:
- * @name: name of an existing macvtap/macvlan device
- *
- * Set the value of virNetDevMacV(Lan|Tap)LastID to assure that any
- * new device created with an autogenerated name will use a number
- * higher than the number in the given device name.
- *
- * Returns nothing.
- */
-void
-virNetDevMacVLanReserveName(const char *name)
-{
-    virMutexLock(&virNetDevMacVLanCreateMutex);
-    virNetDevMacVLanReserveNameInternal(name);
-    virMutexUnlock(&virNetDevMacVLanCreateMutex);
-}
-
-
-/**
- * virNetDevMacVLanGenerateName:
- * @ifname: pointer to pointer to string containing template
- * @lastID: counter to add to the template to form the name
- *
- * generate a new (currently unused) name for a new macvtap/macvlan
- * device based on the template string in @ifname - replace %d with
- * ++(*counter), and keep trying new values until one is found
- * that doesn't already exist, or we've tried 10000 different
- * names. Once a usable name is found, replace the template with the
- * actual name.
- *
- * Returns 0 on success, -1 on failure.
- */
-static int
-virNetDevMacVLanGenerateName(char **ifname, unsigned int flags)
-{
-    const char *prefix;
-    const char *iftemplate;
-    int *lastID;
-    int id;
-    double maxIDd;
-    int maxID = INT_MAX;
-    int attempts = 0;
-
-    if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) {
-        prefix = VIR_NET_GENERATED_MACVTAP_PREFIX;
-        iftemplate = VIR_NET_GENERATED_MACVTAP_PREFIX "%d";
-        lastID = &virNetDevMacVTapLastID;
-    } else {
-        prefix = VIR_NET_GENERATED_MACVLAN_PREFIX;
-        iftemplate = VIR_NET_GENERATED_MACVLAN_PREFIX "%d";
-        lastID = &virNetDevMacVLanLastID;
-    }
-
-    maxIDd = pow(10, IFNAMSIZ - 1 - strlen(prefix));
-    if (maxIDd <= (double)INT_MAX)
-        maxID = (int)maxIDd;
-
-    do {
-        g_autofree char *try = NULL;
-
-        id = ++(*lastID);
-
-        /* reset before overflow */
-        if (*lastID == maxID)
-            *lastID = -1;
-
-        try = g_strdup_printf(iftemplate, id);
-
-        if (!virNetDevExists(try)) {
-            g_free(*ifname);
-            *ifname = g_steal_pointer(&try);
-            return 0;
-        }
-    } while (++attempts < 10000);
-
-    virReportError(VIR_ERR_INTERNAL_ERROR,
-                   _("no unused %s names available"),
-                   *ifname);
-    return -1;
-}
-
 
 /**
  * virNetDevMacVLanIsMacvtap:
@@ -209,13 +85,10 @@ virNetDevMacVLanIsMacvtap(const char *ifname)
  * virNetDevMacVLanCreate:
  *
  * @ifname: The name the interface is supposed to have; optional parameter
- * @type: The type of device, i.e., "macvtap", "macvlan"
  * @macaddress: The MAC address of the device
  * @srcdev: The name of the 'link' device
  * @macvlan_mode: The macvlan mode to use
- * @retry: Pointer to integer that will be '1' upon return if an interface
- *         with the same name already exists and it is worth to try
- *         again with a different name
+ * @flags: OR of virNetDevMacVLanCreateFlags.
  *
  * Create a macvtap device with the given properties.
  *
@@ -223,19 +96,21 @@ virNetDevMacVLanIsMacvtap(const char *ifname)
  */
 int
 virNetDevMacVLanCreate(const char *ifname,
-                       const char *type,
                        const virMacAddr *macaddress,
                        const char *srcdev,
-                       uint32_t macvlan_mode)
+                       uint32_t macvlan_mode,
+                       unsigned int flags)
 {
     int error = 0;
     int ifindex = 0;
+    const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+                       VIR_NET_GENERATED_MACVTAP_PREFIX :
+                       VIR_NET_GENERATED_MACVLAN_PREFIX;
     virNetlinkNewLinkData data = {
         .macvlan_mode = &macvlan_mode,
         .mac = macaddress,
     };
 
-
     if (virNetDevGetIndex(srcdev, &ifindex) < 0)
         return -1;
 
@@ -795,7 +670,6 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
                                        size_t tapfdSize,
                                        unsigned int flags)
 {
-    const char *type = VIR_NET_GENERATED_PREFIX;
     g_autofree char *ifname = NULL;
     uint32_t macvtapMode;
     int vf = -1;
@@ -832,8 +706,6 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
            return -1;
     }
 
-    virMutexLock(&virNetDevMacVLanCreateMutex);
-
     if (ifnameRequested) {
         int rc;
         bool isAutoName
@@ -842,10 +714,8 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
 
         VIR_INFO("Requested macvtap device name: %s", ifnameRequested);
 
-        if ((rc = virNetDevExists(ifnameRequested)) < 0) {
-            virMutexUnlock(&virNetDevMacVLanCreateMutex);
+        if ((rc = virNetDevExists(ifnameRequested)) < 0)
             return -1;
-        }
 
         if (rc) {
             /* ifnameRequested is already being used */
@@ -854,17 +724,16 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
                 virReportSystemError(EEXIST,
                                      _("Unable to create device '%s'"),
                                      ifnameRequested);
-                virMutexUnlock(&virNetDevMacVLanCreateMutex);
                 return -1;
             }
         } else {
 
             /* ifnameRequested is available. try to open it */
 
-            virNetDevMacVLanReserveNameInternal(ifnameRequested);
+            virNetDevReserveName(ifnameRequested);
 
-            if (virNetDevMacVLanCreate(ifnameRequested, type, macaddress,
-                                       linkdev, macvtapMode) == 0) {
+            if (virNetDevMacVLanCreate(ifnameRequested, macaddress,
+                                       linkdev, macvtapMode, flags) == 0) {
 
                 /* virNetDevMacVLanCreate() was successful - use this name */
                 ifname = g_strdup(ifnameRequested);
@@ -874,7 +743,6 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
                  * autogenerated named, so there is nothing else to
                  * try - fail and return.
                  */
-                virMutexUnlock(&virNetDevMacVLanCreateMutex);
                 return -1;
             }
         }
@@ -885,16 +753,19 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,
          * autogenerated name, so now we look for an unused
          * autogenerated name.
          */
-        if (virNetDevMacVLanGenerateName(&ifname, flags) < 0 ||
-            virNetDevMacVLanCreate(ifname, type, macaddress,
-                                   linkdev, macvtapMode) < 0) {
-            virMutexUnlock(&virNetDevMacVLanCreateMutex);
+        virNetDevGenNameType type;
+        if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP)
+            type = VIR_NET_DEV_GEN_NAME_MACVTAP;
+        else
+            type = VIR_NET_DEV_GEN_NAME_MACVLAN;
+
+        if (virNetDevGenerateName(&ifname, type) < 0 ||
+            virNetDevMacVLanCreate(ifname, macaddress,
+                                   linkdev, macvtapMode, flags) < 0)
             return -1;
-        }
     }
 
     /* all done creating the device */
-    virMutexUnlock(&virNetDevMacVLanCreateMutex);
 
     if (virNetDevVPortProfileAssociate(ifname,
                                        virtPortProfile,
@@ -1050,10 +921,10 @@ bool virNetDevMacVLanIsMacvtap(const char *ifname G_GNUC_UNUSED)
 }
 
 int virNetDevMacVLanCreate(const char *ifname G_GNUC_UNUSED,
-                           const char *type G_GNUC_UNUSED,
                            const virMacAddr *macaddress G_GNUC_UNUSED,
                            const char *srcdev G_GNUC_UNUSED,
-                           uint32_t macvlan_mode G_GNUC_UNUSED)
+                           uint32_t macvlan_mode G_GNUC_UNUSED,
+                           unsigned int fflags G_GNUC_UNUSED)
 {
     virReportSystemError(ENOSYS, "%s",
                          _("Cannot create macvlan devices on this platform"));
@@ -1141,10 +1012,4 @@ int virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname G_GNUC_UNUSE
                          _("Cannot create macvlan devices on this platform"));
     return -1;
 }
-
-void virNetDevMacVLanReserveName(const char *name G_GNUC_UNUSED)
-{
-    virReportSystemError(ENOSYS, "%s",
-                         _("Cannot create macvlan devices on this platform"));
-}
 #endif /* ! WITH_LIBNL */
diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h
index 48800a8f..0a013fc2 100644
--- a/src/util/virnetdevmacvlan.h
+++ b/src/util/virnetdevmacvlan.h
@@ -48,23 +48,15 @@ typedef enum {
    VIR_NETDEV_MACVLAN_VNET_HDR          = 1 << 2,
 } virNetDevMacVLanCreateFlags;
 
-/* libvirt will start macvtap/macvlan interface names with one of
- * these prefixes when it auto-generates the name
- */
-#define VIR_NET_GENERATED_MACVTAP_PREFIX "macvtap"
-#define VIR_NET_GENERATED_MACVLAN_PREFIX "macvlan"
-
-void virNetDevMacVLanReserveName(const char *name);
-
 bool virNetDevMacVLanIsMacvtap(const char *ifname)
    ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT G_GNUC_NO_INLINE;
 
 int virNetDevMacVLanCreate(const char *ifname,
-                           const char *type,
                            const virMacAddr *macaddress,
                            const char *srcdev,
-                           uint32_t macvlan_mode)
-    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
+                           uint32_t macvlan_mode,
+                           unsigned int flags)
+    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
     G_GNUC_WARN_UNUSED_RESULT;
 
 int virNetDevMacVLanDelete(const char *ifname)
-- 
2.25.1





More information about the libvir-list mailing list