[libvirt] [PATCH 14/17] nwfilter: Add @refs logic to __virNWFilterObj

John Ferlan jferlan at redhat.com
Fri Jun 2 10:25:35 UTC 2017


"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 at redhat.com>
---
 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




More information about the libvir-list mailing list