7:12 a.m.
Use the recently implemented VIR_LOCK_GUARD and VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
This made a solitary "virMutexUnlock()" call without previous call to
"virMutexLock()" in ch_driver.c obvious, which is removed in patch #8.
Tim Wiederhake (11):
virthreadpool: Use automatic memory management
virthreadpool: Cleanup
libxl: Use automatic memory management
lxc: Prepare virLXCDriverGetCapabilities for automatic mutex
management
lxc: Use automatic mutex management
ch: Prepare virCHDriverGetCapabilities for automatic mutex management
ch: Use automatic mutex management
ch: Remove solitary virMutexUnlock
network: Use automatic mutex management
bhyve_driver: Use automatic mutex management
node_device: Use automatic mutex management
src/bhyve/bhyve_conf.c | 7 +--
src/bhyve/bhyve_driver.c | 12 ----
src/bhyve/bhyve_utils.h | 3 -
src/ch/ch_conf.c | 31 +++++-----
src/ch/ch_conf.h | 10 ----
src/ch/ch_driver.c | 29 +++++----
src/libxl/libxl_conf.c | 8 +--
src/libxl/libxl_conf.h | 12 ----
src/libxl/libxl_logger.c | 20 +++----
src/lxc/lxc_conf.c | 41 ++++++-------
src/lxc/lxc_conf.h | 9 ---
src/lxc/lxc_controller.c | 25 ++++----
src/lxc/lxc_fuse.c | 12 ++--
src/network/bridge_driver.c | 41 ++++---------
src/node_device/node_device_driver.c | 23 ++-----
src/node_device/node_device_udev.c | 52 +++++++---------
src/util/virthreadpool.c | 90 ++++++++--------------------
17 files changed, 138 insertions(+), 287 deletions(-)
--
2.31.1
7:12 a.m.
New subject: [libvirt PATCH 01/11] virthreadpool: Use automatic memory management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/util/virthreadpool.c | 39 +++++++++++++--------------------------
1 file changed, 13 insertions(+), 26 deletions(-)
diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c
index 426840e435..b6d154802a 100644
--- a/src/util/virthreadpool.c
+++ b/src/util/virthreadpool.c
@@ -306,15 +306,13 @@ void virThreadPoolFree(virThreadPool *pool)
if (!pool)
return;
- virMutexLock(&pool->mutex);
- virThreadPoolDrainLocked(pool);
+ virThreadPoolDrain(pool);
if (pool->identity)
g_object_unref(pool->identity);
g_free(pool->jobName);
g_free(pool->workers);
- virMutexUnlock(&pool->mutex);
virMutexDestroy(&pool->mutex);
virCondDestroy(&pool->quit_cond);
virCondDestroy(&pool->cond);
@@ -326,66 +324,60 @@ void virThreadPoolFree(virThreadPool *pool)
size_t virThreadPoolGetMinWorkers(virThreadPool *pool)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
size_t ret;
- virMutexLock(&pool->mutex);
ret = pool->minWorkers;
- virMutexUnlock(&pool->mutex);
return ret;
}
size_t virThreadPoolGetMaxWorkers(virThreadPool *pool)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
size_t ret;
- virMutexLock(&pool->mutex);
ret = pool->maxWorkers;
- virMutexUnlock(&pool->mutex);
return ret;
}
size_t virThreadPoolGetPriorityWorkers(virThreadPool *pool)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
size_t ret;
- virMutexLock(&pool->mutex);
ret = pool->nPrioWorkers;
- virMutexUnlock(&pool->mutex);
return ret;
}
size_t virThreadPoolGetCurrentWorkers(virThreadPool *pool)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
size_t ret;
- virMutexLock(&pool->mutex);
ret = pool->nWorkers;
- virMutexUnlock(&pool->mutex);
return ret;
}
size_t virThreadPoolGetFreeWorkers(virThreadPool *pool)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
size_t ret;
- virMutexLock(&pool->mutex);
ret = pool->freeWorkers;
- virMutexUnlock(&pool->mutex);
return ret;
}
size_t virThreadPoolGetJobQueueDepth(virThreadPool *pool)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
size_t ret;
- virMutexLock(&pool->mutex);
ret = pool->jobQueueDepth;
- virMutexUnlock(&pool->mutex);
return ret;
}
@@ -398,9 +390,9 @@ int virThreadPoolSendJob(virThreadPool *pool,
unsigned int priority,
void *jobData)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
virThreadPoolJob *job;
- virMutexLock(&pool->mutex);
if (pool->quit)
goto error;
@@ -431,11 +423,9 @@ int virThreadPoolSendJob(virThreadPool *pool,
if (priority)
virCondSignal(&pool->prioCond);
- virMutexUnlock(&pool->mutex);
return 0;
error:
- virMutexUnlock(&pool->mutex);
return -1;
}
@@ -445,11 +435,10 @@ virThreadPoolSetParameters(virThreadPool *pool,
long long int maxWorkers,
long long int prioWorkers)
{
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
size_t max;
size_t min;
- virMutexLock(&pool->mutex);
-
max = maxWorkers >= 0 ? maxWorkers : pool->maxWorkers;
min = minWorkers >= 0 ? minWorkers : pool->minWorkers;
if (min > max) {
@@ -490,26 +479,24 @@ virThreadPoolSetParameters(virThreadPool *pool,
pool->maxPrioWorkers = prioWorkers;
}
- virMutexUnlock(&pool->mutex);
return 0;
error:
- virMutexUnlock(&pool->mutex);
return -1;
}
void
virThreadPoolStop(virThreadPool *pool)
{
- virMutexLock(&pool->mutex);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
+
virThreadPoolStopLocked(pool);
- virMutexUnlock(&pool->mutex);
}
void
virThreadPoolDrain(virThreadPool *pool)
{
- virMutexLock(&pool->mutex);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
+
virThreadPoolDrainLocked(pool);
- virMutexUnlock(&pool->mutex);
}
--
2.31.1
7:57 a.m.
New subject: [libvirt PATCH 01/11] virthreadpool: Use automatic memory management
On 2/7/22 14:12, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
s/memory/lock/ in $SUBJ.
---
src/util/virthreadpool.c | 39 +++++++++++++--------------------------
1 file changed, 13 insertions(+), 26 deletions(-)
diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c
index 426840e435..b6d154802a 100644
--- a/src/util/virthreadpool.c
+++ b/src/util/virthreadpool.c
@@ -306,15 +306,13 @@ void virThreadPoolFree(virThreadPool *pool)
if (!pool)
return;
- virMutexLock(&pool->mutex);
Look at this made me realize we need to check pool->quit in
virThreadPoolSetParameters(). I was trying to come up with a path where
removing this lock acquire would create a problem (because removing a
mutex makes all the control lights flash for me), but failed. I'm still
trying to convince myself this is safe thing to do.
On the other hand, if we used automatic mutex management, then ...
- virThreadPoolDrainLocked(pool);
+ virThreadPoolDrain(pool);
if (pool->identity)
g_object_unref(pool->identity);
g_free(pool->jobName);
g_free(pool->workers);
- virMutexUnlock(&pool->mutex);
virMutexDestroy(&pool->mutex);
.. this would destroy a locked mutex. So I guess your code is right.
virCondDestroy(&pool->quit_cond);
virCondDestroy(&pool->cond);
Michal
7:12 a.m.
New subject: [libvirt PATCH 02/11] virthreadpool: Cleanup
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/util/virthreadpool.c | 51 ++++++++++------------------------------
1 file changed, 13 insertions(+), 38 deletions(-)
diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c
index b6d154802a..2f1d49d665 100644
--- a/src/util/virthreadpool.c
+++ b/src/util/virthreadpool.c
@@ -325,61 +325,43 @@ void virThreadPoolFree(virThreadPool *pool)
size_t virThreadPoolGetMinWorkers(virThreadPool *pool)
{
VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
- size_t ret;
- ret = pool->minWorkers;
-
- return ret;
+ return pool->minWorkers;
}
size_t virThreadPoolGetMaxWorkers(virThreadPool *pool)
{
VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
- size_t ret;
-
- ret = pool->maxWorkers;
- return ret;
+ return pool->maxWorkers;
}
size_t virThreadPoolGetPriorityWorkers(virThreadPool *pool)
{
VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
- size_t ret;
-
- ret = pool->nPrioWorkers;
- return ret;
+ return pool->nPrioWorkers;
}
size_t virThreadPoolGetCurrentWorkers(virThreadPool *pool)
{
VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
- size_t ret;
- ret = pool->nWorkers;
-
- return ret;
+ return pool->nWorkers;
}
size_t virThreadPoolGetFreeWorkers(virThreadPool *pool)
{
VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
- size_t ret;
-
- ret = pool->freeWorkers;
- return ret;
+ return pool->freeWorkers;
}
size_t virThreadPoolGetJobQueueDepth(virThreadPool *pool)
{
VIR_LOCK_GUARD lock = virLockGuardLock(&pool->mutex);
- size_t ret;
- ret = pool->jobQueueDepth;
-
- return ret;
+ return pool->jobQueueDepth;
}
/*
@@ -394,12 +376,12 @@ int virThreadPoolSendJob(virThreadPool *pool,
virThreadPoolJob *job;
if (pool->quit)
- goto error;
+ return -1;
if (pool->freeWorkers - pool->jobQueueDepth <= 0 &&
pool->nWorkers < pool->maxWorkers &&
virThreadPoolExpand(pool, 1, false) < 0)
- goto error;
+ return -1;
job = g_new0(virThreadPoolJob, 1);
@@ -424,9 +406,6 @@ int virThreadPoolSendJob(virThreadPool *pool,
virCondSignal(&pool->prioCond);
return 0;
-
- error:
- return -1;
}
int
@@ -444,7 +423,7 @@ virThreadPoolSetParameters(virThreadPool *pool,
if (min > max) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("minWorkers cannot be larger than maxWorkers"));
- goto error;
+ return -1;
}
if ((maxWorkers == 0 && pool->maxWorkers > 0) ||
@@ -452,14 +431,13 @@ virThreadPoolSetParameters(virThreadPool *pool,
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("maxWorkers must not be switched from zero to
non-zero"
" and vice versa"));
- goto error;
+ return -1;
}
if (minWorkers >= 0) {
if ((size_t) minWorkers > pool->nWorkers &&
- virThreadPoolExpand(pool, minWorkers - pool->nWorkers,
- false) < 0)
- goto error;
+ virThreadPoolExpand(pool, minWorkers - pool->nWorkers, false) < 0)
+ return -1;
pool->minWorkers = minWorkers;
}
@@ -474,15 +452,12 @@ virThreadPoolSetParameters(virThreadPool *pool,
} else if ((size_t) prioWorkers > pool->nPrioWorkers &&
virThreadPoolExpand(pool, prioWorkers - pool->nPrioWorkers,
true) < 0) {
- goto error;
+ return -1;
}
pool->maxPrioWorkers = prioWorkers;
}
return 0;
-
- error:
- return -1;
}
void
--
2.31.1
7:12 a.m.
New subject: [libvirt PATCH 03/11] libxl: Use automatic memory management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/libxl/libxl_conf.c | 8 ++------
src/libxl/libxl_conf.h | 12 ------------
src/libxl/libxl_logger.c | 20 +++++++++-----------
3 files changed, 11 insertions(+), 29 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f062f8e958..a0fc51c74a 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1815,12 +1815,8 @@ libxlDriverConfigInit(libxlDriverConfig *cfg)
libxlDriverConfig *
libxlDriverConfigGet(libxlDriverPrivate *driver)
{
- libxlDriverConfig *cfg;
-
- libxlDriverLock(driver);
- cfg = virObjectRef(driver->config);
- libxlDriverUnlock(driver);
- return cfg;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
+ return virObjectRef(driver->config);
}
int libxlDriverConfigLoadFile(libxlDriverConfig *cfg,
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index b74f455b69..16a7195bfd 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -219,15 +219,3 @@ libxlBuildDomainConfig(virPortAllocatorRange *graphicsports,
virDomainDef *def,
libxlDriverConfig *cfg,
libxl_domain_config *d_config);
-
-static inline void
-libxlDriverLock(libxlDriverPrivate *driver)
-{
- virMutexLock(&driver->lock);
-}
-
-static inline void
-libxlDriverUnlock(libxlDriverPrivate *driver)
-{
- virMutexUnlock(&driver->lock);
-}
diff --git a/src/libxl/libxl_logger.c b/src/libxl/libxl_logger.c
index 4708553f29..45cdc32f38 100644
--- a/src/libxl/libxl_logger.c
+++ b/src/libxl/libxl_logger.c
@@ -79,16 +79,16 @@ libvirt_vmessage(xentoollog_logger *logger_in,
/* Should we print to a domain-specific log file? */
if ((start = strstr(message, ": Domain ")) &&
(end = strstr(start + 9, ":"))) {
- FILE *domainLogFile;
+ FILE *domainLogFile = NULL;
VIR_DEBUG("Found domain log message");
start = start + 9;
*end = '\0';
- virMutexLock(&lg->tableLock);
- domainLogFile = virHashLookup(lg->files, start);
- virMutexUnlock(&lg->tableLock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&lg->tableLock) {
+ domainLogFile = virHashLookup(lg->files, start);
+ }
if (domainLogFile)
logFile = domainLogFile;
@@ -197,9 +197,9 @@ libxlLoggerOpenFile(libxlLogger *logger,
path, g_strerror(errno));
return;
}
- virMutexLock(&logger->tableLock);
- ignore_value(virHashAddEntry(logger->files, domidstr, logFile));
- virMutexUnlock(&logger->tableLock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&logger->tableLock) {
+ ignore_value(virHashAddEntry(logger->files, domidstr, logFile));
+ }
/* domain_config is non NULL only when starting a new domain */
if (domain_config) {
@@ -211,10 +211,8 @@ libxlLoggerOpenFile(libxlLogger *logger,
void
libxlLoggerCloseFile(libxlLogger *logger, int id)
{
- g_autofree char *domidstr = NULL;
- domidstr = g_strdup_printf("%d", id);
+ g_autofree char *domidstr = g_strdup_printf("%d", id);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&logger->tableLock);
- virMutexLock(&logger->tableLock);
ignore_value(virHashRemoveEntry(logger->files, domidstr));
- virMutexUnlock(&logger->tableLock);
}
--
2.31.1
7:12 a.m.
New subject: [libvirt PATCH 04/11] lxc: Prepare virLXCDriverGetCapabilities for automatic mutex management
No functional change intended. This change makes the recfatoring to
automatic mutex management easier to follow.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/lxc/lxc_conf.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index 8955578d54..cf6679711a 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -156,28 +156,28 @@ virCaps *virLXCDriverCapsInit(virLXCDriver *driver)
virCaps *virLXCDriverGetCapabilities(virLXCDriver *driver,
bool refresh)
{
- virCaps *ret;
- if (refresh) {
- virCaps *caps = NULL;
- if ((caps = virLXCDriverCapsInit(driver)) == NULL)
- return NULL;
+ virCaps *ret = NULL;
+ virCaps *caps = NULL;
+
+ lxcDriverLock(driver);
+ if (!refresh && !driver->caps) {
+ VIR_DEBUG("Capabilities didn't detect any guests. Forcing a
refresh.");
+ refresh = true;
+ }
+ lxcDriverUnlock(driver);
- lxcDriverLock(driver);
+ if (refresh && !(caps = virLXCDriverCapsInit(driver)))
+ return NULL;
+
+ lxcDriverLock(driver);
+ if (refresh) {
virObjectUnref(driver->caps);
driver->caps = caps;
- } else {
- lxcDriverLock(driver);
-
- if (driver->caps == NULL) {
- VIR_DEBUG("Capabilities didn't detect any guests. Forcing a "
- "refresh.");
- lxcDriverUnlock(driver);
- return virLXCDriverGetCapabilities(driver, true);
- }
}
ret = virObjectRef(driver->caps);
lxcDriverUnlock(driver);
+
return ret;
}
--
2.31.1
7:12 a.m.
New subject: [libvirt PATCH 05/11] lxc: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/lxc/lxc_conf.c | 31 ++++++++++++++-----------------
src/lxc/lxc_conf.h | 9 ---------
src/lxc/lxc_controller.c | 25 ++++++++++---------------
src/lxc/lxc_fuse.c | 12 ++++++------
4 files changed, 30 insertions(+), 47 deletions(-)
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index cf6679711a..3ab4d61fe3 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -159,24 +159,24 @@ virCaps *virLXCDriverGetCapabilities(virLXCDriver *driver,
virCaps *ret = NULL;
virCaps *caps = NULL;
- lxcDriverLock(driver);
- if (!refresh && !driver->caps) {
- VIR_DEBUG("Capabilities didn't detect any guests. Forcing a
refresh.");
- refresh = true;
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ if (!refresh && !driver->caps) {
+ VIR_DEBUG("Capabilities didn't detect any guests. Forcing a
refresh.");
+ refresh = true;
+ }
}
- lxcDriverUnlock(driver);
if (refresh && !(caps = virLXCDriverCapsInit(driver)))
return NULL;
- lxcDriverLock(driver);
- if (refresh) {
- virObjectUnref(driver->caps);
- driver->caps = caps;
- }
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ if (refresh) {
+ virObjectUnref(driver->caps);
+ driver->caps = caps;
+ }
- ret = virObjectRef(driver->caps);
- lxcDriverUnlock(driver);
+ ret = virObjectRef(driver->caps);
+ }
return ret;
}
@@ -248,11 +248,8 @@ virLXCLoadDriverConfig(virLXCDriverConfig *cfg,
virLXCDriverConfig *virLXCDriverGetConfig(virLXCDriver *driver)
{
- virLXCDriverConfig *cfg;
- lxcDriverLock(driver);
- cfg = virObjectRef(driver->config);
- lxcDriverUnlock(driver);
- return cfg;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
+ return virObjectRef(driver->config);
}
static void
diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
index 5a1351bd63..04e51aa954 100644
--- a/src/lxc/lxc_conf.h
+++ b/src/lxc/lxc_conf.h
@@ -113,12 +113,3 @@ virCaps *virLXCDriverGetCapabilities(virLXCDriver *driver,
bool refresh);
virDomainXMLOption *lxcDomainXMLConfInit(virLXCDriver *driver,
const char *defsecmodel);
-
-static inline void lxcDriverLock(virLXCDriver *driver)
-{
- virMutexLock(&driver->lock);
-}
-static inline void lxcDriverUnlock(virLXCDriver *driver)
-{
- virMutexUnlock(&driver->lock);
-}
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index c4e3b66751..b5289b2448 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -995,7 +995,7 @@ static int lxcControllerClearCapabilities(void)
}
static bool wantReboot;
-static virMutex lock = VIR_MUTEX_INITIALIZER;
+static virMutex mutex = VIR_MUTEX_INITIALIZER;
static int
virLXCControllerEventSendExit(virLXCController *ctrl,
@@ -1012,13 +1012,13 @@ static void virLXCControllerSignalChildIO(virNetDaemon *dmn
G_GNUC_UNUSED,
ret = waitpid(-1, &status, WNOHANG);
VIR_DEBUG("Got sig child %d vs %lld", ret, (long long)ctrl->initpid);
if (ret == ctrl->initpid) {
- virMutexLock(&lock);
- if (WIFSIGNALED(status) &&
- WTERMSIG(status) == SIGHUP) {
- VIR_DEBUG("Status indicates reboot");
- wantReboot = true;
+ VIR_WITH_MUTEX_LOCK_GUARD(&mutex) {
+ if (WIFSIGNALED(status) &&
+ WTERMSIG(status) == SIGHUP) {
+ VIR_DEBUG("Status indicates reboot");
+ wantReboot = true;
+ }
}
- virMutexUnlock(&lock);
virLXCControllerEventSendExit(ctrl, wantReboot ? 1 : 0);
}
}
@@ -1132,8 +1132,8 @@ static void
virLXCControllerConsoleUpdateWatch(virLXCControllerConsole *console)
static void virLXCControllerConsoleEPoll(int watch, int fd, int events, void *opaque)
{
virLXCControllerConsole *console = opaque;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&mutex);
- virMutexLock(&lock);
VIR_DEBUG("IO event watch=%d fd=%d events=%d fromHost=%zu fromcont=%zu",
watch, fd, events,
console->fromHostLen,
@@ -1149,7 +1149,7 @@ static void virLXCControllerConsoleEPoll(int watch, int fd, int
events, void *op
virReportSystemError(errno, "%s",
_("Unable to wait on epoll"));
virNetDaemonQuit(console->daemon);
- goto cleanup;
+ return;
}
if (ret == 0)
@@ -1171,16 +1171,13 @@ static void virLXCControllerConsoleEPoll(int watch, int fd, int
events, void *op
break;
}
}
-
- cleanup:
- virMutexUnlock(&lock);
}
static void virLXCControllerConsoleIO(int watch, int fd, int events, void *opaque)
{
virLXCControllerConsole *console = opaque;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&mutex);
- virMutexLock(&lock);
VIR_DEBUG("IO event watch=%d fd=%d events=%d fromHost=%zu fromcont=%zu",
watch, fd, events,
console->fromHostLen,
@@ -1254,7 +1251,6 @@ static void virLXCControllerConsoleIO(int watch, int fd, int events,
void *opaqu
}
virLXCControllerConsoleUpdateWatch(console);
- virMutexUnlock(&lock);
return;
error:
@@ -1262,7 +1258,6 @@ static void virLXCControllerConsoleIO(int watch, int fd, int events,
void *opaqu
virEventRemoveHandle(console->hostWatch);
console->contWatch = console->hostWatch = -1;
virNetDaemonQuit(console->daemon);
- virMutexUnlock(&lock);
}
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index c2fe8f0e60..2e600e4402 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -260,11 +260,11 @@ static struct fuse_operations lxcProcOper = {
static void lxcFuseDestroy(struct virLXCFuse *fuse)
{
- virMutexLock(&fuse->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&fuse->lock);
+
fuse_unmount(fuse->mountpoint, fuse->ch);
fuse_destroy(fuse->fuse);
fuse->fuse = NULL;
- virMutexUnlock(&fuse->lock);
}
static void lxcFuseRun(void *opaque)
@@ -346,10 +346,10 @@ void lxcFreeFuse(struct virLXCFuse **f)
if (fuse) {
/* exit fuse_loop, lxcFuseRun thread may try to destroy
* fuse->fuse at the same time,so add a lock here. */
- virMutexLock(&fuse->lock);
- if (fuse->fuse)
- fuse_exit(fuse->fuse);
- virMutexUnlock(&fuse->lock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&fuse->lock) {
+ if (fuse->fuse)
+ fuse_exit(fuse->fuse);
+ }
g_free(fuse->mountpoint);
g_free(*f);
--
2.31.1
7:12 a.m.
New subject: [libvirt PATCH 06/11] ch: Prepare virCHDriverGetCapabilities for automatic mutex management
No functional change intended. This change makes the refactoring to
automatic mutex management easier to follow.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/ch/ch_conf.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
index be12934dcd..cdf69e3e70 100644
--- a/src/ch/ch_conf.c
+++ b/src/ch/ch_conf.c
@@ -87,21 +87,22 @@ virCaps *virCHDriverCapsInit(void)
virCaps *virCHDriverGetCapabilities(virCHDriver *driver,
bool refresh)
{
- virCaps *ret;
- if (refresh) {
- virCaps *caps = NULL;
- if ((caps = virCHDriverCapsInit()) == NULL)
- return NULL;
+ virCaps *ret = NULL;
+ virCaps *caps = NULL;
- chDriverLock(driver);
+ if (refresh && !(caps = virCHDriverCapsInit()))
+ return NULL;
+
+ chDriverLock(driver);
+
+ if (refresh) {
virObjectUnref(driver->caps);
driver->caps = caps;
- } else {
- chDriverLock(driver);
}
ret = virObjectRef(driver->caps);
chDriverUnlock(driver);
+
return ret;
}
--
2.31.1
7:12 a.m.
New subject: [libvirt PATCH 07/11] ch: Use automatic mutex management
This leaves a bogus `virMutexUnlock` in `chDomainCreateXML`.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/ch/ch_conf.c | 20 ++++++++------------
src/ch/ch_conf.h | 10 ----------
src/ch/ch_driver.c | 30 +++++++++++++++---------------
3 files changed, 23 insertions(+), 37 deletions(-)
diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
index cdf69e3e70..ccc01ecdab 100644
--- a/src/ch/ch_conf.c
+++ b/src/ch/ch_conf.c
@@ -93,16 +93,15 @@ virCaps *virCHDriverGetCapabilities(virCHDriver *driver,
if (refresh && !(caps = virCHDriverCapsInit()))
return NULL;
- chDriverLock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ if (refresh) {
+ virObjectUnref(driver->caps);
+ driver->caps = caps;
+ }
- if (refresh) {
- virObjectUnref(driver->caps);
- driver->caps = caps;
+ ret = virObjectRef(driver->caps);
}
- ret = virObjectRef(driver->caps);
- chDriverUnlock(driver);
-
return ret;
}
@@ -159,11 +158,8 @@ virCHDriverConfigNew(bool privileged)
virCHDriverConfig *virCHDriverGetConfig(virCHDriver *driver)
{
- virCHDriverConfig *cfg;
- chDriverLock(driver);
- cfg = virObjectRef(driver->config);
- chDriverUnlock(driver);
- return cfg;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
+ return virObjectRef(driver->config);
}
static void
diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h
index c56caa3f5f..b927621a97 100644
--- a/src/ch/ch_conf.h
+++ b/src/ch/ch_conf.h
@@ -78,13 +78,3 @@ virDomainXMLOption *chDomainXMLConfInit(virCHDriver *driver);
virCHDriverConfig *virCHDriverConfigNew(bool privileged);
virCHDriverConfig *virCHDriverGetConfig(virCHDriver *driver);
int chExtractVersion(virCHDriver *driver);
-
-static inline void chDriverLock(virCHDriver *driver)
-{
- virMutexLock(&driver->lock);
-}
-
-static inline void chDriverUnlock(virCHDriver *driver)
-{
- virMutexUnlock(&driver->lock);
-}
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 3223f31367..945a1aa225 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -106,9 +106,10 @@ static int chConnectGetVersion(virConnectPtr conn,
if (virConnectGetVersionEnsureACL(conn) < 0)
return -1;
- chDriverLock(driver);
- *version = driver->version;
- chDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ *version = driver->version;
+ }
+
return 0;
}
@@ -241,7 +242,7 @@ chDomainCreateXML(virConnectPtr conn,
virDomainObjListRemove(driver->domains, vm);
}
virDomainObjEndAPI(&vm);
- chDriverUnlock(driver);
+ virMutexUnlock(&driver->lock);
return dom;
}
@@ -368,8 +369,8 @@ static int chDomainIsActive(virDomainPtr dom)
virCHDriver *driver = dom->conn->privateData;
virDomainObj *vm;
int ret = -1;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
- chDriverLock(driver);
if (!(vm = virCHDomainObjFromDomain(dom)))
goto cleanup;
@@ -380,7 +381,6 @@ static int chDomainIsActive(virDomainPtr dom)
cleanup:
virDomainObjEndAPI(&vm);
- chDriverUnlock(driver);
return ret;
}
@@ -638,9 +638,9 @@ static virDomainPtr chDomainLookupByID(virConnectPtr conn,
virDomainObj *vm;
virDomainPtr dom = NULL;
- chDriverLock(driver);
- vm = virDomainObjListFindByID(driver->domains, id);
- chDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ vm = virDomainObjListFindByID(driver->domains, id);
+ }
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN,
@@ -665,9 +665,9 @@ static virDomainPtr chDomainLookupByName(virConnectPtr conn,
virDomainObj *vm;
virDomainPtr dom = NULL;
- chDriverLock(driver);
- vm = virDomainObjListFindByName(driver->domains, name);
- chDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ vm = virDomainObjListFindByName(driver->domains, name);
+ }
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN,
@@ -692,9 +692,9 @@ static virDomainPtr chDomainLookupByUUID(virConnectPtr conn,
virDomainObj *vm;
virDomainPtr dom = NULL;
- chDriverLock(driver);
- vm = virDomainObjListFindByUUID(driver->domains, uuid);
- chDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ vm = virDomainObjListFindByUUID(driver->domains, uuid);
+ }
if (!vm) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
--
2.31.1
7:57 a.m.
New subject: [libvirt PATCH 07/11] ch: Use automatic mutex management
On 2/7/22 14:12, Tim Wiederhake wrote:
This leaves a bogus `virMutexUnlock` in `chDomainCreateXML`.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/ch/ch_conf.c | 20 ++++++++------------
src/ch/ch_conf.h | 10 ----------
src/ch/ch_driver.c | 30 +++++++++++++++---------------
3 files changed, 23 insertions(+), 37 deletions(-)
diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
index cdf69e3e70..ccc01ecdab 100644
--- a/src/ch/ch_conf.c
+++ b/src/ch/ch_conf.c
@@ -93,16 +93,15 @@ virCaps *virCHDriverGetCapabilities(virCHDriver *driver,
if (refresh && !(caps = virCHDriverCapsInit()))
return NULL;
- chDriverLock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ if (refresh) {
+ virObjectUnref(driver->caps);
+ driver->caps = caps;
+ }
- if (refresh) {
- virObjectUnref(driver->caps);
- driver->caps = caps;
+ ret = virObjectRef(driver->caps);
}
- ret = virObjectRef(driver->caps);
- chDriverUnlock(driver);
-
return ret;
}
@@ -159,11 +158,8 @@ virCHDriverConfigNew(bool privileged)
virCHDriverConfig *virCHDriverGetConfig(virCHDriver *driver)
{
- virCHDriverConfig *cfg;
- chDriverLock(driver);
- cfg = virObjectRef(driver->config);
- chDriverUnlock(driver);
- return cfg;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
+ return virObjectRef(driver->config);
}
static void
diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h
index c56caa3f5f..b927621a97 100644
--- a/src/ch/ch_conf.h
+++ b/src/ch/ch_conf.h
@@ -78,13 +78,3 @@ virDomainXMLOption *chDomainXMLConfInit(virCHDriver *driver);
virCHDriverConfig *virCHDriverConfigNew(bool privileged);
virCHDriverConfig *virCHDriverGetConfig(virCHDriver *driver);
int chExtractVersion(virCHDriver *driver);
-
-static inline void chDriverLock(virCHDriver *driver)
-{
- virMutexLock(&driver->lock);
-}
-
-static inline void chDriverUnlock(virCHDriver *driver)
-{
- virMutexUnlock(&driver->lock);
-}
Until here I have no objections. But the code below, I mean the
pre-existing one is total mess and we should clean it up first, because
it will drop some locking.
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 3223f31367..945a1aa225 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -106,9 +106,10 @@ static int chConnectGetVersion(virConnectPtr conn,
if (virConnectGetVersionEnsureACL(conn) < 0)
return -1;
- chDriverLock(driver);
- *version = driver->version;
- chDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ *version = driver->version;
+ }
+
I see no point in locking the driver here. Firstly, the @version member
is set in chExtractVersion() which is called exactly once from
chStateInitialize() and driver init happens in a single thread (for a
good reason). And even if it didn't, the initialize function never locks
the driver, so this lock here guards nothing.
return 0;
}
@@ -241,7 +242,7 @@ chDomainCreateXML(virConnectPtr conn,
virDomainObjListRemove(driver->domains, vm);
}
virDomainObjEndAPI(&vm);
- chDriverUnlock(driver);
+ virMutexUnlock(&driver->lock);
I'm sorry, but what? I know, not your fault, but this is clearly an
imbalance in mutex locking. Not to mention that this function doesn't
end job properly (typical 'goto error' instead of 'goto endjob' problem).
return dom;
}
@@ -368,8 +369,8 @@ static int chDomainIsActive(virDomainPtr dom)
virCHDriver *driver = dom->conn->privateData;
virDomainObj *vm;
int ret = -1;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
- chDriverLock(driver);
I'm failing to see the purpose of this locking in the first place.
Again, neither of these problems are your fault, I'm just pointing them out.
if (!(vm = virCHDomainObjFromDomain(dom)))
goto cleanup;
@@ -380,7 +381,6 @@ static int chDomainIsActive(virDomainPtr dom)
cleanup:
virDomainObjEndAPI(&vm);
- chDriverUnlock(driver);
return ret;
}
@@ -638,9 +638,9 @@ static virDomainPtr chDomainLookupByID(virConnectPtr conn,
virDomainObj *vm;
virDomainPtr dom = NULL;
- chDriverLock(driver);
- vm = virDomainObjListFindByID(driver->domains, id);
- chDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ vm = virDomainObjListFindByID(driver->domains, id);
+ }
This is very suboptimal thing to do. I've specifically introduced RW
locks to virDomainObjList so that we don't have to lock the whole driver
when accessing the list.
Alright, let me post patches for these and then get back to you. Patches
until here look good.
Michal
7:12 a.m.
New subject: [libvirt PATCH 08/11] ch: Remove solitary virMutexUnlock
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/ch/ch_driver.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 945a1aa225..4c1a9bae77 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -242,7 +242,6 @@ chDomainCreateXML(virConnectPtr conn,
virDomainObjListRemove(driver->domains, vm);
}
virDomainObjEndAPI(&vm);
- virMutexUnlock(&driver->lock);
return dom;
}
--
2.31.1
7:12 a.m.
New subject: [libvirt PATCH 09/11] network: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/network/bridge_driver.c | 41 +++++++++----------------------------
1 file changed, 10 insertions(+), 31 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 23d9ed4226..3750da7962 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -100,28 +100,11 @@ networkGetDriver(void)
}
-static void
-networkDriverLock(virNetworkDriverState *driver)
-{
- virMutexLock(&driver->lock);
-}
-
-
-static void
-networkDriverUnlock(virNetworkDriverState *driver)
-{
- virMutexUnlock(&driver->lock);
-}
-
-
static dnsmasqCaps *
networkGetDnsmasqCaps(virNetworkDriverState *driver)
{
- dnsmasqCaps *ret;
- networkDriverLock(driver);
- ret = virObjectRef(driver->dnsmasqCaps);
- networkDriverUnlock(driver);
- return ret;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
+ return virObjectRef(driver->dnsmasqCaps);
}
@@ -133,10 +116,11 @@ networkDnsmasqCapsRefresh(virNetworkDriverState *driver)
if (!(caps = dnsmasqCapsNewFromBinary()))
return -1;
- networkDriverLock(driver);
- virObjectUnref(driver->dnsmasqCaps);
- driver->dnsmasqCaps = caps;
- networkDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ virObjectUnref(driver->dnsmasqCaps);
+ driver->dnsmasqCaps = caps;
+ }
+
return 0;
}
@@ -2725,27 +2709,22 @@ static int
networkBridgeNameValidate(virNetworkObjList *nets,
virNetworkDef *def)
{
- virMutexLock(&bridgeNameValidateMutex);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&bridgeNameValidateMutex);
if (def->bridge && !strstr(def->bridge, "%d")) {
if (virNetworkObjBridgeInUse(nets, def->bridge, def->name)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("bridge name '%s' already in use."),
def->bridge);
- goto error;
+ return -1;
}
} else {
/* Allocate a bridge name */
if (networkFindUnusedBridgeName(nets, def) < 0)
- goto error;
+ return -1;
}
- virMutexUnlock(&bridgeNameValidateMutex);
return 0;
-
- error:
- virMutexUnlock(&bridgeNameValidateMutex);
- return -1;
}
--
2.31.1
7:12 a.m.
New subject: [libvirt PATCH 10/11] bhyve_driver: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/bhyve/bhyve_conf.c | 7 ++-----
src/bhyve/bhyve_driver.c | 12 ------------
src/bhyve/bhyve_utils.h | 3 ---
3 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/src/bhyve/bhyve_conf.c b/src/bhyve/bhyve_conf.c
index 5eca0855f0..28981c90b7 100644
--- a/src/bhyve/bhyve_conf.c
+++ b/src/bhyve/bhyve_conf.c
@@ -85,11 +85,8 @@ virBhyveLoadDriverConfig(struct _virBhyveDriverConfig *cfg,
struct _virBhyveDriverConfig *
virBhyveDriverGetConfig(struct _bhyveConn *driver)
{
- struct _virBhyveDriverConfig *cfg;
- bhyveDriverLock(driver);
- cfg = virObjectRef(driver->config);
- bhyveDriverUnlock(driver);
- return cfg;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
+ return virObjectRef(driver->config);
}
static void
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 47ee98e650..f580e87419 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -70,18 +70,6 @@ VIR_LOG_INIT("bhyve.bhyve_driver");
struct _bhyveConn *bhyve_driver = NULL;
-void
-bhyveDriverLock(struct _bhyveConn *driver)
-{
- virMutexLock(&driver->lock);
-}
-
-void
-bhyveDriverUnlock(struct _bhyveConn *driver)
-{
- virMutexUnlock(&driver->lock);
-}
-
static int
bhyveAutostartDomain(virDomainObj *vm, void *opaque)
{
diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h
index af7b15486a..5d6e198b09 100644
--- a/src/bhyve/bhyve_utils.h
+++ b/src/bhyve/bhyve_utils.h
@@ -73,6 +73,3 @@ struct bhyveAutostartData {
struct _bhyveConn *driver;
virConnectPtr conn;
};
-
-void bhyveDriverLock(struct _bhyveConn *driver);
-void bhyveDriverUnlock(struct _bhyveConn *driver);
--
2.31.1
7:12 a.m.
New subject: [libvirt PATCH 11/11] node_device: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/node_device/node_device_driver.c | 23 +++---------
src/node_device/node_device_udev.c | 52 ++++++++++++----------------
2 files changed, 26 insertions(+), 49 deletions(-)
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index d19ed7d948..7b2fb3d953 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -156,33 +156,18 @@ nodeDeviceUpdateDriverName(virNodeDeviceDef *def G_GNUC_UNUSED)
#endif
-void
-nodeDeviceLock(void)
-{
- virMutexLock(&driver->lock);
-}
-
-
-void
-nodeDeviceUnlock(void)
-{
- virMutexUnlock(&driver->lock);
-}
-
-
static int
nodeDeviceInitWait(void)
{
- nodeDeviceLock();
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
+
while (!driver->initialized) {
if (virCondWait(&driver->initCond, &driver->lock) < 0) {
- virReportSystemError(errno, "%s",
- _("failed to wait on condition"));
- nodeDeviceUnlock();
+ virReportSystemError(errno, "%s", _("failed to wait on
condition"));
return -1;
}
}
- nodeDeviceUnlock();
+
return 0;
}
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index a9e8bf10da..3d5e25424a 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -93,9 +93,9 @@ udevEventDataDispose(void *obj)
udev_monitor_unref(priv->udev_monitor);
udev_unref(udev);
- virMutexLock(&priv->mdevctlLock);
- g_list_free_full(priv->mdevctlMonitors, g_object_unref);
- virMutexUnlock(&priv->mdevctlLock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&priv->mdevctlLock) {
+ g_list_free_full(priv->mdevctlMonitors, g_object_unref);
+ }
virMutexDestroy(&priv->mdevctlLock);
virCondDestroy(&priv->threadCond);
@@ -348,13 +348,9 @@ udevTranslatePCIIds(unsigned int vendor,
m.match_data = 0;
/* pci_get_strings returns void and unfortunately is not thread safe. */
- virMutexLock(&pciaccessMutex);
- pci_get_strings(&m,
- &device_name,
- &vendor_name,
- NULL,
- NULL);
- virMutexUnlock(&pciaccessMutex);
+ VIR_WITH_MUTEX_LOCK_GUARD(&pciaccessMutex) {
+ pci_get_strings(&m, &device_name, &vendor_name, NULL, NULL);
+ }
*vendor_string = g_strdup(vendor_name);
*product_string = g_strdup(device_name);
@@ -373,11 +369,11 @@ udevProcessPCI(struct udev_device *device,
virPCIDeviceAddress devAddr;
int ret = -1;
char *p;
- bool privileged;
+ bool privileged = false;
- nodeDeviceLock();
- privileged = driver->privileged;
- nodeDeviceUnlock();
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ privileged = driver->privileged;
+ }
pci_dev->klass = -1;
if (udevGetIntProperty(device, "PCI_CLASS", &pci_dev->klass, 16)
< 0)
@@ -1989,10 +1985,10 @@ nodeStateInitializeEnumerate(void *opaque)
goto error;
cleanup:
- nodeDeviceLock();
- driver->initialized = true;
- virCondBroadcast(&driver->initCond);
- nodeDeviceUnlock();
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ driver->initialized = true;
+ virCondBroadcast(&driver->initCond);
+ }
return;
@@ -2036,12 +2032,10 @@ static void
mdevctlHandlerThread(void *opaque G_GNUC_UNUSED)
{
udevEventData *priv = driver->privateData;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->mdevctlLock);
- /* ensure only a single thread can query mdevctl at a time */
- virMutexLock(&priv->mdevctlLock);
if (nodeDeviceUpdateMediatedDevices() < 0)
VIR_WARN("mdevctl failed to updated mediated devices");
- virMutexUnlock(&priv->mdevctlLock);
}
@@ -2143,13 +2137,10 @@ mdevctlEnableMonitor(udevEventData *priv)
* mdevctl configuration is stored in a directory tree within
* /etc/mdevctl.d/. There is a directory for each parent device, which
* contains a file defining each mediated device */
- virMutexLock(&priv->mdevctlLock);
- if (!(priv->mdevctlMonitors = monitorFileRecursively(priv,
- mdevctlConfigDir))) {
- virMutexUnlock(&priv->mdevctlLock);
- return -1;
+ VIR_WITH_MUTEX_LOCK_GUARD(&priv->mdevctlLock) {
+ if (!(priv->mdevctlMonitors = monitorFileRecursively(priv,
mdevctlConfigDir)))
+ return -1;
}
- virMutexUnlock(&priv->mdevctlLock);
return 0;
}
@@ -2171,9 +2162,10 @@ mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED,
if (file_type == G_FILE_TYPE_DIRECTORY) {
GList *newmonitors = monitorFileRecursively(priv, file);
- virMutexLock(&priv->mdevctlLock);
- priv->mdevctlMonitors = g_list_concat(priv->mdevctlMonitors,
newmonitors);
- virMutexUnlock(&priv->mdevctlLock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&priv->mdevctlLock) {
+ priv->mdevctlMonitors = g_list_concat(priv->mdevctlMonitors,
+ newmonitors);
+ }
}
}
--
2.31.1
3:58 a.m.
On 2/7/22 14:12, Tim Wiederhake wrote:
Use the recently implemented VIR_LOCK_GUARD and
VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
This made a solitary "virMutexUnlock()" call without previous call to
"virMutexLock()" in ch_driver.c obvious, which is removed in patch #8.
Tim Wiederhake (11):
virthreadpool: Use automatic memory management
virthreadpool: Cleanup
libxl: Use automatic memory management
lxc: Prepare virLXCDriverGetCapabilities for automatic mutex
management
lxc: Use automatic mutex management
ch: Prepare virCHDriverGetCapabilities for automatic mutex management
ch: Use automatic mutex management
ch: Remove solitary virMutexUnlock
network: Use automatic mutex management
bhyve_driver: Use automatic mutex management
node_device: Use automatic mutex management
src/bhyve/bhyve_conf.c | 7 +--
src/bhyve/bhyve_driver.c | 12 ----
src/bhyve/bhyve_utils.h | 3 -
src/ch/ch_conf.c | 31 +++++-----
src/ch/ch_conf.h | 10 ----
src/ch/ch_driver.c | 29 +++++----
src/libxl/libxl_conf.c | 8 +--
src/libxl/libxl_conf.h | 12 ----
src/libxl/libxl_logger.c | 20 +++----
src/lxc/lxc_conf.c | 41 ++++++-------
src/lxc/lxc_conf.h | 9 ---
src/lxc/lxc_controller.c | 25 ++++----
src/lxc/lxc_fuse.c | 12 ++--
src/network/bridge_driver.c | 41 ++++---------
src/node_device/node_device_driver.c | 23 ++-----
src/node_device/node_device_udev.c | 52 +++++++---------
src/util/virthreadpool.c | 90 ++++++++--------------------
17 files changed, 138 insertions(+), 287 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
And sorry for the conflict, but it's trivial enough to resolve.
Michal
1051
days inactive
1055
days old
14 comments
2 participants
participants (2)
-
Michal Prívozník -
Tim Wiederhake