[PATCHv3 0/5] netdev: Extract GenerateName/ReserveName as common functions
V2 here: https://www.redhat.com/archives/libvir-list/2020-December/msg00563.html Since V2: * Fix libxl driver for missing changing virNetDevMacVLanReserveName 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/libxl/libxl_driver.c | 2 +- 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 +++++-------------- 15 files changed, 218 insertions(+), 400 deletions(-) -- 2.25.1
Extract ReserveName/GenerateName from netdevtap and netdevmacvlan as common helper functions. Signed-off-by: Shi Lei <shi_lei@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
Simplify GenerateName/ReserveName for netdevtap by using common functions. Signed-off-by: Shi Lei <shi_lei@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
Simplify ReserveName/GenerateName for macvlan and macvtap by using common functions. Signed-off-by: Shi Lei <shi_lei@massclouds.com> --- src/libvirt_private.syms | 1 - src/libxl/libxl_driver.c | 2 +- src/lxc/lxc_process.c | 2 +- src/qemu/qemu_process.c | 2 +- src/util/virnetdevmacvlan.c | 177 +++++------------------------------- src/util/virnetdevmacvlan.h | 14 +-- 6 files changed, 27 insertions(+), 171 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 64ef01e1..4d6ae84b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2655,7 +2655,6 @@ virNetDevMacVLanDelete; virNetDevMacVLanDeleteWithVPortProfile; virNetDevMacVLanIsMacvtap; virNetDevMacVLanModeTypeFromString; -virNetDevMacVLanReserveName; virNetDevMacVLanRestartWithVPortProfile; virNetDevMacVLanTapOpen; virNetDevMacVLanTapSetup; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 6af274cb..3488d7ec 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -364,7 +364,7 @@ libxlReconnectNotifyNets(virDomainDefPtr def) * impolite. */ if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) - virNetDevMacVLanReserveName(net->ifname); + virNetDevReserveName(net->ifname); if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { if (!conn && !(conn = virGetConnectNetwork())) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 0f818e2e..eb29431e 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1641,7 +1641,7 @@ virLXCProcessReconnectNotifyNets(virDomainDefPtr def) * impolite. */ if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) - virNetDevMacVLanReserveName(net->ifname); + virNetDevReserveName(net->ifname); if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { if (!conn && !(conn = virGetConnectNetwork())) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1d54f201..6244ade4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3390,7 +3390,7 @@ qemuProcessNotifyNets(virDomainDefPtr def) */ switch (virDomainNetGetActualType(net)) { case VIR_DOMAIN_NET_TYPE_DIRECT: - virNetDevMacVLanReserveName(net->ifname); + virNetDevReserveName(net->ifname); break; case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_NETWORK: diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 72f0d670..36b13133 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -45,7 +45,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, # include <net/if.h> # include <linux/if_tun.h> -# include <math.h> # include "viralloc.h" # include "virlog.h" @@ -59,129 +58,6 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_LOG_INIT("util.netdevmacvlan"); -# define VIR_NET_GENERATED_MACVTAP_PATTERN VIR_NET_GENERATED_MACVTAP_PREFIX "%d" -# define VIR_NET_GENERATED_MACVLAN_PATTERN VIR_NET_GENERATED_MACVLAN_PREFIX "%d" -# define VIR_NET_GENERATED_PREFIX \ - ((flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ? \ - VIR_NET_GENERATED_MACVTAP_PREFIX : VIR_NET_GENERATED_MACVLAN_PREFIX) - - -virMutex virNetDevMacVLanCreateMutex = VIR_MUTEX_INITIALIZER; -static int virNetDevMacVTapLastID = -1; -static int virNetDevMacVLanLastID = -1; - - -static void -virNetDevMacVLanReserveNameInternal(const char *name) -{ - unsigned int id; - const char *idstr = NULL; - int *lastID = NULL; - int len; - - if (STRPREFIX(name, VIR_NET_GENERATED_MACVTAP_PREFIX)) { - lastID = &virNetDevMacVTapLastID; - len = strlen(VIR_NET_GENERATED_MACVTAP_PREFIX); - } else if (STRPREFIX(name, VIR_NET_GENERATED_MACVLAN_PREFIX)) { - lastID = &virNetDevMacVTapLastID; - len = strlen(VIR_NET_GENERATED_MACVLAN_PREFIX); - } else { - return; - } - - VIR_INFO("marking device in use: '%s'", name); - - idstr = name + len; - - if (virStrToLong_ui(idstr, NULL, 10, &id) >= 0) { - if (*lastID < (int)id) - *lastID = id; - } -} - - -/** - * virNetDevMacVLanReserveName: - * @name: name of an existing macvtap/macvlan device - * - * Set the value of virNetDevMacV(Lan|Tap)LastID to assure that any - * new device created with an autogenerated name will use a number - * higher than the number in the given device name. - * - * Returns nothing. - */ -void -virNetDevMacVLanReserveName(const char *name) -{ - virMutexLock(&virNetDevMacVLanCreateMutex); - virNetDevMacVLanReserveNameInternal(name); - virMutexUnlock(&virNetDevMacVLanCreateMutex); -} - - -/** - * virNetDevMacVLanGenerateName: - * @ifname: pointer to pointer to string containing template - * @lastID: counter to add to the template to form the name - * - * generate a new (currently unused) name for a new macvtap/macvlan - * device based on the template string in @ifname - replace %d with - * ++(*counter), and keep trying new values until one is found - * that doesn't already exist, or we've tried 10000 different - * names. Once a usable name is found, replace the template with the - * actual name. - * - * Returns 0 on success, -1 on failure. - */ -static int -virNetDevMacVLanGenerateName(char **ifname, unsigned int flags) -{ - const char *prefix; - const char *iftemplate; - int *lastID; - int id; - double maxIDd; - int maxID = INT_MAX; - int attempts = 0; - - if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) { - prefix = VIR_NET_GENERATED_MACVTAP_PREFIX; - iftemplate = VIR_NET_GENERATED_MACVTAP_PREFIX "%d"; - lastID = &virNetDevMacVTapLastID; - } else { - prefix = VIR_NET_GENERATED_MACVLAN_PREFIX; - iftemplate = VIR_NET_GENERATED_MACVLAN_PREFIX "%d"; - lastID = &virNetDevMacVLanLastID; - } - - maxIDd = pow(10, IFNAMSIZ - 1 - strlen(prefix)); - if (maxIDd <= (double)INT_MAX) - maxID = (int)maxIDd; - - do { - g_autofree char *try = NULL; - - id = ++(*lastID); - - /* reset before overflow */ - if (*lastID == maxID) - *lastID = -1; - - try = g_strdup_printf(iftemplate, id); - - if (!virNetDevExists(try)) { - g_free(*ifname); - *ifname = g_steal_pointer(&try); - return 0; - } - } while (++attempts < 10000); - - virReportError(VIR_ERR_INTERNAL_ERROR, - _("no unused %s names available"), - *ifname); - return -1; -} - /** * virNetDevMacVLanIsMacvtap: @@ -209,13 +85,10 @@ virNetDevMacVLanIsMacvtap(const char *ifname) * virNetDevMacVLanCreate: * * @ifname: The name the interface is supposed to have; optional parameter - * @type: The type of device, i.e., "macvtap", "macvlan" * @macaddress: The MAC address of the device * @srcdev: The name of the 'link' device * @macvlan_mode: The macvlan mode to use - * @retry: Pointer to integer that will be '1' upon return if an interface - * with the same name already exists and it is worth to try - * again with a different name + * @flags: OR of virNetDevMacVLanCreateFlags. * * Create a macvtap device with the given properties. * @@ -223,19 +96,21 @@ virNetDevMacVLanIsMacvtap(const char *ifname) */ int virNetDevMacVLanCreate(const char *ifname, - const char *type, const virMacAddr *macaddress, const char *srcdev, - uint32_t macvlan_mode) + uint32_t macvlan_mode, + unsigned int flags) { int error = 0; int ifindex = 0; + const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ? + VIR_NET_GENERATED_MACVTAP_PREFIX : + VIR_NET_GENERATED_MACVLAN_PREFIX; virNetlinkNewLinkData data = { .macvlan_mode = &macvlan_mode, .mac = macaddress, }; - if (virNetDevGetIndex(srcdev, &ifindex) < 0) return -1; @@ -795,7 +670,6 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, size_t tapfdSize, unsigned int flags) { - const char *type = VIR_NET_GENERATED_PREFIX; g_autofree char *ifname = NULL; uint32_t macvtapMode; int vf = -1; @@ -832,8 +706,6 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, return -1; } - virMutexLock(&virNetDevMacVLanCreateMutex); - if (ifnameRequested) { int rc; bool isAutoName @@ -842,10 +714,8 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, VIR_INFO("Requested macvtap device name: %s", ifnameRequested); - if ((rc = virNetDevExists(ifnameRequested)) < 0) { - virMutexUnlock(&virNetDevMacVLanCreateMutex); + if ((rc = virNetDevExists(ifnameRequested)) < 0) return -1; - } if (rc) { /* ifnameRequested is already being used */ @@ -854,17 +724,16 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, virReportSystemError(EEXIST, _("Unable to create device '%s'"), ifnameRequested); - virMutexUnlock(&virNetDevMacVLanCreateMutex); return -1; } } else { /* ifnameRequested is available. try to open it */ - virNetDevMacVLanReserveNameInternal(ifnameRequested); + virNetDevReserveName(ifnameRequested); - if (virNetDevMacVLanCreate(ifnameRequested, type, macaddress, - linkdev, macvtapMode) == 0) { + if (virNetDevMacVLanCreate(ifnameRequested, macaddress, + linkdev, macvtapMode, flags) == 0) { /* virNetDevMacVLanCreate() was successful - use this name */ ifname = g_strdup(ifnameRequested); @@ -874,7 +743,6 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, * autogenerated named, so there is nothing else to * try - fail and return. */ - virMutexUnlock(&virNetDevMacVLanCreateMutex); return -1; } } @@ -885,16 +753,19 @@ virNetDevMacVLanCreateWithVPortProfile(const char *ifnameRequested, * autogenerated name, so now we look for an unused * autogenerated name. */ - if (virNetDevMacVLanGenerateName(&ifname, flags) < 0 || - virNetDevMacVLanCreate(ifname, type, macaddress, - linkdev, macvtapMode) < 0) { - virMutexUnlock(&virNetDevMacVLanCreateMutex); + virNetDevGenNameType type; + if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) + type = VIR_NET_DEV_GEN_NAME_MACVTAP; + else + type = VIR_NET_DEV_GEN_NAME_MACVLAN; + + if (virNetDevGenerateName(&ifname, type) < 0 || + virNetDevMacVLanCreate(ifname, macaddress, + linkdev, macvtapMode, flags) < 0) return -1; - } } /* all done creating the device */ - virMutexUnlock(&virNetDevMacVLanCreateMutex); if (virNetDevVPortProfileAssociate(ifname, virtPortProfile, @@ -1050,10 +921,10 @@ bool virNetDevMacVLanIsMacvtap(const char *ifname G_GNUC_UNUSED) } int virNetDevMacVLanCreate(const char *ifname G_GNUC_UNUSED, - const char *type G_GNUC_UNUSED, const virMacAddr *macaddress G_GNUC_UNUSED, const char *srcdev G_GNUC_UNUSED, - uint32_t macvlan_mode G_GNUC_UNUSED) + uint32_t macvlan_mode G_GNUC_UNUSED, + unsigned int fflags G_GNUC_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Cannot create macvlan devices on this platform")); @@ -1141,10 +1012,4 @@ int virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname G_GNUC_UNUSE _("Cannot create macvlan devices on this platform")); return -1; } - -void virNetDevMacVLanReserveName(const char *name G_GNUC_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", - _("Cannot create macvlan devices on this platform")); -} #endif /* ! WITH_LIBNL */ diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h index 48800a8f..0a013fc2 100644 --- a/src/util/virnetdevmacvlan.h +++ b/src/util/virnetdevmacvlan.h @@ -48,23 +48,15 @@ typedef enum { VIR_NETDEV_MACVLAN_VNET_HDR = 1 << 2, } virNetDevMacVLanCreateFlags; -/* libvirt will start macvtap/macvlan interface names with one of - * these prefixes when it auto-generates the name - */ -#define VIR_NET_GENERATED_MACVTAP_PREFIX "macvtap" -#define VIR_NET_GENERATED_MACVLAN_PREFIX "macvlan" - -void virNetDevMacVLanReserveName(const char *name); - bool virNetDevMacVLanIsMacvtap(const char *ifname) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT G_GNUC_NO_INLINE; int virNetDevMacVLanCreate(const char *ifname, - const char *type, const virMacAddr *macaddress, const char *srcdev, - uint32_t macvlan_mode) - ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) + uint32_t macvlan_mode, + unsigned int flags) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT; int virNetDevMacVLanDelete(const char *ifname) -- 2.25.1
Simplify virNetDevVethCreate by using common GenerateName/ReserveName functions. Signed-off-by: Shi Lei <shi_lei@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
On 12/13/20 8:50 PM, Shi Lei wrote:
Simplify virNetDevVethCreate by using common GenerateName/ReserveName functions.
Signed-off-by: Shi Lei <shi_lei@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); +
I think this is unnecessary (since a user-provided name shouldn't be using the auto-generate pattern, and would have been deleted by the parser anyway (see src/conf/domain_conf.c:12038, and comments in my reply to patch 5/5). So the only possible string here would be a string that would pass through virNetDevReserveName() with no action taken anyway.
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; + }
Both of these don't need the "if (!*vethN) { ... }. Remember that virNetDevGenerateName() is a NOP if the input ifname != NULL. Otherwise good. I can squash in these two changes if you approve.
- 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; }
/**
On 2020-12-15 at 11:09, Laine Stump wrote:
On 12/13/20 8:50 PM, Shi Lei wrote:
Simplify virNetDevVethCreate by using common GenerateName/ReserveName functions.
Signed-off-by: Shi Lei <shi_lei@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); +
I think this is unnecessary (since a user-provided name shouldn't be using the auto-generate pattern, and would have been deleted by the parser anyway (see src/conf/domain_conf.c:12038, and comments in my reply to patch 5/5). So the only possible string here would be a string that would pass through virNetDevReserveName() with no action taken anyway.
Okay.
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; + }
Both of these don't need the "if (!*vethN) { ... }. Remember that virNetDevGenerateName() is a NOP if the input ifname != NULL.
Otherwise good. I can squash in these two changes if you approve.
Okay. Shi Lei
- 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; } /**
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@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
(NB: As I looked further into the details of this, I decided it would be simpler for me to just post a few patches to take the place of this one patch. I've left in the reasoning for a couple of those patches which I had typed before I made the decision to just make a new series, but this patch can just be dropped, as the series I just sent here: https://www.redhat.com/archives/libvir-list/2020-December/msg00744.html takes care of everything done here, plus some other things.) On 12/13/20 8:50 PM, Shi Lei wrote:
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@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"); }
Actually 1) this entire clause can be eliminated - from the start if the "if (" to the "}" just before my comment - it is a remnant of code added all the way back in 2007 (commit a8977b62ba), but we have been doing the same clearing of "vnetN" generated names in the parser itself since commit 282d135d (2008), and this was made more complete/consistent in commit 77f72a861 (August 2019) which gives more details of the history (in case you're interested, which you probably aren't, and shouldn't be :-)). This is in Patch 2 of the patchset linked above. 2) Hmm. But I just noticed that in the case of the virNetDevCreateTap() used by FreeBSD/OpenBSD, we actually had forgotten to switch it to use virNetDevGenerateName() in your patches! I've remedied that in Patch 1 of the new patchset I linked above).
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;
This bool was added by commit 2b1f67d418 from 2009, and is used to clear out the autogenerated ifname in case of failure. We do want to preserve that behavior (in order to avoid some unwanted "cleaning up what isn't there" during failure), so while we can get rid of everything about adding VIR_NET_GENERATED_VNET_PREFIX + "%d", we need to remember that we generated this name, and clear it during a failure exit from the function (see Patch 3 of my new patchset)
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;
On 12/13/20 10:50 PM, Shi Lei wrote:
V2 here: https://www.redhat.com/archives/libvir-list/2020-December/msg00563.html
Since V2: * Fix libxl driver for missing changing virNetDevMacVLanReserveName
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
Series LGTM: Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
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/libxl/libxl_driver.c | 2 +- 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 +++++-------------- 15 files changed, 218 insertions(+), 400 deletions(-)
On 12/14/20 12:49 PM, Daniel Henrique Barboza wrote:
On 12/13/20 10:50 PM, Shi Lei wrote:
V2 here: https://www.redhat.com/archives/libvir-list/2020-December/msg00563.html
Since V2: * Fix libxl driver for missing changing virNetDevMacVLanReserveName
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
Series LGTM:
There are still a couple problems with this that we're working on (including an infinitely repeating segfault on restart if there are any running lxc domains with a type='direct' interface), so please don't push it!
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
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/libxl/libxl_driver.c | 2 +- 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 +++++-------------- 15 files changed, 218 insertions(+), 400 deletions(-)
On 12/13/20 8:50 PM, Shi Lei wrote:
V2 here: https://www.redhat.com/archives/libvir-list/2020-December/msg00563.html
Since V2: * Fix libxl driver for missing changing virNetDevMacVLanReserveName
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.
Thanks for working through all of those. For Patches 1-4: Reviewed-by: Laine Stump <laine@redhat.com> and pushed. Thanks for your contribution! For Patch 5 - it turned out simpler to just write a short series of patches to replace that, which I posted as https://www.redhat.com/archives/libvir-list/2020-December/msg00744.html
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/libxl/libxl_driver.c | 2 +- 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 +++++-------------- 15 files changed, 218 insertions(+), 400 deletions(-)
participants (3)
-
Daniel Henrique Barboza -
Laine Stump -
Shi Lei