From nobody Sun May 5 04:21:44 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 1500411693018351.60331845555413; Tue, 18 Jul 2017 14:01:33 -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 953EE80481; Tue, 18 Jul 2017 21:01:29 +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 35E5A19E30; Tue, 18 Jul 2017 21:01:29 +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 E2C093FAE1; Tue, 18 Jul 2017 21:01:28 +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 v6IKvwd1019248 for ; Tue, 18 Jul 2017 16:57:58 -0400 Received: by smtp.corp.redhat.com (Postfix) id 3A3FB7A24A; Tue, 18 Jul 2017 20:57:58 +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 DFB2D17538 for ; Tue, 18 Jul 2017 20:57:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 953EE80481 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=pass smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 953EE80481 From: John Ferlan To: libvir-list@redhat.com Date: Tue, 18 Jul 2017 16:57:45 -0400 Message-Id: <20170718205750.14503-2-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 1/6] nwfilter: Add @refs logic to __virNWFilterObj 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]); Tue, 18 Jul 2017 21:01:30 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" "Simple" conversion to the virObjectLockable object isn't quite possible yet because the nwfilter lock requires usage of recursive locking due to algorithms handing filter ""'s and ""'s. The list search logic would also benefit from using hash tables for lookups. So this patch is step 1 in the process - add the @refs to _virNWFilterObj and modify the algorithms to use (a temporary) virNWFilterObj{Ref|Unref} in order to set things up for the list logic to utilize virObjectLockable hash tables. Signed-off-by: John Ferlan --- src/conf/virnwfilterobj.c | 75 ++++++++++++++++++++++++++----= ---- src/conf/virnwfilterobj.h | 15 ++++--- src/libvirt_private.syms | 5 ++- src/nwfilter/nwfilter_driver.c | 15 +++---- src/nwfilter/nwfilter_gentech_driver.c | 11 +++-- 5 files changed, 83 insertions(+), 38 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 93072be..ecc7653 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -23,6 +23,7 @@ #include "datatypes.h" =20 #include "viralloc.h" +#include "viratomic.h" #include "virerror.h" #include "virfile.h" #include "virlog.h" @@ -33,8 +34,15 @@ =20 VIR_LOG_INIT("conf.virnwfilterobj"); =20 +static void +virNWFilterObjLock(virNWFilterObjPtr obj); + +static void +virNWFilterObjUnlock(virNWFilterObjPtr obj); + struct _virNWFilterObj { virMutex lock; + int refs; =20 bool wantRemoved; =20 @@ -64,10 +72,24 @@ virNWFilterObjNew(void) } =20 virNWFilterObjLock(obj); + virAtomicIntSet(&obj->refs, 1); + return obj; } =20 =20 +void +virNWFilterObjEndAPI(virNWFilterObjPtr *obj) +{ + if (!*obj) + return; + + virNWFilterObjUnlock(*obj); + virNWFilterObjUnref(*obj); + *obj =3D NULL; +} + + virNWFilterDefPtr virNWFilterObjGetDef(virNWFilterObjPtr obj) { @@ -104,12 +126,33 @@ virNWFilterObjFree(virNWFilterObjPtr obj) } =20 =20 +virNWFilterObjPtr +virNWFilterObjRef(virNWFilterObjPtr obj) +{ + if (obj) + virAtomicIntInc(&obj->refs); + return obj; +} + + +bool +virNWFilterObjUnref(virNWFilterObjPtr obj) +{ + bool lastRef; + if (!obj) + return false; + if ((lastRef =3D virAtomicIntDecAndTest(&obj->refs))) + virNWFilterObjFree(obj); + return !lastRef; +} + + void virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) { size_t i; for (i =3D 0; i < nwfilters->count; i++) - virNWFilterObjFree(nwfilters->objs[i]); + virNWFilterObjUnref(nwfilters->objs[i]); VIR_FREE(nwfilters->objs); VIR_FREE(nwfilters); } @@ -138,7 +181,7 @@ virNWFilterObjListRemove(virNWFilterObjListPtr nwfilter= s, virNWFilterObjLock(nwfilters->objs[i]); if (nwfilters->objs[i] =3D=3D obj) { virNWFilterObjUnlock(nwfilters->objs[i]); - virNWFilterObjFree(nwfilters->objs[i]); + virNWFilterObjUnref(nwfilters->objs[i]); =20 VIR_DELETE_ELEMENT(nwfilters->objs, i, nwfilters->count); break; @@ -161,7 +204,7 @@ virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfi= lters, virNWFilterObjLock(obj); def =3D obj->def; if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN)) - return obj; + return virNWFilterObjRef(obj); virNWFilterObjUnlock(obj); } =20 @@ -182,7 +225,7 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nwfi= lters, virNWFilterObjLock(obj); def =3D obj->def; if (STREQ_NULLABLE(def->name, name)) - return obj; + return virNWFilterObjRef(obj); virNWFilterObjUnlock(obj); } =20 @@ -205,8 +248,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 +282,7 @@ _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr = nwfilters, if (obj) { rc =3D _virNWFilterObjListDefLoopDetect(nwfilters, obj->de= f, filtername); - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); if (rc < 0) break; } @@ -332,10 +374,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]; @@ -345,7 +387,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; } } @@ -356,7 +398,6 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, return NULL; } =20 - if ((obj =3D virNWFilterObjListFindByName(nwfilters, def->name))) { =20 objdef =3D obj->def; @@ -370,7 +411,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, /* trigger the update on VMs referencing the filter */ if (virNWFilterTriggerVMFilterRebuild() < 0) { obj->newDef =3D NULL; - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return NULL; } =20 @@ -391,7 +432,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, } obj->def =3D def; =20 - return obj; + return virNWFilterObjRef(obj); } =20 =20 @@ -561,8 +602,8 @@ virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr = nwfilters, continue; =20 obj =3D virNWFilterObjListLoadConfig(nwfilters, configDir, entry->= d_name); - if (obj) - virNWFilterObjUnlock(obj); + + virNWFilterObjEndAPI(&obj); } =20 VIR_DIR_CLOSE(dir); @@ -570,14 +611,14 @@ virNWFilterObjListLoadAllConfigs(virNWFilterObjListPt= r nwfilters, } =20 =20 -void +static void virNWFilterObjLock(virNWFilterObjPtr obj) { virMutexLock(&obj->lock); } =20 =20 -void +static void virNWFilterObjUnlock(virNWFilterObjPtr obj) { virMutexUnlock(&obj->lock); diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 509e8dc..a78d8cd 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 @@ -50,6 +53,12 @@ virNWFilterObjGetNewDef(virNWFilterObjPtr obj); bool virNWFilterObjWantRemoved(virNWFilterObjPtr obj); =20 +virNWFilterObjPtr +virNWFilterObjRef(virNWFilterObjPtr obj); + +bool +virNWFilterObjUnref(virNWFilterObjPtr obj); + virNWFilterObjListPtr virNWFilterObjListNew(void); =20 @@ -105,10 +114,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 187b12b..41531b5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -980,6 +980,7 @@ virNodeDeviceObjListRemove; =20 =20 # conf/virnwfilterobj.h +virNWFilterObjEndAPI; virNWFilterObjGetDef; virNWFilterObjGetNewDef; virNWFilterObjListAssignDef; @@ -993,9 +994,9 @@ virNWFilterObjListLoadAllConfigs; virNWFilterObjListNew; virNWFilterObjListNumOfNWFilters; virNWFilterObjListRemove; -virNWFilterObjLock; +virNWFilterObjRef; virNWFilterObjTestUnassignDef; -virNWFilterObjUnlock; +virNWFilterObjUnref; virNWFilterObjWantRemoved; =20 =20 diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 2f9a51c..cb7b330 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); + virNWFilterObjUnref(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); + virNWFilterObjUnref(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 6758200..608cfbc 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 @@ -545,7 +544,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr fi= lter, /* create a temporary hashmap for depth-first tree traversal */ if (!(tmpvars =3D virNWFilterCreateVarsFrom(inc->params, vars)= )) { rc =3D -1; - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); break; } =20 @@ -569,7 +568,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr fi= lter, =20 virNWFilterHashTableFree(tmpvars); =20 - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); if (rc < 0) break; } @@ -843,7 +842,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.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Sun May 5 04:21:44 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 1500411701155160.14651629893888; Tue, 18 Jul 2017 14:01:41 -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 B36C17CE04; Tue, 18 Jul 2017 21:01:37 +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 4BD5E61F52; Tue, 18 Jul 2017 21:01:37 +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 D2AB33FAE3; Tue, 18 Jul 2017 21:01:36 +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 v6IKvwBo019261 for ; Tue, 18 Jul 2017 16:57:58 -0400 Received: by smtp.corp.redhat.com (Postfix) id A30277A24A; Tue, 18 Jul 2017 20:57:58 +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 688A717538 for ; Tue, 18 Jul 2017 20:57:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B36C17CE04 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=pass smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com B36C17CE04 From: John Ferlan To: libvir-list@redhat.com Date: Tue, 18 Jul 2017 16:57:46 -0400 Message-Id: <20170718205750.14503-3-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 2/6] 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.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 18 Jul 2017 21:01:38 +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 --- 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 ecc7653..1e8fd36 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -329,18 +329,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))) @@ -349,9 +342,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 @@ -401,7 +391,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.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Sun May 5 04:21:44 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 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 From nobody Sun May 5 04:21:44 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 1500411708077899.7170727277352; Tue, 18 Jul 2017 14:01:48 -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 072387D0DB; Tue, 18 Jul 2017 21:01:45 +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 D21FB5B812; Tue, 18 Jul 2017 21:01:44 +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 68FC91853E39; Tue, 18 Jul 2017 21:01:44 +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 v6IKw1NV019288 for ; Tue, 18 Jul 2017 16:58:01 -0400 Received: by smtp.corp.redhat.com (Postfix) id 852A417538; Tue, 18 Jul 2017 20:58:01 +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 3984C7A253 for ; Tue, 18 Jul 2017 20:58:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 072387D0DB Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.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 072387D0DB From: John Ferlan To: libvir-list@redhat.com Date: Tue, 18 Jul 2017 16:57:48 -0400 Message-Id: <20170718205750.14503-5-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 4/6] nwfilter: Remove recursive locking for nwfilter instantiation 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.27]); Tue, 18 Jul 2017 21:01:45 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The current algorithm required usage of recursive locks because there was no other mechanism available to ensure that the object wasn't deleted whilst the instantiation was taking place. Unfortunately common objects (virObjectLockable) do not allow for recursive locks; however, since this is a very specific use case, let's roll our own version by using pthread_mutex_trylock and handle the return status based on what the API tells us. In order to do this, introduce a virMutexTryLock call and modify the virNWFilterObjListFindInstantiateFilter in order to consume and check the returned status. For starters let's only ever worry about the good status (e.g. we were the first to get the lock) and the pseudo recursive view of the world that our current thread was the thread that tried to get the lock when EBUSY is returned. An EBUSY is returned when either this thread or some thread has the lock. This involves keeping track of which thread was able to take out the first lock and if it's the same as the current thread, then allow the lock to succeed; otherwise, we'll need to retry as some other thread is currently holding the lock. Still we don't want that retry to last forever, so in order to avoid potential deadlock if two threads are doing a define at the same time, add retry lock that lasts for about 2 seconds which should be plenty of time.For any other errors, cause failure for the calling thread. Modify the callers to use a new virNWFilterObjEndInstAPI which handles the comparison of the number of try locks that were successful for the thread and only truly unlocks the object resource when the last lock is removed. Additionally to avoid the possibility that we do something untoward to ourselves and because we are not taking out the higher level lock, introduce an instLock to ensure that changes to the saved depth and tid are single threaded. Signed-off-by: John Ferlan --- src/conf/virnwfilterobj.c | 156 +++++++++++++++++++++++++++++= +++- src/conf/virnwfilterobj.h | 3 + src/libvirt_private.syms | 2 + src/nwfilter/nwfilter_gentech_driver.c | 65 ++++++++++---- src/util/virthread.c | 5 ++ src/util/virthread.h | 1 + 6 files changed, 211 insertions(+), 21 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 5d34851..d4fa98b 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -48,6 +48,12 @@ struct _virNWFilterObj { =20 virNWFilterDefPtr def; virNWFilterDefPtr newDef; + + /* Instantiation locking */ + virMutex instLock; + int instDepth; + int instRetry; + pthread_t instTID; }; =20 struct _virNWFilterObjList { @@ -88,7 +94,7 @@ virNWFilterObjNew(void) if (VIR_ALLOC(obj) < 0) return NULL; =20 - if (virMutexInitRecursive(&obj->lock) < 0) { + if (virMutexInit(&obj->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize mutex")); VIR_FREE(obj); @@ -98,6 +104,12 @@ virNWFilterObjNew(void) virNWFilterObjLock(obj); virAtomicIntSet(&obj->refs, 1); =20 + if (virMutexInit(&obj->instLock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize instantiation mutex")); + virNWFilterObjEndAPI(&obj); + } + return obj; } =20 @@ -114,6 +126,96 @@ virNWFilterObjEndAPI(virNWFilterObjPtr *obj) } =20 =20 +/** + * virNWFilterObjTryLock(obj) + * virNWFilterObjEndInstAPI(&obj) + * @obj: nwfilter object + * + * Rather than use recursive locks, nwfilter instantiation uses a + * modified version that will use NORMAL mutex locks except for the + * locking mechanism in virNWFilterObjListFindInstantiateFilter which + * uses virMutexTryLock processing to lock the object "recursively" + * for this thread only. + * + * In order to do this "safely", the processing of the TryLock and EndInst= API + * will be protected by an "instLock" so that we can track which thread by + * pthread id has the lock and how many levels of depth have been taken wh= en + * the lock is taken. + * + * This way when we go to release the lock (EndInstAPI) we can check our l= evel + * and once the last lock has been released, call virNWFilterObjUnlock to + * release the lock. If we called it too soon, then we'd potentially relea= se + * a lock we still need. + * + * Returns 0 on success and an errno value on failure + */ +static int +virNWFilterObjTryLock(virNWFilterObjPtr obj) +{ + int err; + pthread_t thisTID =3D pthread_self(); + + virMutexLock(&obj->instLock); + + do { + err =3D virMutexTryLock(&obj->lock); + if (err =3D=3D 0) { + /* We are the first, then just like virMutexLock and we + * set our markers, instDepth =3D 1 and thisTID */ + obj->instDepth =3D 1; + obj->instRetry =3D 0; + obj->instTID =3D thisTID; + goto cleanup; + } else if (err =3D=3D EBUSY) { + /* EBUSY indicates this thread or some other thread owns the l= ock + * If it's us, then excellent, similar to recursion. + * Else it's some other thread, let's retry for a bit until + * it is us. If we cannot get it, then avoid deadlock */ + if (obj->instTID =3D=3D thisTID) { + obj->instDepth++; + obj->instRetry =3D 0; + err =3D 0; + goto cleanup; + } + if (++obj->instRetry > 20) + goto cleanup; + usleep(100 * 1000); /* Give the owner a chance */ + } else { + /* Don't handle other errors - return failure */ + goto cleanup; + } + } while (1); + + cleanup: + virMutexUnlock(&obj->instLock); + return err; +} + + +void +virNWFilterObjEndInstAPI(virNWFilterObjPtr *obj) +{ + if (!*obj) + return; + + virMutexLock(&(*obj)->instLock); + + /* Once the last locker has called this EndInstAPI function, then + * we can clear the instTID and really release the Lock; otherwise, + * we'll just decrement the Refcnt for the object and Unlock our + * instLock which protects this code from ourselves. */ + if (--(*obj)->instDepth =3D=3D 0) { + (*obj)->instTID =3D 0; + virNWFilterObjUnlock(*obj); + } + + virNWFilterObjUnref(*obj); + virMutexUnlock(&(*obj)->instLock); + + *obj =3D NULL; +} + + virNWFilterDefPtr virNWFilterObjGetDef(virNWFilterObjPtr obj) { @@ -298,13 +400,30 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nw= filters, } =20 =20 +/** + * To avoid the need to have recursive locks as a result of how the + * virNWFilterInstantiateFilter processing works, this API uses the + * virNWFilterObjTryLock processing in order to perform the pseudo + * recursive locking operation. + * + * NB: Use virNWFilterObjListFindByNameLocked since when called via + * virNWFilterObjListAssignDef path, the lock will already be held. + * For other callers, the single threaded driver level locking via + * virNWFilterWriteLockFilterUpdates ensure that the AssignDef, + * Undefine, and Reload will provide protection that the list cannot + * be adjusted whilst we're processing. Additionally the driver has + * a virNWFilterCallbackDriversLock which gets set to ensure other + * consumers of the NWFilter data cannot be called whilst the + * AssignDef, Undefine, or Reload occurs. + */ virNWFilterObjPtr virNWFilterObjListFindInstantiateFilter(virNWFilterObjListPtr nwfilters, const char *filtername) { virNWFilterObjPtr obj; + int err; =20 - if (!(obj =3D virNWFilterObjListFindByName(nwfilters, filtername))) { + if (!(obj =3D virNWFilterObjListFindByNameLocked(nwfilters, filtername= ))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("referenced filter '%s' is missing"), filtername); return NULL; @@ -313,7 +432,19 @@ virNWFilterObjListFindInstantiateFilter(virNWFilterObj= ListPtr nwfilters, if (virNWFilterObjWantRemoved(obj)) { virReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), filtername); - virNWFilterObjEndAPI(&obj); + virNWFilterObjUnref(obj); + return NULL; + } + + if ((err =3D virNWFilterObjTryLock(obj)) !=3D 0) { + virReportSystemError(err, + _("Unable to get mutex for '%s' depth=3D%d " + "tid=3D%llu mytid=3D%llu"), + filtername, obj->instDepth, + (unsigned long long)obj->instTID, + (unsigned long long)pthread_self()); + virNWFilterObjUnref(obj); + return NULL; } =20 return obj; @@ -424,6 +555,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, virNWFilterObjPtr obj; virNWFilterDefPtr objdef; char uuidstr[VIR_UUID_STRING_BUFLEN]; + int rebuild_ret; =20 virObjectLock(nwfilters); =20 @@ -474,8 +606,24 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfi= lters, } =20 obj->newDef =3D def; + + /* Since we have the obj lock, update the instTID + * and instDepth because the Rebuild will trigger + * the driver to start calling FindInstantiateFilter, + * which uses the TryLock processing in order to + * acquire and release locks. */ + obj->instTID =3D pthread_self(); + obj->instDepth =3D 1; + /* trigger the update on VMs referencing the filter */ - if (virNWFilterTriggerVMFilterRebuild() < 0) { + rebuild_ret =3D virNWFilterTriggerVMFilterRebuild(); + + /* We're done, we still hold the original lock, let's reset + * the instTID and instDepth for the next consumer */ + obj->instTID =3D 0; + obj->instDepth =3D 0; + + if (rebuild_ret < 0) { obj->newDef =3D NULL; virNWFilterObjEndAPI(&obj); virObjectUnlock(nwfilters); diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index a78d8cd..9f738fe 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -44,6 +44,9 @@ struct _virNWFilterDriverState { void virNWFilterObjEndAPI(virNWFilterObjPtr *obj); =20 +void +virNWFilterObjEndInstAPI(virNWFilterObjPtr *obj); + virNWFilterDefPtr virNWFilterObjGetDef(virNWFilterObjPtr obj); =20 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 41531b5..3b9640d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -981,6 +981,7 @@ virNodeDeviceObjListRemove; =20 # conf/virnwfilterobj.h virNWFilterObjEndAPI; +virNWFilterObjEndInstAPI; virNWFilterObjGetDef; virNWFilterObjGetNewDef; virNWFilterObjListAssignDef; @@ -2725,6 +2726,7 @@ virMutexDestroy; virMutexInit; virMutexInitRecursive; virMutexLock; +virMutexTryLock; virMutexUnlock; virOnce; virRWLockDestroy; diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter= _gentech_driver.c index 608cfbc..0f7f026 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -57,19 +57,50 @@ static virNWFilterTechDriverPtr filter_tech_drivers[] = =3D { NULL }; =20 -/* Serializes instantiation of filters. This is necessary - * to avoid lock ordering deadlocks. eg virNWFilterInstantiateFilterUpdate - * will hold a lock on a virNWFilterObjPtr. This in turn invokes - * virNWFilterDoInstantiate which invokes virNWFilterDetermineMissingVarsR= ec - * which invokes virNWFilterObjListFindInstantiateFilter. This iterates ov= er - * every single virNWFilterObjPtr in the list. So if 2 threads try to - * instantiate a filter in parallel, they'll both hold 1 lock at the top l= evel - * in virNWFilterInstantiateFilterUpdate which will cause the other thread - * to deadlock in virNWFilterObjListFindInstantiateFilter. +/* NB: Upon return from virNWFilterObjListFindInstantiateFilter the nwfilt= er + * object will be locked via an NWFilter only call to virObjectTryLock as a + * way to implement recursive locking, but without requiring the usage of + * recursive lock for the object. This mechanism allows the nwfilter object + * to use the common virLockableObject API's rather than having to have + * recursive mutexes and lock/ref API's for the majority of calls while + * leaving the very special case of instantiation to be handled via its + * own recursive methodolgy (all handled in the nwfilter object code). * - * XXX better long term solution is to make virNWFilterObjList use a - * hash table as is done for virDomainObjList. You can then get - * lockless lookup of objects by name. + * The virNWFilterObjListFindInstantiateFilter consumers are a complicated + * set of API's that are serialized via the updateMutex. For some consumer= s, + * a simple/shared read lock will suffice, while others will require the + * write lock. Serialization is also handled via a pair of driver mutexes + * virNWFilterWriteLockFilterUpdates and virNWFilterCallbackDriversLock. + * + * Processing of the instantiation code can be triggered via a direct + * nwfilterInstantiateFilter call or it may be triggered via a driver + * callback mechanism. The implementation is an interesting compilation of + * recursively called API's in order to handle the filter objects @def + * elements and which define the ordering and usage + * of the filters. + * + * Since virNWFilterObjListFindInstantiateFilter provides a single entry + * reference point, it was modified to use the TryLock processing to + * check if the attempt to get the lock was being done by the same thread + * that originally obtained the lock. The corollary for the instantiation + * lock processing is usage of virNWFilterObjEndInstAPI when done with + * the object instead of virNWFilterObjEndAPI. The virNWFilterObjEndInstAPI + * must be used for the nwfilter lock obtained as a result of the call to + * virNWFilterObjListFindInstantiateFilter. + * + * XXX + * Some day perhaps the "matrix" of recursive callers and ways to get ones= elf + * very lost in this code will be "fixed" to be more orderly. For example, + * during virNWFilterDefToInst processing an object is fetched and placed + * into the inst->filters lookaside list during virNWFilterIncludeDefToRul= eInst + * processing which then recursively calls virNWFilterDefToInst. The object + * is removed during virNWFilterInstReset, but in the mean time it's also + * possible to call virNWFilterDoInstantiate that has a completely separate + * path to calling virNWFilterObjListFindInstantiateFilter via the call to + * virNWFilterDetermineMissingVarsRec. This tangled web of interconnected + * callers also inludes virNWFilterInstantiateFilterUpdate as well. Each + * of these callers will use virNWFilterObjEndInstAPI in order to undo the + * lock and reference taken. */ static virMutex updateMutex; =20 @@ -316,7 +347,7 @@ virNWFilterInstReset(virNWFilterInstPtr inst) size_t i; =20 for (i =3D 0; i < inst->nfilters; i++) - virNWFilterObjEndAPI(&inst->filters[i]); + virNWFilterObjEndInstAPI(&inst->filters[i]); VIR_FREE(inst->filters); inst->nfilters =3D 0; =20 @@ -426,7 +457,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStateP= tr driver, if (ret < 0) virNWFilterInstReset(inst); virNWFilterHashTableFree(tmpvars); - virNWFilterObjEndAPI(&obj); + virNWFilterObjEndInstAPI(&obj); return ret; } =20 @@ -544,7 +575,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr fi= lter, /* create a temporary hashmap for depth-first tree traversal */ if (!(tmpvars =3D virNWFilterCreateVarsFrom(inc->params, vars)= )) { rc =3D -1; - virNWFilterObjEndAPI(&obj); + virNWFilterObjEndInstAPI(&obj); break; } =20 @@ -568,7 +599,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr fi= lter, =20 virNWFilterHashTableFree(tmpvars); =20 - virNWFilterObjEndAPI(&obj); + virNWFilterObjEndInstAPI(&obj); if (rc < 0) break; } @@ -842,7 +873,7 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverSta= tePtr driver, virNWFilterHashTableFree(vars1); =20 err_exit: - virNWFilterObjEndAPI(&obj); + virNWFilterObjEndInstAPI(&obj); =20 VIR_FREE(str_ipaddr); VIR_FREE(str_macaddr); diff --git a/src/util/virthread.c b/src/util/virthread.c index 6c49515..1da6606 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -84,6 +84,11 @@ void virMutexDestroy(virMutexPtr m) pthread_mutex_destroy(&m->lock); } =20 +int virMutexTryLock(virMutexPtr m) +{ + return pthread_mutex_trylock(&m->lock); +} + void virMutexLock(virMutexPtr m) { pthread_mutex_lock(&m->lock); diff --git a/src/util/virthread.h b/src/util/virthread.h index e466d9b..03c9fd6 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -132,6 +132,7 @@ int virMutexInitRecursive(virMutexPtr m) ATTRIBUTE_RETU= RN_CHECK; void virMutexDestroy(virMutexPtr m); =20 void virMutexLock(virMutexPtr m); +int virMutexTryLock(virMutexPtr m); void virMutexUnlock(virMutexPtr m); =20 =20 --=20 2.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Sun May 5 04:21:44 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 1500411712860502.3059075469811; Tue, 18 Jul 2017 14:01:52 -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 E704DC058EB3; Tue, 18 Jul 2017 21:01:49 +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 AF2F019EFF; Tue, 18 Jul 2017 21:01:49 +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 4ED9C3FAE4; Tue, 18 Jul 2017 21:01:49 +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 v6IKw4Eb019306 for ; Tue, 18 Jul 2017 16:58:04 -0400 Received: by smtp.corp.redhat.com (Postfix) id 89D487A2EC; Tue, 18 Jul 2017 20:58:04 +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 401B47A24A for ; Tue, 18 Jul 2017 20:58:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E704DC058EB3 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.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 E704DC058EB3 From: John Ferlan To: libvir-list@redhat.com Date: Tue, 18 Jul 2017 16:57:49 -0400 Message-Id: <20170718205750.14503-6-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 5/6] nwfilter: Convert virNWFilterObj to use 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.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Tue, 18 Jul 2017 21:01:50 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Now that we have a bit more control, let's convert our object into a lockable object and let that magic handle the create, lock/unlock, and reference counting. Because we have the need for instantiation in a recursive manner, introduce the virObjectTryLock to handle the virNWFilterObjTryLock processing. It'll just be the shim into virMutexTryLock, but adds an extra error value EFAULT for when the incoming @obj is not determined to be a LockableObject. Signed-off-by: John Ferlan --- src/conf/virnwfilterobj.c | 132 +++++++++++++------------------------= ---- src/conf/virnwfilterobj.h | 6 -- src/libvirt_private.syms | 3 +- src/nwfilter/nwfilter_driver.c | 4 +- src/util/virobject.c | 24 ++++++++ src/util/virobject.h | 4 ++ 6 files changed, 71 insertions(+), 102 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index d4fa98b..4792f9a 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -23,7 +23,6 @@ #include "datatypes.h" =20 #include "viralloc.h" -#include "viratomic.h" #include "virerror.h" #include "virfile.h" #include "virlog.h" @@ -34,15 +33,8 @@ =20 VIR_LOG_INIT("conf.virnwfilterobj"); =20 -static void -virNWFilterObjLock(virNWFilterObjPtr obj); - -static void -virNWFilterObjUnlock(virNWFilterObjPtr obj); - struct _virNWFilterObj { - virMutex lock; - int refs; + virObjectLockable parent; =20 bool wantRemoved; =20 @@ -68,12 +60,20 @@ struct _virNWFilterObjList { virHashTable *objsName; }; =20 +static virClassPtr virNWFilterObjClass; static virClassPtr virNWFilterObjListClass; +static void virNWFilterObjDispose(void *opaque); static void virNWFilterObjListDispose(void *opaque); =20 static int virNWFilterObjOnceInit(void) { + if (!(virNWFilterObjClass =3D virClassNew(virClassForObjectLockable(), + "virNWFilterObj", + sizeof(virNWFilterObj), + virNWFilterObjDispose))) + return -1; + if (!(virNWFilterObjListClass =3D virClassNew(virClassForObjectLockabl= e(), "virNWFilterObjList", sizeof(virNWFilterObjList), @@ -91,18 +91,13 @@ virNWFilterObjNew(void) { virNWFilterObjPtr obj; =20 - if (VIR_ALLOC(obj) < 0) + if (virNWFilterObjInitialize() < 0) return NULL; =20 - if (virMutexInit(&obj->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot initialize mutex")); - VIR_FREE(obj); + if (!(obj =3D virObjectLockableNew(virNWFilterObjClass))) return NULL; - } =20 - virNWFilterObjLock(obj); - virAtomicIntSet(&obj->refs, 1); + virObjectLock(obj); =20 if (virMutexInit(&obj->instLock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -120,8 +115,8 @@ virNWFilterObjEndAPI(virNWFilterObjPtr *obj) if (!*obj) return; =20 - virNWFilterObjUnlock(*obj); - virNWFilterObjUnref(*obj); + virObjectUnlock(*obj); + virObjectUnref(*obj); *obj =3D NULL; } =20 @@ -158,7 +153,7 @@ virNWFilterObjTryLock(virNWFilterObjPtr obj) virMutexLock(&obj->instLock); =20 do { - err =3D virMutexTryLock(&obj->lock); + err =3D virObjectTryLock(obj); if (err =3D=3D 0) { /* We are the first, then just like virMutexLock and we * set our markers, instDepth =3D 1 and thisTID */ @@ -206,10 +201,10 @@ virNWFilterObjEndInstAPI(virNWFilterObjPtr *obj) * instLock which protects this code from ourselves. */ if (--(*obj)->instDepth =3D=3D 0) { (*obj)->instTID =3D 0; - virNWFilterObjUnlock(*obj); + virObjectUnlock(*obj); } =20 - virNWFilterObjUnref(*obj); + virObjectUnref(*obj); virMutexUnlock(&(*obj)->instLock); =20 *obj =3D NULL; @@ -238,38 +233,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); -} - - -virNWFilterObjPtr -virNWFilterObjRef(virNWFilterObjPtr obj) -{ - if (obj) - virAtomicIntInc(&obj->refs); - return obj; -} - - -bool -virNWFilterObjUnref(virNWFilterObjPtr obj) -{ - bool lastRef; - if (!obj) - return false; - if ((lastRef =3D virAtomicIntDecAndTest(&obj->refs))) - virNWFilterObjFree(obj); - return !lastRef; } =20 =20 @@ -290,16 +262,6 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) } =20 =20 -static void -virNWFilterObjListObjsFreeData(void *payload, - const void *name ATTRIBUTE_UNUSED) -{ - virNWFilterObjPtr obj =3D payload; - - virNWFilterObjUnref(obj); -} - - virNWFilterObjListPtr virNWFilterObjListNew(void) { @@ -311,12 +273,12 @@ virNWFilterObjListNew(void) if (!(nwfilters =3D virObjectLockableNew(virNWFilterObjListClass))) return NULL; =20 - if (!(nwfilters->objs =3D virHashCreate(10, virNWFilterObjListObjsFree= Data))) { + if (!(nwfilters->objs =3D virHashCreate(10, virObjectFreeHashData))) { virObjectUnref(nwfilters); return NULL; } =20 - if (!(nwfilters->objsName =3D virHashCreate(10, virNWFilterObjListObjs= FreeData))) { + if (!(nwfilters->objsName =3D virHashCreate(10, virObjectFreeHashData)= )) { virHashFree(nwfilters->objs); virObjectUnref(nwfilters); return NULL; @@ -338,14 +300,14 @@ virNWFilterObjListRemove(virNWFilterObjListPtr nwfilt= ers, def =3D obj->def; =20 virUUIDFormat(def->uuid, uuidstr); - virNWFilterObjRef(obj); - virNWFilterObjUnlock(obj); + virObjectRef(obj); + virObjectUnlock(obj); virObjectLock(nwfilters); - virNWFilterObjLock(obj); + virObjectLock(obj); virHashRemoveEntry(nwfilters->objs, uuidstr); virHashRemoveEntry(nwfilters->objsName, def->name); - virNWFilterObjUnlock(obj); - virNWFilterObjUnref(obj); + virObjectUnlock(obj); + virObjectUnref(obj); virObjectUnlock(nwfilters); } =20 @@ -357,7 +319,7 @@ virNWFilterObjListFindByUUIDLocked(virNWFilterObjListPt= r nwfilters, char uuidstr[VIR_UUID_STRING_BUFLEN]; =20 virUUIDFormat(uuid, uuidstr); - return virNWFilterObjRef(virHashLookup(nwfilters->objs, uuidstr)); + return virObjectRef(virHashLookup(nwfilters->objs, uuidstr)); } =20 =20 @@ -371,7 +333,7 @@ virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfi= lters, obj =3D virNWFilterObjListFindByUUIDLocked(nwfilters, uuid); virObjectUnlock(nwfilters); if (obj) - virNWFilterObjLock(obj); + virObjectLock(obj); return obj; } =20 @@ -380,7 +342,7 @@ static virNWFilterObjPtr virNWFilterObjListFindByNameLocked(virNWFilterObjListPtr nwfilters, const char *name) { - return virNWFilterObjRef(virHashLookup(nwfilters->objsName, name)); + return virObjectRef(virHashLookup(nwfilters->objsName, name)); } =20 =20 @@ -394,7 +356,7 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nwfi= lters, obj =3D virNWFilterObjListFindByNameLocked(nwfilters, name); virObjectUnlock(nwfilters); if (obj) - virNWFilterObjLock(obj); + virObjectLock(obj); =20 return obj; } @@ -432,7 +394,7 @@ virNWFilterObjListFindInstantiateFilter(virNWFilterObjL= istPtr nwfilters, if (virNWFilterObjWantRemoved(obj)) { virReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), filtername); - virNWFilterObjUnref(obj); + virObjectUnref(obj); return NULL; } =20 @@ -443,7 +405,7 @@ virNWFilterObjListFindInstantiateFilter(virNWFilterObjL= istPtr nwfilters, filtername, obj->instDepth, (unsigned long long)obj->instTID, (unsigned long long)pthread_self()); - virNWFilterObjUnref(obj); + virObjectUnref(obj); return NULL; } =20 @@ -560,7 +522,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, virObjectLock(nwfilters); =20 if ((obj =3D virNWFilterObjListFindByUUIDLocked(nwfilters, def->uuid))= ) { - virNWFilterObjLock(obj); + virObjectLock(obj); objdef =3D obj->def; =20 if (STRNEQ(def->name, objdef->name)) { @@ -576,7 +538,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, } else { if ((obj =3D virNWFilterObjListFindByNameLocked(nwfilters, def->na= me))) { =20 - virNWFilterObjLock(obj); + virObjectLock(obj); objdef =3D obj->def; virUUIDFormat(objdef->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, @@ -597,7 +559,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, =20 =20 if ((obj =3D virNWFilterObjListFindByNameLocked(nwfilters, def->name))= ) { - virNWFilterObjLock(obj); + virObjectLock(obj); objdef =3D obj->def; if (virNWFilterDefEqual(def, objdef)) { virNWFilterDefFree(objdef); @@ -642,7 +604,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfil= ters, virUUIDFormat(def->uuid, uuidstr); if (virHashAddEntry(nwfilters->objs, uuidstr, obj) < 0) goto error; - virNWFilterObjRef(obj); + virObjectRef(obj); =20 if (virHashAddEntry(nwfilters->objsName, def->name, obj) < 0) { virHashRemoveEntry(nwfilters->objs, uuidstr); @@ -650,15 +612,15 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwf= ilters, } =20 obj->def =3D def; - virNWFilterObjRef(obj); + virObjectRef(obj); =20 cleanup: virObjectUnlock(nwfilters); return obj; =20 error: - virNWFilterObjUnlock(obj); - virNWFilterObjUnref(obj); + virObjectUnlock(obj); + virObjectUnref(obj); virObjectUnlock(nwfilters); return NULL; } @@ -917,17 +879,3 @@ virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr= nwfilters, VIR_DIR_CLOSE(dir); return ret; } - - -static void -virNWFilterObjLock(virNWFilterObjPtr obj) -{ - virMutexLock(&obj->lock); -} - - -static void -virNWFilterObjUnlock(virNWFilterObjPtr obj) -{ - virMutexUnlock(&obj->lock); -} diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 9f738fe..73bb9b9 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -56,12 +56,6 @@ virNWFilterObjGetNewDef(virNWFilterObjPtr obj); bool virNWFilterObjWantRemoved(virNWFilterObjPtr obj); =20 -virNWFilterObjPtr -virNWFilterObjRef(virNWFilterObjPtr obj); - -bool -virNWFilterObjUnref(virNWFilterObjPtr obj); - virNWFilterObjListPtr virNWFilterObjListNew(void); =20 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3b9640d..c10960d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -995,9 +995,7 @@ virNWFilterObjListLoadAllConfigs; virNWFilterObjListNew; virNWFilterObjListNumOfNWFilters; virNWFilterObjListRemove; -virNWFilterObjRef; virNWFilterObjTestUnassignDef; -virNWFilterObjUnref; virNWFilterObjWantRemoved; =20 =20 @@ -2296,6 +2294,7 @@ virObjectLock; virObjectLockableNew; virObjectNew; virObjectRef; +virObjectTryLock; virObjectUnlock; virObjectUnref; =20 diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index cb7b330..a83f5cf 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -517,7 +517,7 @@ nwfilterDefineXML(virConnectPtr conn, =20 if (virNWFilterSaveConfig(driver->configDir, objdef) < 0) { virNWFilterObjListRemove(driver->nwfilters, obj); - virNWFilterObjUnref(obj); + virObjectUnref(obj); obj =3D NULL; goto cleanup; } @@ -564,7 +564,7 @@ nwfilterUndefine(virNWFilterPtr nwfilter) goto cleanup; =20 virNWFilterObjListRemove(driver->nwfilters, obj); - virNWFilterObjUnref(obj); + virObjectUnref(obj); obj =3D NULL; ret =3D 0; =20 diff --git a/src/util/virobject.c b/src/util/virobject.c index 34805d3..3ced1b4 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -350,6 +350,30 @@ virObjectLock(void *anyobj) =20 =20 /** + * virObjectTryLock: + * @anyobj: any instance of virObjectLockablePtr + * + * Acquire a lock on @anyobj. The lock must be + * released by virObjectUnlock. + * + * The caller is expected to have acquired a reference + * on the object before locking it (eg virObjectRef). + * The object must be unlocked before releasing this + * reference. + */ +int +virObjectTryLock(void *anyobj) +{ + virObjectLockablePtr obj =3D virObjectGetLockableObj(anyobj); + + if (!obj) + return EFAULT; + + return virMutexTryLock(&obj->lock); +} + + +/** * virObjectUnlock: * @anyobj: any instance of virObjectLockablePtr * diff --git a/src/util/virobject.h b/src/util/virobject.h index f4c292b..452b6b2 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -112,6 +112,10 @@ void virObjectLock(void *lockableobj) ATTRIBUTE_NONNULL(1); =20 +int +virObjectTryLock(void *lockableobj) + ATTRIBUTE_NONNULL(1); + void virObjectUnlock(void *lockableobj) ATTRIBUTE_NONNULL(1); --=20 2.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Sun May 5 04:21:44 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 1500411699909508.7157097745808; Tue, 18 Jul 2017 14:01:39 -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 00F8D7CB9E; Tue, 18 Jul 2017 21:01:36 +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 C190D19E30; Tue, 18 Jul 2017 21:01:35 +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 546681853E34; Tue, 18 Jul 2017 21:01:35 +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 v6IKw5aS019311 for ; Tue, 18 Jul 2017 16:58:05 -0400 Received: by smtp.corp.redhat.com (Postfix) id 008BF7A24A; Tue, 18 Jul 2017 20:58:05 +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 B63647A2EC for ; Tue, 18 Jul 2017 20:58:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 00F8D7CB9E 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=pass smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 00F8D7CB9E From: John Ferlan To: libvir-list@redhat.com Date: Tue, 18 Jul 2017 16:57:50 -0400 Message-Id: <20170718205750.14503-7-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 6/6] 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.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 18 Jul 2017 21:01:36 +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 a83f5cf..fba2c79 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(); @@ -386,11 +390,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 @@ -413,11 +413,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; @@ -451,17 +447,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 @@ -470,19 +461,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 @@ -500,6 +485,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(); @@ -542,6 +531,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(); @@ -588,11 +581,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.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list