diff --git a/libvirt.spec b/libvirt.spec index 759785119f8a98baf2c23b1890ac43aab962943a..b4aa18cd20dbd89c2cc29439be36970777a3b626 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -101,7 +101,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 6.2.0 -Release: 36 +Release: 37 License: LGPLv2+ URL: https://libvirt.org/ @@ -229,6 +229,7 @@ Patch0116: qemu-Fix-libvirt-hang-due-to-early-TPM-device-stop.patch Patch0117: qemu_tpm-Move-logfile-path-generation-into-a-separat.patch Patch0118: qemu_tpm-Generate-log-file-path-among-with-storage-p.patch Patch0119: virtpm-Fix-path-handling-in-virTPMEmulatorInit.patch + Patch0120: qemuMonitorJSONSetMigrationParams-Take-double-pointe.patch Patch0121: qemuMonitorJSONAddObject-Take-double-pointer-for-pro.patch Patch0122: qemuMonitorJSONMakeCommandInternal-Clear-arguments-w.patch @@ -246,6 +247,7 @@ Patch0133: qemu-command-Use-JSON-for-QAPIfied-object-directly.patch Patch0134: tests-qemuxml2argv-Validate-generation-of-JSON-props.patch Patch0135: qemu-capabilities-Enable-detection-of-QEMU_CAPS_OBJE.patch Patch0136: apparmor-Permit-new-capabilities-required-by-libvirt.patch +Patch0137: util-replace-macvtap-name-reservation-bitmap.patch Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-daemon-config-network = %{version}-%{release} @@ -1980,6 +1982,9 @@ exit 0 %changelog +* Wed Mar 30 2022 lijunwei +- util: replace macvtap name reservation bitmap with a simple counter + * Thu Mar 24 2022 yezengruan - apparmor: Permit new capabilities required by libvirtd diff --git a/util-replace-macvtap-name-reservation-bitmap.patch b/util-replace-macvtap-name-reservation-bitmap.patch new file mode 100644 index 0000000000000000000000000000000000000000..e75e91ddcecf12ba9b25511a53e297ad8f290ba2 --- /dev/null +++ b/util-replace-macvtap-name-reservation-bitmap.patch @@ -0,0 +1,729 @@ +From 6c37f42d08e8e45cfedd6abf441749a5f26defee Mon Sep 17 00:00:00 2001 +From: Laine Stump +Date: Sun, 23 Aug 2020 14:57:19 -0400 +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 +Reviewed-by: Michal Privoznik +--- + 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 e276f55bb1..d16129196d 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 7ec4fcc3d1..ca32c1d14d 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 5199f3806e..61d8a1a6e7 100644 +--- a/src/lxc/lxc_process.c ++++ b/src/lxc/lxc_process.c +@@ -1643,7 +1643,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 6b9f6fb860..e1b2d28da2 100644 +--- a/src/qemu/qemu_process.c ++++ b/src/qemu/qemu_process.c +@@ -3311,7 +3311,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 dcea93a5fe..66d2dafb56 100644 +--- a/src/util/virnetdevmacvlan.c ++++ b/src/util/virnetdevmacvlan.c +@@ -45,6 +45,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, + + # include + # include ++# include + + /* Older kernels lacked this enum value. */ + # if !HAVE_DECL_MACVLAN_MODE_PASSTHRU +@@ -69,211 +70,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; + } + + +@@ -320,8 +231,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; +@@ -330,7 +240,6 @@ virNetDevMacVLanCreate(const char *ifname, + .mac = macaddress, + }; + +- *retry = 0; + + if (virNetDevGetIndex(srcdev, &ifindex) < 0) + return -1; +@@ -338,17 +247,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; + } + +@@ -363,6 +270,7 @@ virNetDevMacVLanCreate(const char *ifname, + */ + int virNetDevMacVLanDelete(const char *ifname) + { ++ VIR_INFO("delete device: '%s'", ifname); + return virNetlinkDelLink(ifname, NULL); + } + +@@ -903,13 +811,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; + +@@ -944,6 +847,8 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, + return -1; + } + ++ virMutexLock(&virNetDevMacVLanCreateMutex); ++ + if (ifnameRequested) { + int rc; + bool isAutoName +@@ -951,97 +856,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 || +@@ -1050,17 +939,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, +@@ -1070,9 +960,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; + } + +@@ -1106,7 +994,6 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, + ret = -1; + if (virNetDevMacVLanDelete(ifname) < 0) + ret = -1; +- virNetDevMacVLanReleaseName(ifname); + } + + if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) { +@@ -1181,8 +1068,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")); +@@ -1271,18 +1157,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); +-int virNetDevMacVLanReleaseName(const char *name); ++void virNetDevMacVLanReserveName(const char *name); + + bool virNetDevMacVLanIsMacvtap(const char *ifname) + ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT G_GNUC_NO_INLINE; +@@ -64,8 +63,7 @@ int virNetDevMacVLanCreate(const char *ifname, + const char *type, + const virMacAddr *macaddress, + const char *srcdev, +- uint32_t macvlan_mode, +- int *retry) ++ uint32_t macvlan_mode) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) + G_GNUC_WARN_UNUSED_RESULT; + +-- +2.11.0 +