[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

On 8/24/20 6:23 AM, Laine Stump wrote:
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

(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

Signed-off-by: Laine Stump <laine redhat com>
  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).
-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)
+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;

Hopefully, we won't use any of these in a forked off process because these are not async-signal safe anymore.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]