[libvirt] [PATCH v5 08/15] secret: Use virObjectLookupHash
John Ferlan
jferlan at redhat.com
Wed Aug 23 21:22:04 UTC 2017
Use the virObjectLookupHash in _virSecretObjList. Convert the code
to use the LookupHash object and APIs rather than local code and
usage of the virHash* calls.
Since the _virSecretObjList only now contains the @parent
object, the virClassNew must be removed from OnceInit because
instantiation would fail since the object size would be the
same as the parent object size.
Usage of HashLookup{Find|Search} API's returns a locked/reffed
object so need to remove virObjectLock after FindBy*Locked calls.
The only function requiring a taking a lock is the Add function
since it needs to be synchronized in such a way to avoid
multiple threads attempting to add the same secret via UUID
or UsageID at the same time.
NB: We cannot make usageID a LookupHash key because it's possible
that the usageType is NONE and thus usageID is NULL, thus leaving
the only "unique" element the UUID.
The NumOfSecretsCallback and GetUUIDsCallback can use the same
callback function with the only difference being the filling in
of the @uuids array from the passed @data structure if it exists.
Signed-off-by: John Ferlan <jferlan at redhat.com>
---
src/conf/virsecretobj.c | 251 ++++++++++++------------------------------------
1 file changed, 64 insertions(+), 187 deletions(-)
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 4dca152..21a5ab7 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -47,16 +47,10 @@ struct _virSecretObj {
};
static virClassPtr virSecretObjClass;
-static virClassPtr virSecretObjListClass;
static void virSecretObjDispose(void *obj);
-static void virSecretObjListDispose(void *obj);
struct _virSecretObjList {
- virObjectLockable parent;
-
- /* uuid string -> virSecretObj mapping
- * for O(1), lockless lookup-by-uuid */
- virHashTable *objs;
+ virObjectLookupHash parent;
};
struct virSecretSearchData {
@@ -74,12 +68,6 @@ virSecretObjOnceInit(void)
virSecretObjDispose)))
return -1;
- if (!(virSecretObjListClass = virClassNew(virClassForObjectLockable(),
- "virSecretObjList",
- sizeof(virSecretObjList),
- virSecretObjListDispose)))
- return -1;
-
return 0;
}
@@ -118,20 +106,8 @@ virSecretObjEndAPI(virSecretObjPtr *obj)
virSecretObjListPtr
virSecretObjListNew(void)
{
- virSecretObjListPtr secrets;
-
- if (virSecretObjInitialize() < 0)
- return NULL;
-
- if (!(secrets = virObjectLockableNew(virSecretObjListClass)))
- return NULL;
-
- if (!(secrets->objs = virHashCreate(50, virObjectFreeHashData))) {
- virObjectUnref(secrets);
- return NULL;
- }
-
- return secrets;
+ return virObjectLookupHashNew(virClassForObjectLookupHash(), 50,
+ VIR_OBJECT_LOOKUP_HASH_UUID);
}
@@ -151,15 +127,6 @@ virSecretObjDispose(void *opaque)
}
-static void
-virSecretObjListDispose(void *obj)
-{
- virSecretObjListPtr secrets = obj;
-
- virHashFree(secrets->objs);
-}
-
-
/**
* virSecretObjFindByUUIDLocked:
* @secrets: list of secret objects
@@ -173,7 +140,7 @@ static virSecretObjPtr
virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
const char *uuidstr)
{
- return virObjectRef(virHashLookup(secrets->objs, uuidstr));
+ return virObjectLookupHashFindLocked(secrets, uuidstr);
}
@@ -191,14 +158,7 @@ virSecretObjPtr
virSecretObjListFindByUUID(virSecretObjListPtr secrets,
const char *uuidstr)
{
- virSecretObjPtr obj;
-
- virObjectLock(secrets);
- obj = virSecretObjListFindByUUIDLocked(secrets, uuidstr);
- virObjectUnlock(secrets);
- if (obj)
- virObjectLock(obj);
- return obj;
+ return virObjectLookupHashFind(secrets, uuidstr);
}
@@ -243,14 +203,11 @@ virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
int usageType,
const char *usageID)
{
- virSecretObjPtr obj = NULL;
struct virSecretSearchData data = { .usageType = usageType,
.usageID = usageID };
- obj = virHashSearch(secrets->objs, virSecretObjSearchName, &data, NULL);
- if (obj)
- virObjectRef(obj);
- return obj;
+ return virObjectLookupHashSearchUUIDLocked(secrets, virSecretObjSearchName,
+ &data);
}
@@ -263,6 +220,12 @@ virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
* This function locks @secrets and finds the secret object which
* corresponds to @usageID of @usageType.
*
+ * NB: The usageID cannot be used as a hash table key because
+ * virSecretDefParseUsage will not fill in the def->usage_id
+ * if the def->usage_type is VIR_SECRET_USAGE_TYPE_NONE, thus
+ * we cannot use def->usage_id as a key since both keys must
+ * be present in every object in order to be valid.
+ *
* Returns: locked and ref'd secret object.
*/
virSecretObjPtr
@@ -270,21 +233,17 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
int usageType,
const char *usageID)
{
- virSecretObjPtr obj;
+ struct virSecretSearchData data = { .usageType = usageType,
+ .usageID = usageID };
- virObjectLock(secrets);
- obj = virSecretObjListFindByUsageLocked(secrets, usageType, usageID);
- virObjectUnlock(secrets);
- if (obj)
- virObjectLock(obj);
- return obj;
+ return virObjectLookupHashSearchUUID(secrets, virSecretObjSearchName, &data);
}
/*
* virSecretObjListRemove:
* @secrets: list of secret objects
- * @secret: a secret object
+ * @obj: a locked secret object
*
* Remove the object from the hash table. The caller must hold the lock
* on the driver owning @secrets and must have also locked @secret to
@@ -295,22 +254,12 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
virSecretObjPtr obj)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
- virSecretDefPtr def;
if (!obj)
return;
- def = obj->def;
- virUUIDFormat(def->uuid, uuidstr);
- virObjectRef(obj);
- virObjectUnlock(obj);
-
- virObjectLock(secrets);
- virObjectLock(obj);
- virHashRemoveEntry(secrets->objs, uuidstr);
- virObjectUnlock(obj);
- virObjectUnref(obj);
- virObjectUnlock(secrets);
+ virUUIDFormat(obj->def->uuid, uuidstr);
+ virObjectLookupHashRemove(secrets, obj, uuidstr, NULL);
}
@@ -336,7 +285,7 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
virSecretObjPtr ret = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- virObjectLock(secrets);
+ virObjectRWLockWrite(secrets);
if (oldDef)
*oldDef = NULL;
@@ -345,7 +294,6 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
/* Is there a secret already matching this UUID */
if ((obj = virSecretObjListFindByUUIDLocked(secrets, uuidstr))) {
- virObjectLock(obj);
objdef = obj->def;
if (STRNEQ_NULLABLE(objdef->usage_id, newdef->usage_id)) {
@@ -373,7 +321,6 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
if ((obj = virSecretObjListFindByUsageLocked(secrets,
newdef->usage_type,
newdef->usage_id))) {
- virObjectLock(obj);
objdef = obj->def;
virUUIDFormat(objdef->uuid, uuidstr);
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -393,11 +340,10 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
!(obj->base64File = virFileBuildPath(configDir, uuidstr, ".base64")))
goto cleanup;
- if (virHashAddEntry(secrets->objs, uuidstr, obj) < 0)
+ if (virObjectLookupHashAdd(secrets, obj, uuidstr, NULL) < 0)
goto cleanup;
obj->def = newdef;
- virObjectRef(obj);
}
ret = obj;
@@ -405,72 +351,35 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
cleanup:
virSecretObjEndAPI(&obj);
- virObjectUnlock(secrets);
+ virObjectRWUnlock(secrets);
return ret;
}
-struct virSecretCountData {
- virConnectPtr conn;
- virSecretObjListACLFilter filter;
- int count;
-};
-
static int
-virSecretObjListNumOfSecretsCallback(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *opaque)
+virSecretObjListGetHelper(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;
- bool error;
-};
-
-
-static int
-virSecretObjListGetUUIDsCallback(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *opaque)
-{
- struct virSecretListData *data = opaque;
virSecretObjPtr obj = payload;
+ virObjectLookupHashForEachDataPtr data = opaque;
+ virSecretObjListACLFilter filter = data->filter;
+ char **uuids = (char **)data->elems;
virSecretDefPtr def;
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 (filter && !filter(data->conn, def))
goto cleanup;
- if (data->uuids) {
+ if (uuids) {
char *uuidstr;
if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0) {
@@ -479,7 +388,9 @@ virSecretObjListGetUUIDsCallback(void *payload,
}
virUUIDFormat(def->uuid, uuidstr);
- data->uuids[data->nuuids++] = uuidstr;
+ uuids[data->nElems++] = uuidstr;
+ } else {
+ data->nElems++;
}
cleanup:
@@ -493,14 +404,12 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
virSecretObjListACLFilter filter,
virConnectPtr conn)
{
- struct virSecretCountData data = {
- .conn = conn, .filter = filter, .count = 0 };
-
- virObjectLock(secrets);
- virHashForEach(secrets->objs, virSecretObjListNumOfSecretsCallback, &data);
- virObjectUnlock(secrets);
+ virObjectLookupHashForEachData data = {
+ .conn = conn, .filter = filter, .error = false, .nElems = 0,
+ .elems = NULL, .maxElems = -2 };
- return data.count;
+ return virObjectLookupHashForEachUUID(secrets, virSecretObjListGetHelper,
+ &data);
}
@@ -532,22 +441,15 @@ 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;
+ virObjectLookupHashForEachDataPtr data = opaque;
+ virSecretPtr *secrets = (virSecretPtr *)data->elems;
+ virSecretObjListACLFilter filter = data->filter;
virSecretDefPtr def;
virSecretPtr secret = NULL;
@@ -557,25 +459,24 @@ virSecretObjListExportCallback(void *payload,
virObjectLock(obj);
def = obj->def;
- if (data->filter && !data->filter(data->conn, def))
+ if (filter && !filter(data->conn, def))
goto cleanup;
if (!virSecretObjMatchFlags(obj, data->flags))
goto cleanup;
- if (!data->secrets) {
- data->nsecrets++;
+ if (!secrets) {
+ data->nElems++;
goto cleanup;
}
if (!(secret = virGetSecret(data->conn, def->uuid,
- def->usage_type,
- def->usage_id))) {
+ def->usage_type, def->usage_id))) {
data->error = true;
goto cleanup;
}
- data->secrets[data->nsecrets++] = secret;
+ secrets[data->nElems++] = secret;
cleanup:
virObjectUnlock(obj);
@@ -590,35 +491,22 @@ virSecretObjListExport(virConnectPtr conn,
virSecretObjListACLFilter filter,
unsigned int flags)
{
- struct virSecretObjListData data = {
- .conn = conn, .secrets = NULL,
- .filter = filter, .flags = flags,
- .nsecrets = 0, .error = false };
-
- virObjectLock(secretobjs);
- if (secrets &&
- VIR_ALLOC_N(data.secrets, virHashSize(secretobjs->objs) + 1) < 0) {
- virObjectUnlock(secretobjs);
- return -1;
- }
+ int ret;
- virHashForEach(secretobjs->objs, virSecretObjListExportCallback, &data);
- virObjectUnlock(secretobjs);
+ virObjectLookupHashForEachData data = {
+ .conn = conn, .filter = filter, .error = false, .nElems = 0,
+ .elems = NULL, .maxElems = 0, .flags = flags };
- if (data.error)
- goto error;
-
- if (data.secrets) {
- /* trim the array to the final size */
- ignore_value(VIR_REALLOC_N(data.secrets, data.nsecrets + 1));
- *secrets = data.secrets;
- }
+ if (secrets)
+ data.maxElems = -1;
- return data.nsecrets;
+ ret = virObjectLookupHashForEachUUID(secretobjs,
+ virSecretObjListExportCallback,
+ &data);
+ if (secrets)
+ *secrets = (virSecretPtr *)data.elems;
- error:
- virObjectListFree(data.secrets);
- return -1;
+ return ret;
}
@@ -629,23 +517,12 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
virSecretObjListACLFilter filter,
virConnectPtr conn)
{
- struct virSecretListData data = {
- .conn = conn, .filter = filter, .uuids = uuids, .nuuids = 0,
- .maxuuids = maxuuids, .error = false };
+ virObjectLookupHashForEachData data = {
+ .conn = conn, .filter = filter, .error = false, .nElems = 0,
+ .elems = (void **)uuids, .maxElems = maxuuids };
- virObjectLock(secrets);
- virHashForEach(secrets->objs, virSecretObjListGetUUIDsCallback, &data);
- virObjectUnlock(secrets);
-
- if (data.error)
- goto error;
-
- return data.nuuids;
-
- error:
- while (--data.nuuids)
- VIR_FREE(data.uuids[data.nuuids]);
- return -1;
+ return virObjectLookupHashForEachUUID(secrets, virSecretObjListGetHelper,
+ &data);
}
--
2.9.5
More information about the libvir-list
mailing list