From nobody Fri May 3 15:37:52 2024 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 1512741773481499.3898429639347; Fri, 8 Dec 2017 06:02:53 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4663574860; Fri, 8 Dec 2017 14:02:51 +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 BEC7492310; Fri, 8 Dec 2017 14:02:50 +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 C12E8180884F; Fri, 8 Dec 2017 14:02:47 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id vB8E2jM0022995 for ; Fri, 8 Dec 2017 09:02:45 -0500 Received: by smtp.corp.redhat.com (Postfix) id EB482183AD; Fri, 8 Dec 2017 14:02:45 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-134.phx2.redhat.com [10.3.116.134]) by smtp.corp.redhat.com (Postfix) with ESMTP id B1F9B18B26 for ; Fri, 8 Dec 2017 14:02:45 +0000 (UTC) From: John Ferlan To: libvir-list@redhat.com Date: Fri, 8 Dec 2017 09:01:38 -0500 Message-Id: <20171208140141.11491-2-jferlan@redhat.com> In-Reply-To: <20171208140141.11491-1-jferlan@redhat.com> References: <20171208140141.11491-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v4 1/4] nwfilter: Remove unnecessary UUID comparison bypass 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.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 08 Dec 2017 14:02:52 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Commit id '46a811db07' added code to check if the filter by Name already existed with a different UUID, to go along with the existing found filter by UUID and compare the Names match thus making it impossible to reach this find by Name condition without both the Name and UUID already matching, so remove the need to "filter" out the UUID for the virNWFilterDefEqual. Signed-off-by: John Ferlan Reviewed-by: Laine Stump Reviewed-by: Stefan Berger --- src/conf/virnwfilterobj.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 408e575ca..87d7e7270 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -287,18 +287,11 @@ virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj) =20 static bool virNWFilterDefEqual(const virNWFilterDef *def1, - virNWFilterDefPtr def2, - bool cmpUUIDs) + virNWFilterDefPtr def2) { bool ret =3D false; - unsigned char rem_uuid[VIR_UUID_BUFLEN]; - char *xml1, *xml2 =3D NULL; - - if (!cmpUUIDs) { - /* make sure the UUIDs are equal */ - memcpy(rem_uuid, def2->uuid, sizeof(rem_uuid)); - memcpy(def2->uuid, def1->uuid, sizeof(def2->uuid)); - } + char *xml1 =3D NULL; + char *xml2 =3D NULL; =20 if (!(xml1 =3D virNWFilterDefFormat(def1)) || !(xml2 =3D virNWFilterDefFormat(def2))) @@ -307,9 +300,6 @@ virNWFilterDefEqual(const virNWFilterDef *def1, ret =3D STREQ(xml1, xml2); =20 cleanup: - if (!cmpUUIDs) - memcpy(def2->uuid, rem_uuid, sizeof(rem_uuid)); - VIR_FREE(xml1); VIR_FREE(xml2); =20 @@ -360,7 +350,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, if ((obj =3D virNWFilterObjListFindByName(nwfilters, def->name))) { =20 objdef =3D obj->def; - if (virNWFilterDefEqual(def, objdef, false)) { + if (virNWFilterDefEqual(def, objdef)) { virNWFilterDefFree(objdef); obj->def =3D def; return obj; --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri May 3 15:37:52 2024 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 1512741795870182.9370181343004; Fri, 8 Dec 2017 06:03:15 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2D9105214D; Fri, 8 Dec 2017 14:03:14 +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 001B818673; Fri, 8 Dec 2017 14:03:13 +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 B9142389B; Fri, 8 Dec 2017 14:03:13 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id vB8E2kx5023000 for ; Fri, 8 Dec 2017 09:02:46 -0500 Received: by smtp.corp.redhat.com (Postfix) id 62CE018B33; Fri, 8 Dec 2017 14:02:46 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-134.phx2.redhat.com [10.3.116.134]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1905E60C8A for ; Fri, 8 Dec 2017 14:02:46 +0000 (UTC) From: John Ferlan To: libvir-list@redhat.com Date: Fri, 8 Dec 2017 09:01:39 -0500 Message-Id: <20171208140141.11491-3-jferlan@redhat.com> In-Reply-To: <20171208140141.11491-1-jferlan@redhat.com> References: <20171208140141.11491-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v4 2/4] nwfilter: Convert _virNWFilterObj 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.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 08 Dec 2017 14:03:14 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Unlike it's counterparts, the nwfilter object code needs to be able to support recursive read locks while processing and checking new filters against the existing environment. Thus instead of using a virObjectLockable which uses pure mutexes, use the virObjectRWLockable and primarily use RWLockRead when obtaining the object lock since RWLockRead locks can be recursively obtained (up to a limit) as long as there's a corresponding unlock. Since all the object management is within the virnwfilterobj code, we can limit the number of Write locks on the object to very small areas of code to ensure we don't run into deadlock's with other threads and domain code that will check/use the filters (it's a very delicate balance). This limits the write locks to AssignDef and Remove processing. This patch will introduce a new API virNWFilterObjEndAPI to unlock and deref the object. This patch introduces two helpers to promote/demote the read/write lock. Signed-off-by: John Ferlan --- src/conf/virnwfilterobj.c | 191 +++++++++++++++++++++++------= ---- src/conf/virnwfilterobj.h | 9 +- src/libvirt_private.syms | 3 +- src/nwfilter/nwfilter_driver.c | 15 +-- src/nwfilter/nwfilter_gentech_driver.c | 11 +- 5 files changed, 149 insertions(+), 80 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 87d7e7270..6b4758656 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -34,7 +34,7 @@ VIR_LOG_INIT("conf.virnwfilterobj"); =20 struct _virNWFilterObj { - virMutex lock; + virObjectRWLockable parent; =20 bool wantRemoved; =20 @@ -47,27 +47,69 @@ struct _virNWFilterObjList { virNWFilterObjPtr *objs; }; =20 +static virClassPtr virNWFilterObjClass; +static void virNWFilterObjDispose(void *opaque); + + +static int +virNWFilterObjOnceInit(void) +{ + if (!(virNWFilterObjClass =3D virClassNew(virClassForObjectRWLockable(= ), + "virNWFilterObj", + sizeof(virNWFilterObj), + virNWFilterObjDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNWFilterObj) + =20 static virNWFilterObjPtr virNWFilterObjNew(void) { virNWFilterObjPtr obj; =20 - if (VIR_ALLOC(obj) < 0) + if (virNWFilterObjInitialize() < 0) return NULL; =20 - if (virMutexInitRecursive(&obj->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot initialize mutex")); - VIR_FREE(obj); + if (!(obj =3D virObjectRWLockableNew(virNWFilterObjClass))) return NULL; - } =20 - virNWFilterObjLock(obj); + virObjectRWLockWrite(obj); return obj; } =20 =20 +static void +virNWFilterObjPromoteToWrite(virNWFilterObjPtr obj) +{ + virObjectRWUnlock(obj); + virObjectRWLockWrite(obj); +} + + +static void +virNWFilterObjDemoteFromWrite(virNWFilterObjPtr obj) +{ + virObjectRWUnlock(obj); + virObjectRWLockRead(obj); +} + + +void +virNWFilterObjEndAPI(virNWFilterObjPtr *obj) +{ + if (!*obj) + return; + + virObjectRWUnlock(*obj); + virObjectUnref(*obj); + *obj =3D NULL; +} + + virNWFilterDefPtr virNWFilterObjGetDef(virNWFilterObjPtr obj) { @@ -90,17 +132,15 @@ virNWFilterObjWantRemoved(virNWFilterObjPtr obj) =20 =20 static void -virNWFilterObjFree(virNWFilterObjPtr obj) +virNWFilterObjDispose(void *opaque) { + virNWFilterObjPtr obj =3D opaque; + if (!obj) return; =20 virNWFilterDefFree(obj->def); virNWFilterDefFree(obj->newDef); - - virMutexDestroy(&obj->lock); - - VIR_FREE(obj); } =20 =20 @@ -109,7 +149,7 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) { size_t i; for (i =3D 0; i < nwfilters->count; i++) - virNWFilterObjFree(nwfilters->objs[i]); + virObjectUnref(nwfilters->objs[i]); VIR_FREE(nwfilters->objs); VIR_FREE(nwfilters); } @@ -132,22 +172,32 @@ virNWFilterObjListRemove(virNWFilterObjListPtr nwfilt= ers, { size_t i; =20 - virNWFilterObjUnlock(obj); + virObjectRWUnlock(obj); =20 for (i =3D 0; i < nwfilters->count; i++) { - virNWFilterObjLock(nwfilters->objs[i]); + virObjectRWLockWrite(nwfilters->objs[i]); if (nwfilters->objs[i] =3D=3D obj) { - virNWFilterObjUnlock(nwfilters->objs[i]); - virNWFilterObjFree(nwfilters->objs[i]); + virObjectRWUnlock(nwfilters->objs[i]); + virObjectUnref(nwfilters->objs[i]); =20 VIR_DELETE_ELEMENT(nwfilters->objs, i, nwfilters->count); break; } - virNWFilterObjUnlock(nwfilters->objs[i]); + virObjectRWUnlock(nwfilters->objs[i]); } } =20 =20 +/** + * virNWFilterObjListFindByUUID + * @nwfilters: Pointer to filter list + * @uuid: UUID to use to lookup the object + * + * Search for the object by uuidstr in the hash table and return a read + * locked copy of the object. + * + * Returns: A reffed and read locked object or NULL on error + */ virNWFilterObjPtr virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters, const unsigned char *uuid) @@ -158,17 +208,27 @@ virNWFilterObjListFindByUUID(virNWFilterObjListPtr nw= filters, =20 for (i =3D 0; i < nwfilters->count; i++) { obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); + virObjectRWLockRead(obj); def =3D obj->def; if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN)) - return obj; - virNWFilterObjUnlock(obj); + return virObjectRef(obj); + virObjectRWUnlock(obj); } =20 return NULL; } =20 =20 +/** + * virNWFilterObjListFindByName + * @nwfilters: Pointer to filter list + * @name: filter name to use to lookup the object + * + * Search for the object by name in the hash table and return a read + * locked copy of the object. + * + * Returns: A reffed and read locked object or NULL on error + */ virNWFilterObjPtr virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters, const char *name) @@ -179,11 +239,11 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nw= filters, =20 for (i =3D 0; i < nwfilters->count; i++) { obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); + virObjectRWLockRead(obj); def =3D obj->def; if (STREQ_NULLABLE(def->name, name)) - return obj; - virNWFilterObjUnlock(obj); + return virObjectRef(obj); + virObjectRWUnlock(obj); } =20 return NULL; @@ -205,8 +265,7 @@ virNWFilterObjListFindInstantiateFilter(virNWFilterObjL= istPtr nwfilters, if (virNWFilterObjWantRemoved(obj)) { virReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), filtername); - virNWFilterObjUnlock(obj); - return NULL; + virNWFilterObjEndAPI(&obj); } =20 return obj; @@ -240,7 +299,7 @@ _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr = nwfilters, if (obj) { rc =3D _virNWFilterObjListDefLoopDetect(nwfilters, obj->de= f, filtername); - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); if (rc < 0) break; } @@ -269,17 +328,36 @@ virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr= nwfilters, } =20 =20 +/* virNWFilterObjTestUnassignDef + * @obj: A read locked nwfilter object + * + * Cause the rebuild to occur because we're about to undefine the nwfilter. + * The rebuild code is designed to check if the filter is currently in use + * by a domain and thus disallow the unassign. + * + * NB: Although we enter with the UPDATE lock from UNDEFINE, let's still + * promote to a WRITE lock before changing *this* object's wantRemoved + * value that will be used in the virNWFilterObjListFindInstantiateFil= ter + * processing to determine whether we can really remove this filter or= not. + * + * Returns 0 if we can continue with the unassign, -1 if it's in use + */ int virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj) { int rc =3D 0; =20 + virNWFilterObjPromoteToWrite(obj); obj->wantRemoved =3D true; + virNWFilterObjDemoteFromWrite(obj); + /* trigger the update on VMs referencing the filter */ if (virNWFilterTriggerVMFilterRebuild() < 0) rc =3D -1; =20 + virNWFilterObjPromoteToWrite(obj); obj->wantRemoved =3D false; + virNWFilterObjDemoteFromWrite(obj); =20 return rc; } @@ -322,10 +400,10 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwf= ilters, _("filter with same UUID but different name " "('%s') already exists"), objdef->name); - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return NULL; } - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); } else { if ((obj =3D virNWFilterObjListFindByName(nwfilters, def->name))) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -335,7 +413,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, virReportError(VIR_ERR_OPERATION_FAILED, _("filter '%s' already exists with uuid %s"), def->name, uuidstr); - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return NULL; } } @@ -347,7 +425,10 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfi= lters, } =20 =20 + /* Get a READ lock and immediately promote to WRITE while we adjust + * data within. */ if ((obj =3D virNWFilterObjListFindByName(nwfilters, def->name))) { + virNWFilterObjPromoteToWrite(obj); =20 objdef =3D obj->def; if (virNWFilterDefEqual(def, objdef)) { @@ -357,13 +438,20 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwf= ilters, } =20 obj->newDef =3D def; + + /* Demote while the trigger runs since it may need to grab a read + * lock on this object and promote before returning. */ + virNWFilterObjDemoteFromWrite(obj); + /* trigger the update on VMs referencing the filter */ if (virNWFilterTriggerVMFilterRebuild() < 0) { + virNWFilterObjPromoteToWrite(obj); obj->newDef =3D NULL; - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return NULL; } =20 + virNWFilterObjPromoteToWrite(obj); virNWFilterDefFree(objdef); obj->def =3D def; obj->newDef =3D NULL; @@ -375,13 +463,12 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwf= ilters, =20 if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, nwfilters->count, obj) < 0) { - virNWFilterObjUnlock(obj); - virNWFilterObjFree(obj); + virNWFilterObjEndAPI(&obj); return NULL; } obj->def =3D def; =20 - return obj; + return virObjectRef(obj); } =20 =20 @@ -395,10 +482,10 @@ virNWFilterObjListNumOfNWFilters(virNWFilterObjListPt= r nwfilters, =20 for (i =3D 0; i < nwfilters->count; i++) { virNWFilterObjPtr obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); + virObjectRWLockRead(obj); if (!filter || filter(conn, obj->def)) nfilters++; - virNWFilterObjUnlock(obj); + virObjectRWUnlock(obj); } =20 return nfilters; @@ -418,16 +505,16 @@ virNWFilterObjListGetNames(virNWFilterObjListPtr nwfi= lters, =20 for (i =3D 0; i < nwfilters->count && nnames < maxnames; i++) { virNWFilterObjPtr obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); + virObjectRWLockRead(obj); def =3D obj->def; if (!filter || filter(conn, def)) { if (VIR_STRDUP(names[nnames], def->name) < 0) { - virNWFilterObjUnlock(obj); + virObjectRWUnlock(obj); goto failure; } nnames++; } - virNWFilterObjUnlock(obj); + virObjectRWUnlock(obj); } =20 return nnames; @@ -464,16 +551,16 @@ virNWFilterObjListExport(virConnectPtr conn, =20 for (i =3D 0; i < nwfilters->count; i++) { obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); + virObjectRWLockRead(obj); def =3D obj->def; if (!filter || filter(conn, def)) { if (!(nwfilter =3D virGetNWFilter(conn, def->name, def->uuid))= ) { - virNWFilterObjUnlock(obj); + virObjectRWUnlock(obj); goto cleanup; } tmp_filters[nfilters++] =3D nwfilter; } - virNWFilterObjUnlock(obj); + virObjectRWUnlock(obj); } =20 *filters =3D tmp_filters; @@ -551,24 +638,10 @@ virNWFilterObjListLoadAllConfigs(virNWFilterObjListPt= r nwfilters, continue; =20 obj =3D virNWFilterObjListLoadConfig(nwfilters, configDir, entry->= d_name); - if (obj) - virNWFilterObjUnlock(obj); + + virNWFilterObjEndAPI(&obj); } =20 VIR_DIR_CLOSE(dir); return ret; } - - -void -virNWFilterObjLock(virNWFilterObjPtr obj) -{ - virMutexLock(&obj->lock); -} - - -void -virNWFilterObjUnlock(virNWFilterObjPtr obj) -{ - virMutexUnlock(&obj->lock); -} diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 8e79518ed..0281bc5f5 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -41,6 +41,9 @@ struct _virNWFilterDriverState { bool watchingFirewallD; }; =20 +void +virNWFilterObjEndAPI(virNWFilterObjPtr *obj); + virNWFilterDefPtr virNWFilterObjGetDef(virNWFilterObjPtr obj); =20 @@ -105,10 +108,4 @@ int virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters, const char *configDir); =20 -void -virNWFilterObjLock(virNWFilterObjPtr obj); - -void -virNWFilterObjUnlock(virNWFilterObjPtr obj); - #endif /* VIRNWFILTEROBJ_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index de4ec4d44..132244878 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1022,6 +1022,7 @@ virNodeDeviceObjListRemove; =20 =20 # conf/virnwfilterobj.h +virNWFilterObjEndAPI; virNWFilterObjGetDef; virNWFilterObjGetNewDef; virNWFilterObjListAssignDef; @@ -1035,9 +1036,7 @@ virNWFilterObjListLoadAllConfigs; virNWFilterObjListNew; virNWFilterObjListNumOfNWFilters; virNWFilterObjListRemove; -virNWFilterObjLock; virNWFilterObjTestUnassignDef; -virNWFilterObjUnlock; virNWFilterObjWantRemoved; =20 =20 diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 885dbcc28..b38740b15 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -400,7 +400,7 @@ nwfilterLookupByUUID(virConnectPtr conn, nwfilter =3D virGetNWFilter(conn, def->name, def->uuid); =20 cleanup: - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return nwfilter; } =20 @@ -430,7 +430,7 @@ nwfilterLookupByName(virConnectPtr conn, nwfilter =3D virGetNWFilter(conn, def->name, def->uuid); =20 cleanup: - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return nwfilter; } =20 @@ -517,6 +517,8 @@ nwfilterDefineXML(virConnectPtr conn, =20 if (virNWFilterSaveConfig(driver->configDir, objdef) < 0) { virNWFilterObjListRemove(driver->nwfilters, obj); + virObjectUnref(obj); + obj =3D NULL; goto cleanup; } =20 @@ -524,8 +526,7 @@ nwfilterDefineXML(virConnectPtr conn, =20 cleanup: virNWFilterDefFree(def); - if (obj) - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); =20 virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); @@ -563,12 +564,12 @@ nwfilterUndefine(virNWFilterPtr nwfilter) goto cleanup; =20 virNWFilterObjListRemove(driver->nwfilters, obj); + virObjectUnref(obj); obj =3D NULL; ret =3D 0; =20 cleanup: - if (obj) - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); =20 virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); @@ -601,7 +602,7 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, ret =3D virNWFilterDefFormat(def); =20 cleanup: - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return ret; } =20 diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter= _gentech_driver.c index 840d419bb..48d0e1769 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -316,7 +316,7 @@ virNWFilterInstReset(virNWFilterInstPtr inst) size_t i; =20 for (i =3D 0; i < inst->nfilters; i++) - virNWFilterObjUnlock(inst->filters[i]); + virNWFilterObjEndAPI(&inst->filters[i]); VIR_FREE(inst->filters); inst->nfilters =3D 0; =20 @@ -426,8 +426,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStateP= tr driver, if (ret < 0) virNWFilterInstReset(inst); virNWFilterHashTableFree(tmpvars); - if (obj) - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return ret; } =20 @@ -541,7 +540,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr fi= lter, =20 /* create a temporary hashmap for depth-first tree traversal */ if (!(tmpvars =3D virNWFilterCreateVarsFrom(inc->params, vars)= )) { - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return -1; } =20 @@ -565,7 +564,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr fi= lter, =20 virNWFilterHashTableFree(tmpvars); =20 - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); if (rc < 0) return -1; } @@ -839,7 +838,7 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverSta= tePtr driver, virNWFilterHashTableFree(vars1); =20 err_exit: - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); =20 VIR_FREE(str_ipaddr); VIR_FREE(str_macaddr); --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri May 3 15:37:52 2024 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 1512741776501371.52986404122703; Fri, 8 Dec 2017 06:02:56 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DCB56D9659; Fri, 8 Dec 2017 14:02:54 +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 8C81D92310; Fri, 8 Dec 2017 14:02:54 +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 4A4B61800FC6; Fri, 8 Dec 2017 14:02:54 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id vB8E2kbQ023005 for ; Fri, 8 Dec 2017 09:02:46 -0500 Received: by smtp.corp.redhat.com (Postfix) id D0DE918B28; Fri, 8 Dec 2017 14:02:46 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-134.phx2.redhat.com [10.3.116.134]) by smtp.corp.redhat.com (Postfix) with ESMTP id 85E0118B3B for ; Fri, 8 Dec 2017 14:02:46 +0000 (UTC) From: John Ferlan To: libvir-list@redhat.com Date: Fri, 8 Dec 2017 09:01:40 -0500 Message-Id: <20171208140141.11491-4-jferlan@redhat.com> In-Reply-To: <20171208140141.11491-1-jferlan@redhat.com> References: <20171208140141.11491-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v4 3/4] 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.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Fri, 08 Dec 2017 14:02:55 +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 6b4758656..a4e6a03d2 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 0281bc5f5..cabb42a71 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 b38740b15..b24f59379 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -369,11 +369,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 From nobody Fri May 3 15:37:52 2024 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 1512741795864658.8872476093255; Fri, 8 Dec 2017 06:03:15 -0800 (PST) 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 30861C0798FD; Fri, 8 Dec 2017 14:03:14 +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 08A5118B2F; Fri, 8 Dec 2017 14:03:14 +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 C0ADF410A9; Fri, 8 Dec 2017 14:03:13 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id vB8E2lEe023010 for ; Fri, 8 Dec 2017 09:02:47 -0500 Received: by smtp.corp.redhat.com (Postfix) id 3973A60C8A; Fri, 8 Dec 2017 14:02:47 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-134.phx2.redhat.com [10.3.116.134]) by smtp.corp.redhat.com (Postfix) with ESMTP id F3A5318B28 for ; Fri, 8 Dec 2017 14:02:46 +0000 (UTC) From: John Ferlan To: libvir-list@redhat.com Date: Fri, 8 Dec 2017 09:01:41 -0500 Message-Id: <20171208140141.11491-5-jferlan@redhat.com> In-Reply-To: <20171208140141.11491-1-jferlan@redhat.com> References: <20171208140141.11491-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v4 4/4] nwfilter: Remove need for nwfilterDriverLock in some API's 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.31]); Fri, 08 Dec 2017 14:03:14 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Now that nwfilters object list is self locking, it's no longer necessary to hold the driver level lock for certain API's. However, for the DefineXML, Undefine, and Reload processing keeping that lock ensures for serialization required in order to process the filter Instantiation properly. Signed-off-by: John Ferlan --- src/nwfilter/nwfilter_driver.c | 51 +++++++++++++++++---------------------= ---- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index b24f59379..5b7ba1fcd 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -295,6 +295,10 @@ nwfilterStateReload(void) /* shut down all threads -- they will be restarted if necessary */ virNWFilterLearnThreadsTerminate(true); =20 + /* Serialization of virNWFilterObjListLoadAllConfigs is extremely + * important as it relates to virNWFilterObjListFindInstantiateFilter + * processing via virNWFilterTriggerVMFilterRebuild that occurs during + * virNWFilterObjListAssignDef */ nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); @@ -385,11 +389,7 @@ nwfilterLookupByUUID(virConnectPtr conn, virNWFilterDefPtr def; virNWFilterPtr nwfilter =3D NULL; =20 - nwfilterDriverLock(); - obj =3D nwfilterObjFromNWFilter(uuid); - nwfilterDriverUnlock(); - - if (!obj) + if (!(obj =3D nwfilterObjFromNWFilter(uuid))) return NULL; def =3D virNWFilterObjGetDef(obj); =20 @@ -412,11 +412,7 @@ nwfilterLookupByName(virConnectPtr conn, virNWFilterDefPtr def; virNWFilterPtr nwfilter =3D NULL; =20 - nwfilterDriverLock(); - obj =3D virNWFilterObjListFindByName(driver->nwfilters, name); - nwfilterDriverUnlock(); - - if (!obj) { + if (!(obj =3D virNWFilterObjListFindByName(driver->nwfilters, name))) { virReportError(VIR_ERR_NO_NWFILTER, _("no nwfilter with matching name '%s'"), name); return NULL; @@ -450,17 +446,12 @@ nwfilterConnectListNWFilters(virConnectPtr conn, char **const names, int maxnames) { - int nnames; - if (virConnectListNWFiltersEnsureACL(conn) < 0) return -1; =20 - nwfilterDriverLock(); - nnames =3D virNWFilterObjListGetNames(driver->nwfilters, conn, - virConnectListNWFiltersCheckACL, - names, maxnames); - nwfilterDriverUnlock(); - return nnames; + return virNWFilterObjListGetNames(driver->nwfilters, conn, + virConnectListNWFiltersCheckACL, + names, maxnames); } =20 =20 @@ -469,19 +460,13 @@ nwfilterConnectListAllNWFilters(virConnectPtr conn, virNWFilterPtr **nwfilters, unsigned int flags) { - int ret; - virCheckFlags(0, -1); =20 if (virConnectListAllNWFiltersEnsureACL(conn) < 0) return -1; =20 - nwfilterDriverLock(); - ret =3D virNWFilterObjListExport(conn, driver->nwfilters, nwfilters, - virConnectListAllNWFiltersCheckACL); - nwfilterDriverUnlock(); - - return ret; + return virNWFilterObjListExport(conn, driver->nwfilters, nwfilters, + virConnectListAllNWFiltersCheckACL); } =20 static virNWFilterPtr @@ -499,6 +484,10 @@ nwfilterDefineXML(virConnectPtr conn, return NULL; } =20 + /* Serialization of *one* DefineXML consumer is extremely important + * as it relates to virNWFilterObjListFindInstantiateFilter processing + * via virNWFilterTriggerVMFilterRebuild that occurs during + * virNWFilterObjListAssignDef */ nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); @@ -541,6 +530,10 @@ nwfilterUndefine(virNWFilterPtr nwfilter) virNWFilterDefPtr def; int ret =3D -1; =20 + /* Serialization of *one* Undefine consumer is extremely important + * as it relates to virNWFilterObjListFindInstantiateFilter processing + * via virNWFilterTriggerVMFilterRebuild that occurs during + * virNWFilterObjTestUnassignDef */ nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); @@ -587,11 +580,7 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, =20 virCheckFlags(0, NULL); =20 - nwfilterDriverLock(); - obj =3D nwfilterObjFromNWFilter(nwfilter->uuid); - nwfilterDriverUnlock(); - - if (!obj) + if (!(obj =3D nwfilterObjFromNWFilter(nwfilter->uuid))) return NULL; def =3D virNWFilterObjGetDef(obj); =20 --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list