From 4238e5f0783c63802de79bc5ed2a1f49673ef2a3 Mon Sep 17 00:00:00 2001

Message-Id: <4238e5f0783c63802de79bc5ed2a1f49673ef2a3@dist-git>

From: Laine Stump <laine@redhat.com>

Date: Sat, 12 Dec 2020 22:04:51 -0500

Subject: [PATCH] util: replace macvtap name reservation bitmap with a simple

 counter

There have been some reports that, due to libvirt always trying to

assign the lowest numbered macvtap / tap device name possible, a new

guest would sometimes be started using the same tap device name as

previously used by another guest that is in the process of being

destroyed *as the new guest is starting.

In some cases this has led to, for example, the old guest's

qemuProcessStop() code deleting a port from an OVS switch that had

just been re-added by the new guest (because the port name is based on

only the device name using the port). Similar problems can happen (and

I believe have) with nwfilter rules and bandwidth rules (which are

both instantiated based on the name of the tap device).

A couple patches have been previously proposed to change the ordering

of startup and shutdown processing, or to put a mutex around

everything related to the tap/macvtap device name usage, but in the

end no matter what you do there will still be possible holes, because

the device could be deleted outside libvirt's control (for example,

regular tap devices are automatically deleted when the qemu process

terminates, and that isn't always initiated by libvirt but could

instead happen completely asynchronously - libvirt then has no control

over the ordering of shutdown operations, and no opportunity to

protect it with a mutex.)

But this only happens if a new device is created at the same time as

one is being deleted. We can effectively eliminate the chance of this

happening if we end the practice of always looking for the lowest

numbered available device name, and instead just keep an integer that

is incremented each time we need a new device name. At some point it

will need to wrap back around to 0 (in order to avoid the IFNAMSIZ 15

character limit if nothing else), and we can't guarantee that the new

name really will be the *least* recently used name, but "math"

suggests that it will be *much* less common that we'll try to re-use

the *most* recently used name.

This patch implements such a counter for macvtap/macvlan, replacing

the existing, and much more complicated, "ID reservation" system. The

counter is set according to whatever macvtap/macvlan devices are

already in use by guests when libvirtd is started, incremented each

time a new device name is needed, and wraps back to 0 when either

INT_MAX is reached, or when the resulting device name would be longer

than IFNAMSIZ-1 characters (which actually is what happens when the

template for the device name is "maccvtap%d"). The result is that no

macvtap name will be re-used until the host has created (and possibly

destroyed) 99,999,999 devices.

Signed-off-by: Laine Stump <laine@redhat.com>

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

(cherry picked from commit d7f38beb2ee072f1f19bb91fbafc9182ce9b069e)

https://bugzilla.redhat.com/1874304

Signed-off-by: Laine Stump <laine@redhat.com>

Message-Id: <20201213030453.48851-2-laine@redhat.com>

Reviewed-by: Michal Privoznik <mprivozn@redhat.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 | 403 +++++++++++++-----------------------

 src/util/virnetdevmacvlan.h |   6 +-

 6 files changed, 145 insertions(+), 271 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms

index fdd104cd25..1c66c40f86 100644

--- a/src/libvirt_private.syms

+++ b/src/libvirt_private.syms

@@ -2604,7 +2604,6 @@ virNetDevMacVLanDelete;

 virNetDevMacVLanDeleteWithVPortProfile;

 virNetDevMacVLanIsMacvtap;

 virNetDevMacVLanModeTypeFromString;

-virNetDevMacVLanReleaseName;

 virNetDevMacVLanReserveName;

 virNetDevMacVLanRestartWithVPortProfile;

 virNetDevMacVLanTapOpen;

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c

index 1449795494..9269e9b475 100644

--- a/src/libxl/libxl_driver.c

+++ b/src/libxl/libxl_driver.c

@@ -367,7 +367,7 @@ libxlReconnectNotifyNets(virDomainDefPtr def)

          * impolite.

          */

         if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)

-           ignore_value(virNetDevMacVLanReserveName(net->ifname, false));

+            virNetDevMacVLanReserveName(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 0a9ccdf9ec..114c40b0ea 100644

--- a/src/lxc/lxc_process.c

+++ b/src/lxc/lxc_process.c

@@ -1638,7 +1638,7 @@ virLXCProcessReconnectNotifyNets(virDomainDefPtr def)

          * impolite.

          */

         if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)

