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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list