On 12/9/20 10:00 PM, Shi Lei wrote:
Extract ReserveName/GenerateName from netdevtap and netdevmacvlan as
common helper functions.
Signed-off-by: Shi Lei <shi_lei@xxxxxxxxxxxxxx>
---
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)
This would still attempt to generate a name for something that had
multiple format specifiers in it, e.g. "vnet%d%n", which could lead to
"Bad Things(tm)". I *think* it would be sufficient to avoid this if we
just checked for multiple occurences of %, something like this:
if (*ifname &&
(strchr(*ifname, '%') != strrchr(*ifname, '%') ||
strstr(*ifname, "%d") == NULL)) {
return 0;
}
(The idea here is that if strchr and strrchr are the same, that means
there's only a single '%'. So if we get past this check, we know that
either the string is empty, or that it contains a single %d and no other
format specifiers).
If you're okay with me squashing that change, I can just do that and
push it, or if you'd rather do it some other way and re-post, that's
fine too - just let me know.
+ 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;
}
