On 02/28/2013 08:25 AM, 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.
> ---
> 
> @@ -870,23 +880,39 @@ int virNetDevMacVLanCreateWithVPortProfile(const char 
> *tgifname,
>              return -1;
>      } else {
>  create_name:
> -        retries = 5;
> +        if (virNetDevMacVLanCreateMutexInitialize() < 0) {
> +            virReportSystemError(errno, "%s", _("unable to init mutext"));

s/mutext/mutex/

> +            return -1;
> +        }
> +
> +        retries = 20;

Do we really need to bump retries up, if the point of the mutex was to
prevent the need for that?

> +        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;
> +        }

Seems reasonable, but you might want a second reviewer since we are so
close to release.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to