Re: [PATCHv2 1/5] netdev: Introduce several helper functions for generating unique netdev name

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