Extract ReserveName/GenerateName from netdevtap and netdevmacvlan as common helper functions. Signed-off-by: Shi Lei <shi_lei(a)massclouds.com&gt; --- src/bhyve/bhyve_command.c | 4 +- src/conf/domain_conf.c | 4 +- src/interface/interface_backend_udev.c | 2 +- src/libvirt_private.syms | 2 + src/qemu/qemu_interface.c | 8 +- src/util/virnetdev.c | 116 +++++++++++++++++++++++++ src/util/virnetdev.h | 27 +++++- src/util/virnetdevtap.c | 10 +-- 8 files changed, 158 insertions(+), 15 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index acf3a5a4..4cf98c0e 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -80,10 +80,10 @@ bhyveBuildNetArgStr(const virDomainDef *def, } if (!net->ifname || - STRPREFIX(net->ifname, VIR_NET_GENERATED_TAP_PREFIX) || + STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) || strchr(net->ifname, '%')) { VIR_FREE(net->ifname); - net->ifname = g_strdup(VIR_NET_GENERATED_TAP_PREFIX "%d"); + net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d"); } if (!dryRun) { diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 23415b32..403ecab8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12037,7 +12037,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, if (def->managed_tap != VIR_TRISTATE_BOOL_NO && ifname && (flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && - (STRPREFIX(ifname, VIR_NET_GENERATED_TAP_PREFIX) || + (STRPREFIX(ifname, VIR_NET_GENERATED_VNET_PREFIX) || STRPREFIX(ifname, VIR_NET_GENERATED_MACVTAP_PREFIX) || STRPREFIX(ifname, VIR_NET_GENERATED_MACVLAN_PREFIX) || (prefix && STRPREFIX(ifname, prefix)))) { @@ -26460,7 +26460,7 @@ virDomainNetDefFormat(virBufferPtr buf, if (def->ifname && (def->managed_tap == VIR_TRISTATE_BOOL_NO || !((flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) && - (STRPREFIX(def->ifname, VIR_NET_GENERATED_TAP_PREFIX) || + (STRPREFIX(def->ifname, VIR_NET_GENERATED_VNET_PREFIX) || STRPREFIX(def->ifname, VIR_NET_GENERATED_MACVTAP_PREFIX) || STRPREFIX(def->ifname, VIR_NET_GENERATED_MACVLAN_PREFIX) || (prefix && STRPREFIX(def->ifname, prefix)))))) { diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index 173c4fc3..6a94a450 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -544,7 +544,7 @@ udevBridgeScanDirFilter(const struct dirent *entry) * vnet%d. Improvements to this check are welcome. */ if (strlen(entry->d_name) >= 5) { - if (STRPREFIX(entry->d_name, VIR_NET_GENERATED_TAP_PREFIX) && + if (STRPREFIX(entry->d_name, VIR_NET_GENERATED_VNET_PREFIX) && g_ascii_isdigit(entry->d_name[4])) return 0; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 992488f7..c0f50856 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2551,6 +2551,7 @@ virNetDevDelMulti; virNetDevExists; virNetDevFeatureTypeFromString; virNetDevFeatureTypeToString; +virNetDevGenerateName; virNetDevGetFeatures; virNetDevGetIndex; virNetDevGetLinkInfo; @@ -2574,6 +2575,7 @@ virNetDevIfStateTypeToString; virNetDevIsVirtualFunction; virNetDevPFGetVF; virNetDevReadNetConfig; +virNetDevReserveName; virNetDevRunEthernetScript; virNetDevRxFilterFree; virNetDevRxFilterModeTypeFromString; diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index 32b397d2..197c0aa2 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -456,10 +456,10 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, } } else { if (!net->ifname || - STRPREFIX(net->ifname, VIR_NET_GENERATED_TAP_PREFIX) || + STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) || strchr(net->ifname, '%')) { VIR_FREE(net->ifname); - net->ifname = g_strdup(VIR_NET_GENERATED_TAP_PREFIX "%d"); + net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d"); /* avoid exposing vnet%d in getXMLDesc or error outputs */ template_ifname = true; } @@ -560,10 +560,10 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def, } if (!net->ifname || - STRPREFIX(net->ifname, VIR_NET_GENERATED_TAP_PREFIX) || + STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) || strchr(net->ifname, '%')) { VIR_FREE(net->ifname); - net->ifname = g_strdup(VIR_NET_GENERATED_TAP_PREFIX "%d"); + net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d"); /* avoid exposing vnet%d in getXMLDesc or error outputs */ template_ifname = true; } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 5104bbe7..bd1ca1d8 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -17,6 +17,7 @@ */ #include <config.h> +#include <math.h> #include "virnetdev.h" #include "viralloc.h" @@ -95,6 +96,14 @@ VIR_LOG_INIT("util.netdev"); (FEATURE_WORD(blocks, index, field) & FEATURE_FIELD_FLAG(index)) #endif + +static virNetDevGenName +virNetDevGenNames[VIR_NET_DEV_GEN_NAME_LAST] = { + {-1, VIR_NET_GENERATED_VNET_PREFIX, VIR_MUTEX_INITIALIZER}, + {-1, VIR_NET_GENERATED_MACVTAP_PREFIX, VIR_MUTEX_INITIALIZER}, + {-1, VIR_NET_GENERATED_MACVLAN_PREFIX, VIR_MUTEX_INITIALIZER}, +}; + typedef enum { VIR_MCAST_TYPE_INDEX_TOKEN, VIR_MCAST_TYPE_NAME_TOKEN, @@ -3516,3 +3525,110 @@ virNetDevSetRootQDisc(const char *ifname, return 0; } + + +/** + * virNetDevReserveName: + * @name: name of an existing network device + * + * Reserve a network device name, so that any new network device + * created with an autogenerated name will use a number higher + * than the number in the given device name. + * + * Returns nothing. + */ +void +virNetDevReserveName(const char *name) +{ + unsigned int id; + const char *idstr = NULL; + virNetDevGenNameType type; + + if (STRPREFIX(name, VIR_NET_GENERATED_VNET_PREFIX)) + type = VIR_NET_DEV_GEN_NAME_VNET; + else if (STRPREFIX(name, VIR_NET_GENERATED_MACVTAP_PREFIX)) + type = VIR_NET_DEV_GEN_NAME_MACVTAP; + else if (STRPREFIX(name, VIR_NET_GENERATED_MACVLAN_PREFIX)) + type = VIR_NET_DEV_GEN_NAME_MACVLAN; + else + return; + + VIR_INFO("marking device in use: '%s'", name); + + idstr = name + strlen(virNetDevGenNames[type].prefix); + + if (virStrToLong_ui(idstr, NULL, 10, &id) >= 0) { + virMutexLock(&virNetDevGenNames[type].mutex); + + if (virNetDevGenNames[type].lastID < (int)id) + virNetDevGenNames[type].lastID = id; + + virMutexUnlock(&virNetDevGenNames[type].mutex); + } +} + + +/** + * virNetDevGenerateName: + * @ifname: pointer to pointer to string which can be a template, + * NULL or user-provided name. + * @type: type of the network device + * + * generate a new (currently unused) name for a new network device based + * on @ifname. If string pointed by @ifname is a template, replace %d + * with the reserved id; if that string is NULL, just generate a new + * name. 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. + * + * Note: if string pointed by @ifname is NOT a template or NULL, leave + * it unchanged and return it directly. + * + * Returns 0 on success, -1 on failure. + */ +int +virNetDevGenerateName(char **ifname, virNetDevGenNameType type) +{ + int id; + const char *prefix = virNetDevGenNames[type].prefix; + double maxIDd = pow(10, IFNAMSIZ - 1 - strlen(prefix)); + int maxID = INT_MAX; + int attempts = 0; + + /* The @ifname is not a template, leave it unchanged. */ + if (*ifname && strstr(*ifname, "%d") == NULL) + return 0; + + if (maxIDd <= (double)INT_MAX) + maxID = (int)maxIDd; + + do { + g_autofree char *try = NULL; + + virMutexLock(&virNetDevGenNames[type].mutex); + + id = ++virNetDevGenNames[type].lastID; + + /* reset before overflow */ + if (virNetDevGenNames[type].lastID >= maxID) + virNetDevGenNames[type].lastID = -1; + + virMutexUnlock(&virNetDevGenNames[type].mutex); + + if (*ifname) + try = g_strdup_printf(*ifname, id); + else + try = g_strdup_printf("%s%d", prefix, 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"), + prefix); + return -1; +} diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 53e606c6..f0160127 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -38,7 +38,13 @@ typedef void virIfreq; /* Used for prefix of ifname of any tap device name generated * dynamically by libvirt, cannot be used for a persistent network name. */ -#define VIR_NET_GENERATED_TAP_PREFIX "vnet" +#define VIR_NET_GENERATED_VNET_PREFIX "vnet" + +/* 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" typedef enum { VIR_NETDEV_RX_FILTER_MODE_NONE = 0, @@ -145,6 +151,21 @@ struct _virNetDevCoalesce { uint32_t rate_sample_interval; }; +typedef enum { + VIR_NET_DEV_GEN_NAME_VNET, + VIR_NET_DEV_GEN_NAME_MACVTAP, + VIR_NET_DEV_GEN_NAME_MACVLAN, + VIR_NET_DEV_GEN_NAME_LAST +} virNetDevGenNameType; + +typedef struct _virNetDevGenName virNetDevGenName; +typedef virNetDevGenName *virNetDevGenNamePtr; +struct _virNetDevGenName { + int lastID; /* not "unsigned" because callers use %d */ + const char *prefix; + virMutex mutex; +}; + int virNetDevSetupControl(const char *ifname, virIfreq *ifr) @@ -321,3 +342,7 @@ int virNetDevVFInterfaceStats(virPCIDeviceAddressPtr vfAddr, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevRxFilter, virNetDevRxFilterFree); + +void virNetDevReserveName(const char *name); + +int virNetDevGenerateName(char **ifname, virNetDevGenNameType type); diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 198607b5..9354cc10 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -76,11 +76,11 @@ virNetDevTapReserveName(const char *name) const char *idstr = NULL; - if (STRPREFIX(name, VIR_NET_GENERATED_TAP_PREFIX)) { + if (STRPREFIX(name, VIR_NET_GENERATED_VNET_PREFIX)) { VIR_INFO("marking device in use: '%s'", name); - idstr = name + strlen(VIR_NET_GENERATED_TAP_PREFIX); + idstr = name + strlen(VIR_NET_GENERATED_VNET_PREFIX); if (virStrToLong_ui(idstr, NULL, 10, &id) >= 0) { virMutexLock(&virNetDevTapCreateMutex); @@ -200,7 +200,7 @@ static int virNetDevTapGenerateName(char **ifname) { int id; - double maxIDd = pow(10, IFNAMSIZ - 1 - strlen(VIR_NET_GENERATED_TAP_PREFIX)); + double maxIDd = pow(10, IFNAMSIZ - 1 - strlen(VIR_NET_GENERATED_VNET_PREFIX)); int maxID = INT_MAX; int attempts = 0; @@ -227,7 +227,7 @@ virNetDevTapGenerateName(char **ifname) virReportError(VIR_ERR_INTERNAL_ERROR, _("no unused %s names available"), - VIR_NET_GENERATED_TAP_PREFIX); + VIR_NET_GENERATED_VNET_PREFIX); return -1; } @@ -270,7 +270,7 @@ int virNetDevTapCreate(char **ifname, * immediately re-using names that have just been released, which * can lead to race conditions). */ - if (STREQ(*ifname, VIR_NET_GENERATED_TAP_PREFIX "%d") && + if (STREQ(*ifname, VIR_NET_GENERATED_VNET_PREFIX "%d") && virNetDevTapGenerateName(ifname) < 0) { goto cleanup; } -- 2.25.1

