[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[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 redhat com>
---
 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]