11:28 a.m.
Use the recently implemented VIR_LOCK_GUARD and VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
v1: https://listman.redhat.com/archives/libvir-list/2022-February/msg00674.html
Changed since v1:
* Removed locking / unlocking in storage driver initialization and cleanup
instead of working around the issue of the lifetime of the mutex.
Tim Wiederhake (10):
test: Use automatic mutex management
openvz: Use automatic mutex management
remote_daemon_dispatch: Use automatic mutex management
netdev: Use automatic mutex management
nodesuspend: Use automatic mutex management
admin: Use automatic mutex management
esx_stream: Use automatic mutex management
esx_vi: Use automatic mutex management
storage: Removing mutex locking in initialization and cleanup
storage: Use automatic mutex management
src/admin/admin_server_dispatch.c | 3 +-
src/esx/esx_stream.c | 65 ++++------
src/esx/esx_vi.c | 109 +++++++---------
src/openvz/openvz_driver.c | 91 +++++---------
src/remote/remote_daemon_dispatch.c | 187 +++++++++-------------------
src/storage/storage_driver.c | 32 ++---
src/test/test_driver.c | 15 +--
src/util/virnetdev.c | 20 ++-
src/util/virnodesuspend.c | 54 +++-----
9 files changed, 193 insertions(+), 383 deletions(-)
--
2.31.1
11:28 a.m.
New subject: [libvirt PATCH v2 01/10] test: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/test/test_driver.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4eca5c4a65..34e4652375 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1386,16 +1386,14 @@ testOpenFromFile(virConnectPtr conn, const char *file)
static int
testOpenDefault(virConnectPtr conn)
{
- int ret = VIR_DRV_OPEN_ERROR;
testDriver *privconn = NULL;
g_autoptr(xmlDoc) doc = NULL;
g_autoptr(xmlXPathContext) ctxt = NULL;
size_t i;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&defaultLock);
- virMutexLock(&defaultLock);
if (defaultPrivconn) {
conn->privateData = virObjectRef(defaultPrivconn);
- virMutexUnlock(&defaultLock);
return VIR_DRV_OPEN_SUCCESS;
}
@@ -1434,15 +1432,12 @@ testOpenDefault(virConnectPtr conn)
goto error;
defaultPrivconn = privconn;
- ret = VIR_DRV_OPEN_SUCCESS;
- cleanup:
- virMutexUnlock(&defaultLock);
- return ret;
+ return VIR_DRV_OPEN_SUCCESS;
error:
virObjectUnref(privconn);
conn->privateData = NULL;
- goto cleanup;
+ return VIR_DRV_OPEN_ERROR;
}
static int
@@ -1500,12 +1495,12 @@ testConnectAuthenticate(virConnectPtr conn,
static void
testDriverCloseInternal(testDriver *driver)
{
- virMutexLock(&defaultLock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&defaultLock);
+
testDriverDisposed = false;
virObjectUnref(driver);
if (testDriverDisposed && driver == defaultPrivconn)
defaultPrivconn = NULL;
- virMutexUnlock(&defaultLock);
}
--
2.31.1
11:28 a.m.
New subject: [libvirt PATCH v2 02/10] openvz: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/openvz/openvz_driver.c | 91 ++++++++++++--------------------------
1 file changed, 28 insertions(+), 63 deletions(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index aa1db09540..d8e4dd7f8b 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -65,16 +65,6 @@ static int openvzDomainSetMemoryInternal(virDomainObj *vm,
unsigned long long memory);
static int openvzGetVEStatus(virDomainObj *vm, int *status, int *reason);
-static void openvzDriverLock(struct openvz_driver *driver)
-{
- virMutexLock(&driver->lock);
-}
-
-static void openvzDriverUnlock(struct openvz_driver *driver)
-{
- virMutexUnlock(&driver->lock);
-}
-
struct openvz_driver ovz_driver;
@@ -101,12 +91,9 @@ static virDomainObj *
openvzDomObjFromDomain(struct openvz_driver *driver,
const unsigned char *uuid)
{
- virDomainObj *vm;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
- openvzDriverLock(driver);
- vm = openvzDomObjFromDomainLocked(driver, uuid);
- openvzDriverUnlock(driver);
- return vm;
+ return openvzDomObjFromDomainLocked(driver, uuid);
}
@@ -262,12 +249,12 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr conn,
int id)
{
struct openvz_driver *driver = conn->privateData;
- virDomainObj *vm;
+ virDomainObj *vm = NULL;
virDomainPtr dom = NULL;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByID(driver->domains, id);
- openvzDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ vm = virDomainObjListFindByID(driver->domains, id);
+ }
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN,
@@ -285,9 +272,9 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr conn,
static int openvzConnectGetVersion(virConnectPtr conn, unsigned long *version)
{
struct openvz_driver *driver = conn->privateData;
- openvzDriverLock(driver);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
+
*version = driver->version;
- openvzDriverUnlock(driver);
return 0;
}
@@ -334,12 +321,12 @@ static virDomainPtr openvzDomainLookupByName(virConnectPtr conn,
const char *name)
{
struct openvz_driver *driver = conn->privateData;
- virDomainObj *vm;
+ virDomainObj *vm = NULL;
virDomainPtr dom = NULL;
- openvzDriverLock(driver);
- vm = virDomainObjListFindByName(driver->domains, name);
- openvzDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ vm = virDomainObjListFindByName(driver->domains, name);
+ }
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN,
@@ -808,13 +795,13 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml,
unsigned int fla
virDomainObj *vm = NULL;
virDomainPtr dom = NULL;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL);
if (flags & VIR_DOMAIN_DEFINE_VALIDATE)
parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA;
- openvzDriverLock(driver);
if ((vmdef = virDomainDefParseString(xml, driver->xmlopt,
NULL, parse_flags)) == NULL)
goto cleanup;
@@ -876,7 +863,6 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml,
unsigned int fla
cleanup:
virDomainObjEndAPI(&vm);
- openvzDriverUnlock(driver);
return dom;
}
@@ -896,13 +882,13 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml,
virDomainObj *vm = NULL;
virDomainPtr dom = NULL;
unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL);
if (flags & VIR_DOMAIN_START_VALIDATE)
parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA;
- openvzDriverLock(driver);
if ((vmdef = virDomainDefParseString(xml, driver->xmlopt,
NULL, parse_flags)) == NULL)
goto cleanup;
@@ -963,7 +949,6 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml,
cleanup:
virDomainObjEndAPI(&vm);
- openvzDriverUnlock(driver);
return dom;
}
@@ -972,15 +957,15 @@ openvzDomainCreateWithFlags(virDomainPtr dom, unsigned int flags)
{
g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet",
"start", NULL);
struct openvz_driver *driver = dom->conn->privateData;
- virDomainObj *vm;
+ virDomainObj *vm = NULL;
int ret = -1;
int status;
virCheckFlags(0, -1);
- openvzDriverLock(driver);
- vm = virDomainObjListFindByName(driver->domains, dom->name);
- openvzDriverUnlock(driver);
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ vm = virDomainObjListFindByName(driver->domains, dom->name);
+ }
if (!vm) {
virReportError(VIR_ERR_NO_DOMAIN,
@@ -1028,10 +1013,10 @@ openvzDomainUndefineFlags(virDomainPtr dom,
virDomainObj *vm;
int ret = -1;
int status;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
virCheckFlags(0, -1);
- openvzDriverLock(driver);
if (!(vm = openvzDomObjFromDomainLocked(driver, dom->uuid)))
goto cleanup;
@@ -1052,7 +1037,6 @@ openvzDomainUndefineFlags(virDomainPtr dom,
cleanup:
virDomainObjEndAPI(&vm);
- openvzDriverUnlock(driver);
return ret;
}
@@ -1321,13 +1305,9 @@ openvzConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED)
static char *openvzConnectGetCapabilities(virConnectPtr conn) {
struct openvz_driver *driver = conn->privateData;
- char *ret;
-
- openvzDriverLock(driver);
- ret = virCapabilitiesFormatXML(driver->caps);
- openvzDriverUnlock(driver);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
- return ret;
+ return virCapabilitiesFormatXML(driver->caps);
}
static int openvzConnectListDomains(virConnectPtr conn G_GNUC_UNUSED,
@@ -1370,13 +1350,9 @@ static int openvzConnectListDomains(virConnectPtr conn
G_GNUC_UNUSED,
static int openvzConnectNumOfDomains(virConnectPtr conn)
{
struct openvz_driver *driver = conn->privateData;
- int n;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
- openvzDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, true, NULL, NULL);
- openvzDriverUnlock(driver);
-
- return n;
+ return virDomainObjListNumOfDomains(driver->domains, true, NULL, NULL);
}
static int openvzConnectListDefinedDomains(virConnectPtr conn G_GNUC_UNUSED,
@@ -1480,13 +1456,9 @@ Version: 2.2
static int openvzConnectNumOfDefinedDomains(virConnectPtr conn)
{
struct openvz_driver *driver = conn->privateData;
- int n;
-
- openvzDriverLock(driver);
- n = virDomainObjListNumOfDomains(driver->domains, false, NULL, NULL);
- openvzDriverUnlock(driver);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
- return n;
+ return virDomainObjListNumOfDomains(driver->domains, false, NULL, NULL);
}
static int
@@ -1818,11 +1790,11 @@ openvzDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
virDomainObj *vm = NULL;
virDomainDef *def = NULL;
bool persist = false;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
- openvzDriverLock(driver);
if (!(vm = openvzDomObjFromDomainLocked(driver, dom->uuid)))
goto cleanup;
@@ -1849,7 +1821,6 @@ openvzDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml,
ret = 0;
cleanup:
- openvzDriverUnlock(driver);
virDomainDeviceDefFree(dev);
virDomainObjEndAPI(&vm);
return ret;
@@ -1861,16 +1832,10 @@ openvzConnectListAllDomains(virConnectPtr conn,
unsigned int flags)
{
struct openvz_driver *driver = conn->privateData;
- int ret = -1;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ALL, -1);
-
- openvzDriverLock(driver);
- ret = virDomainObjListExport(driver->domains, conn, domains,
- NULL, flags);
- openvzDriverUnlock(driver);
-
- return ret;
+ return virDomainObjListExport(driver->domains, conn, domains, NULL, flags);
}
--
2.31.1
11:28 a.m.
New subject: [libvirt PATCH v2 03/10] remote_daemon_dispatch: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/remote/remote_daemon_dispatch.c | 187 +++++++++-------------------
1 file changed, 58 insertions(+), 129 deletions(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 510856024c..1a73976831 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -2057,14 +2057,13 @@ remoteDispatchConnectOpen(virNetServer *server G_GNUC_UNUSED,
#endif
unsigned int flags;
struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
- int rv = -1;
#ifdef MODULE_NAME
const char *type = NULL;
#endif /* !MODULE_NAME */
bool preserveIdentity = false;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
VIR_DEBUG("priv=%p conn=%p", priv, priv->conn);
- virMutexLock(&priv->lock);
/* Already opened? */
if (priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection already
open"));
@@ -2184,17 +2183,14 @@ remoteDispatchConnectOpen(virNetServer *server G_GNUC_UNUSED,
* by default, but do accept RO flags, e.g. TCP
*/
virNetServerClientSetReadonly(client, (flags & VIR_CONNECT_RO));
- rv = 0;
+ return 0;
cleanup:
- if (rv < 0) {
- virNetMessageSaveError(rerr);
- if (priv->conn) {
- g_clear_pointer(&priv->conn, virObjectUnref);
- }
+ virNetMessageSaveError(rerr);
+ if (priv->conn) {
+ g_clear_pointer(&priv->conn, virObjectUnref);
}
- virMutexUnlock(&priv->lock);
- return rv;
+ return -1;
}
@@ -3656,8 +3652,7 @@ remoteDispatchAuthSaslInit(virNetServer *server G_GNUC_UNUSED,
virNetSASLSession *sasl = NULL;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
VIR_DEBUG("Initialize SASL auth %d", virNetServerClientGetFD(client));
if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_SASL ||
@@ -3702,7 +3697,6 @@ remoteDispatchAuthSaslInit(virNetServer *server G_GNUC_UNUSED,
VIR_DEBUG("Available mechanisms for client: '%s'",
ret->mechlist);
priv->sasl = sasl;
- virMutexUnlock(&priv->lock);
return 0;
authfail:
@@ -3714,7 +3708,6 @@ remoteDispatchAuthSaslInit(virNetServer *server G_GNUC_UNUSED,
"client=%p auth=%d",
client, REMOTE_AUTH_SASL);
virObjectUnref(sasl);
- virMutexUnlock(&priv->lock);
return -1;
}
@@ -3783,8 +3776,7 @@ remoteDispatchAuthSaslStart(virNetServer *server,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
const char *identity;
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
VIR_DEBUG("Start SASL auth %d", virNetServerClientGetFD(client));
if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_SASL ||
@@ -3836,7 +3828,6 @@ remoteDispatchAuthSaslStart(virNetServer *server,
ret->complete = 1;
}
- virMutexUnlock(&priv->lock);
return 0;
authfail:
@@ -3858,7 +3849,6 @@ remoteDispatchAuthSaslStart(virNetServer *server,
virReportError(VIR_ERR_AUTH_FAILED, "%s",
_("authentication failed"));
virNetMessageSaveError(rerr);
- virMutexUnlock(&priv->lock);
return -1;
}
@@ -3877,8 +3867,7 @@ remoteDispatchAuthSaslStep(virNetServer *server,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
const char *identity;
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
VIR_DEBUG("Step SASL auth %d", virNetServerClientGetFD(client));
if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_SASL ||
@@ -3930,7 +3919,6 @@ remoteDispatchAuthSaslStep(virNetServer *server,
ret->complete = 1;
}
- virMutexUnlock(&priv->lock);
return 0;
authfail:
@@ -3952,7 +3940,6 @@ remoteDispatchAuthSaslStep(virNetServer *server,
virReportError(VIR_ERR_AUTH_FAILED, "%s",
_("authentication failed"));
virNetMessageSaveError(rerr);
- virMutexUnlock(&priv->lock);
return -1;
}
#else
@@ -4017,8 +4004,8 @@ remoteDispatchAuthPolkit(virNetServer *server,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
int rv;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
- virMutexLock(&priv->lock);
action = virNetServerClientGetReadonly(client) ?
"org.libvirt.unix.monitor" :
"org.libvirt.unix.manage";
@@ -4062,13 +4049,10 @@ remoteDispatchAuthPolkit(virNetServer *server,
ret->complete = 1;
virNetServerSetClientAuthenticated(server, client);
- virMutexUnlock(&priv->lock);
-
return 0;
error:
virNetMessageSaveError(rerr);
- virMutexUnlock(&priv->lock);
return -1;
authfail:
@@ -4129,12 +4113,10 @@ remoteDispatchConnectRegisterCloseCallback(virNetServer *server
G_GNUC_UNUSED,
virNetMessage *msg G_GNUC_UNUSED,
struct virNetMessageError *rerr)
{
- int rv = -1;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -4145,13 +4127,11 @@ remoteDispatchConnectRegisterCloseCallback(virNetServer *server
G_GNUC_UNUSED,
goto cleanup;
priv->closeRegistered = true;
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
static int
@@ -4160,12 +4140,10 @@ remoteDispatchConnectUnregisterCloseCallback(virNetServer *server
G_GNUC_UNUSED,
virNetMessage *msg G_GNUC_UNUSED,
struct virNetMessageError *rerr)
{
- int rv = -1;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -4175,13 +4153,11 @@ remoteDispatchConnectUnregisterCloseCallback(virNetServer *server
G_GNUC_UNUSED,
goto cleanup;
priv->closeRegistered = false;
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
static int
@@ -4198,8 +4174,7 @@ remoteDispatchConnectDomainEventRegister(virNetServer *server
G_GNUC_UNUSED,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -4240,7 +4215,6 @@ remoteDispatchConnectDomainEventRegister(virNetServer *server
G_GNUC_UNUSED,
rv = 0;
cleanup:
- virMutexUnlock(&priv->lock);
remoteEventCallbackFree(callback);
if (rv < 0)
virNetMessageSaveError(rerr);
@@ -4255,13 +4229,11 @@ remoteDispatchConnectDomainEventDeregister(virNetServer *server
G_GNUC_UNUSED,
remote_connect_domain_event_deregister_ret
*ret G_GNUC_UNUSED)
{
int callbackID = -1;
- int rv = -1;
size_t i;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -4286,13 +4258,11 @@ remoteDispatchConnectDomainEventDeregister(virNetServer *server
G_GNUC_UNUSED,
VIR_DELETE_ELEMENT(priv->domainEventCallbacks, i,
priv->ndomainEventCallbacks);
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
static void
@@ -4417,8 +4387,7 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServer *server
G_GNUC_UNUSED,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -4467,7 +4436,6 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServer *server
G_GNUC_UNUSED,
rv = 0;
cleanup:
- virMutexUnlock(&priv->lock);
remoteEventCallbackFree(callback);
if (rv < 0)
virNetMessageSaveError(rerr);
@@ -4491,8 +4459,7 @@ remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServer
*server G_GNUC_
virNetServerClientGetPrivateData(client);
virDomainPtr dom = NULL;
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -4541,7 +4508,6 @@ remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServer
*server G_GNUC_
rv = 0;
cleanup:
- virMutexUnlock(&priv->lock);
remoteEventCallbackFree(callback);
if (rv < 0)
virNetMessageSaveError(rerr);
@@ -4558,13 +4524,11 @@ remoteDispatchConnectDomainEventDeregisterAny(virNetServer *server
G_GNUC_UNUSED
remote_connect_domain_event_deregister_any_args *args)
{
int callbackID = -1;
- int rv = -1;
size_t i;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -4597,13 +4561,11 @@ remoteDispatchConnectDomainEventDeregisterAny(virNetServer *server
G_GNUC_UNUSED
VIR_DELETE_ELEMENT(priv->domainEventCallbacks, i,
priv->ndomainEventCallbacks);
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
@@ -4614,13 +4576,11 @@ remoteDispatchConnectDomainEventCallbackDeregisterAny(virNetServer
*server G_GNU
struct virNetMessageError *rerr
G_GNUC_UNUSED,
remote_connect_domain_event_callback_deregister_any_args *args)
{
- int rv = -1;
size_t i;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -4642,13 +4602,11 @@ remoteDispatchConnectDomainEventCallbackDeregisterAny(virNetServer
*server G_GNU
VIR_DELETE_ELEMENT(priv->domainEventCallbacks, i,
priv->ndomainEventCallbacks);
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
@@ -6027,8 +5985,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServer *server
G_GNUC_UNUSED,
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetNetworkConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6077,7 +6034,6 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServer *server
G_GNUC_UNUSED,
rv = 0;
cleanup:
- virMutexUnlock(&priv->lock);
remoteEventCallbackFree(callback);
if (rv < 0)
virNetMessageSaveError(rerr);
@@ -6093,13 +6049,11 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServer
*server G_GNUC_UNUSE
struct virNetMessageError *rerr
G_GNUC_UNUSED,
remote_connect_network_event_deregister_any_args *args)
{
- int rv = -1;
size_t i;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetNetworkConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6121,13 +6075,11 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServer
*server G_GNUC_UNUSE
VIR_DELETE_ELEMENT(priv->networkEventCallbacks, i,
priv->nnetworkEventCallbacks);
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
static int
@@ -6146,8 +6098,7 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServer
*server G_GNUC_UNU
virNetServerClientGetPrivateData(client);
virStoragePoolPtr pool = NULL;
virConnectPtr conn = remoteGetStorageConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6196,7 +6147,6 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServer
*server G_GNUC_UNU
rv = 0;
cleanup:
- virMutexUnlock(&priv->lock);
remoteEventCallbackFree(callback);
if (rv < 0)
virNetMessageSaveError(rerr);
@@ -6211,13 +6161,12 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServer
*server G_GNUC_U
struct virNetMessageError *rerr
G_GNUC_UNUSED,
remote_connect_storage_pool_event_deregister_any_args *args)
{
- int rv = -1;
size_t i;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetStorageConn(client);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
- virMutexLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6239,13 +6188,11 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServer
*server G_GNUC_U
VIR_DELETE_ELEMENT(priv->storageEventCallbacks, i,
priv->nstorageEventCallbacks);
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
static int
@@ -6264,8 +6211,7 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServer *server
G_GNUC_UNUS
virNetServerClientGetPrivateData(client);
virNodeDevicePtr dev = NULL;
virConnectPtr conn = remoteGetNodeDevConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6314,7 +6260,6 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServer *server
G_GNUC_UNUS
rv = 0;
cleanup:
- virMutexUnlock(&priv->lock);
remoteEventCallbackFree(callback);
if (rv < 0)
virNetMessageSaveError(rerr);
@@ -6329,13 +6274,11 @@ remoteDispatchConnectNodeDeviceEventDeregisterAny(virNetServer
*server G_GNUC_UN
struct virNetMessageError *rerr
G_GNUC_UNUSED,
remote_connect_node_device_event_deregister_any_args *args)
{
- int rv = -1;
size_t i;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetNodeDevConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6357,13 +6300,11 @@ remoteDispatchConnectNodeDeviceEventDeregisterAny(virNetServer
*server G_GNUC_UN
VIR_DELETE_ELEMENT(priv->nodeDeviceEventCallbacks, i,
priv->nnodeDeviceEventCallbacks);
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
static int
@@ -6382,8 +6323,7 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServer *server
G_GNUC_UNUSED,
virNetServerClientGetPrivateData(client);
virSecretPtr secret = NULL;
virConnectPtr conn = remoteGetSecretConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6432,7 +6372,6 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServer *server
G_GNUC_UNUSED,
rv = 0;
cleanup:
- virMutexUnlock(&priv->lock);
remoteEventCallbackFree(callback);
if (rv < 0)
virNetMessageSaveError(rerr);
@@ -6447,13 +6386,11 @@ remoteDispatchConnectSecretEventDeregisterAny(virNetServer *server
G_GNUC_UNUSED
struct virNetMessageError *rerr
G_GNUC_UNUSED,
remote_connect_secret_event_deregister_any_args *args)
{
- int rv = -1;
size_t i;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetSecretConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6475,13 +6412,11 @@ remoteDispatchConnectSecretEventDeregisterAny(virNetServer *server
G_GNUC_UNUSED
VIR_DELETE_ELEMENT(priv->secretEventCallbacks, i,
priv->nsecretEventCallbacks);
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
static int
@@ -6501,8 +6436,7 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServer *server
G_GNUC_UNUSED
virDomainPtr dom = NULL;
const char *event = args->event ? *args->event : NULL;
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6546,7 +6480,6 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServer *server
G_GNUC_UNUSED
rv = 0;
cleanup:
- virMutexUnlock(&priv->lock);
remoteEventCallbackFree(callback);
if (rv < 0)
virNetMessageSaveError(rerr);
@@ -6562,13 +6495,11 @@ qemuDispatchConnectDomainMonitorEventDeregister(virNetServer
*server G_GNUC_UNUS
struct virNetMessageError *rerr
G_GNUC_UNUSED,
qemu_connect_domain_monitor_event_deregister_args *args)
{
- int rv = -1;
size_t i;
struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client);
virConnectPtr conn = remoteGetHypervisorConn(client);
-
- virMutexLock(&priv->lock);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
if (!conn)
goto cleanup;
@@ -6591,13 +6522,11 @@ qemuDispatchConnectDomainMonitorEventDeregister(virNetServer
*server G_GNUC_UNUS
VIR_DELETE_ELEMENT(priv->qemuEventCallbacks, i,
priv->nqemuEventCallbacks);
- rv = 0;
+ return 0;
cleanup:
- virMutexUnlock(&priv->lock);
- if (rv < 0)
- virNetMessageSaveError(rerr);
- return rv;
+ virNetMessageSaveError(rerr);
+ return -1;
}
static int
--
2.31.1
4:38 a.m.
New subject: [libvirt PATCH v2 03/10] remote_daemon_dispatch: Use automatic mutex management
On 3/4/22 18:28, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/remote/remote_daemon_dispatch.c | 187 +++++++++-------------------
1 file changed, 58 insertions(+), 129 deletions(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 510856024c..1a73976831 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -2057,14 +2057,13 @@ remoteDispatchConnectOpen(virNetServer *server G_GNUC_UNUSED,
#endif
unsigned int flags;
struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);
- int rv = -1;
#ifdef MODULE_NAME
const char *type = NULL;
#endif /* !MODULE_NAME */
bool preserveIdentity = false;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
VIR_DEBUG("priv=%p conn=%p", priv, priv->conn);
- virMutexLock(&priv->lock);
/* Already opened? */
if (priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection
already open"));
@@ -2184,17 +2183,14 @@ remoteDispatchConnectOpen(virNetServer *server G_GNUC_UNUSED,
* by default, but do accept RO flags, e.g. TCP
*/
virNetServerClientSetReadonly(client, (flags & VIR_CONNECT_RO));
- rv = 0;
+ return 0;
cleanup:
Here, and in the rest of the patch:
This label is now used only in case of failure, so it should be renamed
to 'error'. But it's okay to do that in a follow up patch.
- if (rv < 0) {
- virNetMessageSaveError(rerr);
- if (priv->conn) {
- g_clear_pointer(&priv->conn, virObjectUnref);
- }
+ virNetMessageSaveError(rerr);
+ if (priv->conn) {
+ g_clear_pointer(&priv->conn, virObjectUnref);
}
- virMutexUnlock(&priv->lock);
- return rv;
+ return -1;
}
Michal
11:28 a.m.
New subject: [libvirt PATCH v2 04/10] netdev: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/util/virnetdev.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index fcf679ec37..5df48af60c 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3568,12 +3568,10 @@ virNetDevReserveName(const char *name)
idstr = name + strlen(virNetDevGenNames[type].prefix);
if (virStrToLong_ui(idstr, NULL, 10, &id) >= 0) {
- virMutexLock(&virNetDevGenNames[type].mutex);
+ VIR_LOCK_GUARD lock = virLockGuardLock(&virNetDevGenNames[type].mutex);
if (virNetDevGenNames[type].lastID < (int)id)
virNetDevGenNames[type].lastID = id;
-
- virMutexUnlock(&virNetDevGenNames[type].mutex);
}
}
@@ -3599,7 +3597,6 @@ virNetDevReserveName(const char *name)
int
virNetDevGenerateName(char **ifname, virNetDevGenNameType type)
{
- int id;
const char *prefix = virNetDevGenNames[type].prefix;
double maxIDd = pow(10, IFNAMSIZ - 1 - strlen(prefix));
int maxID = INT_MAX;
@@ -3617,16 +3614,15 @@ virNetDevGenerateName(char **ifname, virNetDevGenNameType type)
do {
g_autofree char *try = NULL;
+ int id = 0;
- virMutexLock(&virNetDevGenNames[type].mutex);
-
- id = ++virNetDevGenNames[type].lastID;
+ VIR_WITH_OBJECT_LOCK_GUARD(&virNetDevGenNames[type].mutex) {
+ id = ++virNetDevGenNames[type].lastID;
- /* reset before overflow */
- if (virNetDevGenNames[type].lastID >= maxID)
- virNetDevGenNames[type].lastID = -1;
-
- virMutexUnlock(&virNetDevGenNames[type].mutex);
+ /* reset before overflow */
+ if (virNetDevGenNames[type].lastID >= maxID)
+ virNetDevGenNames[type].lastID = -1;
+ }
if (*ifname)
try = g_strdup_printf(*ifname, id);
--
2.31.1
11:28 a.m.
New subject: [libvirt PATCH v2 05/10] nodesuspend: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/util/virnodesuspend.c | 54 +++++++++++++--------------------------
1 file changed, 18 insertions(+), 36 deletions(-)
diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c
index e1167605ca..5feef79c43 100644
--- a/src/util/virnodesuspend.c
+++ b/src/util/virnodesuspend.c
@@ -53,17 +53,6 @@ static virMutex virNodeSuspendMutex = VIR_MUTEX_INITIALIZER;
static bool aboutToSuspend;
-static void virNodeSuspendLock(void)
-{
- virMutexLock(&virNodeSuspendMutex);
-}
-
-static void virNodeSuspendUnlock(void)
-{
- virMutexUnlock(&virNodeSuspendMutex);
-}
-
-
/**
* virNodeSuspendSetNodeWakeup:
* @alarmTime: time in seconds from now, at which the RTC alarm has to be set.
@@ -116,9 +105,9 @@ static void virNodeSuspendHelper(void *cmdString)
* Now that we have resumed from suspend or the suspend failed,
* reset 'aboutToSuspend' flag.
*/
- virNodeSuspendLock();
- aboutToSuspend = false;
- virNodeSuspendUnlock();
+ VIR_WITH_MUTEX_LOCK_GUARD(&virNodeSuspendMutex) {
+ aboutToSuspend = false;
+ }
}
/**
@@ -154,8 +143,8 @@ int virNodeSuspend(unsigned int target,
{
static virThread thread;
const char *cmdString = NULL;
- int ret = -1;
unsigned int supported;
+ VIR_LOCK_GUARD lock = { NULL };
virCheckFlags(0, -1);
@@ -166,13 +155,13 @@ int virNodeSuspend(unsigned int target,
* Ensure that we are the only ones trying to suspend.
* Fail if somebody has already initiated a suspend.
*/
- virNodeSuspendLock();
+ lock = virLockGuardLock(&virNodeSuspendMutex);
if (aboutToSuspend) {
/* A suspend operation is already in progress */
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Suspend operation already in progress"));
- goto cleanup;
+ return -1;
}
/* Check if the host supports the requested suspend target */
@@ -180,7 +169,7 @@ int virNodeSuspend(unsigned int target,
case VIR_NODE_SUSPEND_TARGET_MEM:
if (!(supported & (1 << VIR_NODE_SUSPEND_TARGET_MEM))) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("Suspend-to-RAM"));
- goto cleanup;
+ return -1;
}
cmdString = "pm-suspend";
break;
@@ -188,7 +177,7 @@ int virNodeSuspend(unsigned int target,
case VIR_NODE_SUSPEND_TARGET_DISK:
if (!(supported & (1 << VIR_NODE_SUSPEND_TARGET_DISK))) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("Suspend-to-Disk"));
- goto cleanup;
+ return -1;
}
cmdString = "pm-hibernate";
break;
@@ -196,19 +185,19 @@ int virNodeSuspend(unsigned int target,
case VIR_NODE_SUSPEND_TARGET_HYBRID:
if (!(supported & (1 << VIR_NODE_SUSPEND_TARGET_HYBRID))) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("Hybrid-Suspend"));
- goto cleanup;
+ return -1;
}
cmdString = "pm-suspend-hybrid";
break;
default:
virReportError(VIR_ERR_INVALID_ARG, "%s", _("Invalid suspend
target"));
- goto cleanup;
+ return -1;
}
/* Just set the RTC alarm. Don't suspend yet. */
if (virNodeSuspendSetNodeWakeup(duration) < 0)
- goto cleanup;
+ return -1;
if (virThreadCreateFull(&thread, false,
virNodeSuspendHelper,
@@ -217,14 +206,11 @@ int virNodeSuspend(unsigned int target,
(void *)cmdString) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to create thread to suspend the host"));
- goto cleanup;
+ return -1;
}
aboutToSuspend = true;
- ret = 0;
- cleanup:
- virNodeSuspendUnlock();
- return ret;
+ return 0;
}
#ifdef WITH_PM_UTILS
@@ -348,11 +334,10 @@ virNodeSuspendSupportsTarget(unsigned int target, bool *supported)
int
virNodeSuspendGetTargetMask(unsigned int *bitmask)
{
- int ret = -1;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&virNodeSuspendMutex);
*bitmask = 0;
- virNodeSuspendLock();
/* Get the power management capabilities supported by the host */
if (!nodeSuspendTargetMaskInit) {
bool supported;
@@ -360,19 +345,19 @@ virNodeSuspendGetTargetMask(unsigned int *bitmask)
/* Check support for Suspend-to-RAM (S3) */
if (virNodeSuspendSupportsTarget(VIR_NODE_SUSPEND_TARGET_MEM, &supported)
< 0)
- goto cleanup;
+ return -1;
if (supported)
nodeSuspendTargetMask |= (1 << VIR_NODE_SUSPEND_TARGET_MEM);
/* Check support for Suspend-to-Disk (S4) */
if (virNodeSuspendSupportsTarget(VIR_NODE_SUSPEND_TARGET_DISK, &supported)
< 0)
- goto cleanup;
+ return -1;
if (supported)
nodeSuspendTargetMask |= (1 << VIR_NODE_SUSPEND_TARGET_DISK);
/* Check support for Hybrid-Suspend */
if (virNodeSuspendSupportsTarget(VIR_NODE_SUSPEND_TARGET_HYBRID, &supported)
< 0)
- goto cleanup;
+ return -1;
if (supported)
nodeSuspendTargetMask |= (1 << VIR_NODE_SUSPEND_TARGET_HYBRID);
@@ -380,8 +365,5 @@ virNodeSuspendGetTargetMask(unsigned int *bitmask)
}
*bitmask = nodeSuspendTargetMask;
- ret = 0;
- cleanup:
- virNodeSuspendUnlock();
- return ret;
+ return 0;
}
--
2.31.1
11:28 a.m.
New subject: [libvirt PATCH v2 06/10] admin: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/admin/admin_server_dispatch.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c
index f533fcf539..893c7f1de2 100644
--- a/src/admin/admin_server_dispatch.c
+++ b/src/admin/admin_server_dispatch.c
@@ -165,9 +165,9 @@ adminDispatchConnectOpen(virNetServer *server G_GNUC_UNUSED,
struct daemonAdmClientPrivate *priv =
virNetServerClientGetPrivateData(client);
int ret = -1;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
VIR_DEBUG("priv=%p dmn=%p", priv, priv->dmn);
- virMutexLock(&priv->lock);
flags = args->flags;
virCheckFlagsGoto(0, cleanup);
@@ -176,7 +176,6 @@ adminDispatchConnectOpen(virNetServer *server G_GNUC_UNUSED,
cleanup:
if (ret < 0)
virNetMessageSaveError(rerr);
- virMutexUnlock(&priv->lock);
return ret;
}
--
2.31.1
4:38 a.m.
New subject: [libvirt PATCH v2 06/10] admin: Use automatic mutex management
On 3/4/22 18:28, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/admin/admin_server_dispatch.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c
index f533fcf539..893c7f1de2 100644
--- a/src/admin/admin_server_dispatch.c
+++ b/src/admin/admin_server_dispatch.c
@@ -165,9 +165,9 @@ adminDispatchConnectOpen(virNetServer *server G_GNUC_UNUSED,
struct daemonAdmClientPrivate *priv =
virNetServerClientGetPrivateData(client);
int ret = -1;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->lock);
VIR_DEBUG("priv=%p dmn=%p", priv, priv->dmn);
- virMutexLock(&priv->lock);
flags = args->flags;
virCheckFlagsGoto(0, cleanup);
@@ -176,7 +176,6 @@ adminDispatchConnectOpen(virNetServer *server G_GNUC_UNUSED,
cleanup:
if (ret < 0)
virNetMessageSaveError(rerr);
- virMutexUnlock(&priv->lock);
return ret;
}
I'm not sure why was the locking here in the first place, but it made me
smile. Thank you for that! :-)
Michal
11:28 a.m.
New subject: [libvirt PATCH v2 07/10] esx_stream: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/esx/esx_stream.c | 65 ++++++++++++++------------------------------
1 file changed, 21 insertions(+), 44 deletions(-)
diff --git a/src/esx/esx_stream.c b/src/esx/esx_stream.c
index 5b20804bb1..2b49c8dd12 100644
--- a/src/esx/esx_stream.c
+++ b/src/esx/esx_stream.c
@@ -198,9 +198,8 @@ esxStreamTransfer(esxStreamPrivate *priv, bool blocking)
static int
esxStreamSend(virStreamPtr stream, const char *data, size_t nbytes)
{
- int result = -1;
esxStreamPrivate *priv = stream->privateData;
- int status;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->curl->lock);
if (nbytes == 0)
return 0;
@@ -215,38 +214,29 @@ esxStreamSend(virStreamPtr stream, const char *data, size_t nbytes)
return -1;
}
- virMutexLock(&priv->curl->lock);
-
priv->buffer = (char *)data;
priv->buffer_size = nbytes;
priv->buffer_used = nbytes;
if (stream->flags & VIR_STREAM_NONBLOCK) {
if (esxStreamTransfer(priv, false) < 0)
- goto cleanup;
+ return -1;
- if (priv->buffer_used < priv->buffer_size)
- result = priv->buffer_size - priv->buffer_used;
- else
- result = -2;
+ if (priv->buffer_used >= priv->buffer_size)
+ return -2;
} else /* blocking */ {
do {
- status = esxStreamTransfer(priv, true);
+ int status = esxStreamTransfer(priv, true);
if (status < 0)
- goto cleanup;
+ return -1;
if (status > 0)
break;
} while (priv->buffer_used > 0);
-
- result = priv->buffer_size - priv->buffer_used;
}
- cleanup:
- virMutexUnlock(&priv->curl->lock);
-
- return result;
+ return priv->buffer_size - priv->buffer_used;
}
static int
@@ -255,9 +245,8 @@ esxStreamRecvFlags(virStreamPtr stream,
size_t nbytes,
unsigned int flags)
{
- int result = -1;
esxStreamPrivate *priv = stream->privateData;
- int status;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->curl->lock);
virCheckFlags(0, -1);
@@ -274,8 +263,6 @@ esxStreamRecvFlags(virStreamPtr stream,
return -1;
}
- virMutexLock(&priv->curl->lock);
-
priv->buffer = data;
priv->buffer_size = nbytes;
priv->buffer_used = 0;
@@ -291,33 +278,25 @@ esxStreamRecvFlags(virStreamPtr stream,
priv->backlog_used - priv->buffer_used);
priv->backlog_used -= priv->buffer_used;
- result = priv->buffer_used;
} else if (stream->flags & VIR_STREAM_NONBLOCK) {
if (esxStreamTransfer(priv, false) < 0)
- goto cleanup;
+ return -1;
- if (priv->buffer_used > 0)
- result = priv->buffer_used;
- else
- result = -2;
+ if (priv->buffer_used <= 0)
+ return -2;
} else /* blocking */ {
do {
- status = esxStreamTransfer(priv, true);
+ int status = esxStreamTransfer(priv, true);
if (status < 0)
- goto cleanup;
+ return -1;
if (status > 0)
break;
} while (priv->buffer_used < priv->buffer_size);
-
- result = priv->buffer_used;
}
- cleanup:
- virMutexUnlock(&priv->curl->lock);
-
- return result;
+ return priv->buffer_used;
}
static int
@@ -348,18 +327,16 @@ esxStreamClose(virStreamPtr stream, bool finish)
if (!priv)
return 0;
- virMutexLock(&priv->curl->lock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&priv->curl->lock) {
+ if (finish && priv->backlog_used > 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Stream has untransferred data left"));
+ result = -1;
+ }
- if (finish && priv->backlog_used > 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Stream has untransferred data left"));
- result = -1;
+ stream->privateData = NULL;
}
- stream->privateData = NULL;
-
- virMutexUnlock(&priv->curl->lock);
-
esxFreeStreamPrivate(&priv);
return result;
--
2.31.1
4:44 a.m.
New subject: [libvirt PATCH v2 07/10] esx_stream: Use automatic mutex management
On Fri, Mar 04, 2022 at 06:28:37PM +0100, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/esx/esx_stream.c | 65 ++++++++++++++------------------------------
1 file changed, 21 insertions(+), 44 deletions(-)
diff --git a/src/esx/esx_stream.c b/src/esx/esx_stream.c
index 5b20804bb1..2b49c8dd12 100644
--- a/src/esx/esx_stream.c
+++ b/src/esx/esx_stream.c
@@ -198,9 +198,8 @@ esxStreamTransfer(esxStreamPrivate *priv, bool blocking)
static int
esxStreamSend(virStreamPtr stream, const char *data, size_t nbytes)
{
- int result = -1;
esxStreamPrivate *priv = stream->privateData;
- int status;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->curl->lock);
Coverity discovered that this change is not correct:
/src/esx/esx_stream.c: 207 in esxStreamSend()
201 esxStreamPrivate *priv = stream->privateData;
202 VIR_LOCK_GUARD lock = virLockGuardLock(&priv->curl->lock);
203
204 if (nbytes == 0)
205 return 0;
206
>> CID 389978: Null pointer dereferences (REVERSE_INULL)
>> Null-checking "priv" suggests that it may be null, but it has
already been dereferenced on all paths leading to the check.
207 if (!priv)
{
208 virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Stream is
not open"));
209 return -1;
210 }
211
212 if (priv->mode != ESX_STREAM_MODE_UPLOAD) {
Previously the mutex was initialized after the !priv check, now we will
dereference priv before this check.
if (nbytes == 0)
return 0;
@@ -215,38 +214,29 @@ esxStreamSend(virStreamPtr stream, const char *data, size_t
nbytes)
return -1;
}
- virMutexLock(&priv->curl->lock);
-
priv->buffer = (char *)data;
priv->buffer_size = nbytes;
priv->buffer_used = nbytes;
if (stream->flags & VIR_STREAM_NONBLOCK) {
if (esxStreamTransfer(priv, false) < 0)
- goto cleanup;
+ return -1;
- if (priv->buffer_used < priv->buffer_size)
- result = priv->buffer_size - priv->buffer_used;
- else
- result = -2;
+ if (priv->buffer_used >= priv->buffer_size)
+ return -2;
} else /* blocking */ {
do {
- status = esxStreamTransfer(priv, true);
+ int status = esxStreamTransfer(priv, true);
if (status < 0)
- goto cleanup;
+ return -1;
if (status > 0)
break;
} while (priv->buffer_used > 0);
-
- result = priv->buffer_size - priv->buffer_used;
}
- cleanup:
- virMutexUnlock(&priv->curl->lock);
-
- return result;
+ return priv->buffer_size - priv->buffer_used;
}
static int
@@ -255,9 +245,8 @@ esxStreamRecvFlags(virStreamPtr stream,
size_t nbytes,
unsigned int flags)
{
- int result = -1;
esxStreamPrivate *priv = stream->privateData;
- int status;
+ VIR_LOCK_GUARD lock = virLockGuardLock(&priv->curl->lock);
And same happens here.
Pavel
virCheckFlags(0, -1);
@@ -274,8 +263,6 @@ esxStreamRecvFlags(virStreamPtr stream,
return -1;
}
- virMutexLock(&priv->curl->lock);
-
priv->buffer = data;
priv->buffer_size = nbytes;
priv->buffer_used = 0;
@@ -291,33 +278,25 @@ esxStreamRecvFlags(virStreamPtr stream,
priv->backlog_used - priv->buffer_used);
priv->backlog_used -= priv->buffer_used;
- result = priv->buffer_used;
} else if (stream->flags & VIR_STREAM_NONBLOCK) {
if (esxStreamTransfer(priv, false) < 0)
- goto cleanup;
+ return -1;
- if (priv->buffer_used > 0)
- result = priv->buffer_used;
- else
- result = -2;
+ if (priv->buffer_used <= 0)
+ return -2;
} else /* blocking */ {
do {
- status = esxStreamTransfer(priv, true);
+ int status = esxStreamTransfer(priv, true);
if (status < 0)
- goto cleanup;
+ return -1;
if (status > 0)
break;
} while (priv->buffer_used < priv->buffer_size);
-
- result = priv->buffer_used;
}
- cleanup:
- virMutexUnlock(&priv->curl->lock);
-
- return result;
+ return priv->buffer_used;
}
static int
@@ -348,18 +327,16 @@ esxStreamClose(virStreamPtr stream, bool finish)
if (!priv)
return 0;
- virMutexLock(&priv->curl->lock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&priv->curl->lock) {
+ if (finish && priv->backlog_used > 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Stream has untransferred data left"));
+ result = -1;
+ }
- if (finish && priv->backlog_used > 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Stream has untransferred data left"));
- result = -1;
+ stream->privateData = NULL;
}
- stream->privateData = NULL;
-
- virMutexUnlock(&priv->curl->lock);
-
esxFreeStreamPrivate(&priv);
return result;
--
2.31.1
5:29 a.m.
New subject: [libvirt PATCH v2 07/10] esx_stream: Use automatic mutex management
On Thu, 2022-03-17 at 10:44 +0100, Pavel Hrdina wrote:
On Fri, Mar 04, 2022 at 06:28:37PM +0100, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
> ---
> src/esx/esx_stream.c | 65 ++++++++++++++--------------------------
> ----
> 1 file changed, 21 insertions(+), 44 deletions(-)
>
> diff --git a/src/esx/esx_stream.c b/src/esx/esx_stream.c
> index 5b20804bb1..2b49c8dd12 100644
> --- a/src/esx/esx_stream.c
> +++ b/src/esx/esx_stream.c
> @@ -198,9 +198,8 @@ esxStreamTransfer(esxStreamPrivate *priv, bool
> blocking)
> static int
> esxStreamSend(virStreamPtr stream, const char *data, size_t
> nbytes)
> {
> - int result = -1;
> esxStreamPrivate *priv = stream->privateData;
> - int status;
> + VIR_LOCK_GUARD lock = virLockGuardLock(&priv->curl->lock);
Coverity discovered that this change is not correct:
/src/esx/esx_stream.c: 207 in esxStreamSend()
201 esxStreamPrivate *priv = stream->privateData;
202 VIR_LOCK_GUARD lock = virLockGuardLock(&priv->curl-
>lock);
203
204 if (nbytes == 0)
205 return 0;
206
> > > CID 389978: Null pointer dereferences (REVERSE_INULL)
> > > Null-checking "priv" suggests that it may be null, but it
> > > has already been dereferenced on all paths leading to the
> > > check.
207 if (!priv) {
208 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Stream is not open"));
209 return -1;
210 }
211
212 if (priv->mode != ESX_STREAM_MODE_UPLOAD) {
Previously the mutex was initialized after the !priv check, now we
will
dereference priv before this check.
> if (nbytes == 0)
> return 0;
> @@ -215,38 +214,29 @@ esxStreamSend(virStreamPtr stream, const char
> *data, size_t nbytes)
> return -1;
> }
>
> - virMutexLock(&priv->curl->lock);
> -
> priv->buffer = (char *)data;
> priv->buffer_size = nbytes;
> priv->buffer_used = nbytes;
>
> if (stream->flags & VIR_STREAM_NONBLOCK) {
> if (esxStreamTransfer(priv, false) < 0)
> - goto cleanup;
> + return -1;
>
> - if (priv->buffer_used < priv->buffer_size)
> - result = priv->buffer_size - priv->buffer_used;
> - else
> - result = -2;
> + if (priv->buffer_used >= priv->buffer_size)
> + return -2;
> } else /* blocking */ {
> do {
> - status = esxStreamTransfer(priv, true);
> + int status = esxStreamTransfer(priv, true);
>
> if (status < 0)
> - goto cleanup;
> + return -1;
>
> if (status > 0)
> break;
> } while (priv->buffer_used > 0);
> -
> - result = priv->buffer_size - priv->buffer_used;
> }
>
> - cleanup:
> - virMutexUnlock(&priv->curl->lock);
> -
> - return result;
> + return priv->buffer_size - priv->buffer_used;
> }
>
> static int
> @@ -255,9 +245,8 @@ esxStreamRecvFlags(virStreamPtr stream,
> size_t nbytes,
> unsigned int flags)
> {
> - int result = -1;
> esxStreamPrivate *priv = stream->privateData;
> - int status;
> + VIR_LOCK_GUARD lock = virLockGuardLock(&priv->curl->lock);
And same happens here.
Pavel
Thanks for making me aware of that mistake. I will set out to fix this
immediately.
Tim
>
> virCheckFlags(0, -1);
>
> @@ -274,8 +263,6 @@ esxStreamRecvFlags(virStreamPtr stream,
> return -1;
> }
>
> - virMutexLock(&priv->curl->lock);
> -
> priv->buffer = data;
> priv->buffer_size = nbytes;
> priv->buffer_used = 0;
> @@ -291,33 +278,25 @@ esxStreamRecvFlags(virStreamPtr stream,
> priv->backlog_used - priv->buffer_used);
>
> priv->backlog_used -= priv->buffer_used;
> - result = priv->buffer_used;
> } else if (stream->flags & VIR_STREAM_NONBLOCK) {
> if (esxStreamTransfer(priv, false) < 0)
> - goto cleanup;
> + return -1;
>
> - if (priv->buffer_used > 0)
> - result = priv->buffer_used;
> - else
> - result = -2;
> + if (priv->buffer_used <= 0)
> + return -2;
> } else /* blocking */ {
> do {
> - status = esxStreamTransfer(priv, true);
> + int status = esxStreamTransfer(priv, true);
>
> if (status < 0)
> - goto cleanup;
> + return -1;
>
> if (status > 0)
> break;
> } while (priv->buffer_used < priv->buffer_size);
> -
> - result = priv->buffer_used;
> }
>
> - cleanup:
> - virMutexUnlock(&priv->curl->lock);
> -
> - return result;
> + return priv->buffer_used;
> }
>
> static int
> @@ -348,18 +327,16 @@ esxStreamClose(virStreamPtr stream, bool
> finish)
> if (!priv)
> return 0;
>
> - virMutexLock(&priv->curl->lock);
> + VIR_WITH_MUTEX_LOCK_GUARD(&priv->curl->lock) {
> + if (finish && priv->backlog_used > 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Stream has untransferred data
> left"));
> + result = -1;
> + }
>
> - if (finish && priv->backlog_used > 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Stream has untransferred data left"));
> - result = -1;
> + stream->privateData = NULL;
> }
>
> - stream->privateData = NULL;
> -
> - virMutexUnlock(&priv->curl->lock);
> -
> esxFreeStreamPrivate(&priv);
>
> return result;
> --
> 2.31.1
>
11:28 a.m.
New subject: [libvirt PATCH v2 08/10] esx_vi: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/esx/esx_vi.c | 109 ++++++++++++++++++++---------------------------
1 file changed, 47 insertions(+), 62 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 36e9dc1d2c..b56d84fc43 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -380,17 +380,15 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url, char
**content,
range = g_strdup_printf("%llu-", offset);
}
- virMutexLock(&curl->lock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&curl->lock) {
+ curl_easy_setopt(curl->handle, CURLOPT_URL, url);
+ curl_easy_setopt(curl->handle, CURLOPT_RANGE, range);
+ curl_easy_setopt(curl->handle, CURLOPT_WRITEDATA, &buffer);
+ curl_easy_setopt(curl->handle, CURLOPT_UPLOAD, 0);
+ curl_easy_setopt(curl->handle, CURLOPT_HTTPGET, 1);
- curl_easy_setopt(curl->handle, CURLOPT_URL, url);
- curl_easy_setopt(curl->handle, CURLOPT_RANGE, range);
- curl_easy_setopt(curl->handle, CURLOPT_WRITEDATA, &buffer);
- curl_easy_setopt(curl->handle, CURLOPT_UPLOAD, 0);
- curl_easy_setopt(curl->handle, CURLOPT_HTTPGET, 1);
-
- responseCode = esxVI_CURL_Perform(curl, url);
-
- virMutexUnlock(&curl->lock);
+ responseCode = esxVI_CURL_Perform(curl, url);
+ }
if (responseCode < 0) {
return -1;
@@ -422,17 +420,15 @@ esxVI_CURL_Upload(esxVI_CURL *curl, const char *url, const char
*content)
return -1;
}
- virMutexLock(&curl->lock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&curl->lock) {
+ curl_easy_setopt(curl->handle, CURLOPT_URL, url);
+ curl_easy_setopt(curl->handle, CURLOPT_RANGE, NULL);
+ curl_easy_setopt(curl->handle, CURLOPT_READDATA, &content);
+ curl_easy_setopt(curl->handle, CURLOPT_UPLOAD, 1);
+ curl_easy_setopt(curl->handle, CURLOPT_INFILESIZE, strlen(content));
- curl_easy_setopt(curl->handle, CURLOPT_URL, url);
- curl_easy_setopt(curl->handle, CURLOPT_RANGE, NULL);
- curl_easy_setopt(curl->handle, CURLOPT_READDATA, &content);
- curl_easy_setopt(curl->handle, CURLOPT_UPLOAD, 1);
- curl_easy_setopt(curl->handle, CURLOPT_INFILESIZE, strlen(content));
-
- responseCode = esxVI_CURL_Perform(curl, url);
-
- virMutexUnlock(&curl->lock);
+ responseCode = esxVI_CURL_Perform(curl, url);
+ }
if (responseCode < 0) {
return -1;
@@ -574,14 +570,12 @@ esxVI_SharedCURL_Add(esxVI_SharedCURL *shared, esxVI_CURL *curl)
}
}
- virMutexLock(&curl->lock);
+ VIR_WITH_MUTEX_LOCK_GUARD(&curl->lock) {
+ curl_easy_setopt(curl->handle, CURLOPT_SHARE, shared->handle);
- curl_easy_setopt(curl->handle, CURLOPT_SHARE, shared->handle);
-
- curl->shared = shared;
- ++shared->count;
-
- virMutexUnlock(&curl->lock);
+ curl->shared = shared;
+ ++shared->count;
+ }
return 0;
}
@@ -606,14 +600,12 @@ esxVI_SharedCURL_Remove(esxVI_SharedCURL *shared, esxVI_CURL *curl)
return -1;
}
- virMutexLock(&curl->lock);
-
- curl_easy_setopt(curl->handle, CURLOPT_SHARE, NULL);
-
- curl->shared = NULL;
- --shared->count;
+ VIR_WITH_MUTEX_LOCK_GUARD(&curl->lock) {
+ curl_easy_setopt(curl->handle, CURLOPT_SHARE, NULL);
- virMutexUnlock(&curl->lock);
+ curl->shared = NULL;
+ --shared->count;
+ }
return 0;
}
@@ -667,14 +659,12 @@ esxVI_MultiCURL_Add(esxVI_MultiCURL *multi, esxVI_CURL *curl)
}
- virMutexLock(&curl->lock);
-
- curl_multi_add_handle(multi->handle, curl->handle);
-
- curl->multi = multi;
- ++multi->count;
+ VIR_WITH_MUTEX_LOCK_GUARD(&curl->lock) {
+ curl_multi_add_handle(multi->handle, curl->handle);
- virMutexUnlock(&curl->lock);
+ curl->multi = multi;
+ ++multi->count;
+ }
return 0;
}
@@ -701,14 +691,12 @@ esxVI_MultiCURL_Remove(esxVI_MultiCURL *multi, esxVI_CURL *curl)
return -1;
}
- virMutexLock(&curl->lock);
-
- curl_multi_remove_handle(multi->handle, curl->handle);
-
- curl->multi = NULL;
- --multi->count;
+ VIR_WITH_MUTEX_LOCK_GUARD(&curl->lock) {
+ curl_multi_remove_handle(multi->handle, curl->handle);
- virMutexUnlock(&curl->lock);
+ curl->multi = NULL;
+ --multi->count;
+ }
return 0;
}
@@ -1232,18 +1220,16 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName,
if (esxVI_Response_Alloc(response) < 0)
return -1;
- virMutexLock(&ctx->curl->lock);
-
- curl_easy_setopt(ctx->curl->handle, CURLOPT_URL, ctx->url);
- curl_easy_setopt(ctx->curl->handle, CURLOPT_RANGE, NULL);
- curl_easy_setopt(ctx->curl->handle, CURLOPT_WRITEDATA, &buffer);
- curl_easy_setopt(ctx->curl->handle, CURLOPT_UPLOAD, 0);
- curl_easy_setopt(ctx->curl->handle, CURLOPT_POSTFIELDS, request);
- curl_easy_setopt(ctx->curl->handle, CURLOPT_POSTFIELDSIZE, strlen(request));
-
- (*response)->responseCode = esxVI_CURL_Perform(ctx->curl, ctx->url);
+ VIR_WITH_MUTEX_LOCK_GUARD(&ctx->curl->lock) {
+ curl_easy_setopt(ctx->curl->handle, CURLOPT_URL, ctx->url);
+ curl_easy_setopt(ctx->curl->handle, CURLOPT_RANGE, NULL);
+ curl_easy_setopt(ctx->curl->handle, CURLOPT_WRITEDATA, &buffer);
+ curl_easy_setopt(ctx->curl->handle, CURLOPT_UPLOAD, 0);
+ curl_easy_setopt(ctx->curl->handle, CURLOPT_POSTFIELDS, request);
+ curl_easy_setopt(ctx->curl->handle, CURLOPT_POSTFIELDSIZE,
strlen(request));
- virMutexUnlock(&ctx->curl->lock);
+ (*response)->responseCode = esxVI_CURL_Perform(ctx->curl, ctx->url);
+ }
if ((*response)->responseCode < 0)
goto cleanup;
@@ -1872,13 +1858,14 @@ esxVI_EnsureSession(esxVI_Context *ctx)
esxVI_DynamicProperty *dynamicProperty = NULL;
esxVI_UserSession *currentSession = NULL;
g_autofree char *escapedPassword = NULL;
+ VIR_LOCK_GUARD lock = { NULL };
if (!ctx->sessionLock) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call, no
mutex"));
return -1;
}
- virMutexLock(ctx->sessionLock);
+ lock = virLockGuardLock(ctx->sessionLock);
if (!ctx->session) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid call, no
session"));
@@ -1933,8 +1920,6 @@ esxVI_EnsureSession(esxVI_Context *ctx)
result = 0;
cleanup:
- virMutexUnlock(ctx->sessionLock);
-
esxVI_String_Free(&propertyNameList);
esxVI_ObjectContent_Free(&sessionManager);
esxVI_UserSession_Free(¤tSession);
--
2.31.1
11:28 a.m.
New subject: [libvirt PATCH v2 09/10] storage: Removing mutex locking in initialization and cleanup
These functions are only ever called in a single threaded
environment and the mutex would not have prevented concurrent
access anyway.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/storage/storage_driver.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 97e0d9b3a0..ee710f6b76 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -274,7 +274,6 @@ storageStateInitialize(bool privileged,
VIR_FREE(driver);
return VIR_DRV_STATE_INIT_ERROR;
}
- storageDriverLock();
if (!(driver->pools = virStoragePoolObjListNew()))
goto error;
@@ -330,12 +329,9 @@ storageStateInitialize(bool privileged,
if (!(driver->caps = virStorageBackendGetCapabilities()))
goto error;
- storageDriverUnlock();
-
return VIR_DRV_STATE_INIT_COMPLETE;
error:
- storageDriverUnlock();
storageStateCleanup();
return VIR_DRV_STATE_INIT_ERROR;
}
@@ -376,8 +372,6 @@ storageStateCleanup(void)
if (!driver)
return -1;
- storageDriverLock();
-
virObjectUnref(driver->caps);
virObjectUnref(driver->storageEventState);
@@ -391,7 +385,6 @@ storageStateCleanup(void)
VIR_FREE(driver->configDir);
VIR_FREE(driver->autostartDir);
VIR_FREE(driver->stateDir);
- storageDriverUnlock();
virMutexDestroy(&driver->lock);
VIR_FREE(driver);
--
2.31.1
11:28 a.m.
New subject: [libvirt PATCH v2 10/10] storage: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/storage/storage_driver.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index ee710f6b76..e3e1604311 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -65,16 +65,6 @@ struct _virStorageVolStreamInfo {
char *vol_path;
};
-static void storageDriverLock(void)
-{
- virMutexLock(&driver->lock);
-}
-static void storageDriverUnlock(void)
-{
- virMutexUnlock(&driver->lock);
-}
-
-
static void
storagePoolRefreshFailCleanup(virStorageBackend *backend,
virStoragePoolObj *obj,
@@ -348,14 +338,13 @@ storageStateReload(void)
if (!driver)
return -1;
- storageDriverLock();
- virStoragePoolObjLoadAllState(driver->pools,
- driver->stateDir);
- virStoragePoolObjLoadAllConfigs(driver->pools,
- driver->configDir,
- driver->autostartDir);
- storageDriverAutostart();
- storageDriverUnlock();
+ VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
+ virStoragePoolObjLoadAllState(driver->pools, driver->stateDir);
+ virStoragePoolObjLoadAllConfigs(driver->pools,
+ driver->configDir,
+ driver->autostartDir);
+ storageDriverAutostart();
+ }
return 0;
}
--
2.31.1
3:58 a.m.
ping
On Fri, 2022-03-04 at 18:28 +0100, Tim Wiederhake wrote:
Use the recently implemented VIR_LOCK_GUARD and
VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
v1:
https://listman.redhat.com/archives/libvir-list/2022-February/msg00674.html
Changed since v1:
* Removed locking / unlocking in storage driver initialization and
cleanup
instead of working around the issue of the lifetime of the mutex.
Tim Wiederhake (10):
test: Use automatic mutex management
openvz: Use automatic mutex management
remote_daemon_dispatch: Use automatic mutex management
netdev: Use automatic mutex management
nodesuspend: Use automatic mutex management
admin: Use automatic mutex management
esx_stream: Use automatic mutex management
esx_vi: Use automatic mutex management
storage: Removing mutex locking in initialization and cleanup
storage: Use automatic mutex management
src/admin/admin_server_dispatch.c | 3 +-
src/esx/esx_stream.c | 65 ++++------
src/esx/esx_vi.c | 109 +++++++---------
src/openvz/openvz_driver.c | 91 +++++---------
src/remote/remote_daemon_dispatch.c | 187 +++++++++-----------------
--
src/storage/storage_driver.c | 32 ++---
src/test/test_driver.c | 15 +--
src/util/virnetdev.c | 20 ++-
src/util/virnodesuspend.c | 54 +++-----
9 files changed, 193 insertions(+), 383 deletions(-)
--
2.31.1
4:38 a.m.
On 3/4/22 18:28, Tim Wiederhake wrote:
Use the recently implemented VIR_LOCK_GUARD and
VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
v1: https://listman.redhat.com/archives/libvir-list/2022-February/msg00674.html
Changed since v1:
* Removed locking / unlocking in storage driver initialization and cleanup
instead of working around the issue of the lifetime of the mutex.
Tim Wiederhake (10):
test: Use automatic mutex management
openvz: Use automatic mutex management
remote_daemon_dispatch: Use automatic mutex management
netdev: Use automatic mutex management
nodesuspend: Use automatic mutex management
admin: Use automatic mutex management
esx_stream: Use automatic mutex management
esx_vi: Use automatic mutex management
storage: Removing mutex locking in initialization and cleanup
storage: Use automatic mutex management
src/admin/admin_server_dispatch.c | 3 +-
src/esx/esx_stream.c | 65 ++++------
src/esx/esx_vi.c | 109 +++++++---------
src/openvz/openvz_driver.c | 91 +++++---------
src/remote/remote_daemon_dispatch.c | 187 +++++++++-------------------
src/storage/storage_driver.c | 32 ++---
src/test/test_driver.c | 15 +--
src/util/virnetdev.c | 20 ++-
src/util/virnodesuspend.c | 54 +++-----
9 files changed, 193 insertions(+), 383 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
921
days inactive
934
days old
16 comments
3 participants
participants (3)
-
Michal Prívozník -
Pavel Hrdina -
Tim Wiederhake