1:57 p.m.
Patches should speak for themselves.
John Ferlan (2):
secrets: Convert to use ObjectRWLockable
secrets: Introduce virSecretObjListForEachCb
src/conf/virsecretobj.c | 203 +++++++++++++++++-------------------------------
1 file changed, 72 insertions(+), 131 deletions(-)
--
2.13.6
1:57 p.m.
New subject: [libvirt] [PATCH 1/2] secrets: Convert to use ObjectRWLockable
Let's use the ObjectRWLockable for the various list lock mgmt.
Only time need Write lock will be for Add/Remove logic.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virsecretobj.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 4dca152d48..47e0b28968 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -52,7 +52,7 @@ static void virSecretObjDispose(void *obj);
static void virSecretObjListDispose(void *obj);
struct _virSecretObjList {
- virObjectLockable parent;
+ virObjectRWLockable parent;
/* uuid string -> virSecretObj mapping
* for O(1), lockless lookup-by-uuid */
@@ -74,7 +74,7 @@ virSecretObjOnceInit(void)
virSecretObjDispose)))
return -1;
- if (!(virSecretObjListClass = virClassNew(virClassForObjectLockable(),
+ if (!(virSecretObjListClass = virClassNew(virClassForObjectRWLockable(),
"virSecretObjList",
sizeof(virSecretObjList),
virSecretObjListDispose)))
@@ -123,7 +123,7 @@ virSecretObjListNew(void)
if (virSecretObjInitialize() < 0)
return NULL;
- if (!(secrets = virObjectLockableNew(virSecretObjListClass)))
+ if (!(secrets = virObjectRWLockableNew(virSecretObjListClass)))
return NULL;
if (!(secrets->objs = virHashCreate(50, virObjectFreeHashData))) {
@@ -193,9 +193,9 @@ virSecretObjListFindByUUID(virSecretObjListPtr secrets,
{
virSecretObjPtr obj;
- virObjectLock(secrets);
+ virObjectRWLockRead(secrets);
obj = virSecretObjListFindByUUIDLocked(secrets, uuidstr);
- virObjectUnlock(secrets);
+ virObjectRWUnlock(secrets);
if (obj)
virObjectLock(obj);
return obj;
@@ -272,9 +272,9 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
{
virSecretObjPtr obj;
- virObjectLock(secrets);
+ virObjectRWLockRead(secrets);
obj = virSecretObjListFindByUsageLocked(secrets, usageType, usageID);
- virObjectUnlock(secrets);
+ virObjectRWUnlock(secrets);
if (obj)
virObjectLock(obj);
return obj;
@@ -305,12 +305,12 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
virObjectRef(obj);
virObjectUnlock(obj);
- virObjectLock(secrets);
+ virObjectRWLockWrite(secrets);
virObjectLock(obj);
virHashRemoveEntry(secrets->objs, uuidstr);
virObjectUnlock(obj);
virObjectUnref(obj);
- virObjectUnlock(secrets);
+ virObjectRWUnlock(secrets);
}
@@ -336,7 +336,7 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
virSecretObjPtr ret = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- virObjectLock(secrets);
+ virObjectRWLockWrite(secrets);
if (oldDef)
*oldDef = NULL;
@@ -405,7 +405,7 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
cleanup:
virSecretObjEndAPI(&obj);
- virObjectUnlock(secrets);
+ virObjectRWUnlock(secrets);
return ret;
}
@@ -496,9 +496,9 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
struct virSecretCountData data = {
.conn = conn, .filter = filter, .count = 0 };
- virObjectLock(secrets);
+ virObjectRWLockRead(secrets);
virHashForEach(secrets->objs, virSecretObjListNumOfSecretsCallback, &data);
- virObjectUnlock(secrets);
+ virObjectRWUnlock(secrets);
return data.count;
}
@@ -595,15 +595,15 @@ virSecretObjListExport(virConnectPtr conn,
.filter = filter, .flags = flags,
.nsecrets = 0, .error = false };
- virObjectLock(secretobjs);
+ virObjectRWLockRead(secretobjs);
if (secrets &&
VIR_ALLOC_N(data.secrets, virHashSize(secretobjs->objs) + 1) < 0) {
- virObjectUnlock(secretobjs);
+ virObjectRWUnlock(secretobjs);
return -1;
}
virHashForEach(secretobjs->objs, virSecretObjListExportCallback, &data);
- virObjectUnlock(secretobjs);
+ virObjectRWUnlock(secretobjs);
if (data.error)
goto error;
@@ -633,9 +633,9 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
.conn = conn, .filter = filter, .uuids = uuids, .nuuids = 0,
.maxuuids = maxuuids, .error = false };
- virObjectLock(secrets);
+ virObjectRWLockRead(secrets);
virHashForEach(secrets->objs, virSecretObjListGetUUIDsCallback, &data);
- virObjectUnlock(secrets);
+ virObjectRWUnlock(secrets);
if (data.error)
goto error;
--
2.13.6
8:48 a.m.
New subject: [libvirt] [PATCH 1/2] secrets: Convert to use ObjectRWLockable
On Mon, Oct 09, 2017 at 02:57:38PM -0400, John Ferlan wrote:
Let's use the ObjectRWLockable for the various list lock mgmt.
Only time need Write lock will be for Add/Remove logic.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
1:57 p.m.
New subject: [libvirt] [PATCH 2/2] secrets: Introduce virSecretObjListForEachCb
Rather than separate functions for NumOfSecrets, GetUUIDs, and
ListExport - let's converge the code into one handler.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virsecretobj.c | 167 ++++++++++++++++--------------------------------
1 file changed, 54 insertions(+), 113 deletions(-)
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 47e0b28968..6046547875 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -410,64 +410,41 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
}
-struct virSecretCountData {
+typedef bool
+(*virSecretObjListMatchFilter)(virSecretObjPtr obj, unsigned int flags);
+struct _virSecretForEachData {
virConnectPtr conn;
virSecretObjListACLFilter filter;
- int count;
-};
-
-static int
-virSecretObjListNumOfSecretsCallback(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *opaque)
-{
- struct virSecretCountData *data = opaque;
- virSecretObjPtr obj = payload;
- virSecretDefPtr def;
-
- virObjectLock(obj);
- def = obj->def;
-
- if (data->filter && !data->filter(data->conn, def))
- goto cleanup;
-
- data->count++;
-
- cleanup:
- virObjectUnlock(obj);
- return 0;
-}
-
-
-struct virSecretListData {
- virConnectPtr conn;
- virSecretObjListACLFilter filter;
- int nuuids;
- char **uuids;
- int maxuuids;
+ virSecretObjListMatchFilter match;
+ unsigned int flags;
bool error;
+ int nElems;
+ int maxElems;
+ char **uuids;
+ virSecretPtr *secrets;
};
-
static int
-virSecretObjListGetUUIDsCallback(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *opaque)
+virSecretObjListForEachCb(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
{
- struct virSecretListData *data = opaque;
+ struct _virSecretForEachData *data = opaque;
virSecretObjPtr obj = payload;
- virSecretDefPtr def;
+ virSecretPtr secret = NULL;
if (data->error)
return 0;
- if (data->maxuuids >= 0 && data->nuuids == data->maxuuids)
+ if (data->maxElems >= 0 && data->nElems == data->maxElems)
return 0;
virObjectLock(obj);
- def = obj->def;
- if (data->filter && !data->filter(data->conn, def))
+ if (data->filter && !data->filter(data->conn, obj->def))
+ goto cleanup;
+
+ if (data->match && !data->match(obj, data->flags))
goto cleanup;
if (data->uuids) {
@@ -478,10 +455,20 @@ virSecretObjListGetUUIDsCallback(void *payload,
goto cleanup;
}
- virUUIDFormat(def->uuid, uuidstr);
- data->uuids[data->nuuids++] = uuidstr;
+ virUUIDFormat(obj->def->uuid, uuidstr);
+ data->uuids[data->nElems] = uuidstr;
+ } else if (data->secrets) {
+ if (!(secret = virGetSecret(data->conn, obj->def->uuid,
+ obj->def->usage_type,
+ obj->def->usage_id))) {
+ data->error = true;
+ goto cleanup;
+ }
+ data->secrets[data->nElems] = secret;
}
+ data->nElems++;
+
cleanup:
virObjectUnlock(obj);
return 0;
@@ -493,14 +480,16 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
virSecretObjListACLFilter filter,
virConnectPtr conn)
{
- struct virSecretCountData data = {
- .conn = conn, .filter = filter, .count = 0 };
+ struct _virSecretForEachData data = {
+ .conn = conn, .filter = filter, .match = NULL,
+ .flags = 0, .error = false, .nElems = 0, .maxElems = -1,
+ .uuids = NULL, .secrets = NULL };
virObjectRWLockRead(secrets);
- virHashForEach(secrets->objs, virSecretObjListNumOfSecretsCallback, &data);
+ virHashForEach(secrets->objs, virSecretObjListForEachCb, &data);
virObjectRWUnlock(secrets);
- return data.count;
+ return data.nElems;
}
@@ -532,57 +521,6 @@ virSecretObjMatchFlags(virSecretObjPtr obj,
#undef MATCH
-struct virSecretObjListData {
- virConnectPtr conn;
- virSecretPtr *secrets;
- virSecretObjListACLFilter filter;
- unsigned int flags;
- int nsecrets;
- bool error;
-};
-
-static int
-virSecretObjListExportCallback(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *opaque)
-{
- struct virSecretObjListData *data = opaque;
- virSecretObjPtr obj = payload;
- virSecretDefPtr def;
- virSecretPtr secret = NULL;
-
- if (data->error)
- return 0;
-
- virObjectLock(obj);
- def = obj->def;
-
- if (data->filter && !data->filter(data->conn, def))
- goto cleanup;
-
- if (!virSecretObjMatchFlags(obj, data->flags))
- goto cleanup;
-
- if (!data->secrets) {
- data->nsecrets++;
- goto cleanup;
- }
-
- if (!(secret = virGetSecret(data->conn, def->uuid,
- def->usage_type,
- def->usage_id))) {
- data->error = true;
- goto cleanup;
- }
-
- data->secrets[data->nsecrets++] = secret;
-
- cleanup:
- virObjectUnlock(obj);
- return 0;
-}
-
-
int
virSecretObjListExport(virConnectPtr conn,
virSecretObjListPtr secretobjs,
@@ -590,10 +528,10 @@ virSecretObjListExport(virConnectPtr conn,
virSecretObjListACLFilter filter,
unsigned int flags)
{
- struct virSecretObjListData data = {
- .conn = conn, .secrets = NULL,
- .filter = filter, .flags = flags,
- .nsecrets = 0, .error = false };
+ struct _virSecretForEachData data = {
+ .conn = conn, .filter = filter, .match = virSecretObjMatchFlags,
+ .flags = flags, .error = false, .nElems = 0, .maxElems = -1,
+ .uuids = NULL, .secrets = NULL };
virObjectRWLockRead(secretobjs);
if (secrets &&
@@ -601,8 +539,10 @@ virSecretObjListExport(virConnectPtr conn,
virObjectRWUnlock(secretobjs);
return -1;
}
+ if (data.secrets)
+ data.maxElems = virHashSize(secretobjs->objs) + 1;
- virHashForEach(secretobjs->objs, virSecretObjListExportCallback, &data);
+ virHashForEach(secretobjs->objs, virSecretObjListForEachCb, &data);
virObjectRWUnlock(secretobjs);
if (data.error)
@@ -610,11 +550,11 @@ virSecretObjListExport(virConnectPtr conn,
if (data.secrets) {
/* trim the array to the final size */
- ignore_value(VIR_REALLOC_N(data.secrets, data.nsecrets + 1));
+ ignore_value(VIR_REALLOC_N(data.secrets, data.nElems + 1));
*secrets = data.secrets;
}
- return data.nsecrets;
+ return data.nElems;
error:
virObjectListFree(data.secrets);
@@ -629,22 +569,23 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
virSecretObjListACLFilter filter,
virConnectPtr conn)
{
- struct virSecretListData data = {
- .conn = conn, .filter = filter, .uuids = uuids, .nuuids = 0,
- .maxuuids = maxuuids, .error = false };
+ struct _virSecretForEachData data = {
+ .conn = conn, .filter = filter, .match = NULL,
+ .flags = 0, .error = false, .nElems = 0, .maxElems = maxuuids,
+ .uuids = uuids, .secrets = NULL };
virObjectRWLockRead(secrets);
- virHashForEach(secrets->objs, virSecretObjListGetUUIDsCallback, &data);
+ virHashForEach(secrets->objs, virSecretObjListForEachCb, &data);
virObjectRWUnlock(secrets);
if (data.error)
goto error;
- return data.nuuids;
+ return data.nElems;
error:
- while (--data.nuuids)
- VIR_FREE(data.uuids[data.nuuids]);
+ while (--data.nElems)
+ VIR_FREE(data.uuids[data.nElems]);
return -1;
}
--
2.13.6
8:50 a.m.
New subject: [libvirt] [PATCH 2/2] secrets: Introduce virSecretObjListForEachCb
On Mon, Oct 09, 2017 at 02:57:39PM -0400, John Ferlan wrote:
Rather than separate functions for NumOfSecrets, GetUUIDs, and
ListExport - let's converge the code into one handler.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
NACK to the idea, linking [1] to avoid repeating the comments.
[1] https://www.redhat.com/archives/libvir-list/2017-October/msg00464.html
Erik
2598
days inactive
2601
days old
4 comments
2 participants
participants (2)
-
Erik Skultety -
John Ferlan