[libvirt PATCH 3/3] util: assign tap device names using a monotonically increasing integer
Laine Stump
laine at redhat.com
Mon Aug 24 04:23:16 UTC 2020
When creating a standard tap device, if provided with an ifname that
contains "%d", rather than taking that literally as the name to use
for the new device, the kernel will instead use that string as a
template, and search for the lowest number that could be put in place
of %d and produce an otherwise unused and unique name for the new
device. For example, if there is no tap device name given in the XML,
libvirt will always send "vnet%d" as the device name, and the kernel
will create new devices named "vnet0", "vnet1", etc. If one of those
devices is deleted, creating a "hole" in the name list, the kernel
will always attempt to reuse the name in the hole first before using a
name with a higher number (i.e. it finds the lowest possible unused
number).
The problem with this, as described in the previous patch dealing with
macvtap device naming, is that it makes "immediate reuse" of a newly
freed tap device name *much* more common, and in the aftermath of
deleting a tap device, there is some other necessary cleanup of things
which are named based on the device name (nwfilter rules, bandwidth
rules, OVS switch ports, to name a few) that could end up stomping
over the top of the setup of a new device of the same name for a
different guest.
Since the kernel "create a name based on a template" functionality for
tap devices doesn't exist for macvtap, this patch is a bit different
from the previous patch - we look for a requested ifname of "vnet%d",
and when we see that, we use it as a sprintf format string to find an
unused device name ourselves, then pass that exact name on to the
kernel; in this way we can avoid the perils of the kernel's "lowest
number first" algorithm, and instead use a "hopefully never-before
used number" algorithm.
(NB: It is still possible for a user to provide their own
parameterized template name (e.g. "mytap%d") in the XML, and libvirt
will just pass that through to the kernel as it always has.)
Signed-off-by: Laine Stump <laine at redhat.com>
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_process.c | 22 ++++++++++-
src/util/virnetdevtap.c | 79 +++++++++++++++++++++++++++++++++++++++-
src/util/virnetdevtap.h | 4 ++
4 files changed, 102 insertions(+), 4 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 01c2e710cd..a9d5af9dde 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2674,6 +2674,7 @@ virNetDevTapGetName;
virNetDevTapGetRealDeviceName;
virNetDevTapInterfaceStats;
virNetDevTapReattachBridge;
+virNetDevTapReserveName;
# util/virnetdevveth.h
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 832b2e6870..2a1c1a3732 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3319,8 +3319,26 @@ qemuProcessNotifyNets(virDomainDefPtr def)
* domain to be unceremoniously killed, which would be *very*
* impolite.
*/
- if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT)
- ignore_value(virNetDevMacVLanReserveName(net->ifname, false));
+ switch (virDomainNetGetActualType(net)) {
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ ignore_value(virNetDevMacVLanReserveName(net->ifname, false));
+ break;
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ virNetDevTapReserveName(net->ifname);
+ break;
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ break;
+ }
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
if (!conn && !(conn = virGetConnectNetwork()))
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index c0a7c3019e..4c3b68e582 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -54,6 +54,45 @@
VIR_LOG_INIT("util.netdevtap");
+#define VIR_TAP_MAX_ID 99999
+
+virMutex virNetDevTapCreateMutex = VIR_MUTEX_INITIALIZER;
+static int virNetDevTapLastID = -1;
+
+
+/**
+ * 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_TAP_PREFIX)) {
+
+ idstr = name + strlen(VIR_NET_GENERATED_TAP_PREFIX);
+
+ if (virStrToLong_ui(idstr, NULL, 10, &id) >= 0) {
+ virMutexLock(&virNetDevTapCreateMutex);
+
+ if (virNetDevTapLastID % (VIR_TAP_MAX_ID + 1) < (int)id)
+ virNetDevTapLastID = id;
+
+ virMutexUnlock(&virNetDevTapCreateMutex);
+ }
+ }
+}
+
+
/**
* virNetDevTapGetName:
* @tapfd: a tun/tap file descriptor
@@ -230,10 +269,45 @@ int virNetDevTapCreate(char **ifname,
size_t tapfdSize,
unsigned int flags)
{
- size_t i;
+ size_t i = 0;
struct ifreq ifr;
int ret = -1;
- int fd;
+ int fd = 0;
+
+ virMutexLock(&virNetDevTapCreateMutex);
+
+ /* if ifname is "vnet%d", then create the actual name to use by
+ * replacing %d with ++virNetDevTapLastID. Keep trying new values
+ * until one is found that doesn't already exist, or we've
+ * completed one full circle of the number space.
+ *
+ */
+
+ if (STREQ(*ifname, VIR_NET_GENERATED_TAP_PREFIX "%d")) {
+ int id;
+ int start = ++virNetDevTapLastID % (VIR_TAP_MAX_ID + 1);
+ bool found = false;
+
+ for (id = start;
+ id + 1 != start;
+ id = ++virNetDevTapLastID % (VIR_TAP_MAX_ID + 1)) {
+
+ g_autofree char *try = g_strdup_printf(*ifname, id);
+
+ if (!virNetDevExists(try)) {
+ g_free(*ifname);
+ *ifname = g_steal_pointer(&try);
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("no unused %s names available"),
+ VIR_NET_GENERATED_TAP_PREFIX);
+ goto cleanup;
+ }
+ }
if (!tunpath)
tunpath = "/dev/net/tun";
@@ -302,6 +376,7 @@ 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 c6bd9285ba..dea8aec3af 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -29,6 +29,10 @@
# 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.26.2
More information about the libvir-list
mailing list