Re: [libvirt] [PATCH v4 4/4] nwfilter: Remove need for nwfilterDriverLock in some API's

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 <jferlan(a)redhat.com&gt; --- 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); + /* 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 = NULL; - nwfilterDriverLock(); - obj = nwfilterObjFromNWFilter(uuid); - nwfilterDriverUnlock(); - - if (!obj) + if (!(obj = nwfilterObjFromNWFilter(uuid))) return NULL; def = virNWFilterObjGetDef(obj); @@ -412,11 +412,7 @@ nwfilterLookupByName(virConnectPtr conn, virNWFilterDefPtr def; virNWFilterPtr nwfilter = NULL; - nwfilterDriverLock(); - obj = virNWFilterObjListFindByName(driver->nwfilters, name); - nwfilterDriverUnlock(); - - if (!obj) { + if (!(obj = 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; - nwfilterDriverLock(); - nnames = virNWFilterObjListGetNames(driver->nwfilters, conn, - virConnectListNWFiltersCheckACL, - names, maxnames); - nwfilterDriverUnlock(); - return nnames; + return virNWFilterObjListGetNames(driver->nwfilters, conn, + virConnectListNWFiltersCheckACL, + names, maxnames); } @@ -469,19 +460,13 @@ nwfilterConnectListAllNWFilters(virConnectPtr conn, virNWFilterPtr **nwfilters, unsigned int flags) { - int ret; - virCheckFlags(0, -1); if (virConnectListAllNWFiltersEnsureACL(conn) < 0) return -1; - nwfilterDriverLock(); - ret = virNWFilterObjListExport(conn, driver->nwfilters, nwfilters, - virConnectListAllNWFiltersCheckACL); - nwfilterDriverUnlock(); - - return ret; + return virNWFilterObjListExport(conn, driver->nwfilters, nwfilters, + virConnectListAllNWFiltersCheckACL); } static virNWFilterPtr @@ -499,6 +484,10 @@ nwfilterDefineXML(virConnectPtr conn, return NULL; } + /* 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 = -1; + /* 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, virCheckFlags(0, NULL); - nwfilterDriverLock(); - obj = nwfilterObjFromNWFilter(nwfilter->uuid); - nwfilterDriverUnlock(); - - if (!obj) + if (!(obj = nwfilterObjFromNWFilter(nwfilter->uuid))) return NULL; def = virNWFilterObjGetDef(obj);