From nobody Tue Feb 10 04:22:33 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 1507248103040843.1382540450223; Thu, 5 Oct 2017 17:01:43 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BAF95883BB; Fri, 6 Oct 2017 00:01:41 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8FE0660C8B; Fri, 6 Oct 2017 00:01:41 +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 50233410B7; Fri, 6 Oct 2017 00:01:41 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v95NWbbV027347 for ; Thu, 5 Oct 2017 19:32:37 -0400 Received: by smtp.corp.redhat.com (Postfix) id 9A4635D765; Thu, 5 Oct 2017 23:32:37 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-17.phx2.redhat.com [10.3.116.17]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5153A5D763 for ; Thu, 5 Oct 2017 23:32:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BAF95883BB Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com From: John Ferlan To: libvir-list@redhat.com Date: Thu, 5 Oct 2017 19:32:30 -0400 Message-Id: <20171005233231.3278-5-jferlan@redhat.com> In-Reply-To: <20171005233231.3278-1-jferlan@redhat.com> References: <20171005233231.3278-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v3 4/5] nwfilter: Convert _virNWFilterObjList to use virObjectRWLockable 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.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 06 Oct 2017 00:01:42 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Implement the self locking object list for nwfilter object lists that uses two hash tables to store the nwfilter object by UUID or by Name. As part of this alter the uuid argument to virNWFilterObjLookupByUUID to expect an already formatted uuidstr. Alter the existing list traversal code to implement the hash table find/lookup/search functionality. Signed-off-by: John Ferlan --- src/conf/virnwfilterobj.c | 402 ++++++++++++++++++++++++++++---------= ---- src/conf/virnwfilterobj.h | 2 +- src/nwfilter/nwfilter_driver.c | 5 +- 3 files changed, 282 insertions(+), 127 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 6b4758656f..a4e6a03d29 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -43,12 +43,21 @@ struct _virNWFilterObj { }; =20 struct _virNWFilterObjList { - size_t count; - virNWFilterObjPtr *objs; + virObjectRWLockable parent; + + /* uuid string -> virNWFilterObj mapping + * for O(1), lockless lookup-by-uuid */ + virHashTable *objs; + + /* name -> virNWFilterObj mapping for O(1), + * lockless lookup-by-name */ + virHashTable *objsName; }; =20 static virClassPtr virNWFilterObjClass; +static virClassPtr virNWFilterObjListClass; static void virNWFilterObjDispose(void *opaque); +static void virNWFilterObjListDispose(void *opaque); =20 =20 static int @@ -60,6 +69,12 @@ virNWFilterObjOnceInit(void) virNWFilterObjDispose))) return -1; =20 + if (!(virNWFilterObjListClass =3D virClassNew(virClassForObjectRWLocka= ble(), + "virNWFilterObjList", + sizeof(virNWFilterObjList), + virNWFilterObjListDispose)= )) + return -1; + return 0; } =20 @@ -144,14 +159,20 @@ virNWFilterObjDispose(void *opaque) } =20 =20 +static void +virNWFilterObjListDispose(void *opaque) +{ + virNWFilterObjListPtr nwfilters =3D opaque; + + virHashFree(nwfilters->objs); + virHashFree(nwfilters->objsName); +} + + void virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) { - size_t i; - for (i =3D 0; i < nwfilters->count; i++) - virObjectUnref(nwfilters->objs[i]); - VIR_FREE(nwfilters->objs); - VIR_FREE(nwfilters); + virObjectUnref(nwfilters); } =20 =20 @@ -160,8 +181,23 @@ virNWFilterObjListNew(void) { virNWFilterObjListPtr nwfilters; =20 - if (VIR_ALLOC(nwfilters) < 0) + if (virNWFilterObjInitialize() < 0) + return NULL; + + if (!(nwfilters =3D virObjectRWLockableNew(virNWFilterObjListClass))) + return NULL; + + if (!(nwfilters->objs =3D virHashCreate(10, virObjectFreeHashData))) { + virObjectUnref(nwfilters); + return NULL; + } + + if (!(nwfilters->objsName =3D virHashCreate(10, virObjectFreeHashData)= )) { + virHashFree(nwfilters->objs); + virObjectUnref(nwfilters); return NULL; + } + return nwfilters; } =20 @@ -170,83 +206,105 @@ void virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters, virNWFilterObjPtr obj) { - size_t i; - - virObjectRWUnlock(obj); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virNWFilterDefPtr def; =20 - for (i =3D 0; i < nwfilters->count; i++) { - virObjectRWLockWrite(nwfilters->objs[i]); - if (nwfilters->objs[i] =3D=3D obj) { - virObjectRWUnlock(nwfilters->objs[i]); - virObjectUnref(nwfilters->objs[i]); + if (!obj) + return; + def =3D obj->def; =20 - VIR_DELETE_ELEMENT(nwfilters->objs, i, nwfilters->count); - break; - } - virObjectRWUnlock(nwfilters->objs[i]); - } + virUUIDFormat(def->uuid, uuidstr); + virObjectRef(obj); + virObjectRWUnlock(obj); + virObjectRWLockWrite(nwfilters); + virObjectRWLockWrite(obj); + virHashRemoveEntry(nwfilters->objs, uuidstr); + virHashRemoveEntry(nwfilters->objsName, def->name); + virObjectRWUnlock(obj); + virObjectUnref(obj); + virObjectRWUnlock(nwfilters); } =20 =20 /** - * virNWFilterObjListFindByUUID + * virNWFilterObjListFindByUUID[Locked] * @nwfilters: Pointer to filter list - * @uuid: UUID to use to lookup the object + * @uuidstr: UUID to use to lookup the object + * + * The static [Locked] version would only be used when the Object List is + * already locked, such as is the case during virNWFilterObjListAssignDef. + * The caller is thus responsible for locking the object. * * Search for the object by uuidstr in the hash table and return a read * locked copy of the object. * + * Returns: A reffed object or NULL on error + */ +static virNWFilterObjPtr +virNWFilterObjListFindByUUIDLocked(virNWFilterObjListPtr nwfilters, + const char *uuidstr) +{ + return virObjectRef(virHashLookup(nwfilters->objs, uuidstr)); +} + + +/* * Returns: A reffed and read locked object or NULL on error */ virNWFilterObjPtr virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters, - const unsigned char *uuid) + const char *uuidstr) { - size_t i; virNWFilterObjPtr obj; - virNWFilterDefPtr def; =20 - for (i =3D 0; i < nwfilters->count; i++) { - obj =3D nwfilters->objs[i]; + virObjectRWLockRead(nwfilters); + obj =3D virNWFilterObjListFindByUUIDLocked(nwfilters, uuidstr); + virObjectRWUnlock(nwfilters); + if (obj) virObjectRWLockRead(obj); - def =3D obj->def; - if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN)) - return virObjectRef(obj); - virObjectRWUnlock(obj); - } =20 - return NULL; + return obj; } =20 =20 /** - * virNWFilterObjListFindByName + * virNWFilterObjListFindByName[Locked] * @nwfilters: Pointer to filter list * @name: filter name to use to lookup the object * + * The static [Locked] version would only be used when the Object List is + * already locked, such as is the case during virNWFilterObjListAssignDef. + * The caller is thus responsible for locking the object. + * * Search for the object by name in the hash table and return a read * locked copy of the object. * + * Returns: A reffed object or NULL on error + */ +static virNWFilterObjPtr +virNWFilterObjListFindByNameLocked(virNWFilterObjListPtr nwfilters, + const char *name) +{ + return virObjectRef(virHashLookup(nwfilters->objsName, name)); +} + + +/* * Returns: A reffed and read locked object or NULL on error */ virNWFilterObjPtr virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters, const char *name) { - size_t i; virNWFilterObjPtr obj; - virNWFilterDefPtr def; =20 - for (i =3D 0; i < nwfilters->count; i++) { - obj =3D nwfilters->objs[i]; + virObjectRWLockRead(nwfilters); + obj =3D virNWFilterObjListFindByNameLocked(nwfilters, name); + virObjectRWUnlock(nwfilters); + if (obj) virObjectRWLockRead(obj); - def =3D obj->def; - if (STREQ_NULLABLE(def->name, name)) - return virObjectRef(obj); - virObjectRWUnlock(obj); - } =20 - return NULL; + return obj; } =20 =20 @@ -391,8 +449,11 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfi= lters, { virNWFilterObjPtr obj; virNWFilterDefPtr objdef; + char uuidstr[VIR_UUID_STRING_BUFLEN]; =20 - if ((obj =3D virNWFilterObjListFindByUUID(nwfilters, def->uuid))) { + virUUIDFormat(def->uuid, uuidstr); + + if ((obj =3D virNWFilterObjListFindByUUID(nwfilters, uuidstr))) { objdef =3D obj->def; =20 if (STRNEQ(def->name, objdef->name)) { @@ -406,10 +467,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfi= lters, virNWFilterObjEndAPI(&obj); } else { if ((obj =3D virNWFilterObjListFindByName(nwfilters, def->name))) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - objdef =3D obj->def; - virUUIDFormat(objdef->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, _("filter '%s' already exists with uuid %s"), def->name, uuidstr); @@ -424,11 +482,13 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwf= ilters, return NULL; } =20 - - /* Get a READ lock and immediately promote to WRITE while we adjust - * data within. */ - if ((obj =3D virNWFilterObjListFindByName(nwfilters, def->name))) { - virNWFilterObjPromoteToWrite(obj); + /* We're about to make some changes to objects on the list - so get + * the list READ lock in order to Find the object and WRITE lock it + * while we adjust data within. */ + virObjectRWLockRead(nwfilters); + if ((obj =3D virNWFilterObjListFindByNameLocked(nwfilters, def->name))= ) { + virObjectRWUnlock(nwfilters); + virObjectRWLockWrite(obj); =20 objdef =3D obj->def; if (virNWFilterDefEqual(def, objdef)) { @@ -458,37 +518,112 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nw= filters, return obj; } =20 + /* Promote the nwfilters to add a new object */ + virObjectRWUnlock(nwfilters); + virObjectRWLockWrite(nwfilters); if (!(obj =3D virNWFilterObjNew())) - return NULL; + goto cleanup; =20 - if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, - nwfilters->count, obj) < 0) { - virNWFilterObjEndAPI(&obj); - return NULL; + if (virHashAddEntry(nwfilters->objs, uuidstr, obj) < 0) + goto error; + virObjectRef(obj); + + if (virHashAddEntry(nwfilters->objsName, def->name, obj) < 0) { + virHashRemoveEntry(nwfilters->objs, uuidstr); + goto error; } + virObjectRef(obj); + obj->def =3D def; =20 - return virObjectRef(obj); + cleanup: + virObjectRWUnlock(nwfilters); + return obj; + + error: + virObjectRWUnlock(obj); + virObjectUnref(obj); + virObjectRWUnlock(nwfilters); + return NULL; } =20 =20 +struct virNWFilterCountData { + virConnectPtr conn; + virNWFilterObjListFilter filter; + int nelems; +}; + +static int +virNWFilterObjListNumOfNWFiltersCallback(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virNWFilterObjPtr obj =3D payload; + struct virNWFilterCountData *data =3D opaque; + + virObjectRWLockRead(obj); + if (!data->filter || data->filter(data->conn, obj->def)) + data->nelems++; + virObjectRWUnlock(obj); + return 0; +} + int virNWFilterObjListNumOfNWFilters(virNWFilterObjListPtr nwfilters, virConnectPtr conn, virNWFilterObjListFilter filter) { - size_t i; - int nfilters =3D 0; + struct virNWFilterCountData data =3D { .conn =3D conn, + .filter =3D filter, .nelems =3D 0 }; =20 - for (i =3D 0; i < nwfilters->count; i++) { - virNWFilterObjPtr obj =3D nwfilters->objs[i]; - virObjectRWLockRead(obj); - if (!filter || filter(conn, obj->def)) - nfilters++; - virObjectRWUnlock(obj); + virObjectRWLockRead(nwfilters); + virHashForEach(nwfilters->objs, virNWFilterObjListNumOfNWFiltersCallba= ck, + &data); + virObjectRWUnlock(nwfilters); + + return data.nelems; +} + + +struct virNWFilterListData { + virConnectPtr conn; + virNWFilterObjListFilter filter; + int nelems; + char **elems; + int maxelems; + bool error; +}; + +static int +virNWFilterObjListGetNamesCallback(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virNWFilterObjPtr obj =3D payload; + virNWFilterDefPtr def; + struct virNWFilterListData *data =3D opaque; + + if (data->error) + return 0; + + if (data->maxelems >=3D 0 && data->nelems =3D=3D data->maxelems) + return 0; + + virObjectRWLockRead(obj); + def =3D obj->def; + + if (!data->filter || data->filter(data->conn, def)) { + if (VIR_STRDUP(data->elems[data->nelems], def->name) < 0) { + data->error =3D true; + goto cleanup; + } + data->nelems++; } =20 - return nfilters; + cleanup: + virObjectRWUnlock(obj); + return 0; } =20 =20 @@ -499,82 +634,103 @@ virNWFilterObjListGetNames(virNWFilterObjListPtr nwf= ilters, char **const names, int maxnames) { - int nnames =3D 0; - size_t i; - virNWFilterDefPtr def; + struct virNWFilterListData data =3D { .conn =3D conn, .filter =3D filt= er, + .nelems =3D 0, .elems =3D names, .maxelems =3D maxnames, .error = =3D false }; =20 - for (i =3D 0; i < nwfilters->count && nnames < maxnames; i++) { - virNWFilterObjPtr obj =3D nwfilters->objs[i]; - virObjectRWLockRead(obj); - def =3D obj->def; - if (!filter || filter(conn, def)) { - if (VIR_STRDUP(names[nnames], def->name) < 0) { - virObjectRWUnlock(obj); - goto failure; - } - nnames++; - } - virObjectRWUnlock(obj); - } + virObjectRWLockRead(nwfilters); + virHashForEach(nwfilters->objs, virNWFilterObjListGetNamesCallback, &d= ata); + virObjectRWUnlock(nwfilters); =20 - return nnames; + if (data.error) + goto error; =20 - failure: - while (--nnames >=3D 0) - VIR_FREE(names[nnames]); + return data.nelems; =20 + error: + while (--data.nelems >=3D 0) + VIR_FREE(data.elems[data.nelems]); return -1; } =20 =20 +struct virNWFilterExportData { + virConnectPtr conn; + virNWFilterObjListFilter filter; + virNWFilterPtr *filters; + int nfilters; + bool error; +}; + +static int +virNWFilterObjListExportCallback(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virNWFilterObjPtr obj =3D payload; + virNWFilterDefPtr def; + + struct virNWFilterExportData *data =3D opaque; + virNWFilterPtr nwfilter; + + if (data->error) + return 0; + + virObjectRWLockRead(obj); + def =3D obj->def; + + if (data->filter && !data->filter(data->conn, def)) + goto cleanup; + + if (!data->filters) { + data->nfilters++; + goto cleanup; + } + + if (!(nwfilter =3D virGetNWFilter(data->conn, def->name, def->uuid))) { + data->error =3D true; + goto cleanup; + } + data->filters[data->nfilters++] =3D nwfilter; + + cleanup: + virObjectRWUnlock(obj); + return 0; +} + + int virNWFilterObjListExport(virConnectPtr conn, virNWFilterObjListPtr nwfilters, virNWFilterPtr **filters, virNWFilterObjListFilter filter) { - virNWFilterPtr *tmp_filters =3D NULL; - int nfilters =3D 0; - virNWFilterPtr nwfilter =3D NULL; - virNWFilterObjPtr obj =3D NULL; - virNWFilterDefPtr def; - size_t i; - int ret =3D -1; + struct virNWFilterExportData data =3D { .conn =3D conn, .filter =3D fi= lter, + .filters =3D NULL, .nfilters =3D 0, .error =3D false }; =20 - if (!filters) { - ret =3D nwfilters->count; - goto cleanup; + virObjectRWLockRead(nwfilters); + if (filters && + VIR_ALLOC_N(data.filters, virHashSize(nwfilters->objs) + 1) < 0) { + virObjectRWUnlock(nwfilters); + return -1; } =20 - if (VIR_ALLOC_N(tmp_filters, nwfilters->count + 1) < 0) - goto cleanup; + virHashForEach(nwfilters->objs, virNWFilterObjListExportCallback, &dat= a); + virObjectRWUnlock(nwfilters); =20 - for (i =3D 0; i < nwfilters->count; i++) { - obj =3D nwfilters->objs[i]; - virObjectRWLockRead(obj); - def =3D obj->def; - if (!filter || filter(conn, def)) { - if (!(nwfilter =3D virGetNWFilter(conn, def->name, def->uuid))= ) { - virObjectRWUnlock(obj); - goto cleanup; - } - tmp_filters[nfilters++] =3D nwfilter; - } - virObjectRWUnlock(obj); + if (data.error) + goto cleanup; + + if (data.filters) { + /* trim the array to the final size */ + ignore_value(VIR_REALLOC_N(data.filters, data.nfilters + 1)); + *filters =3D data.filters; } =20 - *filters =3D tmp_filters; - tmp_filters =3D NULL; - ret =3D nfilters; + return data.nfilters; =20 cleanup: - if (tmp_filters) { - for (i =3D 0; i < nfilters; i ++) - virObjectUnref(tmp_filters[i]); - } - VIR_FREE(tmp_filters); - - return ret; + virObjectListFree(data.filters); + return -1; } =20 =20 diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 0281bc5f56..cabb42a71e 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -65,7 +65,7 @@ virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters, =20 virNWFilterObjPtr virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters, - const unsigned char *uuid); + const char *uuidstr); =20 virNWFilterObjPtr virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters, diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index a1d93f7af0..fee132cfd1 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -375,11 +375,10 @@ nwfilterObjFromNWFilter(const unsigned char *uuid) virNWFilterObjPtr obj; char uuidstr[VIR_UUID_STRING_BUFLEN]; =20 - if (!(obj =3D virNWFilterObjListFindByUUID(driver->nwfilters, uuid))) { - virUUIDFormat(uuid, uuidstr); + virUUIDFormat(uuid, uuidstr); + if (!(obj =3D virNWFilterObjListFindByUUID(driver->nwfilters, uuidstr)= )) virReportError(VIR_ERR_NO_NWFILTER, _("no nwfilter with matching uuid '%s'"), uuidstr); - } return obj; } =20 --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list