-           ignore_value(virNetDevMacVLanReserveName(net->ifname, false));

+            virNetDevMacVLanReserveName(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 95c0315e53..b49a463c02 100644

--- a/src/qemu/qemu_process.c

+++ b/src/qemu/qemu_process.c

@@ -3288,7 +3288,7 @@ qemuProcessNotifyNets(virDomainDefPtr def)

          * impolite.

          */

         if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)

-           ignore_value(virNetDevMacVLanReserveName(net->ifname, false));

+            virNetDevMacVLanReserveName(net->ifname);

         if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {

             if (!conn && !(conn = virGetConnectNetwork()))

diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c

index 3ca568fb44..7046cbb04e 100644

--- a/src/util/virnetdevmacvlan.c

+++ b/src/util/virnetdevmacvlan.c

@@ -47,6 +47,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode,

 # include <net/if.h>

 # include <linux/if_tun.h>

+# include <math.h>

 /* Older kernels lacked this enum value.  */

 # if !HAVE_DECL_MACVLAN_MODE_PASSTHRU

@@ -70,211 +71,121 @@ VIR_LOG_INIT("util.netdevmacvlan");

     ((flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ? \

      VIR_NET_GENERATED_MACVTAP_PREFIX : VIR_NET_GENERATED_MACVLAN_PREFIX)

-# define MACVLAN_MAX_ID 8191

 virMutex virNetDevMacVLanCreateMutex = VIR_MUTEX_INITIALIZER;

-virBitmapPtr macvtapIDs = NULL;

-virBitmapPtr macvlanIDs = NULL;

-

-static int

-virNetDevMacVLanOnceInit(void)

-{

-    if (!macvtapIDs &&

-        !(macvtapIDs = virBitmapNew(MACVLAN_MAX_ID + 1)))

-        return -1;

-    if (!macvlanIDs &&

-        !(macvlanIDs = virBitmapNew(MACVLAN_MAX_ID + 1)))

-        return -1;

-    return 0;

-}

-

-VIR_ONCE_GLOBAL_INIT(virNetDevMacVLan);

+static int virNetDevMacVTapLastID = -1;

+static int virNetDevMacVLanLastID = -1;

-/**

- * virNetDevMacVLanReserveID:

- *

- *  @id: id 0 - MACVLAN_MAX_ID+1 to reserve (or -1 for "first free")

- *  @flags: set VIR_NETDEV_MACVLAN_CREATE_WITH_TAP for macvtapN else macvlanN

- *  @quietFail: don't log an error if this name is already in-use

- *  @nextFree: reserve the next free ID *after* @id rather than @id itself

- *

- *  Reserve the indicated ID in the appropriate bitmap, or find the

- *  first free ID if @id is -1.

- *

- *  Returns newly reserved ID# on success, or -1 to indicate failure.

- */

-static int

-virNetDevMacVLanReserveID(int id, unsigned int flags,

-                          bool quietFail, bool nextFree)

+static void

+virNetDevMacVLanReserveNameInternal(const char *name)

 {

-    virBitmapPtr bitmap;

-

-    if (virNetDevMacVLanInitialize() < 0)

-       return -1;

-

-    bitmap = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?

-        macvtapIDs :  macvlanIDs;

+    unsigned int id;

+    const char *idstr = NULL;

+    int *lastID = NULL;

+    int len;

-    if (id > MACVLAN_MAX_ID) {

-        virReportError(VIR_ERR_INTERNAL_ERROR,

-                       _("can't use name %s%d - out of range 0-%d"),

-                       VIR_NET_GENERATED_PREFIX, id, MACVLAN_MAX_ID);

-        return -1;

+    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;

     }

-    if ((id < 0 || nextFree) &&

-        (id = virBitmapNextClearBit(bitmap, id)) < 0) {

-        virReportError(VIR_ERR_INTERNAL_ERROR,

-                       _("no unused %s names available"),

-                       VIR_NET_GENERATED_PREFIX);

-        return -1;

-    }

+    VIR_INFO("marking device in use: '%s'", name);

-    if (virBitmapIsBitSet(bitmap, id)) {

-        if (quietFail) {

-            VIR_INFO("couldn't reserve name %s%d - already in use",

-                     VIR_NET_GENERATED_PREFIX, id);

-        } else {

-            virReportError(VIR_ERR_INTERNAL_ERROR,

-                           _("couldn't reserve name %s%d - already in use"),

-                           VIR_NET_GENERATED_PREFIX, id);

-        }

-        return -1;

-    }

+    idstr = name + len;

-    if (virBitmapSetBit(bitmap, id) < 0) {

-        virReportError(VIR_ERR_INTERNAL_ERROR,

-                       _("couldn't mark %s%d as used"),

-                       VIR_NET_GENERATED_PREFIX, id);

-        return -1;

+    if (virStrToLong_ui(idstr, NULL, 10, &id) >= 0) {

+        if (*lastID < (int)id)

+            *lastID = id;

     }

-

-    VIR_INFO("reserving device %s%d", VIR_NET_GENERATED_PREFIX, id);

-    return id;

 }

 /**

- * virNetDevMacVLanReleaseID:

- *  @id: id 0 - MACVLAN_MAX_ID+1 to release

+ * virNetDevMacVLanReserveName:

+ * @name: name of an existing macvtap/macvlan device

  *

- *  Returns 0 for success or -1 for failure.

+ * 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.

  */

-static int

-virNetDevMacVLanReleaseID(int id, unsigned int flags)

+void

+virNetDevMacVLanReserveName(const char *name)

 {

-    virBitmapPtr bitmap;

-

-    if (virNetDevMacVLanInitialize() < 0)

-        return 0;

-

-    bitmap = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?

-        macvtapIDs :  macvlanIDs;

-

-    if (id > MACVLAN_MAX_ID) {

-        virReportError(VIR_ERR_INTERNAL_ERROR,

-                       _("can't free name %s%d - out of range 0-%d"),

-                       VIR_NET_GENERATED_PREFIX, id, MACVLAN_MAX_ID);

-        return -1;

-    }

-

-    if (id < 0)

-        return 0;

-

-    VIR_INFO("releasing %sdevice %s%d",

-             virBitmapIsBitSet(bitmap, id) ? "" : "unreserved",

-             VIR_NET_GENERATED_PREFIX, id);

-

-    if (virBitmapClearBit(bitmap, id) < 0) {

-        virReportError(VIR_ERR_INTERNAL_ERROR,

-                       _("couldn't mark %s%d as unused"),

-                       VIR_NET_GENERATED_PREFIX, id);

-        return -1;

-    }

-    return 0;

+    virMutexLock(&virNetDevMacVLanCreateMutex);

+    virNetDevMacVLanReserveNameInternal(name);

+    virMutexUnlock(&virNetDevMacVLanCreateMutex);

 }

 /**

- * virNetDevMacVLanReserveName:

- *

- *  @name: already-known name of device

- *  @quietFail: don't log an error if this name is already in-use

+ * virNetDevMacVLanGenerateName:

+ * @ifname: pointer to pointer to string containing template

+ * @lastID: counter to add to the template to form the name

  *

- *  Extract the device type and id from a macvtap/macvlan device name

- *  and mark the appropriate position as in-use in the appropriate

- *  bitmap.

+ * 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 reserved ID# on success, -1 on failure, -2 if the name

- *  doesn't fit the auto-pattern (so not reserveable).

+ * Returns 0 on success, -1 on failure.

  */

-int

-virNetDevMacVLanReserveName(const char *name, bool quietFail)

+static int

+virNetDevMacVLanGenerateName(char **ifname, unsigned int flags)

 {

-    unsigned int id;

-    unsigned int flags = 0;

-    const char *idstr = NULL;

+    const char *prefix;

+    const char *iftemplate;

+    int *lastID;

+    int id;

+    double maxIDd;

+    int maxID = INT_MAX;

+    int attempts = 0;

-    if (virNetDevMacVLanInitialize() < 0)

-       return -1;

-

-    if (STRPREFIX(name, VIR_NET_GENERATED_MACVTAP_PREFIX)) {

-        idstr = name + strlen(VIR_NET_GENERATED_MACVTAP_PREFIX);

-        flags |= VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;

-    } else if (STRPREFIX(name, VIR_NET_GENERATED_MACVLAN_PREFIX)) {

-        idstr = name + strlen(VIR_NET_GENERATED_MACVLAN_PREFIX);

+    if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) {

+        prefix = VIR_NET_GENERATED_MACVTAP_PREFIX;

+        iftemplate = VIR_NET_GENERATED_MACVTAP_PREFIX "%d";

+        lastID = &virNetDevMacVTapLastID;

     } else {

-        return -2;

+        prefix = VIR_NET_GENERATED_MACVLAN_PREFIX;

+        iftemplate = VIR_NET_GENERATED_MACVLAN_PREFIX "%d";

+        lastID = &virNetDevMacVLanLastID;

     }

-    if (virStrToLong_ui(idstr, NULL, 10, &id) < 0) {

-        virReportError(VIR_ERR_INTERNAL_ERROR,

-                       _("couldn't get id value from macvtap device name %s"),

-                       name);

-        return -1;

-    }

-    return virNetDevMacVLanReserveID(id, flags, quietFail, false);

-}

+    maxIDd = pow(10, IFNAMSIZ - 1 - strlen(prefix));

+    if (maxIDd <= (double)INT_MAX)

+        maxID = (int)maxIDd;

+    do {

+        g_autofree char *try = NULL;

-/**

- * virNetDevMacVLanReleaseName:

- *

- *  @name: already-known name of device

- *

- *  Extract the device type and id from a macvtap/macvlan device name

- *  and mark the appropriate position as in-use in the appropriate

- *  bitmap.

- *

- *  returns 0 on success, -1 on failure

- */

-int

-virNetDevMacVLanReleaseName(const char *name)

-{

-    unsigned int id;

-    unsigned int flags = 0;

-    const char *idstr = NULL;

+        id = ++(*lastID);

-    if (virNetDevMacVLanInitialize() < 0)

-       return -1;

+        /* reset before overflow */

+        if (*lastID == maxID)

+            *lastID = -1;

-    if (STRPREFIX(name, VIR_NET_GENERATED_MACVTAP_PREFIX)) {

-        idstr = name + strlen(VIR_NET_GENERATED_MACVTAP_PREFIX);

-        flags |= VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;

-    } else if (STRPREFIX(name, VIR_NET_GENERATED_MACVLAN_PREFIX)) {

-        idstr = name + strlen(VIR_NET_GENERATED_MACVLAN_PREFIX);

-    } else {

-        return 0;

-    }

+        try = g_strdup_printf(iftemplate, id);

-    if (virStrToLong_ui(idstr, NULL, 10, &id) < 0) {

-        virReportError(VIR_ERR_INTERNAL_ERROR,

-                       _("couldn't get id value from macvtap device name %s"),

-                       name);

-        return -1;

-    }

-    return virNetDevMacVLanReleaseID(id, flags);

+        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;

 }

@@ -321,8 +232,7 @@ virNetDevMacVLanCreate(const char *ifname,

                        const char *type,

                        const virMacAddr *macaddress,

                        const char *srcdev,

-                       uint32_t macvlan_mode,

-                       int *retry)

+                       uint32_t macvlan_mode)

 {

     int error = 0;

     int ifindex = 0;

@@ -331,7 +241,6 @@ virNetDevMacVLanCreate(const char *ifname,

         .mac = macaddress,

     };

-    *retry = 0;

     if (virNetDevGetIndex(srcdev, &ifindex) < 0)

         return -1;

@@ -339,17 +248,15 @@ virNetDevMacVLanCreate(const char *ifname,

     data.ifindex = &ifindex;

     if (virNetlinkNewLink(ifname, type, &data, &error) < 0) {

         char macstr[VIR_MAC_STRING_BUFLEN];

-        if (error == -EEXIST)

-            *retry = 1;

-        else if (error < 0)

-            virReportSystemError(-error,

-                                 _("error creating %s interface %s@%s (%s)"),

-                                 type, ifname, srcdev,

-                                 virMacAddrFormat(macaddress, macstr));

+        virReportSystemError(-error,

+                             _("error creating %s interface %s@%s (%s)"),

+                             type, ifname, srcdev,

+                             virMacAddrFormat(macaddress, macstr));

         return -1;

     }

+    VIR_INFO("created device: '%s'", ifname);

     return 0;

 }

@@ -364,6 +271,7 @@ virNetDevMacVLanCreate(const char *ifname,

  */

 int virNetDevMacVLanDelete(const char *ifname)

 {

+    VIR_INFO("delete device: '%s'", ifname);

     return virNetlinkDelLink(ifname, NULL);

 }

@@ -904,13 +812,8 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,

                                        unsigned int flags)

 {

     const char *type = VIR_NET_GENERATED_PREFIX;

-    const char *pattern = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?

-        VIR_NET_GENERATED_MACVTAP_PATTERN : VIR_NET_GENERATED_MACVLAN_PATTERN;

-    int reservedID = -1;

-    char ifname[IFNAMSIZ];

-    int retries, do_retry = 0;

+    g_autofree char *ifname = NULL;

     uint32_t macvtapMode;

-    const char *ifnameCreated = NULL;

     int vf = -1;

     bool vnet_hdr = flags & VIR_NETDEV_MACVLAN_VNET_HDR;

@@ -945,6 +848,8 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,

            return -1;

     }

+    virMutexLock(&virNetDevMacVLanCreateMutex);

+

     if (ifnameRequested) {

         int rc;

         bool isAutoName

@@ -952,97 +857,81 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,

                STRPREFIX(ifnameRequested, VIR_NET_GENERATED_MACVLAN_PREFIX));

         VIR_INFO("Requested macvtap device name: %s", ifnameRequested);

-        virMutexLock(&virNetDevMacVLanCreateMutex);

         if ((rc = virNetDevExists(ifnameRequested)) < 0) {

             virMutexUnlock(&virNetDevMacVLanCreateMutex);

             return -1;

         }

+

         if (rc) {

-            if (isAutoName)

-                goto create_name;

-            virReportSystemError(EEXIST,

-                                 _("Unable to create %s device %s"),

-                                 type, ifnameRequested);

-            virMutexUnlock(&virNetDevMacVLanCreateMutex);

-            return -1;

-        }

-        if (isAutoName &&

-            (reservedID = virNetDevMacVLanReserveName(ifnameRequested, true)) < 0) {

-            reservedID = -1;

-            goto create_name;

-        }

+            /* ifnameRequested is already being used */

-        if (virNetDevMacVLanCreate(ifnameRequested, type, macaddress,

-                                   linkdev, macvtapMode, &do_retry) < 0) {

-            if (isAutoName) {

-                virNetDevMacVLanReleaseName(ifnameRequested);

-                reservedID = -1;

-                goto create_name;

+            if (!isAutoName) {

+                virReportSystemError(EEXIST,

+                                     _("Unable to create device '%s'"),

+                                     ifnameRequested);

+                virMutexUnlock(&virNetDevMacVLanCreateMutex);

+                return -1;

+            }

+        } else {

+

+            /* ifnameRequested is available. try to open it */

+

+            virNetDevMacVLanReserveNameInternal(ifnameRequested);

+

+            if (virNetDevMacVLanCreate(ifnameRequested, type, macaddress,

+                                       linkdev, macvtapMode) == 0) {

+

+                /* virNetDevMacVLanCreate() was successful - use this name */

+                ifname = g_strdup(ifnameRequested);

+

+            } else if (!isAutoName) {

+                /* coudn't open ifnameRequested, but it wasn't an

+                 * autogenerated named, so there is nothing else to

+                 * try - fail and return.

+                 */

+                virMutexUnlock(&virNetDevMacVLanCreateMutex);

+                return -1;

             }

-            virMutexUnlock(&virNetDevMacVLanCreateMutex);

-            return -1;

         }

-        /* virNetDevMacVLanCreate() was successful - use this name */

-        ifnameCreated = ifnameRequested;

- create_name:

-        virMutexUnlock(&virNetDevMacVLanCreateMutex);

     }

-    retries = MACVLAN_MAX_ID;

-    while (!ifnameCreated && retries) {

-        virMutexLock(&virNetDevMacVLanCreateMutex);

-        reservedID = virNetDevMacVLanReserveID(reservedID, flags, false, true);

-        if (reservedID < 0) {

+    if (!ifname) {

+        /* ifnameRequested was NULL, or it was an already in use

+         * 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);

             return -1;

         }

-        g_snprintf(ifname, sizeof(ifname), pattern, reservedID);

-        if (virNetDevMacVLanCreate(ifname, type, macaddress, linkdev,

-                                   macvtapMode, &do_retry) < 0) {

-            virNetDevMacVLanReleaseID(reservedID, flags);

-            virMutexUnlock(&virNetDevMacVLanCreateMutex);

-            if (!do_retry)

-                return -1;

-            VIR_INFO("Device %s wasn't reserved but already existed, skipping",

-                     ifname);

-            retries--;

-            continue;

-        }

-        ifnameCreated = ifname;

-        virMutexUnlock(&virNetDevMacVLanCreateMutex);

     }

-    if (!ifnameCreated) {

-        virReportError(VIR_ERR_INTERNAL_ERROR,

-                       _("Too many unreserved %s devices in use"),

-                       type);

-        return -1;

-    }

+    /* all done creating the device */

+    virMutexUnlock(&virNetDevMacVLanCreateMutex);

-    if (virNetDevVPortProfileAssociate(ifnameCreated,

+    if (virNetDevVPortProfileAssociate(ifname,

                                        virtPortProfile,

                                        macaddress,

                                        linkdev,

                                        vf,

-                                       vmuuid, vmOp, false) < 0)

+                                       vmuuid, vmOp, false) < 0) {

         goto link_del_exit;

+    }

     if (flags & VIR_NETDEV_MACVLAN_CREATE_IFUP) {

-        if (virNetDevSetOnline(ifnameCreated, true) < 0)

+        if (virNetDevSetOnline(ifname, true) < 0)

             goto disassociate_exit;

     }

     if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) {

-        if (virNetDevMacVLanTapOpen(ifnameCreated, tapfd, tapfdSize) < 0)

+        if (virNetDevMacVLanTapOpen(ifname, tapfd, tapfdSize) < 0)

             goto disassociate_exit;

         if (virNetDevMacVLanTapSetup(tapfd, tapfdSize, vnet_hdr) < 0)

             goto disassociate_exit;

-

-        *ifnameResult = g_strdup(ifnameCreated);

-    } else {

-        *ifnameResult = g_strdup(ifnameCreated);

     }

     if (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_CREATE ||

@@ -1051,17 +940,18 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,

          * a saved image) - migration and libvirtd restart are handled

          * elsewhere.

          */

-        if (virNetDevMacVLanVPortProfileRegisterCallback(ifnameCreated, macaddress,

+        if (virNetDevMacVLanVPortProfileRegisterCallback(ifname, macaddress,

                                                          linkdev, vmuuid,

                                                          virtPortProfile,

                                                          vmOp) < 0)

             goto disassociate_exit;

     }

+    *ifnameResult = g_steal_pointer(&ifname);

     return 0;

  disassociate_exit:

-    ignore_value(virNetDevVPortProfileDisassociate(ifnameCreated,

+    ignore_value(virNetDevVPortProfileDisassociate(ifname,

                                                    virtPortProfile,

                                                    macaddress,

                                                    linkdev,

@@ -1071,9 +961,7 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested,

         VIR_FORCE_CLOSE(tapfd[tapfdSize]);

  link_del_exit:

-    ignore_value(virNetDevMacVLanDelete(ifnameCreated));

-    virNetDevMacVLanReleaseName(ifnameCreated);

-

+    ignore_value(virNetDevMacVLanDelete(ifname));

     return -1;

 }

@@ -1107,7 +995,6 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname,

             ret = -1;

         if (virNetDevMacVLanDelete(ifname) < 0)

             ret = -1;

-        virNetDevMacVLanReleaseName(ifname);

     }

     if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) {

@@ -1182,8 +1069,7 @@ 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,

-                           int *retry G_GNUC_UNUSED)

+                           uint32_t macvlan_mode G_GNUC_UNUSED)

 {

     virReportSystemError(ENOSYS, "%s",

                          _("Cannot create macvlan devices on this platform"));

@@ -1272,18 +1158,9 @@ int virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname G_GNUC_UNUSE

     return -1;

 }

-int virNetDevMacVLanReleaseName(const char *name G_GNUC_UNUSED)

+void virNetDevMacVLanReserveName(const char *name G_GNUC_UNUSED)

 {

     virReportSystemError(ENOSYS, "%s",

                          _("Cannot create macvlan devices on this platform"));

-    return -1;

-}

-

-int virNetDevMacVLanReserveName(const char *name G_GNUC_UNUSED,

-                                bool quietFail G_GNUC_UNUSED)

-{

-    virReportSystemError(ENOSYS, "%s",

-                         _("Cannot create macvlan devices on this platform"));

-    return -1;

 }

 #endif /* ! WITH_MACVTAP */

diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h

index fc1bb018a2..48800a8fcf 100644

--- a/src/util/virnetdevmacvlan.h

+++ b/src/util/virnetdevmacvlan.h

@@ -54,8 +54,7 @@ typedef enum {

 #define VIR_NET_GENERATED_MACVTAP_PREFIX "macvtap"

 #define VIR_NET_GENERATED_MACVLAN_PREFIX "macvlan"

-int virNetDevMacVLanReserveName(const char *name, bool quietfail);