From nobody Fri May 16 03:22:19 2025 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 150041170568170.79001696152545; Tue, 18 Jul 2017 14:01:45 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1DFCBC04B939; Tue, 18 Jul 2017 21:01:41 +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 E55A55D6A2; Tue, 18 Jul 2017 21:01:40 +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 976B21853E36; Tue, 18 Jul 2017 21:01:40 +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 v6IKw09l019279 for ; Tue, 18 Jul 2017 16:58:00 -0400 Received: by smtp.corp.redhat.com (Postfix) id CB59217538; Tue, 18 Jul 2017 20:58:00 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-46.phx2.redhat.com [10.3.117.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7BBB07A25C for ; Tue, 18 Jul 2017 20:57:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 1DFCBC04B939 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 1DFCBC04B939 From: John Ferlan To: libvir-list@redhat.com Date: Tue, 18 Jul 2017 16:57:47 -0400 Message-Id: <20170718205750.14503-4-jferlan@redhat.com> In-Reply-To: <20170718205750.14503-1-jferlan@redhat.com> References: <20170718205750.14503-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 3/6] nwfilter: Convert _virNWFilterObjList to be a virObjectLockable 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.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 18 Jul 2017 21:01:41 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Perhaps a bit out of order from the normal convert driver object to virObjectLockable, then convert the driver object list. However, as it turns out nwfilter objects have been using a recursive mutex for which the common virObject code does not want to use. So, if we convert the nwfilter object list to use virObjectLockable, then it will be more possible to make the necessary adjustments to the virNWFilterObjListFindInstantiateFilter algorithm in order to handle a recursive lock operation required as part of the and (or "inc" list) processing. This patch takes the plunge, modifying the nwfilter object list handling code to utilize hash tables for both the UUID and Name for which an nwfilter def would utilize. This makes lookup by either "key" possible without needing to first lock the object in order to find a match as long as of course the object list itself is locked. Signed-off-by: John Ferlan --- src/conf/virnwfilterobj.c | 404 ++++++++++++++++++++++++++++++++----------= ---- 1 file changed, 287 insertions(+), 117 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 1e8fd36..5d34851 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -51,10 +51,34 @@ struct _virNWFilterObj { }; =20 struct _virNWFilterObjList { - size_t count; - virNWFilterObjPtr *objs; + virObjectLockable 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 virNWFilterObjListClass; +static void virNWFilterObjListDispose(void *opaque); + +static int +virNWFilterObjOnceInit(void) +{ + if (!(virNWFilterObjListClass =3D virClassNew(virClassForObjectLockabl= e(), + "virNWFilterObjList", + sizeof(virNWFilterObjList), + virNWFilterObjListDispose)= )) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNWFilterObj) + =20 static virNWFilterObjPtr virNWFilterObjNew(void) @@ -147,14 +171,30 @@ virNWFilterObjUnref(virNWFilterObjPtr obj) } =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++) - virNWFilterObjUnref(nwfilters->objs[i]); - VIR_FREE(nwfilters->objs); - VIR_FREE(nwfilters); + virObjectUnref(nwfilters); +} + + +static void +virNWFilterObjListObjsFreeData(void *payload, + const void *name ATTRIBUTE_UNUSED) +{ + virNWFilterObjPtr obj =3D payload; + + virNWFilterObjUnref(obj); } =20 =20 @@ -163,8 +203,23 @@ virNWFilterObjListNew(void) { virNWFilterObjListPtr nwfilters; =20 - if (VIR_ALLOC(nwfilters) < 0) + if (virNWFilterObjInitialize() < 0) + return NULL; + + if (!(nwfilters =3D virObjectLockableNew(virNWFilterObjListClass))) + return NULL; + + if (!(nwfilters->objs =3D virHashCreate(10, virNWFilterObjListObjsFree= Data))) { + virObjectUnref(nwfilters); + return NULL; + } + + if (!(nwfilters->objsName =3D virHashCreate(10, virNWFilterObjListObjs= FreeData))) { + virHashFree(nwfilters->objs); + virObjectUnref(nwfilters); return NULL; + } + return nwfilters; } =20 @@ -173,21 +228,34 @@ void virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters, virNWFilterObjPtr obj) { - size_t i; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virNWFilterDefPtr def; =20 + if (!obj) + return; + def =3D obj->def; + + virUUIDFormat(def->uuid, uuidstr); + virNWFilterObjRef(obj); virNWFilterObjUnlock(obj); + virObjectLock(nwfilters); + virNWFilterObjLock(obj); + virHashRemoveEntry(nwfilters->objs, uuidstr); + virHashRemoveEntry(nwfilters->objsName, def->name); + virNWFilterObjUnlock(obj); + virNWFilterObjUnref(obj); + virObjectUnlock(nwfilters); +} =20 - for (i =3D 0; i < nwfilters->count; i++) { - virNWFilterObjLock(nwfilters->objs[i]); - if (nwfilters->objs[i] =3D=3D obj) { - virNWFilterObjUnlock(nwfilters->objs[i]); - virNWFilterObjUnref(nwfilters->objs[i]); =20 - VIR_DELETE_ELEMENT(nwfilters->objs, i, nwfilters->count); - break; - } - virNWFilterObjUnlock(nwfilters->objs[i]); - } +static virNWFilterObjPtr +virNWFilterObjListFindByUUIDLocked(virNWFilterObjListPtr nwfilters, + const unsigned char *uuid) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(uuid, uuidstr); + return virNWFilterObjRef(virHashLookup(nwfilters->objs, uuidstr)); } =20 =20 @@ -195,20 +263,22 @@ virNWFilterObjPtr virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters, const unsigned char *uuid) { - size_t i; virNWFilterObjPtr obj; - virNWFilterDefPtr def; =20 - for (i =3D 0; i < nwfilters->count; i++) { - obj =3D nwfilters->objs[i]; + virObjectLock(nwfilters); + obj =3D virNWFilterObjListFindByUUIDLocked(nwfilters, uuid); + virObjectUnlock(nwfilters); + if (obj) virNWFilterObjLock(obj); - def =3D obj->def; - if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN)) - return virNWFilterObjRef(obj); - virNWFilterObjUnlock(obj); - } + return obj; +} =20 - return NULL; + +static virNWFilterObjPtr +virNWFilterObjListFindByNameLocked(virNWFilterObjListPtr nwfilters, + const char *name) +{ + return virNWFilterObjRef(virHashLookup(nwfilters->objsName, name)); } =20 =20 @@ -216,20 +286,15 @@ 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]; + virObjectLock(nwfilters); + obj =3D virNWFilterObjListFindByNameLocked(nwfilters, name); + virObjectUnlock(nwfilters); + if (obj) virNWFilterObjLock(obj); - def =3D obj->def; - if (STREQ_NULLABLE(def->name, name)) - return virNWFilterObjRef(obj); - virNWFilterObjUnlock(obj); - } =20 - return NULL; + return obj; } =20 =20 @@ -277,9 +342,10 @@ _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr= nwfilters, break; } =20 - obj =3D virNWFilterObjListFindByName(nwfilters, - entry->include->filterref); + obj =3D virNWFilterObjListFindByNameLocked(nwfilters, + entry->include->filte= rref); if (obj) { + virObjectLock(obj); rc =3D _virNWFilterObjListDefLoopDetect(nwfilters, obj->de= f, filtername); virNWFilterObjEndAPI(&obj); @@ -301,6 +367,8 @@ _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr = nwfilters, * Detect a loop introduced through the filters being able to * reference each other. * + * NB: Enter with nwfilters locked + * * Returns 0 in case no loop was detected, -1 otherwise. */ static int @@ -355,8 +423,12 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfi= lters, { virNWFilterObjPtr obj; virNWFilterDefPtr objdef; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virObjectLock(nwfilters); =20 - if ((obj =3D virNWFilterObjListFindByUUID(nwfilters, def->uuid))) { + if ((obj =3D virNWFilterObjListFindByUUIDLocked(nwfilters, def->uuid))= ) { + virNWFilterObjLock(obj); objdef =3D obj->def; =20 if (STRNEQ(def->name, objdef->name)) { @@ -365,19 +437,21 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwf= ilters, "('%s') already exists"), objdef->name); virNWFilterObjEndAPI(&obj); + virObjectUnlock(nwfilters); return NULL; } virNWFilterObjEndAPI(&obj); } else { - if ((obj =3D virNWFilterObjListFindByName(nwfilters, def->name))) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; + if ((obj =3D virNWFilterObjListFindByNameLocked(nwfilters, def->na= me))) { =20 + virNWFilterObjLock(obj); objdef =3D obj->def; virUUIDFormat(objdef->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, _("filter '%s' already exists with uuid %s"), def->name, uuidstr); virNWFilterObjEndAPI(&obj); + virObjectUnlock(nwfilters); return NULL; } } @@ -385,16 +459,18 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwf= ilters, if (virNWFilterObjListDefLoopDetect(nwfilters, def) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("filter would introduce a loop")); + virObjectUnlock(nwfilters); return NULL; } =20 - if ((obj =3D virNWFilterObjListFindByName(nwfilters, def->name))) { =20 + if ((obj =3D virNWFilterObjListFindByNameLocked(nwfilters, def->name))= ) { + virNWFilterObjLock(obj); objdef =3D obj->def; if (virNWFilterDefEqual(def, objdef)) { virNWFilterDefFree(objdef); obj->def =3D def; - return obj; + goto cleanup; } =20 obj->newDef =3D def; @@ -402,27 +478,63 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwf= ilters, if (virNWFilterTriggerVMFilterRebuild() < 0) { obj->newDef =3D NULL; virNWFilterObjEndAPI(&obj); + virObjectUnlock(nwfilters); return NULL; } =20 virNWFilterDefFree(objdef); obj->def =3D def; obj->newDef =3D NULL; - return obj; + goto cleanup; } =20 if (!(obj =3D virNWFilterObjNew())) return NULL; =20 - if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, - nwfilters->count, obj) < 0) { - virNWFilterObjUnlock(obj); - virNWFilterObjFree(obj); - return NULL; + virUUIDFormat(def->uuid, uuidstr); + if (virHashAddEntry(nwfilters->objs, uuidstr, obj) < 0) + goto error; + virNWFilterObjRef(obj); + + if (virHashAddEntry(nwfilters->objsName, def->name, obj) < 0) { + virHashRemoveEntry(nwfilters->objs, uuidstr); + goto error; } + obj->def =3D def; + virNWFilterObjRef(obj); + + cleanup: + virObjectUnlock(nwfilters); + return obj; =20 - return virNWFilterObjRef(obj); + error: + virNWFilterObjUnlock(obj); + virNWFilterObjUnref(obj); + virObjectUnlock(nwfilters); + return NULL; +} + + +struct virNWFilterCountData { + virConnectPtr conn; + virNWFilterObjListFilter aclfilter; + int nelems; +}; + +static int +virNWFilterObjListNumOfNWFiltersCallback(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virNWFilterObjPtr obj =3D payload; + struct virNWFilterCountData *data =3D opaque; + + virObjectLock(obj); + if (!data->aclfilter || data->aclfilter(data->conn, obj->def)) + data->nelems++; + virObjectUnlock(obj); + return 0; } =20 =20 @@ -431,18 +543,56 @@ virNWFilterObjListNumOfNWFilters(virNWFilterObjListPt= r nwfilters, virConnectPtr conn, virNWFilterObjListFilter aclfilter) { - size_t i; - int nfilters =3D 0; + struct virNWFilterCountData data =3D { .conn =3D conn, + .aclfilter =3D aclfilter, .nelems =3D 0 }; =20 - for (i =3D 0; i < nwfilters->count; i++) { - virNWFilterObjPtr obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); - if (!aclfilter || aclfilter(conn, obj->def)) - nfilters++; - virNWFilterObjUnlock(obj); + virObjectLock(nwfilters); + virHashForEach(nwfilters->objs, virNWFilterObjListNumOfNWFiltersCallba= ck, + &data); + virObjectUnlock(nwfilters); + + return data.nelems; +} + + +struct virNWFilterListData { + virConnectPtr conn; + virNWFilterObjListFilter aclfilter; + 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; + + virObjectLock(obj); + def =3D obj->def; + + if (!data->aclfilter || data->aclfilter(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: + virObjectUnlock(obj); + return 0; } =20 =20 @@ -453,82 +603,102 @@ 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, .aclfilter =3D a= clfilter, + .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]; - virNWFilterObjLock(obj); - def =3D obj->def; - if (!aclfilter || aclfilter(conn, def)) { - if (VIR_STRDUP(names[nnames], def->name) < 0) { - virNWFilterObjUnlock(obj); - goto failure; - } - nnames++; - } - virNWFilterObjUnlock(obj); - } + virObjectLock(nwfilters); + virHashForEach(nwfilters->objs, virNWFilterObjListGetNamesCallback, &d= ata); + virObjectUnlock(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 -int -virNWFilterObjListExport(virConnectPtr conn, - virNWFilterObjListPtr nwfilters, - virNWFilterPtr **filters, - virNWFilterObjListFilter aclfilter) +struct virNWFilterExportData { + virConnectPtr conn; + virNWFilterObjListFilter aclfilter; + virNWFilterPtr *filters; + int nfilters; + bool error; +}; + +static int +virNWFilterObjListExportCallback(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) { - virNWFilterPtr *tmp_filters =3D NULL; - int nfilters =3D 0; - virNWFilterPtr filter =3D NULL; - virNWFilterObjPtr obj =3D NULL; + virNWFilterObjPtr obj =3D payload; virNWFilterDefPtr def; - size_t i; - int ret =3D -1; + struct virNWFilterExportData *data =3D opaque; + virNWFilterPtr nwfilter; + + if (data->error) + return 0; + + virObjectLock(obj); + def =3D obj->def; =20 - if (!filters) { - ret =3D nwfilters->count; + if (data->aclfilter && !data->aclfilter(data->conn, def)) + goto cleanup; + + if (!data->filters) { + data->nfilters++; goto cleanup; } =20 - if (VIR_ALLOC_N(tmp_filters, nwfilters->count + 1) < 0) + if (!(nwfilter =3D virGetNWFilter(data->conn, def->name, def->uuid))) { + data->error =3D true; goto cleanup; + } + data->filters[data->nfilters++] =3D nwfilter; =20 - for (i =3D 0; i < nwfilters->count; i++) { - obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); - def =3D obj->def; - if (!aclfilter || aclfilter(conn, def)) { - if (!(filter =3D virGetNWFilter(conn, def->name, def->uuid))) { - virNWFilterObjUnlock(obj); - goto cleanup; - } - tmp_filters[nfilters++] =3D filter; - } - virNWFilterObjUnlock(obj); + cleanup: + virObjectUnlock(obj); + return 0; +} + + +int +virNWFilterObjListExport(virConnectPtr conn, + virNWFilterObjListPtr nwfilters, + virNWFilterPtr **filters, + virNWFilterObjListFilter aclfilter) +{ + struct virNWFilterExportData data =3D { .conn =3D conn, .aclfilter =3D= aclfilter, + .filters =3D NULL, .nfilters =3D 0, .error =3D false }; + + virObjectLock(nwfilters); + if (filters && + VIR_ALLOC_N(data.filters, virHashSize(nwfilters->objs) + 1) < 0) { + virObjectUnlock(nwfilters); + return -1; } =20 - *filters =3D tmp_filters; - tmp_filters =3D NULL; - ret =3D nfilters; + virHashForEach(nwfilters->objs, virNWFilterObjListExportCallback, &dat= a); + virObjectUnlock(nwfilters); =20 - cleanup: - if (tmp_filters) { - for (i =3D 0; i < nfilters; i ++) - virObjectUnref(tmp_filters[i]); + 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; } - VIR_FREE(tmp_filters); =20 - return ret; + return data.nfilters; + + cleanup: + virObjectListFree(data.filters); + return -1; } =20 =20 --=20 2.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list