"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 "<rule>"'s and "<filterref>"'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 <jferlan@xxxxxxxxxx> --- src/conf/virnwfilterobj.c | 75 ++++++++++++++++++++++++++-------- src/conf/virnwfilterobj.h | 15 ++++--- src/libvirt_private.syms | 5 ++- src/nwfilter/nwfilter_driver.c | 13 +++--- src/nwfilter/nwfilter_gentech_driver.c | 11 +++-- 5 files changed, 80 insertions(+), 39 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index b21b570..99be59c 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -23,6 +23,7 @@ #include "datatypes.h" #include "viralloc.h" +#include "viratomic.h" #include "virerror.h" #include "virfile.h" #include "virlog.h" @@ -33,8 +34,15 @@ VIR_LOG_INIT("conf.virnwfilterobj"); +static void +virNWFilterObjLock(virNWFilterObjPtr obj); + +static void +virNWFilterObjUnlock(virNWFilterObjPtr obj); + struct _virNWFilterObj { virMutex lock; + int refs; bool wantRemoved; @@ -67,11 +75,24 @@ virNWFilterObjNew(virNWFilterDefPtr def) virNWFilterObjLock(obj); obj->def = def; + virAtomicIntSet(&obj->refs, 1); return obj; } +void +virNWFilterObjEndAPI(virNWFilterObjPtr *obj) +{ + if (!*obj) + return; + + virNWFilterObjUnlock(*obj); + virNWFilterObjUnref(*obj); + *obj = NULL; +} + + virNWFilterDefPtr virNWFilterObjGetDef(virNWFilterObjPtr obj) { @@ -109,12 +130,33 @@ virNWFilterObjFree(virNWFilterObjPtr obj) } +virNWFilterObjPtr +virNWFilterObjRef(virNWFilterObjPtr obj) +{ + if (obj) + virAtomicIntInc(&obj->refs); + return obj; +} + + +bool +virNWFilterObjUnref(virNWFilterObjPtr obj) +{ + bool lastRef; + if (obj) + return false; + if ((lastRef = virAtomicIntDecAndTest(&obj->refs))) + virNWFilterObjFree(obj); + return !lastRef; +} + + void virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) { size_t i; for (i = 0; i < nwfilters->count; i++) - virNWFilterObjFree(nwfilters->objs[i]); + virNWFilterObjUnref(nwfilters->objs[i]); VIR_FREE(nwfilters->objs); VIR_FREE(nwfilters); } @@ -143,7 +185,7 @@ virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters, virNWFilterObjLock(nwfilters->objs[i]); if (nwfilters->objs[i] == obj) { virNWFilterObjUnlock(nwfilters->objs[i]); - virNWFilterObjFree(nwfilters->objs[i]); + virNWFilterObjUnref(nwfilters->objs[i]); VIR_DELETE_ELEMENT(nwfilters->objs, i, nwfilters->count); break; @@ -166,7 +208,7 @@ virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters, virNWFilterObjLock(obj); def = obj->def; if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN)) - return obj; + return virNWFilterObjRef(obj); virNWFilterObjUnlock(obj); } @@ -187,7 +229,7 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters, virNWFilterObjLock(obj); def = obj->def; if (STREQ_NULLABLE(def->name, name)) - return obj; + return virNWFilterObjRef(obj); virNWFilterObjUnlock(obj); } @@ -210,7 +252,7 @@ virNWFilterObjListFindInstantiateFilter(virNWFilterObjListPtr nwfilters, if (virNWFilterObjWantRemoved(obj)) { virReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), filtername); - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return NULL; } @@ -245,7 +287,7 @@ _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters, if (obj) { rc = _virNWFilterObjListDefLoopDetect(nwfilters, obj->def, filtername); - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); if (rc < 0) break; } @@ -338,10 +380,10 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, _("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 = virNWFilterObjListFindByName(nwfilters, def->name))) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -351,7 +393,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, virReportError(VIR_ERR_OPERATION_FAILED, _("filter '%s' already exists with uuid %s"), def->name, uuidstr); - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return NULL; } } @@ -362,7 +404,6 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, return NULL; } - if ((obj = virNWFilterObjListFindByName(nwfilters, def->name))) { objdef = obj->def; @@ -376,7 +417,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, /* trigger the update on VMs referencing the filter */ if (virNWFilterTriggerVMFilterRebuild() < 0) { obj->newDef = NULL; - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return NULL; } @@ -397,12 +438,12 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, nwfilters->count, obj) < 0) goto error; - return obj; + return virNWFilterObjRef(obj); error: obj->def = NULL; virNWFilterObjUnlock(obj); - virNWFilterObjFree(obj); + virNWFilterObjUnref(obj); return NULL; } @@ -600,8 +641,8 @@ virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters, continue; obj = virNWFilterObjListLoadConfig(nwfilters, configDir, entry->d_name); - if (obj) - virNWFilterObjUnlock(obj); + + virNWFilterObjEndAPI(&obj); } VIR_DIR_CLOSE(dir); @@ -609,14 +650,14 @@ virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters, } -void +static void virNWFilterObjLock(virNWFilterObjPtr obj) { virMutexLock(&obj->lock); } -void +static void virNWFilterObjUnlock(virNWFilterObjPtr obj) { virMutexUnlock(&obj->lock); diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 85c8ead..31aa345 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -41,6 +41,9 @@ struct _virNWFilterDriverState { bool watchingFirewallD; }; +void +virNWFilterObjEndAPI(virNWFilterObjPtr *obj); + virNWFilterDefPtr virNWFilterObjGetDef(virNWFilterObjPtr obj); @@ -50,6 +53,12 @@ virNWFilterObjGetNewDef(virNWFilterObjPtr obj); bool virNWFilterObjWantRemoved(virNWFilterObjPtr obj); +virNWFilterObjPtr +virNWFilterObjRef(virNWFilterObjPtr obj); + +bool +virNWFilterObjUnref(virNWFilterObjPtr obj); + virNWFilterObjListPtr virNWFilterObjListNew(void); @@ -109,10 +118,4 @@ int virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters, const char *configDir); -void -virNWFilterObjLock(virNWFilterObjPtr obj); - -void -virNWFilterObjUnlock(virNWFilterObjPtr obj); - #endif /* VIRNWFILTEROBJ_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ee19cb9..ac0507e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -965,6 +965,7 @@ virNodeDeviceObjUnlock; # conf/virnwfilterobj.h +virNWFilterObjEndAPI; virNWFilterObjGetDef; virNWFilterObjGetNewDef; virNWFilterObjListAssignDef; @@ -978,10 +979,10 @@ virNWFilterObjListLoadAllConfigs; virNWFilterObjListNew; virNWFilterObjListNumOfNWFilters; virNWFilterObjListRemove; -virNWFilterObjLock; +virNWFilterObjRef; virNWFilterObjSaveConfig; virNWFilterObjTestUnassignDef; -virNWFilterObjUnlock; +virNWFilterObjUnref; virNWFilterObjWantRemoved; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 7db4f87..b6e11e6 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -400,7 +400,7 @@ nwfilterLookupByUUID(virConnectPtr conn, nwfilter = virGetNWFilter(conn, def->name, def->uuid); cleanup: - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return nwfilter; } @@ -430,7 +430,7 @@ nwfilterLookupByName(virConnectPtr conn, nwfilter = virGetNWFilter(conn, def->name, def->uuid); cleanup: - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return nwfilter; } @@ -525,8 +525,7 @@ nwfilterDefineXML(virConnectPtr conn, cleanup: virNWFilterDefFree(def); - if (obj) - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); @@ -564,12 +563,10 @@ nwfilterUndefine(virNWFilterPtr nwfilter) goto cleanup; virNWFilterObjListRemove(driver->nwfilters, obj); - obj = NULL; ret = 0; cleanup: - if (obj) - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); @@ -602,7 +599,7 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, ret = virNWFilterDefFormat(def); cleanup: - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return ret; } diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 68a2ddb..4c228ea 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -315,7 +315,7 @@ virNWFilterInstReset(virNWFilterInstPtr inst) size_t i; for (i = 0; i < inst->nfilters; i++) - virNWFilterObjUnlock(inst->filters[i]); + virNWFilterObjEndAPI(&inst->filters[i]); VIR_FREE(inst->filters); inst->nfilters = 0; @@ -425,8 +425,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStatePtr driver, if (ret < 0) virNWFilterInstReset(inst); virNWFilterHashTableFree(tmpvars); - if (obj) - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); return ret; } @@ -547,7 +546,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, vars); if (!tmpvars) { rc = -1; - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); break; } @@ -571,7 +570,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, virNWFilterHashTableFree(tmpvars); - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); if (rc < 0) break; } @@ -845,7 +844,7 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver, virNWFilterHashTableFree(vars1); err_exit: - virNWFilterObjUnlock(obj); + virNWFilterObjEndAPI(&obj); VIR_FREE(str_ipaddr); VIR_FREE(str_macaddr); -- 2.9.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list