7:20 p.m.
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)
>