9:57 a.m.
Use the recently implemented VIR_LOCK_GUARD and VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
Tim Wiederhake (10):
virstorageobj: Replace deprecated virHash functions
virStoragePoolObjAddVol: Simplify error path
virStorageVolObjEndAPI: Remove
virStorageVolObjNew: Move locking to usage site
virstorageobject: Use automatic mutex management
virNetServerProcessClients: Remove goto
virnetserver: Use automatic mutex management
virnetserverclient: Use automatic mutex management
security_manager: Use automatic mutex management
vz_driver: Use automatic mutex management
src/conf/virstorageobj.c | 185 +++++------
src/rpc/virnetserver.c | 295 +++++++-----------
src/rpc/virnetserverclient.c | 432 ++++++++++++--------------
src/security/security_manager.c | 524 +++++++++++++-------------------
src/vz/vz_driver.c | 50 ++-
5 files changed, 604 insertions(+), 882 deletions(-)
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 01/10] virstorageobj: Replace deprecated virHash functions
Checking for duplicate / NULL keys beforehand will simplify error
handling in a later patch significantly.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/virstorageobj.c | 47 ++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index b57c1cb8b2..99aee9ffb4 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -521,8 +521,8 @@ virStoragePoolObjRemove(virStoragePoolObjList *pools,
virObjectUnlock(obj);
virObjectRWLockWrite(pools);
virObjectLock(obj);
- virHashRemoveEntry(pools->objs, uuidstr);
- virHashRemoveEntry(pools->objsName, obj->def->name);
+ g_hash_table_remove(pools->objs, uuidstr);
+ g_hash_table_remove(pools->objsName, obj->def->name);
virObjectUnref(obj);
virObjectRWUnlock(pools);
}
@@ -621,9 +621,9 @@ virStoragePoolObjClearVols(virStoragePoolObj *obj)
if (!obj->volumes)
return;
- virHashRemoveAll(obj->volumes->objsKey);
- virHashRemoveAll(obj->volumes->objsName);
- virHashRemoveAll(obj->volumes->objsPath);
+ g_hash_table_remove_all(obj->volumes->objsKey);
+ g_hash_table_remove_all(obj->volumes->objsName);
+ g_hash_table_remove_all(obj->volumes->objsPath);
}
@@ -639,21 +639,19 @@ virStoragePoolObjAddVol(virStoragePoolObj *obj,
if (!(volobj = virStorageVolObjNew()))
goto error;
- if (virHashAddEntry(volumes->objsKey, voldef->key, volobj) < 0)
+ if (!voldef->key || !voldef->name || !voldef->target.path ||
+ g_hash_table_contains(volumes->objsKey, voldef->key) ||
+ g_hash_table_contains(volumes->objsName, voldef->name) ||
+ g_hash_table_contains(volumes->objsPath, voldef->target.path))
goto error;
+
+ g_hash_table_insert(volumes->objsKey, g_strdup(voldef->key), volobj);
virObjectRef(volobj);
- if (virHashAddEntry(volumes->objsName, voldef->name, volobj) < 0) {
- virHashRemoveEntry(volumes->objsKey, voldef->key);
- goto error;
- }
+ g_hash_table_insert(volumes->objsName, g_strdup(voldef->name), volobj);
virObjectRef(volobj);
- if (virHashAddEntry(volumes->objsPath, voldef->target.path, volobj) < 0) {
- virHashRemoveEntry(volumes->objsKey, voldef->key);
- virHashRemoveEntry(volumes->objsName, voldef->name);
- goto error;
- }
+ g_hash_table_insert(volumes->objsPath, g_strdup(voldef->target.path), volobj);
virObjectRef(volobj);
volobj->voldef = voldef;
@@ -688,9 +686,9 @@ virStoragePoolObjRemoveVol(virStoragePoolObj *obj,
virObjectRef(volobj);
virObjectLock(volobj);
- virHashRemoveEntry(volumes->objsKey, voldef->key);
- virHashRemoveEntry(volumes->objsName, voldef->name);
- virHashRemoveEntry(volumes->objsPath, voldef->target.path);
+ g_hash_table_remove(volumes->objsKey, voldef->key);
+ g_hash_table_remove(volumes->objsName, voldef->name);
+ g_hash_table_remove(volumes->objsPath, voldef->target.path);
virStorageVolObjEndAPI(&volobj);
virObjectRWUnlock(volumes);
@@ -1582,15 +1580,18 @@ virStoragePoolObjListAdd(virStoragePoolObjList *pools,
goto error;
virUUIDFormat((*def)->uuid, uuidstr);
- if (virHashAddEntry(pools->objs, uuidstr, obj) < 0)
+
+ if (!(*def)->name ||
+ g_hash_table_contains(pools->objs, uuidstr) ||
+ g_hash_table_contains(pools->objsName, (*def)->name))
goto error;
+
+ g_hash_table_insert(pools->objs, g_strdup(uuidstr), obj);
virObjectRef(obj);
- if (virHashAddEntry(pools->objsName, (*def)->name, obj) < 0) {
- virHashRemoveEntry(pools->objs, uuidstr);
- goto error;
- }
+ g_hash_table_insert(pools->objsName, g_strdup((*def)->name), obj);
virObjectRef(obj);
+
obj->def = g_steal_pointer(def);
virObjectRWUnlock(pools);
return obj;
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 02/10] virStoragePoolObjAddVol: Simplify error path
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/virstorageobj.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 99aee9ffb4..1ecf35640c 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -636,14 +636,18 @@ virStoragePoolObjAddVol(virStoragePoolObj *obj,
virObjectRWLockWrite(volumes);
- if (!(volobj = virStorageVolObjNew()))
- goto error;
-
if (!voldef->key || !voldef->name || !voldef->target.path ||
g_hash_table_contains(volumes->objsKey, voldef->key) ||
g_hash_table_contains(volumes->objsName, voldef->name) ||
- g_hash_table_contains(volumes->objsPath, voldef->target.path))
- goto error;
+ g_hash_table_contains(volumes->objsPath, voldef->target.path)) {
+ virObjectRWUnlock(volumes);
+ return -1;
+ }
+
+ if (!(volobj = virStorageVolObjNew())) {
+ virObjectRWUnlock(volumes);
+ return -1;
+ }
g_hash_table_insert(volumes->objsKey, g_strdup(voldef->key), volobj);
virObjectRef(volobj);
@@ -655,14 +659,10 @@ virStoragePoolObjAddVol(virStoragePoolObj *obj,
virObjectRef(volobj);
volobj->voldef = voldef;
- virObjectRWUnlock(volumes);
- virStorageVolObjEndAPI(&volobj);
- return 0;
- error:
virStorageVolObjEndAPI(&volobj);
virObjectRWUnlock(volumes);
- return -1;
+ return 0;
}
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 03/10] virStorageVolObjEndAPI: Remove
This allows a later patch to replace virObjectLock/Unlock
pairs with automatic mutex management code.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/virstorageobj.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 1ecf35640c..108ae0aeb1 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -138,17 +138,6 @@ virStorageVolObjNew(void)
}
-static void
-virStorageVolObjEndAPI(virStorageVolObj **obj)
-{
- if (!*obj)
- return;
-
- virObjectUnlock(*obj);
- g_clear_pointer(obj, virObjectUnref);
-}
-
-
static void
virStorageVolObjDispose(void *opaque)
{
@@ -660,7 +649,8 @@ virStoragePoolObjAddVol(virStoragePoolObj *obj,
volobj->voldef = voldef;
- virStorageVolObjEndAPI(&volobj);
+ virObjectUnlock(volobj);
+ virObjectUnref(volobj);
virObjectRWUnlock(volumes);
return 0;
}
@@ -689,8 +679,8 @@ virStoragePoolObjRemoveVol(virStoragePoolObj *obj,
g_hash_table_remove(volumes->objsKey, voldef->key);
g_hash_table_remove(volumes->objsName, voldef->name);
g_hash_table_remove(volumes->objsPath, voldef->target.path);
- virStorageVolObjEndAPI(&volobj);
-
+ virObjectUnlock(volobj);
+ virObjectUnref(volobj);
virObjectRWUnlock(volumes);
}
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 04/10] virStorageVolObjNew: Move locking to usage site
This allows a later patch to replace virObjectLock/Unlock
pairs with automatic mutex management code.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/virstorageobj.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 108ae0aeb1..c837f96ae6 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -125,16 +125,10 @@ VIR_ONCE_GLOBAL_INIT(virStorageVolObj);
static virStorageVolObj *
virStorageVolObjNew(void)
{
- virStorageVolObj *obj;
-
if (virStorageVolObjInitialize() < 0)
return NULL;
- if (!(obj = virObjectLockableNew(virStorageVolObjClass)))
- return NULL;
-
- virObjectLock(obj);
- return obj;
+ return virObjectLockableNew(virStorageVolObjClass);
}
@@ -638,6 +632,8 @@ virStoragePoolObjAddVol(virStoragePoolObj *obj,
return -1;
}
+ virObjectLock(volobj);
+
g_hash_table_insert(volumes->objsKey, g_strdup(voldef->key), volobj);
virObjectRef(volobj);
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 05/10] virstorageobject: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/virstorageobj.c | 116 ++++++++++++++-------------------------
1 file changed, 42 insertions(+), 74 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index c837f96ae6..fe868d25fa 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -455,11 +455,11 @@ virStoragePoolObjListSearchCb(const void *payload,
virStoragePoolObj *obj = (virStoragePoolObj *) payload;
struct _virStoragePoolObjListSearchData *data =
(struct _virStoragePoolObjListSearchData *)opaque;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
- virObjectLock(obj);
if (data->searcher(obj, data->opaque))
return 1;
- virObjectUnlock(obj);
+
return 0;
}
@@ -632,20 +632,19 @@ virStoragePoolObjAddVol(virStoragePoolObj *obj,
return -1;
}
- virObjectLock(volobj);
+ VIR_WITH_OBJECT_LOCK_GUARD(volobj) {
+ g_hash_table_insert(volumes->objsKey, g_strdup(voldef->key), volobj);
+ virObjectRef(volobj);
- g_hash_table_insert(volumes->objsKey, g_strdup(voldef->key), volobj);
- virObjectRef(volobj);
+ g_hash_table_insert(volumes->objsName, g_strdup(voldef->name), volobj);
+ virObjectRef(volobj);
- g_hash_table_insert(volumes->objsName, g_strdup(voldef->name), volobj);
- virObjectRef(volobj);
-
- g_hash_table_insert(volumes->objsPath, g_strdup(voldef->target.path), volobj);
- virObjectRef(volobj);
+ g_hash_table_insert(volumes->objsPath, g_strdup(voldef->target.path),
volobj);
+ virObjectRef(volobj);
- volobj->voldef = voldef;
+ volobj->voldef = voldef;
+ }
- virObjectUnlock(volobj);
virObjectUnref(volobj);
virObjectRWUnlock(volumes);
return 0;
@@ -671,11 +670,11 @@ virStoragePoolObjRemoveVol(virStoragePoolObj *obj,
voldef->name, obj->def->name);
virObjectRef(volobj);
- virObjectLock(volobj);
- g_hash_table_remove(volumes->objsKey, voldef->key);
- g_hash_table_remove(volumes->objsName, voldef->name);
- g_hash_table_remove(volumes->objsPath, voldef->target.path);
- virObjectUnlock(volobj);
+ VIR_WITH_OBJECT_LOCK_GUARD(volobj) {
+ g_hash_table_remove(volumes->objsKey, voldef->key);
+ g_hash_table_remove(volumes->objsName, voldef->name);
+ g_hash_table_remove(volumes->objsPath, voldef->target.path);
+ }
virObjectUnref(volobj);
virObjectRWUnlock(volumes);
}
@@ -704,16 +703,14 @@ virStoragePoolObjForEachVolumeCb(void *payload,
const char *name G_GNUC_UNUSED,
void *opaque)
{
- int ret = 0;
virStorageVolObj *volobj = payload;
struct _virStoragePoolObjForEachVolData *data = opaque;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(volobj);
- virObjectLock(volobj);
if (data->iter(volobj->voldef, data->opaque) < 0)
- ret = -1;
- virObjectUnlock(volobj);
+ return -1;
- return ret;
+ return 0;
}
@@ -746,14 +743,12 @@ virStoragePoolObjSearchVolumeCb(const void *payload,
virStorageVolObj *volobj = (virStorageVolObj *) payload;
struct _virStoragePoolObjSearchVolData *data =
(struct _virStoragePoolObjSearchVolData *) opaque;
- int found = 0;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(volobj);
- virObjectLock(volobj);
if (data->iter(volobj->voldef, data->opaque))
- found = 1;
- virObjectUnlock(volobj);
+ return 1;
- return found;
+ return 0;
}
@@ -842,17 +837,12 @@ virStoragePoolObjNumOfVolumesCb(void *payload,
{
virStorageVolObj *volobj = payload;
struct _virStorageVolObjCountData *data = opaque;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(volobj);
- virObjectLock(volobj);
-
- if (data->filter &&
- !data->filter(data->conn, data->pooldef, volobj->voldef))
- goto cleanup;
+ if (data->filter && !data->filter(data->conn, data->pooldef,
volobj->voldef))
+ return 0;
data->count++;
-
- cleanup:
- virObjectUnlock(volobj);
return 0;
}
@@ -891,6 +881,7 @@ virStoragePoolObjVolumeGetNamesCb(void *payload,
{
virStorageVolObj *volobj = payload;
struct _virStorageVolObjNameData *data = opaque;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(volobj);
if (data->error)
return 0;
@@ -898,19 +889,13 @@ virStoragePoolObjVolumeGetNamesCb(void *payload,
if (data->maxnames >= 0 && data->nnames == data->maxnames)
return 0;
- virObjectLock(volobj);
-
- if (data->filter &&
- !data->filter(data->conn, data->pooldef, volobj->voldef))
- goto cleanup;
+ if (data->filter && !data->filter(data->conn, data->pooldef,
volobj->voldef))
+ return 0;
if (data->names)
data->names[data->nnames] = g_strdup(volobj->voldef->name);
data->nnames++;
-
- cleanup:
- virObjectUnlock(volobj);
return 0;
}
@@ -961,30 +946,25 @@ virStoragePoolObjVolumeListExportCallback(void *payload,
virStorageVolObj *volobj = payload;
virStoragePoolObjVolumeListExportData *data = opaque;
virStorageVolPtr vol = NULL;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(volobj);
if (data->error)
return 0;
- virObjectLock(volobj);
-
- if (data->filter &&
- !data->filter(data->conn, data->pooldef, volobj->voldef))
- goto cleanup;
+ if (data->filter && !data->filter(data->conn, data->pooldef,
volobj->voldef))
+ return 0;
if (data->vols) {
if (!(vol = virGetStorageVol(data->conn, data->pooldef->name,
volobj->voldef->name,
volobj->voldef->key,
NULL, NULL))) {
data->error = true;
- goto cleanup;
+ return 0;
}
data->vols[data->nvols] = vol;
}
data->nvols++;
-
- cleanup:
- virObjectUnlock(volobj);
return 0;
}
@@ -1799,19 +1779,15 @@ virStoragePoolObjNumOfStoragePoolsCb(void *payload,
{
virStoragePoolObj *obj = payload;
struct _virStoragePoolCountData *data = opaque;
-
- virObjectLock(obj);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
if (data->filter && !data->filter(data->conn, obj->def))
- goto cleanup;
+ return 0;
if (data->wantActive != virStoragePoolObjIsActive(obj))
- goto cleanup;
+ return 0;
data->count++;
-
- cleanup:
- virObjectUnlock(obj);
return 0;
}
@@ -1851,6 +1827,7 @@ virStoragePoolObjGetNamesCb(void *payload,
{
virStoragePoolObj *obj = payload;
struct _virStoragePoolNameData *data = opaque;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
if (data->error)
return 0;
@@ -1858,21 +1835,16 @@ virStoragePoolObjGetNamesCb(void *payload,
if (data->maxnames >= 0 && data->nnames == data->maxnames)
return 0;
- virObjectLock(obj);
-
if (data->filter && !data->filter(data->conn, obj->def))
- goto cleanup;
+ return 0;
if (data->wantActive != virStoragePoolObjIsActive(obj))
- goto cleanup;
+ return 0;
if (data->names)
data->names[data->nnames] = g_strdup(obj->def->name);
data->nnames++;
-
- cleanup:
- virObjectUnlock(obj);
return 0;
}
@@ -1994,31 +1966,27 @@ virStoragePoolObjListExportCallback(void *payload,
virStoragePoolObj *obj = payload;
virStoragePoolObjListExportData *data = opaque;
virStoragePoolPtr pool = NULL;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(obj);
if (data->error)
return 0;
- virObjectLock(obj);
-
if (data->filter && !data->filter(data->conn, obj->def))
- goto cleanup;
+ return 0;
if (!virStoragePoolObjMatch(obj, data->flags))
- goto cleanup;
+ return 0;
if (data->pools) {
if (!(pool = virGetStoragePool(data->conn, obj->def->name,
obj->def->uuid, NULL, NULL))) {
data->error = true;
- goto cleanup;
+ return 0;
}
data->pools[data->nPools] = pool;
}
data->nPools++;
-
- cleanup:
- virObjectUnlock(obj);
return 0;
}
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 06/10] virNetServerProcessClients: Remove goto
This gets rid of the goto and prepares the function for automatic
mutex management.
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/rpc/virnetserver.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index d9430a2cfa..7824d121cd 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -960,13 +960,19 @@ virNetServerHasClients(virNetServer *srv)
void
virNetServerProcessClients(virNetServer *srv)
{
- size_t i;
- virNetServerClient *client;
+ size_t i = 0;
- virObjectLock(srv);
+ while (true) {
+ virNetServerClient *client;
+ bool removed = false;
+
+ virObjectLock(srv);
+
+ if (i >= srv->nclients) {
+ virObjectUnlock(srv);
+ return;
+ }
- reprocess:
- for (i = 0; i < srv->nclients; i++) {
client = srv->clients[i];
virObjectLock(client);
if (virNetServerClientWantCloseLocked(client))
@@ -974,24 +980,26 @@ virNetServerProcessClients(virNetServer *srv)
if (virNetServerClientIsClosedLocked(client)) {
VIR_DELETE_ELEMENT(srv->clients, i, srv->nclients);
+ removed = true;
/* Update server authentication tracking */
virNetServerSetClientAuthCompletedLocked(srv, client);
- virObjectUnlock(client);
-
virNetServerCheckLimits(srv);
+ }
+
+ virObjectUnlock(client);
+ if (removed) {
+ i = 0;
virObjectUnlock(srv);
virObjectUnref(client);
- virObjectLock(srv);
-
- goto reprocess;
- } else {
- virObjectUnlock(client);
+ continue;
}
- }
- virObjectUnlock(srv);
+ i++;
+
+ virObjectUnlock(srv);
+ }
}
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 07/10] virnetserver: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/rpc/virnetserver.c | 277 +++++++++++++++--------------------------
1 file changed, 100 insertions(+), 177 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 7824d121cd..9b333f1a6c 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -97,13 +97,9 @@ VIR_ONCE_GLOBAL_INIT(virNetServer);
unsigned long long
virNetServerNextClientID(virNetServer *srv)
{
- unsigned long long val;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
- virObjectLock(srv);
- val = srv->next_client_id++;
- virObjectUnlock(srv);
-
- return val;
+ return srv->next_client_id++;
}
@@ -208,13 +204,13 @@ virNetServerDispatchNewMessage(virNetServerClient *client,
VIR_DEBUG("server=%p client=%p message=%p",
srv, client, msg);
- virObjectLock(srv);
- prog = virNetServerGetProgramLocked(srv, msg);
- /* we can unlock @srv since @prog can only become invalid in case
- * of disposing @srv, but let's grab a ref first to ensure nothing
- * disposes of it before we use it. */
- virObjectRef(srv);
- virObjectUnlock(srv);
+ VIR_WITH_OBJECT_LOCK_GUARD(srv) {
+ prog = virNetServerGetProgramLocked(srv, msg);
+ /* we can unlock @srv since @prog can only become invalid in case
+ * of disposing @srv, but let's grab a ref first to ensure nothing
+ * disposes of it before we use it. */
+ virObjectRef(srv);
+ }
if (virThreadPoolGetMaxWorkers(srv->workers) > 0) {
virNetServerJob *job;
@@ -304,35 +300,28 @@ int
virNetServerAddClient(virNetServer *srv,
virNetServerClient *client)
{
- virObjectLock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
if (virNetServerClientInit(client) < 0)
- goto error;
+ return -1;
VIR_EXPAND_N(srv->clients, srv->nclients, 1);
srv->clients[srv->nclients-1] = virObjectRef(client);
- virObjectLock(client);
- if (virNetServerClientIsAuthPendingLocked(client))
- virNetServerTrackPendingAuthLocked(srv);
- virObjectUnlock(client);
+ VIR_WITH_OBJECT_LOCK_GUARD(client) {
+ if (virNetServerClientIsAuthPendingLocked(client))
+ virNetServerTrackPendingAuthLocked(srv);
+ }
virNetServerCheckLimits(srv);
- virNetServerClientSetDispatcher(client,
- virNetServerDispatchNewMessage,
- srv);
+ virNetServerClientSetDispatcher(client, virNetServerDispatchNewMessage, srv);
if (virNetServerClientInitKeepAlive(client, srv->keepaliveInterval,
srv->keepaliveCount) < 0)
- goto error;
+ return -1;
- virObjectUnlock(srv);
return 0;
-
- error:
- virObjectUnlock(srv);
- return -1;
}
@@ -565,65 +554,62 @@ virNetServerPreExecRestart(virNetServer *srv)
g_autoptr(virJSONValue) clients = virJSONValueNewArray();
g_autoptr(virJSONValue) services = virJSONValueNewArray();
size_t i;
-
- virObjectLock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
if (virJSONValueObjectAppendNumberUint(object, "min_workers",
virThreadPoolGetMinWorkers(srv->workers))
< 0)
- goto error;
+ return NULL;
+
if (virJSONValueObjectAppendNumberUint(object, "max_workers",
virThreadPoolGetMaxWorkers(srv->workers))
< 0)
- goto error;
+ return NULL;
+
if (virJSONValueObjectAppendNumberUint(object, "priority_workers",
virThreadPoolGetPriorityWorkers(srv->workers)) < 0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendNumberUint(object, "max_clients",
srv->nclients_max) < 0)
- goto error;
+ return NULL;
+
if (virJSONValueObjectAppendNumberUint(object, "max_anonymous_clients",
srv->nclients_unauth_max) < 0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendNumberUint(object, "keepaliveInterval",
srv->keepaliveInterval) < 0)
- goto error;
+ return NULL;
+
if (virJSONValueObjectAppendNumberUint(object, "keepaliveCount",
srv->keepaliveCount) < 0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendNumberUlong(object, "next_client_id",
srv->next_client_id) < 0)
- goto error;
+ return NULL;
for (i = 0; i < srv->nservices; i++) {
g_autoptr(virJSONValue) child = NULL;
if (!(child = virNetServerServicePreExecRestart(srv->services[i])))
- goto error;
+ return NULL;
if (virJSONValueArrayAppend(services, &child) < 0)
- goto error;
+ return NULL;
}
if (virJSONValueObjectAppend(object, "services", &services) < 0)
- goto error;
+ return NULL;
for (i = 0; i < srv->nclients; i++) {
g_autoptr(virJSONValue) child = NULL;
if (!(child = virNetServerClientPreExecRestart(srv->clients[i])))
- goto error;
+ return NULL;
if (virJSONValueArrayAppend(clients, &child) < 0)
- goto error;
+ return NULL;
}
if (virJSONValueObjectAppend(object, "clients", &clients) < 0)
- goto error;
-
- virObjectUnlock(srv);
+ return NULL;
return g_steal_pointer(&object);
-
- error:
- virObjectUnlock(srv);
- return NULL;
}
@@ -631,16 +617,12 @@ int
virNetServerAddService(virNetServer *srv,
virNetServerService *svc)
{
- virObjectLock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
VIR_EXPAND_N(srv->services, srv->nservices, 1);
srv->services[srv->nservices-1] = virObjectRef(svc);
- virNetServerServiceSetDispatcher(svc,
- virNetServerDispatchNewClient,
- srv);
-
- virObjectUnlock(srv);
+ virNetServerServiceSetDispatcher(svc, virNetServerDispatchNewClient, srv);
return 0;
}
@@ -795,12 +777,10 @@ int
virNetServerAddProgram(virNetServer *srv,
virNetServerProgram *prog)
{
- virObjectLock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
VIR_EXPAND_N(srv->programs, srv->nprograms, 1);
srv->programs[srv->nprograms-1] = virObjectRef(prog);
-
- virObjectUnlock(srv);
return 0;
}
@@ -846,13 +826,12 @@ void
virNetServerSetClientAuthenticated(virNetServer *srv,
virNetServerClient *client)
{
- virObjectLock(srv);
- virObjectLock(client);
+ VIR_LOCK_GUARD server_lock = virObjectLockGuard(srv);
+ VIR_LOCK_GUARD client_lock = virObjectLockGuard(srv);
+
virNetServerClientSetAuthLocked(client, VIR_NET_SERVER_SERVICE_AUTH_NONE);
virNetServerSetClientAuthCompletedLocked(srv, client);
virNetServerCheckLimits(srv);
- virObjectUnlock(client);
- virObjectUnlock(srv);
}
@@ -871,9 +850,9 @@ void
virNetServerUpdateServices(virNetServer *srv,
bool enabled)
{
- virObjectLock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
+
virNetServerUpdateServicesLocked(srv, enabled);
- virObjectUnlock(srv);
}
@@ -904,22 +883,20 @@ virNetServerDispose(void *obj)
void
virNetServerClose(virNetServer *srv)
{
- size_t i;
-
if (!srv)
return;
- virObjectLock(srv);
+ VIR_WITH_OBJECT_LOCK_GUARD(srv) {
+ size_t i;
- for (i = 0; i < srv->nservices; i++)
- virNetServerServiceClose(srv->services[i]);
-
- for (i = 0; i < srv->nclients; i++)
- virNetServerClientClose(srv->clients[i]);
+ for (i = 0; i < srv->nservices; i++)
+ virNetServerServiceClose(srv->services[i]);
- virThreadPoolStop(srv->workers);
+ for (i = 0; i < srv->nclients; i++)
+ virNetServerClientClose(srv->clients[i]);
- virObjectUnlock(srv);
+ virThreadPoolStop(srv->workers);
+ }
}
@@ -947,13 +924,9 @@ virNetServerTrackCompletedAuthLocked(virNetServer *srv)
bool
virNetServerHasClients(virNetServer *srv)
{
- bool ret;
-
- virObjectLock(srv);
- ret = !!srv->nclients;
- virObjectUnlock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
- return ret;
+ return !!srv->nclients;
}
@@ -963,42 +936,37 @@ virNetServerProcessClients(virNetServer *srv)
size_t i = 0;
while (true) {
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
virNetServerClient *client;
bool removed = false;
- virObjectLock(srv);
-
if (i >= srv->nclients) {
- virObjectUnlock(srv);
return;
}
client = srv->clients[i];
- virObjectLock(client);
- if (virNetServerClientWantCloseLocked(client))
- virNetServerClientCloseLocked(client);
- if (virNetServerClientIsClosedLocked(client)) {
- VIR_DELETE_ELEMENT(srv->clients, i, srv->nclients);
- removed = true;
+ VIR_WITH_OBJECT_LOCK_GUARD(client) {
+ if (virNetServerClientWantCloseLocked(client))
+ virNetServerClientCloseLocked(client);
- /* Update server authentication tracking */
- virNetServerSetClientAuthCompletedLocked(srv, client);
- virNetServerCheckLimits(srv);
- }
+ if (virNetServerClientIsClosedLocked(client)) {
+ VIR_DELETE_ELEMENT(srv->clients, i, srv->nclients);
+ removed = true;
- virObjectUnlock(client);
+ /* Update server authentication tracking */
+ virNetServerSetClientAuthCompletedLocked(srv, client);
+ virNetServerCheckLimits(srv);
+ }
+ }
if (removed) {
i = 0;
- virObjectUnlock(srv);
virObjectUnref(client);
continue;
}
i++;
-
- virObjectUnlock(srv);
}
}
@@ -1019,7 +987,7 @@ virNetServerGetThreadPoolParameters(virNetServer *srv,
size_t *nPrioWorkers,
size_t *jobQueueDepth)
{
- virObjectLock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
*minWorkers = virThreadPoolGetMinWorkers(srv->workers);
*maxWorkers = virThreadPoolGetMaxWorkers(srv->workers);
@@ -1028,7 +996,6 @@ virNetServerGetThreadPoolParameters(virNetServer *srv,
*nPrioWorkers = virThreadPoolGetPriorityWorkers(srv->workers);
*jobQueueDepth = virThreadPoolGetJobQueueDepth(srv->workers);
- virObjectUnlock(srv);
return 0;
}
@@ -1039,66 +1006,46 @@ virNetServerSetThreadPoolParameters(virNetServer *srv,
long long int maxWorkers,
long long int prioWorkers)
{
- int ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
- virObjectLock(srv);
- ret = virThreadPoolSetParameters(srv->workers, minWorkers,
- maxWorkers, prioWorkers);
- virObjectUnlock(srv);
-
- return ret;
+ return virThreadPoolSetParameters(srv->workers, minWorkers,
+ maxWorkers, prioWorkers);
}
size_t
virNetServerGetMaxClients(virNetServer *srv)
{
- size_t ret;
-
- virObjectLock(srv);
- ret = srv->nclients_max;
- virObjectUnlock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
- return ret;
+ return srv->nclients_max;
}
size_t
virNetServerGetCurrentClients(virNetServer *srv)
{
- size_t ret;
-
- virObjectLock(srv);
- ret = srv->nclients;
- virObjectUnlock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
- return ret;
+ return srv->nclients;
}
size_t
virNetServerGetMaxUnauthClients(virNetServer *srv)
{
- size_t ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
- virObjectLock(srv);
- ret = srv->nclients_unauth_max;
- virObjectUnlock(srv);
-
- return ret;
+ return srv->nclients_unauth_max;
}
size_t
virNetServerGetCurrentUnauthClients(virNetServer *srv)
{
- size_t ret;
-
- virObjectLock(srv);
- ret = srv->nclients_unauth;
- virObjectUnlock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
- return ret;
+ return srv->nclients_unauth;
}
@@ -1106,17 +1053,15 @@ bool
virNetServerNeedsAuth(virNetServer *srv,
int auth)
{
- bool ret = false;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
size_t i;
- virObjectLock(srv);
for (i = 0; i < srv->nservices; i++) {
if (virNetServerServiceGetAuth(srv->services[i]) == auth)
- ret = true;
+ return true;
}
- virObjectUnlock(srv);
- return ret;
+ return false;
}
@@ -1127,8 +1072,7 @@ virNetServerGetClients(virNetServer *srv,
size_t i;
size_t nclients = 0;
virNetServerClient **list = NULL;
-
- virObjectLock(srv);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
for (i = 0; i < srv->nclients; i++) {
virNetServerClient *client = virObjectRef(srv->clients[i]);
@@ -1137,8 +1081,6 @@ virNetServerGetClients(virNetServer *srv,
*clts = g_steal_pointer(&list);
- virObjectUnlock(srv);
-
return nclients;
}
@@ -1147,23 +1089,17 @@ virNetServerClient *
virNetServerGetClient(virNetServer *srv,
unsigned long long id)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
size_t i;
- virNetServerClient *ret = NULL;
-
- virObjectLock(srv);
for (i = 0; i < srv->nclients; i++) {
virNetServerClient *client = srv->clients[i];
if (virNetServerClientGetID(client) == id)
- ret = virObjectRef(client);
+ return virObjectRef(client);
}
- virObjectUnlock(srv);
-
- if (!ret)
- virReportError(VIR_ERR_NO_CLIENT,
- _("No client with matching ID '%llu'"), id);
- return ret;
+ virReportError(VIR_ERR_NO_CLIENT, _("No client with matching ID
'%llu'"), id);
+ return NULL;
}
@@ -1172,13 +1108,9 @@ virNetServerSetClientLimits(virNetServer *srv,
long long int maxClients,
long long int maxClientsUnauth)
{
- int ret = -1;
- size_t max, max_unauth;
-
- virObjectLock(srv);
-
- max = maxClients >= 0 ? maxClients : srv->nclients_max;
- max_unauth = maxClientsUnauth >= 0 ?
+ VIR_LOCK_GUARD lock = virObjectLockGuard(srv);
+ size_t max = maxClients >= 0 ? maxClients : srv->nclients_max;
+ size_t max_unauth = maxClientsUnauth >= 0 ?
maxClientsUnauth : srv->nclients_unauth_max;
if (max < max_unauth) {
@@ -1186,7 +1118,7 @@ virNetServerSetClientLimits(virNetServer *srv,
_("The overall maximum number of clients must be "
"greater than the maximum number of clients waiting "
"for authentication"));
- goto cleanup;
+ return -1;
}
if (maxClients >= 0)
@@ -1197,10 +1129,7 @@ virNetServerSetClientLimits(virNetServer *srv,
virNetServerCheckLimits(srv);
- ret = 0;
- cleanup:
- virObjectUnlock(srv);
- return ret;
+ return 0;
}
@@ -1226,30 +1155,24 @@ virNetServerGetTLSContext(virNetServer *srv)
int
virNetServerUpdateTlsFiles(virNetServer *srv)
{
- int ret = -1;
- virNetTLSContext *ctxt = NULL;
bool privileged = geteuid() == 0;
+ virNetTLSContext *ctxt = virNetServerGetTLSContext(srv);
- ctxt = virNetServerGetTLSContext(srv);
if (!ctxt) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("no tls service found, unable to update tls files"));
return -1;
}
- virObjectLock(srv);
- virObjectLock(ctxt);
-
- if (virNetTLSContextReloadForServer(ctxt, !privileged)) {
- VIR_DEBUG("failed to reload server's tls context");
- goto cleanup;
+ VIR_WITH_OBJECT_LOCK_GUARD(srv) {
+ VIR_WITH_OBJECT_LOCK_GUARD(ctxt) {
+ if (virNetTLSContextReloadForServer(ctxt, !privileged)) {
+ VIR_DEBUG("failed to reload server's tls context");
+ return -1;
+ }
+ }
}
VIR_DEBUG("update tls files success");
- ret = 0;
-
- cleanup:
- virObjectUnlock(ctxt);
- virObjectUnlock(srv);
- return ret;
+ return 0;
}
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 08/10] virnetserverclient: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/rpc/virnetserverclient.c | 432 +++++++++++++++--------------------
1 file changed, 186 insertions(+), 246 deletions(-)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 7d5c0965b8..da9956f2b4 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -234,14 +234,12 @@ int virNetServerClientAddFilter(virNetServerClient *client,
virNetServerClientFilterFunc func,
void *opaque)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
virNetServerClientFilter *filter;
virNetServerClientFilter **place;
- int ret;
filter = g_new0(virNetServerClientFilter, 1);
- virObjectLock(client);
-
filter->id = client->nextFilterID++;
filter->func = func;
filter->opaque = opaque;
@@ -251,21 +249,16 @@ int virNetServerClientAddFilter(virNetServerClient *client,
place = &(*place)->next;
*place = filter;
- ret = filter->id;
-
- virObjectUnlock(client);
-
- return ret;
+ return filter->id;
}
void virNetServerClientRemoveFilter(virNetServerClient *client,
int filterID)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
virNetServerClientFilter *tmp;
virNetServerClientFilter *prev;
- virObjectLock(client);
-
prev = NULL;
tmp = client->filters;
while (tmp) {
@@ -281,8 +274,6 @@ void virNetServerClientRemoveFilter(virNetServerClient *client,
prev = tmp;
tmp = tmp->next;
}
-
- virObjectUnlock(client);
}
@@ -322,19 +313,19 @@ virNetServerClientCheckAccess(virNetServerClient *client)
static void virNetServerClientDispatchMessage(virNetServerClient *client,
virNetMessage *msg)
{
- virObjectLock(client);
- if (!client->dispatchFunc) {
- virNetMessageFree(msg);
- client->wantClose = true;
- virObjectUnlock(client);
- } else {
- virObjectUnlock(client);
- /* Accessing 'client' is safe, because virNetServerClientSetDispatcher
- * only permits setting 'dispatchFunc' once, so if non-NULL, it will
- * never change again
- */
- client->dispatchFunc(client, msg, client->dispatchOpaque);
+ VIR_WITH_OBJECT_LOCK_GUARD(client) {
+ if (!client->dispatchFunc) {
+ virNetMessageFree(msg);
+ client->wantClose = true;
+ return;
+ }
}
+
+ /* Accessing 'client' is safe, because virNetServerClientSetDispatcher
+ * only permits setting 'dispatchFunc' once, so if non-NULL, it will
+ * never change again
+ */
+ client->dispatchFunc(client, msg, client->dispatchOpaque);
}
@@ -343,13 +334,14 @@ static void virNetServerClientSockTimerFunc(int timer,
{
virNetServerClient *client = opaque;
virNetMessage *msg = NULL;
- virObjectLock(client);
- virEventUpdateTimeout(timer, -1);
- /* Although client->rx != NULL when this timer is enabled, it might have
- * changed since the client was unlocked in the meantime. */
- if (client->rx)
- msg = virNetServerClientDispatchRead(client);
- virObjectUnlock(client);
+
+ VIR_WITH_OBJECT_LOCK_GUARD(client) {
+ virEventUpdateTimeout(timer, -1);
+ /* Although client->rx != NULL when this timer is enabled, it might have
+ * changed since the client was unlocked in the meantime. */
+ if (client->rx)
+ msg = virNetServerClientDispatchRead(client);
+ }
if (msg)
virNetServerClientDispatchMessage(client, msg);
@@ -587,53 +579,45 @@ virJSONValue *virNetServerClientPreExecRestart(virNetServerClient
*client)
g_autoptr(virJSONValue) object = virJSONValueNewObject();
g_autoptr(virJSONValue) sock = NULL;
g_autoptr(virJSONValue) priv = NULL;
-
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
if (virJSONValueObjectAppendNumberUlong(object, "id", client->id) <
0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendNumberInt(object, "auth", client->auth) <
0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendBoolean(object, "auth_pending",
client->auth_pending) < 0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendBoolean(object, "readonly",
client->readonly) < 0)
- goto error;
+ return NULL;
if (virJSONValueObjectAppendNumberUint(object, "nrequests_max",
client->nrequests_max) < 0)
- goto error;
+ return NULL;
if (client->conn_time &&
virJSONValueObjectAppendNumberLong(object, "conn_time",
client->conn_time) < 0)
- goto error;
+ return NULL;
if (!(sock = virNetSocketPreExecRestart(client->sock)))
- goto error;
+ return NULL;
if (virJSONValueObjectAppend(object, "sock", &sock) < 0)
- goto error;
+ return NULL;
if (!(priv = client->privateDataPreExecRestart(client, client->privateData)))
- goto error;
+ return NULL;
if (virJSONValueObjectAppend(object, "privateData", &priv) < 0)
- goto error;
+ return NULL;
- virObjectUnlock(client);
return g_steal_pointer(&object);
-
- error:
- virObjectUnlock(client);
- return NULL;
}
int virNetServerClientGetAuth(virNetServerClient *client)
{
- int auth;
- virObjectLock(client);
- auth = client->auth;
- virObjectUnlock(client);
- return auth;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ return client->auth;
}
@@ -647,11 +631,9 @@ virNetServerClientSetAuthLocked(virNetServerClient *client,
bool virNetServerClientGetReadonly(virNetServerClient *client)
{
- bool readonly;
- virObjectLock(client);
- readonly = client->readonly;
- virObjectUnlock(client);
- return readonly;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ return client->readonly;
}
@@ -659,9 +641,9 @@ void
virNetServerClientSetReadonly(virNetServerClient *client,
bool readonly)
{
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
client->readonly = readonly;
- virObjectUnlock(client);
}
@@ -677,52 +659,48 @@ long long virNetServerClientGetTimestamp(virNetServerClient
*client)
bool virNetServerClientHasTLSSession(virNetServerClient *client)
{
- bool has;
- virObjectLock(client);
- has = client->tls ? true : false;
- virObjectUnlock(client);
- return has;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ return !!client->tls;
}
virNetTLSSession *virNetServerClientGetTLSSession(virNetServerClient *client)
{
- virNetTLSSession *tls;
- virObjectLock(client);
- tls = client->tls;
- virObjectUnlock(client);
- return tls;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ return client->tls;
}
int virNetServerClientGetTLSKeySize(virNetServerClient *client)
{
- int size = 0;
- virObjectLock(client);
- if (client->tls)
- size = virNetTLSSessionGetKeySize(client->tls);
- virObjectUnlock(client);
- return size;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ if (!client->tls)
+ return 0;
+
+ return virNetTLSSessionGetKeySize(client->tls);
}
int virNetServerClientGetFD(virNetServerClient *client)
{
- int fd = -1;
- virObjectLock(client);
- if (client->sock)
- fd = virNetSocketGetFD(client->sock);
- virObjectUnlock(client);
- return fd;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ if (!client->sock)
+ return -1;
+
+ return virNetSocketGetFD(client->sock);
}
bool virNetServerClientIsLocal(virNetServerClient *client)
{
- bool local = false;
- virObjectLock(client);
- if (client->sock)
- local = virNetSocketIsLocal(client->sock);
- virObjectUnlock(client);
- return local;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ if (!client->sock)
+ return false;
+
+ return virNetSocketIsLocal(client->sock);
}
@@ -730,14 +708,12 @@ int virNetServerClientGetUNIXIdentity(virNetServerClient *client,
uid_t *uid, gid_t *gid, pid_t *pid,
unsigned long long *timestamp)
{
- int ret = -1;
- virObjectLock(client);
- if (client->sock)
- ret = virNetSocketGetUNIXIdentity(client->sock,
- uid, gid, pid,
- timestamp);
- virObjectUnlock(client);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ if (!client->sock)
+ return -1;
+
+ return virNetSocketGetUNIXIdentity(client->sock, uid, gid, pid, timestamp);
}
@@ -806,56 +782,60 @@ virNetServerClientCreateIdentity(virNetServerClient *client)
virIdentity *virNetServerClientGetIdentity(virNetServerClient *client)
{
- virIdentity *ret = NULL;
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
if (!client->identity)
client->identity = virNetServerClientCreateIdentity(client);
- if (client->identity)
- ret = g_object_ref(client->identity);
- virObjectUnlock(client);
- return ret;
+
+ if (!client->identity)
+ return NULL;
+
+ return g_object_ref(client->identity);
}
void virNetServerClientSetIdentity(virNetServerClient *client,
virIdentity *identity)
{
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
g_clear_object(&client->identity);
client->identity = identity;
if (client->identity)
g_object_ref(client->identity);
- virObjectUnlock(client);
}
int virNetServerClientGetSELinuxContext(virNetServerClient *client,
char **context)
{
- int ret = 0;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
*context = NULL;
- virObjectLock(client);
- if (client->sock)
- ret = virNetSocketGetSELinuxContext(client->sock, context);
- virObjectUnlock(client);
- return ret;
+
+ if (!client->sock)
+ return 0;
+
+ return virNetSocketGetSELinuxContext(client->sock, context);
}
bool virNetServerClientIsSecure(virNetServerClient *client)
{
- bool secure = false;
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
if (client->tls)
- secure = true;
+ return true;
+
#if WITH_SASL
if (client->sasl)
- secure = true;
+ return true;
#endif
+
if (client->sock && virNetSocketIsLocal(client->sock))
- secure = true;
- virObjectUnlock(client);
- return secure;
+ return true;
+
+ return false;
}
@@ -863,53 +843,47 @@ bool virNetServerClientIsSecure(virNetServerClient *client)
void virNetServerClientSetSASLSession(virNetServerClient *client,
virNetSASLSession *sasl)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
/* We don't set the sasl session on the socket here
* because we need to send out the auth confirmation
* in the clear. Only once we complete the next 'tx'
* operation do we switch to SASL mode
*/
- virObjectLock(client);
client->sasl = virObjectRef(sasl);
- virObjectUnlock(client);
}
virNetSASLSession *virNetServerClientGetSASLSession(virNetServerClient *client)
{
- virNetSASLSession *sasl;
- virObjectLock(client);
- sasl = client->sasl;
- virObjectUnlock(client);
- return sasl;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ return client->sasl;
}
bool virNetServerClientHasSASLSession(virNetServerClient *client)
{
- bool has = false;
- virObjectLock(client);
- has = !!client->sasl;
- virObjectUnlock(client);
- return has;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ return !!client->sasl;
}
#endif
void *virNetServerClientGetPrivateData(virNetServerClient *client)
{
- void *data;
- virObjectLock(client);
- data = client->privateData;
- virObjectUnlock(client);
- return data;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ return client->privateData;
}
void virNetServerClientSetCloseHook(virNetServerClient *client,
virNetServerClientCloseFunc cf)
{
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
client->privateDataCloseFunc = cf;
- virObjectUnlock(client);
}
@@ -917,7 +891,8 @@ void virNetServerClientSetDispatcher(virNetServerClient *client,
virNetServerClientDispatchFunc func,
void *opaque)
{
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
/* Only set dispatcher if not already set, to avoid race
* with dispatch code that runs without locks held
*/
@@ -925,7 +900,6 @@ void virNetServerClientSetDispatcher(virNetServerClient *client,
client->dispatchFunc = func;
client->dispatchOpaque = opaque;
}
- virObjectUnlock(client);
}
@@ -1042,9 +1016,9 @@ virNetServerClientCloseLocked(virNetServerClient *client)
void
virNetServerClientClose(virNetServerClient *client)
{
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
virNetServerClientCloseLocked(client);
- virObjectUnlock(client);
}
@@ -1057,16 +1031,16 @@ virNetServerClientIsClosedLocked(virNetServerClient *client)
void virNetServerClientDelayedClose(virNetServerClient *client)
{
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
client->delayedClose = true;
- virObjectUnlock(client);
}
void virNetServerClientImmediateClose(virNetServerClient *client)
{
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
client->wantClose = true;
- virObjectUnlock(client);
}
@@ -1079,49 +1053,46 @@ virNetServerClientWantCloseLocked(virNetServerClient *client)
int virNetServerClientInit(virNetServerClient *client)
{
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+ int ret = -1;
if (!client->tlsCtxt) {
/* Plain socket, so prepare to read first message */
if (virNetServerClientRegisterEvent(client) < 0)
goto error;
- } else {
- int ret;
+ return 0;
+ }
- if (!(client->tls = virNetTLSSessionNew(client->tlsCtxt,
- NULL)))
- goto error;
+ if (!(client->tls = virNetTLSSessionNew(client->tlsCtxt, NULL)))
+ goto error;
- virNetSocketSetTLSSession(client->sock,
- client->tls);
+ virNetSocketSetTLSSession(client->sock, client->tls);
- /* Begin the TLS handshake. */
- virObjectLock(client->tlsCtxt);
+ /* Begin the TLS handshake. */
+ VIR_WITH_OBJECT_LOCK_GUARD(client->tlsCtxt) {
ret = virNetTLSSessionHandshake(client->tls);
- virObjectUnlock(client->tlsCtxt);
- if (ret == 0) {
- /* Unlikely, but ... Next step is to check the certificate. */
- if (virNetServerClientCheckAccess(client) < 0)
- goto error;
-
- /* Handshake & cert check OK, so prepare to read first message */
- if (virNetServerClientRegisterEvent(client) < 0)
- goto error;
- } else if (ret > 0) {
- /* Most likely, need to do more handshake data */
- if (virNetServerClientRegisterEvent(client) < 0)
- goto error;
- } else {
+ }
+
+ if (ret == 0) {
+ /* Unlikely, but ... Next step is to check the certificate. */
+ if (virNetServerClientCheckAccess(client) < 0)
goto error;
- }
+
+ /* Handshake & cert check OK, so prepare to read first message */
+ if (virNetServerClientRegisterEvent(client) < 0)
+ goto error;
+ } else if (ret > 0) {
+ /* Most likely, need to do more handshake data */
+ if (virNetServerClientRegisterEvent(client) < 0)
+ goto error;
+ } else {
+ goto error;
}
- virObjectUnlock(client);
return 0;
error:
client->wantClose = true;
- virObjectUnlock(client);
return -1;
}
@@ -1406,11 +1377,13 @@ virNetServerClientDispatchWrite(virNetServerClient *client)
static void
virNetServerClientDispatchHandshake(virNetServerClient *client)
{
- int ret;
+ int ret = -1;
+
/* Continue the handshake. */
- virObjectLock(client->tlsCtxt);
- ret = virNetTLSSessionHandshake(client->tls);
- virObjectUnlock(client->tlsCtxt);
+ VIR_WITH_OBJECT_LOCK_GUARD(client->tlsCtxt) {
+ ret = virNetTLSSessionHandshake(client->tls);
+ }
+
if (ret == 0) {
/* Finished. Next step is to check the certificate. */
if (virNetServerClientCheckAccess(client) < 0)
@@ -1435,36 +1408,29 @@ virNetServerClientDispatchEvent(virNetSocket *sock, int events,
void *opaque)
virNetServerClient *client = opaque;
virNetMessage *msg = NULL;
- virObjectLock(client);
-
- if (client->sock != sock) {
- virNetSocketRemoveIOCallback(sock);
- virObjectUnlock(client);
- return;
- }
-
- if (events & (VIR_EVENT_HANDLE_WRITABLE |
- VIR_EVENT_HANDLE_READABLE)) {
- if (client->tls &&
- virNetTLSSessionGetHandshakeStatus(client->tls) !=
- VIR_NET_TLS_HANDSHAKE_COMPLETE) {
- virNetServerClientDispatchHandshake(client);
- } else {
- if (events & VIR_EVENT_HANDLE_WRITABLE)
- virNetServerClientDispatchWrite(client);
- if (events & VIR_EVENT_HANDLE_READABLE &&
- client->rx)
- msg = virNetServerClientDispatchRead(client);
+ VIR_WITH_OBJECT_LOCK_GUARD(client) {
+ if (client->sock != sock) {
+ virNetSocketRemoveIOCallback(sock);
+ return;
}
- }
- /* NB, will get HANGUP + READABLE at same time upon
- * disconnect */
- if (events & (VIR_EVENT_HANDLE_ERROR |
- VIR_EVENT_HANDLE_HANGUP))
- client->wantClose = true;
+ if (events & (VIR_EVENT_HANDLE_WRITABLE | VIR_EVENT_HANDLE_READABLE)) {
+ if (client->tls &&
+ virNetTLSSessionGetHandshakeStatus(client->tls) !=
+ VIR_NET_TLS_HANDSHAKE_COMPLETE) {
+ virNetServerClientDispatchHandshake(client);
+ } else {
+ if (events & VIR_EVENT_HANDLE_WRITABLE)
+ virNetServerClientDispatchWrite(client);
+ if ((events & VIR_EVENT_HANDLE_READABLE) && client->rx)
+ msg = virNetServerClientDispatchRead(client);
+ }
+ }
- virObjectUnlock(client);
+ /* NB, will get HANGUP + READABLE at same time upon disconnect */
+ if (events & (VIR_EVENT_HANDLE_ERROR | VIR_EVENT_HANDLE_HANGUP))
+ client->wantClose = true;
+ }
if (msg)
virNetServerClientDispatchMessage(client, msg);
@@ -1499,24 +1465,18 @@ virNetServerClientSendMessageLocked(virNetServerClient *client,
int virNetServerClientSendMessage(virNetServerClient *client,
virNetMessage *msg)
{
- int ret;
-
- virObjectLock(client);
- ret = virNetServerClientSendMessageLocked(client, msg);
- virObjectUnlock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
- return ret;
+ return virNetServerClientSendMessageLocked(client, msg);
}
bool
virNetServerClientIsAuthenticated(virNetServerClient *client)
{
- bool authenticated;
- virObjectLock(client);
- authenticated = virNetServerClientAuthMethodImpliesAuthenticated(client->auth);
- virObjectUnlock(client);
- return authenticated;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
+
+ return virNetServerClientAuthMethodImpliesAuthenticated(client->auth);
}
@@ -1556,57 +1516,44 @@ virNetServerClientInitKeepAlive(virNetServerClient *client,
int interval,
unsigned int count)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
virKeepAlive *ka;
- int ret = -1;
-
- virObjectLock(client);
if (!(ka = virKeepAliveNew(interval, count, client,
virNetServerClientKeepAliveSendCB,
virNetServerClientKeepAliveDeadCB,
virObjectFreeCallback)))
- goto cleanup;
+ return -1;
+
/* keepalive object has a reference to client */
virObjectRef(client);
client->keepalive = ka;
- ret = 0;
- cleanup:
- virObjectUnlock(client);
-
- return ret;
+ return 0;
}
int
virNetServerClientStartKeepAlive(virNetServerClient *client)
{
- int ret = -1;
-
- virObjectLock(client);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
/* The connection might have been closed before we got here and thus the
* keepalive object could have been removed too.
*/
if (!client->keepalive) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("connection not open"));
- goto cleanup;
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not
open"));
+ return -1;
}
- ret = virKeepAliveStart(client->keepalive, 0, 0);
-
- cleanup:
- virObjectUnlock(client);
- return ret;
+ return virKeepAliveStart(client->keepalive, 0, 0);
}
int
virNetServerClientGetTransport(virNetServerClient *client)
{
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
int ret = -1;
- virObjectLock(client);
-
if (client->sock && virNetSocketIsLocal(client->sock))
ret = VIR_CLIENT_TRANS_UNIX;
else
@@ -1615,8 +1562,6 @@ virNetServerClientGetTransport(virNetServerClient *client)
if (client->tls)
ret = VIR_CLIENT_TRANS_TLS;
- virObjectUnlock(client);
-
return ret;
}
@@ -1625,16 +1570,15 @@ virNetServerClientGetInfo(virNetServerClient *client,
bool *readonly, char **sock_addr,
virIdentity **identity)
{
- int ret = -1;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(client);
const char *addr;
- virObjectLock(client);
*readonly = client->readonly;
if (!(addr = virNetServerClientRemoteAddrStringURI(client))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("No network socket associated with client"));
- goto cleanup;
+ return -1;
}
*sock_addr = g_strdup(addr);
@@ -1642,15 +1586,11 @@ virNetServerClientGetInfo(virNetServerClient *client,
if (!client->identity) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("No identity information available for client"));
- goto cleanup;
+ return -1;
}
*identity = g_object_ref(client->identity);
-
- ret = 0;
- cleanup:
- virObjectUnlock(client);
- return ret;
+ return 0;
}
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 09/10] security_manager: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/security/security_manager.c | 524 +++++++++++++-------------------
1 file changed, 215 insertions(+), 309 deletions(-)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index f9c942de08..572e400a48 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -254,13 +254,12 @@ virSecurityManagerPostFork(virSecurityManager *mgr)
int
virSecurityManagerTransactionStart(virSecurityManager *mgr)
{
- int ret = 0;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- virObjectLock(mgr);
- if (mgr->drv->transactionStart)
- ret = mgr->drv->transactionStart(mgr);
- virObjectUnlock(mgr);
- return ret;
+ if (!mgr->drv->transactionStart)
+ return 0;
+
+ return mgr->drv->transactionStart(mgr);
}
@@ -291,13 +290,12 @@ virSecurityManagerTransactionCommit(virSecurityManager *mgr,
pid_t pid,
bool lock)
{
- int ret = 0;
+ VIR_LOCK_GUARD lockguard = virObjectLockGuard(mgr);
- virObjectLock(mgr);
- if (mgr->drv->transactionCommit)
- ret = mgr->drv->transactionCommit(mgr, pid, lock);
- virObjectUnlock(mgr);
- return ret;
+ if (!mgr->drv->transactionCommit)
+ return 0;
+
+ return mgr->drv->transactionCommit(mgr, pid, lock);
}
@@ -310,10 +308,10 @@ virSecurityManagerTransactionCommit(virSecurityManager *mgr,
void
virSecurityManagerTransactionAbort(virSecurityManager *mgr)
{
- virObjectLock(mgr);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
if (mgr->drv->transactionAbort)
mgr->drv->transactionAbort(mgr);
- virObjectUnlock(mgr);
}
@@ -341,32 +339,28 @@ virSecurityManagerGetDriver(virSecurityManager *mgr)
const char *
virSecurityManagerGetDOI(virSecurityManager *mgr)
{
- if (mgr->drv->getDOI) {
- const char *ret;
- virObjectLock(mgr);
- ret = mgr->drv->getDOI(mgr);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->getDOI) {
+ virReportUnsupportedError();
+ return NULL;
}
- virReportUnsupportedError();
- return NULL;
+ return mgr->drv->getDOI(mgr);
}
const char *
virSecurityManagerGetModel(virSecurityManager *mgr)
{
- if (mgr->drv->getModel) {
- const char *ret;
- virObjectLock(mgr);
- ret = mgr->drv->getModel(mgr);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->getModel) {
+ virReportUnsupportedError();
+ return NULL;
}
- virReportUnsupportedError();
- return NULL;
+ return mgr->drv->getModel(mgr);
}
@@ -375,15 +369,12 @@ const char *
virSecurityManagerGetBaseLabel(virSecurityManager *mgr,
int virtType)
{
- if (mgr->drv->getBaseLabel) {
- const char *ret;
- virObjectLock(mgr);
- ret = mgr->drv->getBaseLabel(mgr, virtType);
- virObjectUnlock(mgr);
- return ret;
- }
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return NULL;
+ if (!mgr->drv->getBaseLabel)
+ return NULL;
+
+ return mgr->drv->getBaseLabel(mgr, virtType);
}
@@ -425,16 +416,14 @@ virSecurityManagerRestoreImageLabel(virSecurityManager *mgr,
virStorageSource *src,
virSecurityDomainImageLabelFlags flags)
{
- if (mgr->drv->domainRestoreSecurityImageLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, src, flags);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainRestoreSecurityImageLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, src, flags);
}
@@ -464,15 +453,12 @@ virSecurityManagerMoveImageMetadata(virSecurityManager *mgr,
virStorageSource *src,
virStorageSource *dst)
{
- if (mgr->drv->domainMoveImageMetadata) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainMoveImageMetadata(mgr, pid, src, dst);
- virObjectUnlock(mgr);
- return ret;
- }
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return 0;
+ if (!mgr->drv->domainMoveImageMetadata)
+ return 0;
+
+ return mgr->drv->domainMoveImageMetadata(mgr, pid, src, dst);
}
@@ -480,16 +466,14 @@ int
virSecurityManagerSetDaemonSocketLabel(virSecurityManager *mgr,
virDomainDef *vm)
{
- if (mgr->drv->domainSetSecurityDaemonSocketLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityDaemonSocketLabel(mgr, vm);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityDaemonSocketLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityDaemonSocketLabel(mgr, vm);
}
@@ -497,16 +481,14 @@ int
virSecurityManagerSetSocketLabel(virSecurityManager *mgr,
virDomainDef *vm)
{
- if (mgr->drv->domainSetSecuritySocketLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecuritySocketLabel(mgr, vm);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecuritySocketLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecuritySocketLabel(mgr, vm);
}
@@ -514,16 +496,14 @@ int
virSecurityManagerClearSocketLabel(virSecurityManager *mgr,
virDomainDef *vm)
{
- if (mgr->drv->domainClearSecuritySocketLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainClearSecuritySocketLabel(mgr, vm);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainClearSecuritySocketLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainClearSecuritySocketLabel(mgr, vm);
}
@@ -544,16 +524,14 @@ virSecurityManagerSetImageLabel(virSecurityManager *mgr,
virStorageSource *src,
virSecurityDomainImageLabelFlags flags)
{
- if (mgr->drv->domainSetSecurityImageLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, src, flags);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityImageLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityImageLabel(mgr, vm, src, flags);
}
@@ -563,16 +541,14 @@ virSecurityManagerRestoreHostdevLabel(virSecurityManager *mgr,
virDomainHostdevDef *dev,
const char *vroot)
{
- if (mgr->drv->domainRestoreSecurityHostdevLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev, vroot);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainRestoreSecurityHostdevLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev, vroot);
}
@@ -582,16 +558,14 @@ virSecurityManagerSetHostdevLabel(virSecurityManager *mgr,
virDomainHostdevDef *dev,
const char *vroot)
{
- if (mgr->drv->domainSetSecurityHostdevLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityHostdevLabel(mgr, vm, dev, vroot);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityHostdevLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityHostdevLabel(mgr, vm, dev, vroot);
}
@@ -600,15 +574,12 @@ virSecurityManagerSetSavedStateLabel(virSecurityManager *mgr,
virDomainDef *vm,
const char *savefile)
{
- if (mgr->drv->domainSetSavedStateLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSavedStateLabel(mgr, vm, savefile);
- virObjectUnlock(mgr);
- return ret;
- }
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return 0;
+ if (!mgr->drv->domainSetSavedStateLabel)
+ return 0;
+
+ return mgr->drv->domainSetSavedStateLabel(mgr, vm, savefile);
}
@@ -617,15 +588,12 @@ virSecurityManagerRestoreSavedStateLabel(virSecurityManager *mgr,
virDomainDef *vm,
const char *savefile)
{
- if (mgr->drv->domainRestoreSavedStateLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSavedStateLabel(mgr, vm, savefile);
- virObjectUnlock(mgr);
- return ret;
- }
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return 0;
+ if (!mgr->drv->domainRestoreSavedStateLabel)
+ return 0;
+
+ return mgr->drv->domainRestoreSavedStateLabel(mgr, vm, savefile);
}
@@ -713,16 +681,14 @@ virSecurityManagerReserveLabel(virSecurityManager *mgr,
virDomainDef *vm,
pid_t pid)
{
- if (mgr->drv->domainReserveSecurityLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainReserveSecurityLabel(mgr, vm, pid);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainReserveSecurityLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainReserveSecurityLabel(mgr, vm, pid);
}
@@ -730,16 +696,14 @@ int
virSecurityManagerReleaseLabel(virSecurityManager *mgr,
virDomainDef *vm)
{
- if (mgr->drv->domainReleaseSecurityLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainReleaseSecurityLabel(mgr, vm);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainReleaseSecurityLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainReleaseSecurityLabel(mgr, vm);
}
@@ -857,18 +821,15 @@ virSecurityManagerSetAllLabel(virSecurityManager *mgr,
bool chardevStdioLogd,
bool migrated)
{
- if (mgr->drv->domainSetSecurityAllLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityAllLabel(mgr, vm, incomingPath,
- chardevStdioLogd,
- migrated);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityAllLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityAllLabel(mgr, vm, incomingPath,
+ chardevStdioLogd, migrated);
}
@@ -878,17 +839,15 @@ virSecurityManagerRestoreAllLabel(virSecurityManager *mgr,
bool migrated,
bool chardevStdioLogd)
{
- if (mgr->drv->domainRestoreSecurityAllLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated,
- chardevStdioLogd);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainRestoreSecurityAllLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated,
+ chardevStdioLogd);
}
int
@@ -897,16 +856,14 @@ virSecurityManagerGetProcessLabel(virSecurityManager *mgr,
pid_t pid,
virSecurityLabelPtr sec)
{
- if (mgr->drv->domainGetSecurityProcessLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainGetSecurityProcessLabel(mgr, vm, pid, sec);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainGetSecurityProcessLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainGetSecurityProcessLabel(mgr, vm, pid, sec);
}
@@ -914,16 +871,14 @@ int
virSecurityManagerSetProcessLabel(virSecurityManager *mgr,
virDomainDef *vm)
{
- if (mgr->drv->domainSetSecurityProcessLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityProcessLabel(mgr, vm);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityProcessLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityProcessLabel(mgr, vm);
}
@@ -957,12 +912,10 @@ virSecurityManagerVerify(virSecurityManager *mgr,
if (secdef == NULL || secdef->model == NULL)
return 0;
- if (mgr->drv->domainSecurityVerify) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSecurityVerify(mgr, def);
- virObjectUnlock(mgr);
- return ret;
+ VIR_WITH_OBJECT_LOCK_GUARD(mgr) {
+ if (mgr->drv->domainSecurityVerify) {
+ return mgr->drv->domainSecurityVerify(mgr, def);
+ }
}
virReportUnsupportedError();
@@ -975,16 +928,14 @@ virSecurityManagerSetImageFDLabel(virSecurityManager *mgr,
virDomainDef *vm,
int fd)
{
- if (mgr->drv->domainSetSecurityImageFDLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityImageFDLabel(mgr, vm, fd);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityImageFDLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityImageFDLabel(mgr, vm, fd);
}
@@ -993,16 +944,14 @@ virSecurityManagerSetTapFDLabel(virSecurityManager *mgr,
virDomainDef *vm,
int fd)
{
- if (mgr->drv->domainSetSecurityTapFDLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityTapFDLabel(mgr, vm, fd);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityTapFDLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityTapFDLabel(mgr, vm, fd);
}
@@ -1010,16 +959,14 @@ char *
virSecurityManagerGetMountOptions(virSecurityManager *mgr,
virDomainDef *vm)
{
- if (mgr->drv->domainGetSecurityMountOptions) {
- char *ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainGetSecurityMountOptions(mgr, vm);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainGetSecurityMountOptions) {
+ virReportUnsupportedError();
+ return NULL;
}
- virReportUnsupportedError();
- return NULL;
+ return mgr->drv->domainGetSecurityMountOptions(mgr, vm);
}
@@ -1059,15 +1006,12 @@ virSecurityManagerDomainSetPathLabel(virSecurityManager *mgr,
const char *path,
bool allowSubtree)
{
- if (mgr->drv->domainSetPathLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetPathLabel(mgr, vm, path, allowSubtree);
- virObjectUnlock(mgr);
- return ret;
- }
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return 0;
+ if (!mgr->drv->domainSetPathLabel)
+ return 0;
+
+ return mgr->drv->domainSetPathLabel(mgr, vm, path, allowSubtree);
}
@@ -1088,15 +1032,12 @@ virSecurityManagerDomainSetPathLabelRO(virSecurityManager *mgr,
virDomainDef *vm,
const char *path)
{
- if (mgr->drv->domainSetPathLabelRO) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetPathLabelRO(mgr, vm, path);
- virObjectUnlock(mgr);
- return ret;
- }
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return 0;
+ if (!mgr->drv->domainSetPathLabelRO)
+ return 0;
+
+ return mgr->drv->domainSetPathLabelRO(mgr, vm, path);
}
/**
@@ -1115,15 +1056,12 @@ virSecurityManagerDomainRestorePathLabel(virSecurityManager *mgr,
virDomainDef *vm,
const char *path)
{
- if (mgr->drv->domainRestorePathLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainRestorePathLabel(mgr, vm, path);
- virObjectUnlock(mgr);
- return ret;
- }
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return 0;
+ if (!mgr->drv->domainRestorePathLabel)
+ return 0;
+
+ return mgr->drv->domainRestorePathLabel(mgr, vm, path);
}
@@ -1143,16 +1081,14 @@ virSecurityManagerSetMemoryLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainMemoryDef *mem)
{
- if (mgr->drv->domainSetSecurityMemoryLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityMemoryLabel(mgr, vm, mem);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityMemoryLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityMemoryLabel(mgr, vm, mem);
}
@@ -1171,16 +1107,14 @@ virSecurityManagerRestoreMemoryLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainMemoryDef *mem)
{
- if (mgr->drv->domainRestoreSecurityMemoryLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityMemoryLabel(mgr, vm, mem);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainRestoreSecurityMemoryLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainRestoreSecurityMemoryLabel(mgr, vm, mem);
}
@@ -1189,16 +1123,14 @@ virSecurityManagerSetInputLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainInputDef *input)
{
- if (mgr->drv->domainSetSecurityInputLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityInputLabel(mgr, vm, input);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityInputLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityInputLabel(mgr, vm, input);
}
@@ -1207,16 +1139,14 @@ virSecurityManagerRestoreInputLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainInputDef *input)
{
- if (mgr->drv->domainRestoreSecurityInputLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityInputLabel(mgr, vm, input);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainRestoreSecurityInputLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainRestoreSecurityInputLabel(mgr, vm, input);
}
@@ -1226,17 +1156,15 @@ virSecurityManagerSetChardevLabel(virSecurityManager *mgr,
virDomainChrSourceDef *dev_source,
bool chardevStdioLogd)
{
- if (mgr->drv->domainSetSecurityChardevLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityChardevLabel(mgr, def, dev_source,
- chardevStdioLogd);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainSetSecurityChardevLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainSetSecurityChardevLabel(mgr, def, dev_source,
+ chardevStdioLogd);
}
@@ -1246,17 +1174,15 @@ virSecurityManagerRestoreChardevLabel(virSecurityManager *mgr,
virDomainChrSourceDef *dev_source,
bool chardevStdioLogd)
{
- if (mgr->drv->domainRestoreSecurityChardevLabel) {
- int ret;
- virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityChardevLabel(mgr, def, dev_source,
- chardevStdioLogd);
- virObjectUnlock(mgr);
- return ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
+
+ if (!mgr->drv->domainRestoreSecurityChardevLabel) {
+ virReportUnsupportedError();
+ return -1;
}
- virReportUnsupportedError();
- return -1;
+ return mgr->drv->domainRestoreSecurityChardevLabel(mgr, def, dev_source,
+ chardevStdioLogd);
}
@@ -1264,17 +1190,12 @@ int
virSecurityManagerSetTPMLabels(virSecurityManager *mgr,
virDomainDef *vm)
{
- int ret;
-
- if (mgr->drv->domainSetSecurityTPMLabels) {
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityTPMLabels(mgr, vm);
- virObjectUnlock(mgr);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return ret;
- }
+ if (!mgr->drv->domainSetSecurityTPMLabels)
+ return 0;
- return 0;
+ return mgr->drv->domainSetSecurityTPMLabels(mgr, vm);
}
@@ -1282,17 +1203,12 @@ int
virSecurityManagerRestoreTPMLabels(virSecurityManager *mgr,
virDomainDef *vm)
{
- int ret;
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- if (mgr->drv->domainRestoreSecurityTPMLabels) {
- virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityTPMLabels(mgr, vm);
- virObjectUnlock(mgr);
-
- return ret;
- }
+ if (!mgr->drv->domainRestoreSecurityTPMLabels)
+ return 0;
- return 0;
+ return mgr->drv->domainRestoreSecurityTPMLabels(mgr, vm);
}
@@ -1301,17 +1217,12 @@ virSecurityManagerSetNetdevLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainNetDef *net)
{
- int ret;
-
- if (mgr->drv->domainSetSecurityNetdevLabel) {
- virObjectLock(mgr);
- ret = mgr->drv->domainSetSecurityNetdevLabel(mgr, vm, net);
- virObjectUnlock(mgr);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return ret;
- }
+ if (!mgr->drv->domainSetSecurityNetdevLabel)
+ return 0;
- return 0;
+ return mgr->drv->domainSetSecurityNetdevLabel(mgr, vm, net);
}
@@ -1320,17 +1231,12 @@ virSecurityManagerRestoreNetdevLabel(virSecurityManager *mgr,
virDomainDef *vm,
virDomainNetDef *net)
{
- int ret;
-
- if (mgr->drv->domainRestoreSecurityNetdevLabel) {
- virObjectLock(mgr);
- ret = mgr->drv->domainRestoreSecurityNetdevLabel(mgr, vm, net);
- virObjectUnlock(mgr);
+ VIR_LOCK_GUARD lock = virObjectLockGuard(mgr);
- return ret;
- }
+ if (!mgr->drv->domainRestoreSecurityNetdevLabel)
+ return 0;
- return 0;
+ return mgr->drv->domainRestoreSecurityNetdevLabel(mgr, vm, net);
}
--
2.31.1
9:57 a.m.
New subject: [libvirt PATCH 10/10] vz_driver: Use automatic mutex management
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/vz/vz_driver.c | 50 +++++++++++++++++++---------------------------
1 file changed, 20 insertions(+), 30 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 2107785ab2..82323cb1b2 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -2018,53 +2018,43 @@ vzConnectRegisterCloseCallback(virConnectPtr conn,
virFreeCallback freecb)
{
struct _vzConn *privconn = conn->privateData;
- int ret = -1;
if (virConnectRegisterCloseCallbackEnsureACL(conn) < 0)
return -1;
- virObjectLock(privconn->driver);
+ VIR_WITH_OBJECT_LOCK_GUARD(privconn->driver) {
+ if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != NULL)
{
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("A close callback is already registered"));
+ return -1;
+ }
- if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != NULL) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("A close callback is already registered"));
- goto cleanup;
+ virConnectCloseCallbackDataRegister(privconn->closeCallback, conn, cb,
+ opaque, freecb);
}
- virConnectCloseCallbackDataRegister(privconn->closeCallback, conn, cb,
- opaque, freecb);
- ret = 0;
-
- cleanup:
- virObjectUnlock(privconn->driver);
-
- return ret;
+ return 0;
}
static int
vzConnectUnregisterCloseCallback(virConnectPtr conn, virConnectCloseFunc cb)
{
struct _vzConn *privconn = conn->privateData;
- int ret = -1;
if (virConnectUnregisterCloseCallbackEnsureACL(conn) < 0)
return -1;
- virObjectLock(privconn->driver);
+ VIR_WITH_OBJECT_LOCK_GUARD(privconn->driver) {
+ if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != cb) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("A different callback was requested"));
+ return -1;
+ }
- if (virConnectCloseCallbackDataGetCallback(privconn->closeCallback) != cb) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("A different callback was requested"));
- goto cleanup;
+ virConnectCloseCallbackDataUnregister(privconn->closeCallback, cb);
}
- virConnectCloseCallbackDataUnregister(privconn->closeCallback, cb);
- ret = 0;
-
- cleanup:
- virObjectUnlock(privconn->driver);
-
- return ret;
+ return 0;
}
static int vzDomainSetMemoryFlags(virDomainPtr domain, unsigned long memory,
@@ -3795,9 +3785,9 @@ vzConnectGetAllDomainStats(virConnectPtr conn,
virDomainStatsRecordPtr tmp;
virDomainObj *dom = doms[i];
- virObjectLock(dom);
- tmp = vzDomainGetAllStats(conn, dom);
- virObjectUnlock(dom);
+ VIR_WITH_OBJECT_LOCK_GUARD(dom) {
+ tmp = vzDomainGetAllStats(conn, dom);
+ }
if (!tmp)
goto cleanup;
--
2.31.1
9:57 a.m.
On 4/12/22 16:57, Tim Wiederhake wrote:
Use the recently implemented VIR_LOCK_GUARD and
VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
Tim Wiederhake (10):
virstorageobj: Replace deprecated virHash functions
virStoragePoolObjAddVol: Simplify error path
virStorageVolObjEndAPI: Remove
virStorageVolObjNew: Move locking to usage site
virstorageobject: Use automatic mutex management
virNetServerProcessClients: Remove goto
virnetserver: Use automatic mutex management
virnetserverclient: Use automatic mutex management
security_manager: Use automatic mutex management
vz_driver: Use automatic mutex management
src/conf/virstorageobj.c | 185 +++++------
src/rpc/virnetserver.c | 295 +++++++-----------
src/rpc/virnetserverclient.c | 432 ++++++++++++--------------
src/security/security_manager.c | 524 +++++++++++++-------------------
src/vz/vz_driver.c | 50 ++-
5 files changed, 604 insertions(+), 882 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1000
days inactive
1002
days old
11 comments
2 participants
participants (2)
-
Michal Prívozník -
Tim Wiederhake