[PATCHv2 3/5] netdevmacvlan: Use helper function to create unique macvlan/macvtap name

Laine Stump laine at redhat.com
Mon Dec 14 00:58:46 UTC 2020


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 at 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)
> 




More information about the libvir-list mailing list