9 p.m.
V1 here: https://www.redhat.com/archives/libvir-list/2020-December/msg00308.html
Since V1:
(1) Remove virNetDev[Lock|Unlock]GenName.
Only *lastID* needs to be protected. Now we lock *lastID*
inside virNetDevReserveName and virNetDevGenerateName, then lock and
unlock functions are no longer needed.
(2) Shorten the locking range for generating names for tap and macvlan.
Since virNetDevReserveName and virNetDevGenerateName are now with lock,
we can call them directly rather than adding lock outside.
(3) Rename *_GEN_NAME_TAP to *_GEN_NAME_VNET.
(4) Now veth and tap share the same prefix "vnet".
(5) Use name rather than type as the argument of virNetDevReserveName.
Just follow the style of virNetDev[Tap|MacVlan]ReserveName.
(6) Remove those in-between functions for tap and macvlan.
(7) Remove useless ENUM_[DECL|IMPL] for enum virNetDevGenNameType.
(8) Remove the *_NONE item for enum virNetDevGenNameType.
(9) Remove useless <math.h> in virnetdevtap.
(10) When @ifname of virNetDevGenerateName is NOT a template or NULL,
just leave it unchanged.
(11) Take advantage of the "g_strdup_printf(*ifname, id)" in
virNetDevGenerateName, prevent the functions that call virNetDevTapCreate()
from adding in "vnet%d" when ifname is empty.
(12) Use VIR_NETDEV_MACVLAN_CREATE_WITH_TAP to distinguish macvtap and
macvlan and remove those useless macros.
Shi Lei (5):
netdev: Introduce several helper functions for generating unique netdev name
netdevtap: Use common helper function to create unique tap name
netdevmacvlan: Use helper function to create unique macvlan/macvtap name
netdevveth: Simplify virNetDevVethCreate by using virNetDevGenerateName
netdev: Prevent functions that call virNetDevTapCreate from adding 'vnet%d' into
ifname
src/bhyve/bhyve_command.c | 3 +-
src/conf/domain_conf.c | 4 +-
src/interface/interface_backend_udev.c | 2 +-
src/libvirt_private.syms | 4 +-
src/lxc/lxc_process.c | 5 +-
src/qemu/qemu_interface.c | 16 +--
src/qemu/qemu_process.c | 4 +-
src/util/virnetdev.c | 116 ++++++++++++++++
src/util/virnetdev.h | 27 +++-
src/util/virnetdevmacvlan.c | 177 +++----------------------
src/util/virnetdevmacvlan.h | 14 +-
src/util/virnetdevtap.c | 100 +-------------
src/util/virnetdevtap.h | 4 -
src/util/virnetdevveth.c | 140 +++++--------------
14 files changed, 217 insertions(+), 399 deletions(-)
--
2.25.1
9 p.m.
New subject: [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>
---
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
8:10 p.m.
New subject: [PATCHv2 1/5] netdev: Introduce several helper functions for generating unique netdev name
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(a)massclouds.com>
---
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;
}
12:51 a.m.
New subject: [PATCHv2 1/5] netdev: Introduce several helper functions for generating unique netdev name
On 2020-12-14 at 10:10, Laine Stump wrote:
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(a)massclouds.com>
> ---
> 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.
Okay. I agree with your change. Thanks!
> + 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;
> }
9 p.m.
New subject: [PATCHv2 2/5] netdevtap: Use common helper function to create unique tap name
Simplify GenerateName/ReserveName for netdevtap by using common
functions.
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/libvirt_private.syms | 1 -
src/qemu/qemu_process.c | 2 +-
src/util/virnetdevtap.c | 100 ++-------------------------------------
src/util/virnetdevtap.h | 4 --
4 files changed, 5 insertions(+), 102 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c0f50856..64ef01e1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2689,7 +2689,6 @@ virNetDevTapGetName;
virNetDevTapGetRealDeviceName;
virNetDevTapInterfaceStats;
virNetDevTapReattachBridge;
-virNetDevTapReserveName;
# util/virnetdevveth.h
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3b64caa6..1d54f201 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3395,7 +3395,7 @@ qemuProcessNotifyNets(virDomainDefPtr def)
case VIR_DOMAIN_NET_TYPE_BRIDGE:
case VIR_DOMAIN_NET_TYPE_NETWORK:
case VIR_DOMAIN_NET_TYPE_ETHERNET:
- virNetDevTapReserveName(net->ifname);
+ virNetDevReserveName(net->ifname);
break;
case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 9354cc10..88ad3216 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -49,51 +49,11 @@
#if defined(WITH_GETIFADDRS) && defined(AF_LINK)
# include <ifaddrs.h>
#endif
-#include <math.h>
#define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("util.netdevtap");
-virMutex virNetDevTapCreateMutex = VIR_MUTEX_INITIALIZER;
-static int virNetDevTapLastID = -1; /* not "unsigned" because callers use %d
*/
-
-
-/**
- * virNetDevTapReserveName:
- * @name: name of an existing tap device
- *
- * Set the value of virNetDevTapLastID to assure that any new tap
- * device created with an autogenerated name will use a number higher
- * than the number in the given tap device name.
- *
- * Returns nothing.
- */
-void
-virNetDevTapReserveName(const char *name)
-{
- unsigned int id;
- const char *idstr = NULL;
-
-
- if (STRPREFIX(name, VIR_NET_GENERATED_VNET_PREFIX)) {
-
- VIR_INFO("marking device in use: '%s'", name);
-
- idstr = name + strlen(VIR_NET_GENERATED_VNET_PREFIX);
-
- if (virStrToLong_ui(idstr, NULL, 10, &id) >= 0) {
- virMutexLock(&virNetDevTapCreateMutex);
-
- if (virNetDevTapLastID < (int)id)
- virNetDevTapLastID = id;
-
- virMutexUnlock(&virNetDevTapCreateMutex);
- }
- }
-}
-
-
/**
* virNetDevTapGetName:
* @tapfd: a tun/tap file descriptor
@@ -183,55 +143,6 @@ virNetDevTapGetRealDeviceName(char *ifname G_GNUC_UNUSED)
#ifdef TUNSETIFF
-/**
- * virNetDevTapGenerateName:
- * @ifname: pointer to pointer to string containing template
- *
- * generate a new (currently unused) name for a new tap device based
- * on the templace string in @ifname - replace %d with
- * ++virNetDevTapLastID, 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
-virNetDevTapGenerateName(char **ifname)
-{
- int id;
- double maxIDd = pow(10, IFNAMSIZ - 1 - strlen(VIR_NET_GENERATED_VNET_PREFIX));
- int maxID = INT_MAX;
- int attempts = 0;
-
- if (maxIDd <= (double)INT_MAX)
- maxID = (int)maxIDd;
-
- do {
- g_autofree char *try = NULL;
-
- id = ++virNetDevTapLastID;
-
- /* reset before overflow */
- if (virNetDevTapLastID >= maxID)
- virNetDevTapLastID = -1;
-
- try = g_strdup_printf(*ifname, 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"),
- VIR_NET_GENERATED_VNET_PREFIX);
- return -1;
-}
-
-
/**
* virNetDevTapCreate:
* @ifname: the interface name
@@ -263,16 +174,14 @@ int virNetDevTapCreate(char **ifname,
int ret = -1;
int fd = -1;
- virMutexLock(&virNetDevTapCreateMutex);
-
/* if ifname is "vnet%d", then auto-generate a name for the new
* device (the kernel could do this for us, but has a bad habit of
* immediately re-using names that have just been released, which
* can lead to race conditions).
- */
- if (STREQ(*ifname, VIR_NET_GENERATED_VNET_PREFIX "%d") &&
- virNetDevTapGenerateName(ifname) < 0) {
- goto cleanup;
+ * if ifname is just a user-provided name, virNetDevGenerateName
+ * leaves it unchanged. */
+ if (virNetDevGenerateName(ifname, VIR_NET_DEV_GEN_NAME_VNET) < 0) {
+ return -1;
}
if (!tunpath)
@@ -333,7 +242,6 @@ int virNetDevTapCreate(char **ifname,
ret = 0;
cleanup:
- virMutexUnlock(&virNetDevTapCreateMutex);
if (ret < 0) {
VIR_FORCE_CLOSE(fd);
while (i--)
diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
index dea8aec3..c6bd9285 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -29,10 +29,6 @@
# define VIR_NETDEV_TAP_REQUIRE_MANUAL_CLEANUP 1
#endif
-void
-virNetDevTapReserveName(const char *name)
- ATTRIBUTE_NONNULL(1);
-
int virNetDevTapCreate(char **ifname,
const char *tunpath,
int *tapfd,
--
2.25.1
9 p.m.
New subject: [PATCHv2 3/5] netdevmacvlan: Use helper function to create unique macvlan/macvtap name
Simplify ReserveName/GenerateName for macvlan and macvtap by using
common functions.
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/libvirt_private.syms | 1 -
src/lxc/lxc_process.c | 2 +-
src/qemu/qemu_process.c | 2 +-
src/util/virnetdevmacvlan.c | 177 +++++-------------------------------
src/util/virnetdevmacvlan.h | 14 +--
5 files changed, 26 insertions(+), 170 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/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
6:58 p.m.
New subject: [PATCHv2 3/5] netdevmacvlan: Use helper function to create unique macvlan/macvtap name
On 12/9/20 10:00 PM, Shi Lei wrote:
Simplify ReserveName/GenerateName for macvlan and macvtap by using
common functions.
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/libvirt_private.syms | 1 -
src/lxc/lxc_process.c | 2 +-
src/qemu/qemu_process.c | 2 +-
src/util/virnetdevmacvlan.c | 177 +++++-------------------------------
src/util/virnetdevmacvlan.h | 14 +--
5 files changed, 26 insertions(+), 170 deletions(-)
You probably don't have the libxl driver enabled in your builds, so you
missed this change (which I've squashed in):
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 6af274cb1b..3488d7ec08 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);
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/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)
7:20 p.m.
New subject: [PATCHv2 3/5] netdevmacvlan: Use helper function to create unique macvlan/macvtap name
On 2020-12-14 at 08:58, Laine Stump wrote:
On 12/9/20 10:00 PM, Shi Lei wrote:
> Simplify ReserveName/GenerateName for macvlan and macvtap by using
> common functions.
>
> Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
> ---
> src/libvirt_private.syms | 1 -
> src/lxc/lxc_process.c | 2 +-
> src/qemu/qemu_process.c | 2 +-
> src/util/virnetdevmacvlan.c | 177 +++++-------------------------------
> src/util/virnetdevmacvlan.h | 14 +--
> 5 files changed, 26 insertions(+), 170 deletions(-)
You probably don't have the libxl driver enabled in your builds, so you
missed this change (which I've squashed in):
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 6af274cb1b..3488d7ec08 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);
Sorry for that. I fix it and send V3.
Shi Lei
>
> 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/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)
>
8:17 p.m.
New subject: [PATCHv2 3/5] netdevmacvlan: Use helper function to create unique macvlan/macvtap name
On 12/13/20 8:20 PM, Shi Lei wrote:
On 2020-12-14 at 08:58, Laine Stump wrote:
> On 12/9/20 10:00 PM, Shi Lei wrote:
>> Simplify ReserveName/GenerateName for macvlan and macvtap by using
>> common functions.
>>
>> Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
>> ---
>> src/libvirt_private.syms | 1 -
>> src/lxc/lxc_process.c | 2 +-
>> src/qemu/qemu_process.c | 2 +-
>> src/util/virnetdevmacvlan.c | 177 +++++-------------------------------
>> src/util/virnetdevmacvlan.h | 14 +--
>> 5 files changed, 26 insertions(+), 170 deletions(-)
> You probably don't have the libxl driver enabled in your builds, so you
> missed this change (which I've squashed in):
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 6af274cb1b..3488d7ec08 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);
>
Sorry for that. I fix it and send V3.
No reason to send a v3 for something that simple! I could just fix it up
here.
However, if you are going to send a V3, I just sent a comment about an
addition that is needed in Patch 1 that is a bit more than just an
omitted function name change - you'll want to put that (or something
similar) in if you repost.
Also, it occurred to me just now while looking at that - it would be
nice if the name change from VIR_NET_GENERATED_TAP_PREFIX to
VIR_NET_GENERATED_VNET_PREFIX was a patch by itself (prerequisite to all
the other patches). That one isn't really necessary, though.
So if you're okay with me making the change I just posted in my reply
for Patch 1, I can just push it all right now - no need for a V3 (I
think! I'm still looking at patch 4 and 5 one last time - I should be
done looking at them in 30 minutes or so).
Shi Lei
>> 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/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)
>>
9 p.m.
New subject: [PATCHv2 4/5] netdevveth: Simplify virNetDevVethCreate by using virNetDevGenerateName
Simplify virNetDevVethCreate by using common GenerateName/ReserveName
functions.
Signed-off-by: Shi Lei <shi_lei(a)massclouds.com>
---
src/lxc/lxc_process.c | 3 +
src/util/virnetdevveth.c | 140 +++++++++------------------------------
2 files changed, 36 insertions(+), 107 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index eb29431e..2e8ae706 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -307,6 +307,9 @@ virLXCProcessSetupInterfaceTap(virDomainDefPtr vm,
VIR_DEBUG("calling vethCreate()");
parentVeth = net->ifname;
+ if (parentVeth)
+ virNetDevReserveName(parentVeth);
+
if (virNetDevVethCreate(&parentVeth, &containerVeth) < 0)
return NULL;
VIR_DEBUG("parentVeth: %s, containerVeth: %s", parentVeth, containerVeth);
diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c
index b3eee1af..d6932a2e 100644
--- a/src/util/virnetdevveth.c
+++ b/src/util/virnetdevveth.c
@@ -32,48 +32,6 @@
VIR_LOG_INIT("util.netdevveth");
-/* Functions */
-
-virMutex virNetDevVethCreateMutex = VIR_MUTEX_INITIALIZER;
-
-static int virNetDevVethExists(int devNum)
-{
- int ret;
- g_autofree char *path = NULL;
-
- path = g_strdup_printf(SYSFS_NET_DIR "vnet%d/", devNum);
- ret = virFileExists(path) ? 1 : 0;
- VIR_DEBUG("Checked dev vnet%d usage: %d", devNum, ret);
- return ret;
-}
-
-/**
- * virNetDevVethGetFreeNum:
- * @startDev: device number to start at (x in vethx)
- *
- * Looks in /sys/class/net/ to find the first available veth device
- * name.
- *
- * Returns non-negative device number on success or -1 in case of error
- */
-static int virNetDevVethGetFreeNum(int startDev)
-{
- int devNum;
-
-#define MAX_DEV_NUM 65536
-
- for (devNum = startDev; devNum < MAX_DEV_NUM; devNum++) {
- int ret = virNetDevVethExists(devNum);
- if (ret < 0)
- return -1;
- if (ret == 0)
- return devNum;
- }
-
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("No free veth devices available"));
- return -1;
-}
/**
* virNetDevVethCreate:
@@ -102,77 +60,45 @@ static int virNetDevVethGetFreeNum(int startDev)
*/
int virNetDevVethCreate(char** veth1, char** veth2)
{
- int ret = -1;
- int vethNum = 0;
- size_t i;
-
- /*
- * We might race with other containers, but this is reasonably
- * unlikely, so don't do too many retries for device creation
- */
- virMutexLock(&virNetDevVethCreateMutex);
-#define MAX_VETH_RETRIES 10
-
- for (i = 0; i < MAX_VETH_RETRIES; i++) {
- g_autofree char *veth1auto = NULL;
- g_autofree char *veth2auto = NULL;
- g_autoptr(virCommand) cmd = NULL;
-
- int status;
- if (!*veth1) {
- int veth1num;
- if ((veth1num = virNetDevVethGetFreeNum(vethNum)) < 0)
- goto cleanup;
-
- veth1auto = g_strdup_printf("vnet%d", veth1num);
- vethNum = veth1num + 1;
- }
- if (!*veth2) {
- int veth2num;
- if ((veth2num = virNetDevVethGetFreeNum(vethNum)) < 0)
- goto cleanup;
+ int status;
+ g_autofree char *veth1auto = NULL;
+ g_autofree char *veth2auto = NULL;
+ g_autoptr(virCommand) cmd = NULL;
- veth2auto = g_strdup_printf("vnet%d", veth2num);
- vethNum = veth2num + 1;
- }
+ if (!*veth1) {
+ if (virNetDevGenerateName(&veth1auto, VIR_NET_DEV_GEN_NAME_VNET) < 0)
+ return -1;
+ }
+ if (!*veth2) {
+ if (virNetDevGenerateName(&veth2auto, VIR_NET_DEV_GEN_NAME_VNET) < 0)
+ return -1;
+ }
- cmd = virCommandNew("ip");
- virCommandAddArgList(cmd, "link", "add",
- *veth1 ? *veth1 : veth1auto,
- "type", "veth", "peer",
"name",
- *veth2 ? *veth2 : veth2auto,
- NULL);
-
- if (virCommandRun(cmd, &status) < 0)
- goto cleanup;
-
- if (status == 0) {
- if (veth1auto) {
- *veth1 = veth1auto;
- veth1auto = NULL;
- }
- if (veth2auto) {
- *veth2 = veth2auto;
- veth2auto = NULL;
- }
- VIR_DEBUG("Create Host: %s guest: %s", *veth1, *veth2);
- ret = 0;
- goto cleanup;
- }
+ cmd = virCommandNew("ip");
+ virCommandAddArgList(cmd, "link", "add",
+ *veth1 ? *veth1 : veth1auto,
+ "type", "veth", "peer",
"name",
+ *veth2 ? *veth2 : veth2auto,
+ NULL);
- VIR_DEBUG("Failed to create veth host: %s guest: %s: %d",
- *veth1 ? *veth1 : veth1auto,
- *veth2 ? *veth2 : veth2auto,
- status);
+ if (virCommandRun(cmd, &status) < 0 || status) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Failed to allocate free veth
pair"));
+ return -1;
}
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to allocate free veth pair after %d attempts"),
- MAX_VETH_RETRIES);
+ VIR_DEBUG("create veth host: %s guest: %s: %d",
+ *veth1 ? *veth1 : veth1auto,
+ *veth2 ? *veth2 : veth2auto,
+ status);
+
+ if (veth1auto)
+ *veth1 = g_steal_pointer(&veth1auto);
+ if (veth2auto)
+ *veth2 = g_steal_pointer(&veth2auto);
- cleanup:
- virMutexUnlock(&virNetDevVethCreateMutex);
- return ret;
+ VIR_DEBUG("Create Host: %s guest: %s", *veth1, *veth2);
+ return 0;
}
/**
--
2.25.1
9 p.m.
New subject: [PATCHv2 5/5] netdev: Prevent functions that call virNetDevTapCreate from adding 'vnet%d' into ifname
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>
---
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
1440
days inactive
1444
days old
10 comments
2 participants
participants (2)
-
Laine Stump -
Shi Lei