Simplify ReserveName/GenerateName for macvlan and macvtap by using common functions. Signed-off-by: Shi Lei <shi_lei(a)massclouds.com&gt; --- 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

On 12/13/20 8:50 PM, Shi Lei wrote: I think this is unnecessary (since a user-provided name shouldn't be using the auto-generate pattern, and would have been deleted by the parser anyway (see src/conf/domain_conf.c:12038, and comments in my reply to patch 5/5). So the only possible string here would be a string that would pass through virNetDevReserveName() with no action taken anyway. Both of these don't need the "if (!*vethN) { ... }. Remember that virNetDevGenerateName() is a NOP if the input ifname != NULL. Otherwise good. I can squash in these two changes if you approve.

Those functions that call virNetDevTapCreate don't need to adding 'vnet%d' into ifname when it is empty, since virNetDevGenerateName which is in virNetDevTapCreate can deal with it. Signed-off-by: Shi Lei <shi_lei(a)massclouds.com&gt; --- src/bhyve/bhyve_command.c | 1 - src/qemu/qemu_interface.c | 12 ------------ 2 files changed, 13 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 4cf98c0e..92b31a6e 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -83,7 +83,6 @@ bhyveBuildNetArgStr(const virDomainDef *def, STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) || strchr(net->ifname, '%')) { VIR_FREE(net->ifname); - net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d"); } if (!dryRun) { diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index 197c0aa2..87cfb8fc 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -413,7 +413,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, virMacAddr tapmac; int ret = -1; unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP; - bool template_ifname = false; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); const char *tunpath = "/dev/net/tun"; const char *auditdev = tunpath; @@ -459,9 +458,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) || strchr(net->ifname, '%')) { VIR_FREE(net->ifname); - net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d"); - /* avoid exposing vnet%d in getXMLDesc or error outputs */ - template_ifname = true; } if (virNetDevTapCreate(&net->ifname, tunpath, tapfd, tapfdSize, tap_create_flags) < 0) { @@ -512,8 +508,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, virDomainAuditNetDevice(def, net, auditdev, false); for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++) VIR_FORCE_CLOSE(tapfd[i]); - if (template_ifname) - VIR_FREE(net->ifname); } return ret; @@ -541,7 +535,6 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def, const char *brname; int ret = -1; unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP; - bool template_ifname = false; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); const char *tunpath = "/dev/net/tun"; @@ -563,9 +556,6 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def, STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) || strchr(net->ifname, '%')) { VIR_FREE(net->ifname); - net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d"); - /* avoid exposing vnet%d in getXMLDesc or error outputs */ - template_ifname = true; } if (qemuInterfaceIsVnetCompatModel(net)) @@ -630,8 +620,6 @@ qemuInterfaceBridgeConnect(virDomainDefPtr def, size_t i; for (i = 0; i < *tapfdSize && tapfd[i] >= 0; i++) VIR_FORCE_CLOSE(tapfd[i]); - if (template_ifname) - VIR_FREE(net->ifname); } return ret; -- 2.25.1