Re: [libvirt PATCH 1/3] util: make locking versions of virNetDevMacVLan(Reserve|Release)Name()

When these functions are called from within virnetdevmacvlan.c, they are usually called with virNetDevMacVLanCreateMutex held, but when virNetDevMacVLanReserveName() is called from other places (hypervisor drivers keeping track of already-in-use macvlan/macvtap devices) the lock isn't acquired. This could lead to a situation where one thread is setting a bit in the bitmap to notify of a device already in-use, while another thread is checking/setting/clearing a bit while creating a new macvtap device. In practice this *probably* doesn't happen, because the external calls to virNetDevMacVLan() only happen during hypervisor driver init routines when libvirtd is restarted, but there's no harm in protecting ourselves. (NB: virNetDevMacVLanReleaseName() is actually never called from outside virnetdevmacvlan.c, so it could just as well be static, but I'm leaving it as-is for now. This locking version *is* called from within virnetdevmacvlan.c, since there are a couple places that we used to call the unlocked version after the lock was already released.) Signed-off-by: Laine Stump <laine(a)redhat.com&gt; --- src/util/virnetdevmacvlan.c | 42 ++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index dcea93a5fe..69a9c784bb 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -196,7 +196,7 @@ virNetDevMacVLanReleaseID(int id, unsigned int flags) /** - * virNetDevMacVLanReserveName: + * virNetDevMacVLanReserveNameInternal: * * @name: already-known name of device * @quietFail: don't log an error if this name is already in-use @@ -208,8 +208,8 @@ virNetDevMacVLanReleaseID(int id, unsigned int flags) * Returns reserved ID# on success, -1 on failure, -2 if the name * doesn't fit the auto-pattern (so not reserveable). */ -int -virNetDevMacVLanReserveName(const char *name, bool quietFail) +static int +virNetDevMacVLanReserveNameInternal(const char *name, bool quietFail) { unsigned int id; unsigned int flags = 0; @@ -237,8 +237,21 @@ virNetDevMacVLanReserveName(const char *name, bool quietFail) } +int +virNetDevMacVLanReserveName(const char *name, bool quietFail) +{ + /* Call the internal function after locking the macvlan mutex */ + int ret; + + virMutexLock(&virNetDevMacVLanCreateMutex); + ret = virNetDevMacVLanReserveNameInternal(name, quietFail); + virMutexUnlock(&virNetDevMacVLanCreateMutex); + return ret; +}