From nobody Mon Feb 9 05:52:17 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1503523402211472.2039692777996; Wed, 23 Aug 2017 14:23:22 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4FF15800A8; Wed, 23 Aug 2017 21:23:20 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2CF926047C; Wed, 23 Aug 2017 21:23:20 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id DE2AF1806109; Wed, 23 Aug 2017 21:23:19 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v7NLNIYS024116 for ; Wed, 23 Aug 2017 17:23:18 -0400 Received: by smtp.corp.redhat.com (Postfix) id 9B03A66D39; Wed, 23 Aug 2017 21:23:18 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-123-165.rdu2.redhat.com [10.10.123.165]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5783066D36 for ; Wed, 23 Aug 2017 21:23:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4FF15800A8 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com From: John Ferlan To: libvir-list@redhat.com Date: Wed, 23 Aug 2017 17:22:04 -0400 Message-Id: <20170823212211.4693-9-jferlan@redhat.com> In-Reply-To: <20170823212211.4693-1-jferlan@redhat.com> References: <20170823212211.4693-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v5 08/15] secret: Use virObjectLookupHash X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Wed, 23 Aug 2017 21:23:20 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" 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 --- 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 { }; =20 static virClassPtr virSecretObjClass; -static virClassPtr virSecretObjListClass; static void virSecretObjDispose(void *obj); -static void virSecretObjListDispose(void *obj); =20 struct _virSecretObjList { - virObjectLockable parent; - - /* uuid string -> virSecretObj mapping - * for O(1), lockless lookup-by-uuid */ - virHashTable *objs; + virObjectLookupHash parent; }; =20 struct virSecretSearchData { @@ -74,12 +68,6 @@ virSecretObjOnceInit(void) virSecretObjDispose))) return -1; =20 - if (!(virSecretObjListClass =3D virClassNew(virClassForObjectLockable(= ), - "virSecretObjList", - sizeof(virSecretObjList), - virSecretObjListDispose))) - return -1; - return 0; } =20 @@ -118,20 +106,8 @@ virSecretObjEndAPI(virSecretObjPtr *obj) virSecretObjListPtr virSecretObjListNew(void) { - virSecretObjListPtr secrets; - - if (virSecretObjInitialize() < 0) - return NULL; - - if (!(secrets =3D virObjectLockableNew(virSecretObjListClass))) - return NULL; - - if (!(secrets->objs =3D virHashCreate(50, virObjectFreeHashData))) { - virObjectUnref(secrets); - return NULL; - } - - return secrets; + return virObjectLookupHashNew(virClassForObjectLookupHash(), 50, + VIR_OBJECT_LOOKUP_HASH_UUID); } =20 =20 @@ -151,15 +127,6 @@ virSecretObjDispose(void *opaque) } =20 =20 -static void -virSecretObjListDispose(void *obj) -{ - virSecretObjListPtr secrets =3D 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); } =20 =20 @@ -191,14 +158,7 @@ virSecretObjPtr virSecretObjListFindByUUID(virSecretObjListPtr secrets, const char *uuidstr) { - virSecretObjPtr obj; - - virObjectLock(secrets); - obj =3D virSecretObjListFindByUUIDLocked(secrets, uuidstr); - virObjectUnlock(secrets); - if (obj) - virObjectLock(obj); - return obj; + return virObjectLookupHashFind(secrets, uuidstr); } =20 =20 @@ -243,14 +203,11 @@ virSecretObjListFindByUsageLocked(virSecretObjListPtr= secrets, int usageType, const char *usageID) { - virSecretObjPtr obj =3D NULL; struct virSecretSearchData data =3D { .usageType =3D usageType, .usageID =3D usageID }; =20 - obj =3D virHashSearch(secrets->objs, virSecretObjSearchName, &data, NU= LL); - if (obj) - virObjectRef(obj); - return obj; + return virObjectLookupHashSearchUUIDLocked(secrets, virSecretObjSearch= Name, + &data); } =20 =20 @@ -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 secre= ts, int usageType, const char *usageID) { - virSecretObjPtr obj; + struct virSecretSearchData data =3D { .usageType =3D usageType, + .usageID =3D usageID }; =20 - virObjectLock(secrets); - obj =3D virSecretObjListFindByUsageLocked(secrets, usageType, usageID); - virObjectUnlock(secrets); - if (obj) - virObjectLock(obj); - return obj; + return virObjectLookupHashSearchUUID(secrets, virSecretObjSearchName, = &data); } =20 =20 /* * 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; =20 if (!obj) return; - def =3D obj->def; =20 - 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); } =20 =20 @@ -336,7 +285,7 @@ virSecretObjListAdd(virSecretObjListPtr secrets, virSecretObjPtr ret =3D NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; =20 - virObjectLock(secrets); + virObjectRWLockWrite(secrets); =20 if (oldDef) *oldDef =3D NULL; @@ -345,7 +294,6 @@ virSecretObjListAdd(virSecretObjListPtr secrets, =20 /* Is there a secret already matching this UUID */ if ((obj =3D virSecretObjListFindByUUIDLocked(secrets, uuidstr))) { - virObjectLock(obj); objdef =3D obj->def; =20 if (STRNEQ_NULLABLE(objdef->usage_id, newdef->usage_id)) { @@ -373,7 +321,6 @@ virSecretObjListAdd(virSecretObjListPtr secrets, if ((obj =3D virSecretObjListFindByUsageLocked(secrets, newdef->usage_type, newdef->usage_id))) { - virObjectLock(obj); objdef =3D obj->def; virUUIDFormat(objdef->uuid, uuidstr); virReportError(VIR_ERR_INTERNAL_ERROR, @@ -393,11 +340,10 @@ virSecretObjListAdd(virSecretObjListPtr secrets, !(obj->base64File =3D virFileBuildPath(configDir, uuidstr, ".b= ase64"))) goto cleanup; =20 - if (virHashAddEntry(secrets->objs, uuidstr, obj) < 0) + if (virObjectLookupHashAdd(secrets, obj, uuidstr, NULL) < 0) goto cleanup; =20 obj->def =3D newdef; - virObjectRef(obj); } =20 ret =3D obj; @@ -405,72 +351,35 @@ virSecretObjListAdd(virSecretObjListPtr secrets, =20 cleanup: virSecretObjEndAPI(&obj); - virObjectUnlock(secrets); + virObjectRWUnlock(secrets); return ret; } =20 =20 -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 =3D opaque; - virSecretObjPtr obj =3D payload; - virSecretDefPtr def; - - virObjectLock(obj); - def =3D 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 =3D opaque; virSecretObjPtr obj =3D payload; + virObjectLookupHashForEachDataPtr data =3D opaque; + virSecretObjListACLFilter filter =3D data->filter; + char **uuids =3D (char **)data->elems; virSecretDefPtr def; =20 if (data->error) return 0; =20 - if (data->maxuuids >=3D 0 && data->nuuids =3D=3D data->maxuuids) + if (data->maxElems >=3D 0 && data->nElems =3D=3D data->maxElems) return 0; =20 virObjectLock(obj); def =3D obj->def; =20 - if (data->filter && !data->filter(data->conn, def)) + if (filter && !filter(data->conn, def)) goto cleanup; =20 - if (data->uuids) { + if (uuids) { char *uuidstr; =20 if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0) { @@ -479,7 +388,9 @@ virSecretObjListGetUUIDsCallback(void *payload, } =20 virUUIDFormat(def->uuid, uuidstr); - data->uuids[data->nuuids++] =3D uuidstr; + uuids[data->nElems++] =3D uuidstr; + } else { + data->nElems++; } =20 cleanup: @@ -493,14 +404,12 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secr= ets, virSecretObjListACLFilter filter, virConnectPtr conn) { - struct virSecretCountData data =3D { - .conn =3D conn, .filter =3D filter, .count =3D 0 }; - - virObjectLock(secrets); - virHashForEach(secrets->objs, virSecretObjListNumOfSecretsCallback, &d= ata); - virObjectUnlock(secrets); + virObjectLookupHashForEachData data =3D { + .conn =3D conn, .filter =3D filter, .error =3D false, .nElems =3D = 0, + .elems =3D NULL, .maxElems =3D -2 }; =20 - return data.count; + return virObjectLookupHashForEachUUID(secrets, virSecretObjListGetHelp= er, + &data); } =20 =20 @@ -532,22 +441,15 @@ virSecretObjMatchFlags(virSecretObjPtr obj, #undef MATCH =20 =20 -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 =3D opaque; virSecretObjPtr obj =3D payload; + virObjectLookupHashForEachDataPtr data =3D opaque; + virSecretPtr *secrets =3D (virSecretPtr *)data->elems; + virSecretObjListACLFilter filter =3D data->filter; virSecretDefPtr def; virSecretPtr secret =3D NULL; =20 @@ -557,25 +459,24 @@ virSecretObjListExportCallback(void *payload, virObjectLock(obj); def =3D obj->def; =20 - if (data->filter && !data->filter(data->conn, def)) + if (filter && !filter(data->conn, def)) goto cleanup; =20 if (!virSecretObjMatchFlags(obj, data->flags)) goto cleanup; =20 - if (!data->secrets) { - data->nsecrets++; + if (!secrets) { + data->nElems++; goto cleanup; } =20 if (!(secret =3D virGetSecret(data->conn, def->uuid, - def->usage_type, - def->usage_id))) { + def->usage_type, def->usage_id))) { data->error =3D true; goto cleanup; } =20 - data->secrets[data->nsecrets++] =3D secret; + secrets[data->nElems++] =3D secret; =20 cleanup: virObjectUnlock(obj); @@ -590,35 +491,22 @@ virSecretObjListExport(virConnectPtr conn, virSecretObjListACLFilter filter, unsigned int flags) { - struct virSecretObjListData data =3D { - .conn =3D conn, .secrets =3D NULL, - .filter =3D filter, .flags =3D flags, - .nsecrets =3D 0, .error =3D false }; - - virObjectLock(secretobjs); - if (secrets && - VIR_ALLOC_N(data.secrets, virHashSize(secretobjs->objs) + 1) < 0) { - virObjectUnlock(secretobjs); - return -1; - } + int ret; =20 - virHashForEach(secretobjs->objs, virSecretObjListExportCallback, &data= ); - virObjectUnlock(secretobjs); + virObjectLookupHashForEachData data =3D { + .conn =3D conn, .filter =3D filter, .error =3D false, .nElems =3D = 0, + .elems =3D NULL, .maxElems =3D 0, .flags =3D flags }; =20 - 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 =3D data.secrets; - } + if (secrets) + data.maxElems =3D -1; =20 - return data.nsecrets; + ret =3D virObjectLookupHashForEachUUID(secretobjs, + virSecretObjListExportCallback, + &data); + if (secrets) + *secrets =3D (virSecretPtr *)data.elems; =20 - error: - virObjectListFree(data.secrets); - return -1; + return ret; } =20 =20 @@ -629,23 +517,12 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets, virSecretObjListACLFilter filter, virConnectPtr conn) { - struct virSecretListData data =3D { - .conn =3D conn, .filter =3D filter, .uuids =3D uuids, .nuuids =3D = 0, - .maxuuids =3D maxuuids, .error =3D false }; + virObjectLookupHashForEachData data =3D { + .conn =3D conn, .filter =3D filter, .error =3D false, .nElems =3D = 0, + .elems =3D (void **)uuids, .maxElems =3D maxuuids }; =20 - 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, virSecretObjListGetHelp= er, + &data); } =20 =20 --=20 2.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list