[libvirt] [PATCH v2] virnetdevmacvlan.c: Introduce mutex for macvlan creation

[Daniel P. Berrange]

On Fri, Mar 01, 2013 at 11:36:24AM +0100, Michal Privoznik wrote:
> Currently, after we removed the qemu driver lock, it may happen
> that two or more threads will start up a machine with macvlan and
> race over virNetDevMacVLanCreateWithVPortProfile(). However,
> there's a racy section in which we are generating a sequence of
> possible device names and detecting if they exits. If we found
> one which doesn't we try to create a device with that name.
> However, the other thread is doing just the same. Assume it will
> succeed and we must therefore fail. If this happens more than 5
> times (which in massive parallel startup surely will) we return
> -1 without any error reported. This patch is a simple hack to
> both of these problems. It introduces a mutex, so only one thread
> will enter the section, and if it runs out of possibilities,
> error is reported. Moreover, the number of retries is raised to 20.
> ---
> 
> diff to v1:
> -don't increase @retries
> -move error reporting to virNetDevMacVLanCreateMutexOnceInit
> 
>  src/util/virnetdevmacvlan.c | 37 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> index a74db1e..ddea11f 100644
> --- a/src/util/virnetdevmacvlan.c
> +++ b/src/util/virnetdevmacvlan.c
> @@ -31,6 +31,7 @@
>  #include "virmacaddr.h"
>  #include "virutil.h"
>  #include "virerror.h"
> +#include "virthread.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_NET
>  
> @@ -71,6 +72,19 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST,
>  # define MACVLAN_NAME_PREFIX	"macvlan"
>  # define MACVLAN_NAME_PATTERN	"macvlan%d"
>  
> +virMutex virNetDevMacVLanCreateMutex;
> +
> +static int virNetDevMacVLanCreateMutexOnceInit(void)
> +{
> +    if (virMutexInit(&virNetDevMacVLanCreateMutex) < 0) {
> +        virReportSystemError(errno, "%s", _("unable to init mutex"));
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +VIR_ONCE_GLOBAL_INIT(virNetDevMacVLanCreateMutex);
> +
>  /**
>   * virNetDevMacVLanCreate:
>   *
> @@ -829,7 +843,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
>      char ifname[IFNAMSIZ];
>      int retries, do_retry = 0;
>      uint32_t macvtapMode;
> -    const char *cr_ifname;
> +    const char *cr_ifname = NULL;
>      int ret;
>      int vf = -1;
>  
> @@ -871,22 +885,35 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
>      } else {
>  create_name:
>          retries = 5;
> +        if (virNetDevMacVLanCreateMutexInitialize() < 0)
> +            return -1;
> +        virMutexLock(&virNetDevMacVLanCreateMutex);
>          for (c = 0; c < 8192; c++) {
>              snprintf(ifname, sizeof(ifname), pattern, c);
> -            if ((ret = virNetDevExists(ifname)) < 0)
> +            if ((ret = virNetDevExists(ifname)) < 0) {
> +                virMutexUnlock(&virNetDevMacVLanCreateMutex);
>                  return -1;
> +            }
>              if (!ret) {
>                  rc = virNetDevMacVLanCreate(ifname, type, macaddress, linkdev,
>                                              macvtapMode, &do_retry);
> -                if (rc == 0)
> +                if (rc == 0) {
> +                    cr_ifname = ifname;
>                      break;
> +                }
>  
>                  if (do_retry && --retries)
>                      continue;
> -                return -1;
> +                break;
>              }
>          }
> -        cr_ifname = ifname;
> +
> +        virMutexUnlock(&virNetDevMacVLanCreateMutex);
> +        if (!cr_ifname) {
> +            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                           _("Unable to create macvlan device"));
> +            return -1;
> +        }
>      }
>  
>      if (virNetDevVPortProfileAssociate(cr_ifname,

ACK

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|