3:59 a.m.
V1: https://listman.redhat.com/archives/libvir-list/2021-August/msg00823.html
V2: https://listman.redhat.com/archives/libvir-list/2021-September/msg00249.html
V3: https://listman.redhat.com/archives/libvir-list/2021-September/msg00964.html
Changes since V3:
* Remove not strictly necessary heap allocations from virLockGuard
Regards,
Tim
Tim Wiederhake (9):
internal: Add CONCAT macro
virthread: Introduce virLockGuard
virthread: Introduce VIR_WITH_MUTEX_LOCK_GUARD
virobject: Introduce virObjectLockGuard
virobject: Introduce VIR_WITH_OBJECT_LOCK_GUARD
virChrdevFDStreamCloseCb: Use virLockGuardLock
virChrdevFree: Use VIR_WITH_MUTEX_LOCK
bhyveAutostartDomain: Use virObjectLockGuard
lxcDomainDetachDeviceHostdevUSBLive: Use VIR_WITH_OBJECT_LOCK_GUARD
src/bhyve/bhyve_driver.c | 4 ++--
src/conf/virchrdev.c | 12 +++++-------
src/internal.h | 3 +++
src/libvirt_private.syms | 3 +++
src/lxc/lxc_driver.c | 6 +++---
src/util/virobject.c | 16 ++++++++++++++++
src/util/virobject.h | 24 ++++++++++++++++++++++++
src/util/virthread.c | 15 +++++++++++++++
src/util/virthread.h | 30 ++++++++++++++++++++++++++++++
9 files changed, 101 insertions(+), 12 deletions(-)
--
2.31.1
3:59 a.m.
New subject: [libvirt PATCH v4 1/9] internal: Add CONCAT macro
Using the two-step idiom to force resolution of other macros, e.g.:
#define bar BAR
CONCAT_(foo, bar) // foobar
CONCAT(foo, bar) // fooBAR
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/internal.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/internal.h b/src/internal.h
index b6e4332542..4cfb022b41 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -100,6 +100,9 @@
#define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0)
#define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0)
+#define CONCAT_(a, b) a ## b
+#define CONCAT(a, b) CONCAT_(a, b)
+
#ifdef WIN32
# ifndef O_CLOEXEC
# define O_CLOEXEC _O_NOINHERIT
--
2.31.1
3:59 a.m.
New subject: [libvirt PATCH v4 2/9] virthread: Introduce virLockGuard
Locks a virMutex on creation and unlocks it in its destructor.
The VIR_LOCK_GUARD macro is used instead of "g_auto(virLockGuard)" to
work around a clang issue (see https://bugs.llvm.org/show_bug.cgi?id=3888
and https://bugs.llvm.org/show_bug.cgi?id=43482).
Typical usage:
void function(virMutex *m)
{
VIR_LOCK_GUARD lock = virLockGuardLock(m);
/* `m` is locked, and released automatically on scope exit */
...
while (expression) {
VIR_LOCK_GUARD lock2 = virLockGuardLock(...);
/* similar */
}
}
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/util/virthread.c | 15 +++++++++++++++
src/util/virthread.h | 10 ++++++++++
3 files changed, 27 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ba3462d849..4cfe6c17eb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3376,6 +3376,8 @@ virCondInit;
virCondSignal;
virCondWait;
virCondWaitUntil;
+virLockGuardLock;
+virLockGuardUnlock;
virMutexDestroy;
virMutexInit;
virMutexInitRecursive;
diff --git a/src/util/virthread.c b/src/util/virthread.c
index e89c1a09fb..5422bb74fd 100644
--- a/src/util/virthread.c
+++ b/src/util/virthread.c
@@ -96,6 +96,21 @@ void virMutexUnlock(virMutex *m)
pthread_mutex_unlock(&m->lock);
}
+virLockGuard virLockGuardLock(virMutex *m)
+{
+ virLockGuard l = { m };
+ virMutexLock(m);
+ return l;
+}
+
+void virLockGuardUnlock(virLockGuard *l)
+{
+ if (!l || !l->mutex)
+ return;
+
+ virMutexUnlock(g_steal_pointer(&l->mutex));
+}
+
int virRWLockInit(virRWLock *m)
{
diff --git a/src/util/virthread.h b/src/util/virthread.h
index 55c8263ae6..6cdaf2820e 100644
--- a/src/util/virthread.h
+++ b/src/util/virthread.h
@@ -31,6 +31,11 @@ struct virMutex {
pthread_mutex_t lock;
};
+typedef struct virLockGuard virLockGuard;
+struct virLockGuard {
+ virMutex *mutex;
+};
+
typedef struct virRWLock virRWLock;
struct virRWLock {
pthread_rwlock_t lock;
@@ -121,6 +126,11 @@ void virMutexLock(virMutex *m);
void virMutexUnlock(virMutex *m);
+virLockGuard virLockGuardLock(virMutex *m);
+void virLockGuardUnlock(virLockGuard *l);
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(virLockGuard, virLockGuardUnlock);
+#define VIR_LOCK_GUARD g_auto(virLockGuard) G_GNUC_UNUSED
+
int virRWLockInit(virRWLock *m) G_GNUC_WARN_UNUSED_RESULT;
void virRWLockDestroy(virRWLock *m);
--
2.31.1
3:59 a.m.
New subject: [libvirt PATCH v4 3/9] virthread: Introduce VIR_WITH_MUTEX_LOCK_GUARD
Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's include/qemu/lockable.h).
Uses "__LINE__" instead of "__COUNTER__", as the latter is a GNU
extension.
See comment for typical usage.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/util/virthread.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/util/virthread.h b/src/util/virthread.h
index 6cdaf2820e..da17780908 100644
--- a/src/util/virthread.h
+++ b/src/util/virthread.h
@@ -209,3 +209,23 @@ int virThreadLocalSet(virThreadLocal *l, void*)
G_GNUC_WARN_UNUSED_RESULT;
return 0; \
} \
struct classname ## EatSemicolon
+
+/**
+ * VIR_WITH_MUTEX_LOCK_GUARD:
+ *
+ * This macro defines a lock scope such that entering the scope takes the lock
+ * and leaving the scope releases the lock. Return statements are allowed
+ * within the scope and release the lock. Break and continue statements leave
+ * the scope early and release the lock.
+ *
+ * virMutex *mutex = ...;
+ *
+ * VIR_WITH_MUTEX_LOCK_GUARD(mutex) {
+ * // `mutex` is locked, and released automatically on scope exit
+ * ...
+ * }
+ */
+#define VIR_WITH_MUTEX_LOCK_GUARD(m) \
+ for (g_auto(virLockGuard) CONCAT(var, __LINE__) = virLockGuardLock(m); \
+ CONCAT(var, __LINE__).mutex; \
+ CONCAT(var, __LINE__).mutex = (virLockGuardUnlock(& CONCAT(var, __LINE__)),
NULL))
--
2.31.1
4:09 a.m.
New subject: [libvirt PATCH v4 3/9] virthread: Introduce VIR_WITH_MUTEX_LOCK_GUARD
On Fri, Jan 28, 2022 at 10:59:16AM +0100, Tim Wiederhake wrote:
Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's
include/qemu/lockable.h).
Uses "__LINE__" instead of "__COUNTER__", as the latter is a GNU
extension.
GNU extensions are fine to use, as we explicitly only support GCC
or CLang which is compatible with GCC, so I'd stick with __COUNTER__.
See comment for typical usage.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/util/virthread.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/util/virthread.h b/src/util/virthread.h
index 6cdaf2820e..da17780908 100644
--- a/src/util/virthread.h
+++ b/src/util/virthread.h
@@ -209,3 +209,23 @@ int virThreadLocalSet(virThreadLocal *l, void*)
G_GNUC_WARN_UNUSED_RESULT;
return 0; \
} \
struct classname ## EatSemicolon
+
+/**
+ * VIR_WITH_MUTEX_LOCK_GUARD:
+ *
+ * This macro defines a lock scope such that entering the scope takes the lock
+ * and leaving the scope releases the lock. Return statements are allowed
+ * within the scope and release the lock. Break and continue statements leave
+ * the scope early and release the lock.
+ *
+ * virMutex *mutex = ...;
+ *
+ * VIR_WITH_MUTEX_LOCK_GUARD(mutex) {
+ * // `mutex` is locked, and released automatically on scope exit
+ * ...
+ * }
+ */
+#define VIR_WITH_MUTEX_LOCK_GUARD(m) \
+ for (g_auto(virLockGuard) CONCAT(var, __LINE__) = virLockGuardLock(m); \
+ CONCAT(var, __LINE__).mutex; \
+ CONCAT(var, __LINE__).mutex = (virLockGuardUnlock(& CONCAT(var, __LINE__)),
NULL))
--
2.31.1
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
3:59 a.m.
New subject: [libvirt PATCH v4 4/9] virobject: Introduce virObjectLockGuard
Typical usage:
void foobar(virObjectLockable *obj)
{
VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
/* `obj` is locked, and released automatically on scope exit */
...
}
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virobject.c | 16 ++++++++++++++++
src/util/virobject.h | 4 ++++
3 files changed, 21 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4cfe6c17eb..463bb39de3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2944,6 +2944,7 @@ virObjectListFree;
virObjectListFreeCount;
virObjectLock;
virObjectLockableNew;
+virObjectLockGuard;
virObjectNew;
virObjectRef;
virObjectRWLockableNew;
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 588b41802c..74cc84205e 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -426,6 +426,22 @@ virObjectGetRWLockableObj(void *anyobj)
}
+/**
+ * virObjectLockGuard:
+ * @anyobj: any instance of virObjectLockable
+ *
+ * Acquire a lock on @anyobj that will be managed by the virLockGuard object
+ * returned to the caller.
+ */
+virLockGuard
+virObjectLockGuard(void *anyobj)
+{
+ virObjectLockable *obj = virObjectGetLockableObj(anyobj);
+
+ return virLockGuardLock(&obj->lock);
+}
+
+
/**
* virObjectLock:
* @anyobj: any instance of virObjectLockable
diff --git a/src/util/virobject.h b/src/util/virobject.h
index 1e34c77744..dc1ce66a4f 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -118,6 +118,10 @@ void *
virObjectRWLockableNew(virClass *klass)
ATTRIBUTE_NONNULL(1);
+virLockGuard
+virObjectLockGuard(void *lockableobj)
+ ATTRIBUTE_NONNULL(1);
+
void
virObjectLock(void *lockableobj)
ATTRIBUTE_NONNULL(1);
--
2.31.1
3:59 a.m.
New subject: [libvirt PATCH v4 5/9] virobject: Introduce VIR_WITH_OBJECT_LOCK_GUARD
Modeled after "WITH_QEMU_LOCK_GUARD" (see qemu's include/qemu/lockable.h).
Uses "__LINE__" instead of "__COUNTER__", as the latter is a GNU
extension.
See comment for typical usage.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/util/virobject.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/util/virobject.h b/src/util/virobject.h
index dc1ce66a4f..595076bda7 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -148,3 +148,23 @@ virObjectListFree(void *list);
void
virObjectListFreeCount(void *list,
size_t count);
+
+/**
+ * VIR_WITH_OBJECT_LOCK_GUARD:
+ *
+ * This macro defines a lock scope such that entering the scope takes the lock
+ * and leaving the scope releases the lock. Return statements are allowed
+ * within the scope and release the lock. Break and continue statements leave
+ * the scope early and release the lock.
+ *
+ * virObjectLockable *lockable = ...;
+ *
+ * VIR_WITH_OBJECT_LOCK_GUARD(lockable) {
+ * // `lockable` is locked, and released automatically on scope exit
+ * ...
+ * }
+ */
+#define VIR_WITH_OBJECT_LOCK_GUARD(o) \
+ for (g_auto(virLockGuard) CONCAT(var, __LINE__) = virObjectLockGuard(o); \
+ CONCAT(var, __LINE__).mutex; \
+ CONCAT(var, __LINE__).mutex = (virLockGuardUnlock(& CONCAT(var, __LINE__)),
NULL))
--
2.31.1
3:59 a.m.
New subject: [libvirt PATCH v4 6/9] virChrdevFDStreamCloseCb: Use virLockGuardLock
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/virchrdev.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index b5477b03d5..c9b2134e3b 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -237,12 +237,10 @@ static void virChrdevFDStreamCloseCb(virStreamPtr st G_GNUC_UNUSED,
void *opaque)
{
virChrdevStreamInfo *priv = opaque;
- virMutexLock(&priv->devs->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->devs->lock);
/* remove entry from hash */
virHashRemoveEntry(priv->devs->hash, priv->path);
-
- virMutexUnlock(&priv->devs->lock);
}
/**
--
2.31.1
3:59 a.m.
New subject: [libvirt PATCH v4 7/9] virChrdevFree: Use VIR_WITH_MUTEX_LOCK
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/virchrdev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index c9b2134e3b..8610f0ac5c 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -291,10 +291,10 @@ void virChrdevFree(virChrdevs *devs)
if (!devs)
return;
- virMutexLock(&devs->lock);
- virHashForEachSafe(devs->hash, virChrdevFreeClearCallbacks, NULL);
- g_clear_pointer(&devs->hash, g_hash_table_unref);
- virMutexUnlock(&devs->lock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&devs->lock) {
+ virHashForEachSafe(devs->hash, virChrdevFreeClearCallbacks, NULL);
+ g_clear_pointer(&devs->hash, g_hash_table_unref);
+ }
virMutexDestroy(&devs->lock);
g_free(devs);
--
2.31.1
3:59 a.m.
New subject: [libvirt PATCH v4 8/9] bhyveAutostartDomain: Use virObjectLockGuard
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/bhyve/bhyve_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index f291f12e50..47ee98e650 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -87,7 +87,8 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque)
{
const struct bhyveAutostartData *data = opaque;
int ret = 0;
- virObjectLock(vm);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(vm);
+
if (vm->autostart && !virDomainObjIsActive(vm)) {
virResetLastError();
ret = virBhyveProcessStart(data->conn, vm,
@@ -98,7 +99,6 @@ bhyveAutostartDomain(virDomainObj *vm, void *opaque)
vm->def->name, virGetLastErrorMessage());
}
}
- virObjectUnlock(vm);
return ret;
}
--
2.31.1
3:59 a.m.
New subject: [libvirt PATCH v4 9/9] lxcDomainDetachDeviceHostdevUSBLive: Use VIR_WITH_OBJECT_LOCK_GUARD
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/lxc/lxc_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 7bc39120ee..42053de9c3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4045,9 +4045,9 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriver *driver,
VIR_WARN("cannot deny device %s for domain %s: %s",
dst, vm->def->name, virGetLastErrorMessage());
- virObjectLock(hostdev_mgr->activeUSBHostdevs);
- virUSBDeviceListDel(hostdev_mgr->activeUSBHostdevs, usb);
- virObjectUnlock(hostdev_mgr->activeUSBHostdevs);
+ VIR_WITH_OBJECT_LOCK_GUARD(hostdev_mgr->activeUSBHostdevs) {
+ virUSBDeviceListDel(hostdev_mgr->activeUSBHostdevs, usb);
+ }
virDomainHostdevRemove(vm->def, idx);
virDomainHostdevDefFree(def);
--
2.31.1
1030
days inactive
1030
days old
10 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Tim Wiederhake