[libvirt] [PATCH v2 5/6] nwfilter: Convert virNWFilterObj to use virObjectLockable

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 <jferlan(a)redhat.com&gt; --- 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" #include "viralloc.h" -#include "viratomic.h" #include "virerror.h" #include "virfile.h" #include "virlog.h" @@ -34,15 +33,8 @@ VIR_LOG_INIT("conf.virnwfilterobj"); -static void -virNWFilterObjLock(virNWFilterObjPtr obj); - -static void -virNWFilterObjUnlock(virNWFilterObjPtr obj); - struct _virNWFilterObj { - virMutex lock; - int refs; + virObjectLockable parent; bool wantRemoved; @@ -68,12 +60,20 @@ struct _virNWFilterObjList { virHashTable *objsName; }; +static virClassPtr virNWFilterObjClass; static virClassPtr virNWFilterObjListClass; +static void virNWFilterObjDispose(void *opaque); static void virNWFilterObjListDispose(void *opaque); static int virNWFilterObjOnceInit(void) { + if (!(virNWFilterObjClass = virClassNew(virClassForObjectLockable(), + "virNWFilterObj", + sizeof(virNWFilterObj), + virNWFilterObjDispose))) + return -1; + if (!(virNWFilterObjListClass = virClassNew(virClassForObjectLockable(), "virNWFilterObjList", sizeof(virNWFilterObjList), @@ -91,18 +91,13 @@ virNWFilterObjNew(void) { virNWFilterObjPtr obj; - if (VIR_ALLOC(obj) < 0) + if (virNWFilterObjInitialize() < 0) return NULL; - if (virMutexInit(&obj->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot initialize mutex")); - VIR_FREE(obj); + if (!(obj = virObjectLockableNew(virNWFilterObjClass))) return NULL; - } - virNWFilterObjLock(obj); - virAtomicIntSet(&obj->refs, 1); + virObjectLock(obj); if (virMutexInit(&obj->instLock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -120,8 +115,8 @@ virNWFilterObjEndAPI(virNWFilterObjPtr *obj) if (!*obj) return; - virNWFilterObjUnlock(*obj); - virNWFilterObjUnref(*obj); + virObjectUnlock(*obj); + virObjectUnref(*obj); *obj = NULL; } @@ -158,7 +153,7 @@ virNWFilterObjTryLock(virNWFilterObjPtr obj) virMutexLock(&obj->instLock); do { - err = virMutexTryLock(&obj->lock); + err = virObjectTryLock(obj); if (err == 0) { /* We are the first, then just like virMutexLock and we * set our markers, instDepth = 1 and thisTID */ @@ -206,10 +201,10 @@ virNWFilterObjEndInstAPI(virNWFilterObjPtr *obj) * instLock which protects this code from ourselves. */ if (--(*obj)->instDepth == 0) { (*obj)->instTID = 0; - virNWFilterObjUnlock(*obj); + virObjectUnlock(*obj); } - virNWFilterObjUnref(*obj); + virObjectUnref(*obj); virMutexUnlock(&(*obj)->instLock); *obj = NULL; @@ -238,38 +233,15 @@ virNWFilterObjWantRemoved(virNWFilterObjPtr obj) static void -virNWFilterObjFree(virNWFilterObjPtr obj) +virNWFilterObjDispose(void *opaque) { + virNWFilterObjPtr obj = opaque; + if (!obj) return; 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 = virAtomicIntDecAndTest(&obj->refs))) - virNWFilterObjFree(obj); - return !lastRef; } @@ -290,16 +262,6 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) } -static void -virNWFilterObjListObjsFreeData(void *payload, - const void *name ATTRIBUTE_UNUSED) -{ - virNWFilterObjPtr obj = payload; - - virNWFilterObjUnref(obj); -} - - virNWFilterObjListPtr virNWFilterObjListNew(void) { @@ -311,12 +273,12 @@ virNWFilterObjListNew(void) if (!(nwfilters = virObjectLockableNew(virNWFilterObjListClass))) return NULL; - if (!(nwfilters->objs = virHashCreate(10, virNWFilterObjListObjsFreeData))) { + if (!(nwfilters->objs = virHashCreate(10, virObjectFreeHashData))) { virObjectUnref(nwfilters); return NULL; } - if (!(nwfilters->objsName = virHashCreate(10, virNWFilterObjListObjsFreeData))) { + if (!(nwfilters->objsName = virHashCreate(10, virObjectFreeHashData))) { virHashFree(nwfilters->objs); virObjectUnref(nwfilters); return NULL; @@ -338,14 +300,14 @@ virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters, def = obj->def; 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); } @@ -357,7 +319,7 @@ virNWFilterObjListFindByUUIDLocked(virNWFilterObjListPtr nwfilters, char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(uuid, uuidstr); - return virNWFilterObjRef(virHashLookup(nwfilters->objs, uuidstr)); + return virObjectRef(virHashLookup(nwfilters->objs, uuidstr)); } @@ -371,7 +333,7 @@ virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters, obj = virNWFilterObjListFindByUUIDLocked(nwfilters, uuid); virObjectUnlock(nwfilters); if (obj) - virNWFilterObjLock(obj); + virObjectLock(obj); return obj; } @@ -380,7 +342,7 @@ static virNWFilterObjPtr virNWFilterObjListFindByNameLocked(virNWFilterObjListPtr nwfilters, const char *name) { - return virNWFilterObjRef(virHashLookup(nwfilters->objsName, name)); + return virObjectRef(virHashLookup(nwfilters->objsName, name)); } @@ -394,7 +356,7 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters, obj = virNWFilterObjListFindByNameLocked(nwfilters, name); virObjectUnlock(nwfilters); if (obj) - virNWFilterObjLock(obj); + virObjectLock(obj); return obj; } @@ -432,7 +394,7 @@ virNWFilterObjListFindInstantiateFilter(virNWFilterObjListPtr nwfilters, if (virNWFilterObjWantRemoved(obj)) { virReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), filtername); - virNWFilterObjUnref(obj); + virObjectUnref(obj); return NULL; } @@ -443,7 +405,7 @@ virNWFilterObjListFindInstantiateFilter(virNWFilterObjListPtr nwfilters, filtername, obj->instDepth, (unsigned long long)obj->instTID, (unsigned long long)pthread_self()); - virNWFilterObjUnref(obj); + virObjectUnref(obj); return NULL; } @@ -560,7 +522,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, virObjectLock(nwfilters); if ((obj = virNWFilterObjListFindByUUIDLocked(nwfilters, def->uuid))) { - virNWFilterObjLock(obj); + virObjectLock(obj); objdef = obj->def; if (STRNEQ(def->name, objdef->name)) { @@ -576,7 +538,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, } else { if ((obj = virNWFilterObjListFindByNameLocked(nwfilters, def->name))) { - virNWFilterObjLock(obj); + virObjectLock(obj); objdef = obj->def; virUUIDFormat(objdef->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, @@ -597,7 +559,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, if ((obj = virNWFilterObjListFindByNameLocked(nwfilters, def->name))) { - virNWFilterObjLock(obj); + virObjectLock(obj); objdef = obj->def; if (virNWFilterDefEqual(def, objdef)) { virNWFilterDefFree(objdef); @@ -642,7 +604,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, virUUIDFormat(def->uuid, uuidstr); if (virHashAddEntry(nwfilters->objs, uuidstr, obj) < 0) goto error; - virNWFilterObjRef(obj); + virObjectRef(obj); if (virHashAddEntry(nwfilters->objsName, def->name, obj) < 0) { virHashRemoveEntry(nwfilters->objs, uuidstr); @@ -650,15 +612,15 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, } obj->def = def; - virNWFilterObjRef(obj); + virObjectRef(obj); cleanup: virObjectUnlock(nwfilters); return obj; 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); -virNWFilterObjPtr -virNWFilterObjRef(virNWFilterObjPtr obj); - -bool -virNWFilterObjUnref(virNWFilterObjPtr obj); - virNWFilterObjListPtr virNWFilterObjListNew(void); 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; @@ -2296,6 +2294,7 @@ virObjectLock; virObjectLockableNew; virObjectNew; virObjectRef; +virObjectTryLock; virObjectUnlock; virObjectUnref; 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, if (virNWFilterSaveConfig(driver->configDir, objdef) < 0) { virNWFilterObjListRemove(driver->nwfilters, obj); - virNWFilterObjUnref(obj); + virObjectUnref(obj); obj = NULL; goto cleanup; } @@ -564,7 +564,7 @@ nwfilterUndefine(virNWFilterPtr nwfilter) goto cleanup; virNWFilterObjListRemove(driver->nwfilters, obj); - virNWFilterObjUnref(obj); + virObjectUnref(obj); obj = NULL; ret = 0; 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) /** + * 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 = 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); +int +virObjectTryLock(void *lockableobj) + ATTRIBUTE_NONNULL(1); + void virObjectUnlock(void *lockableobj) ATTRIBUTE_NONNULL(1); -- 2.9.4