[libvirt] [PATCH v2 1/6] nwfilter: Add @refs logic to __virNWFilterObj

John Ferlan jferlan at redhat.com
Tue Jul 18 20:57:45 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         | 15 +++----
 src/nwfilter/nwfilter_gentech_driver.c | 11 +++--
 5 files changed, 83 insertions(+), 38 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index 93072be..ecc7653 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;
 
@@ -64,10 +72,24 @@ virNWFilterObjNew(void)
     }
 
     virNWFilterObjLock(obj);
+    virAtomicIntSet(&obj->refs, 1);
+
     return obj;
 }
 
 
+void
+virNWFilterObjEndAPI(virNWFilterObjPtr *obj)
+{
+    if (!*obj)
+        return;
+
+    virNWFilterObjUnlock(*obj);
+    virNWFilterObjUnref(*obj);
+    *obj = NULL;
+}
+
+
 virNWFilterDefPtr
 virNWFilterObjGetDef(virNWFilterObjPtr obj)
 {
@@ -104,12 +126,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);
 }
@@ -138,7 +181,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;
@@ -161,7 +204,7 @@ virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters,
         virNWFilterObjLock(obj);
         def = obj->def;
         if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN))
-            return obj;
+            return virNWFilterObjRef(obj);
         virNWFilterObjUnlock(obj);
     }
 
@@ -182,7 +225,7 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters,
         virNWFilterObjLock(obj);
         def = obj->def;
         if (STREQ_NULLABLE(def->name, name))
-            return obj;
+            return virNWFilterObjRef(obj);
         virNWFilterObjUnlock(obj);
     }
 
@@ -205,8 +248,7 @@ virNWFilterObjListFindInstantiateFilter(virNWFilterObjListPtr nwfilters,
     if (virNWFilterObjWantRemoved(obj)) {
         virReportError(VIR_ERR_NO_NWFILTER,
                        _("Filter '%s' is in use."), filtername);
-        virNWFilterObjUnlock(obj);
-        return NULL;
+        virNWFilterObjEndAPI(&obj);
     }
 
     return obj;
@@ -240,7 +282,7 @@ _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters,
             if (obj) {
                 rc = _virNWFilterObjListDefLoopDetect(nwfilters, obj->def,
                                                       filtername);
-                virNWFilterObjUnlock(obj);
+                virNWFilterObjEndAPI(&obj);
                 if (rc < 0)
                     break;
             }
@@ -332,10 +374,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];
@@ -345,7 +387,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;
         }
     }
@@ -356,7 +398,6 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
         return NULL;
     }
 
-
     if ((obj = virNWFilterObjListFindByName(nwfilters, def->name))) {
 
         objdef = obj->def;
@@ -370,7 +411,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
         /* trigger the update on VMs referencing the filter */
         if (virNWFilterTriggerVMFilterRebuild() < 0) {
             obj->newDef = NULL;
-            virNWFilterObjUnlock(obj);
+            virNWFilterObjEndAPI(&obj);
             return NULL;
         }
 
@@ -391,7 +432,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
     }
     obj->def = def;
 
-    return obj;
+    return virNWFilterObjRef(obj);
 }
 
 
@@ -561,8 +602,8 @@ virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters,
             continue;
 
         obj = virNWFilterObjListLoadConfig(nwfilters, configDir, entry->d_name);
-        if (obj)
-            virNWFilterObjUnlock(obj);
+
+        virNWFilterObjEndAPI(&obj);
     }
 
     VIR_DIR_CLOSE(dir);
@@ -570,14 +611,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 509e8dc..a78d8cd 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);
 
@@ -105,10 +114,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 187b12b..41531b5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -980,6 +980,7 @@ virNodeDeviceObjListRemove;
 
 
 # conf/virnwfilterobj.h
+virNWFilterObjEndAPI;
 virNWFilterObjGetDef;
 virNWFilterObjGetNewDef;
 virNWFilterObjListAssignDef;
@@ -993,9 +994,9 @@ virNWFilterObjListLoadAllConfigs;
 virNWFilterObjListNew;
 virNWFilterObjListNumOfNWFilters;
 virNWFilterObjListRemove;
-virNWFilterObjLock;
+virNWFilterObjRef;
 virNWFilterObjTestUnassignDef;
-virNWFilterObjUnlock;
+virNWFilterObjUnref;
 virNWFilterObjWantRemoved;
 
 
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 2f9a51c..cb7b330 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;
 }
 
@@ -517,6 +517,8 @@ nwfilterDefineXML(virConnectPtr conn,
 
     if (virNWFilterSaveConfig(driver->configDir, objdef) < 0) {
         virNWFilterObjListRemove(driver->nwfilters, obj);
+        virNWFilterObjUnref(obj);
+        obj = NULL;
         goto cleanup;
     }
 
@@ -524,8 +526,7 @@ nwfilterDefineXML(virConnectPtr conn,
 
  cleanup:
     virNWFilterDefFree(def);
-    if (obj)
-        virNWFilterObjUnlock(obj);
+    virNWFilterObjEndAPI(&obj);
 
     virNWFilterCallbackDriversUnlock();
     virNWFilterUnlockFilterUpdates();
@@ -563,12 +564,12 @@ nwfilterUndefine(virNWFilterPtr nwfilter)
         goto cleanup;
 
     virNWFilterObjListRemove(driver->nwfilters, obj);
+    virNWFilterObjUnref(obj);
     obj = NULL;
     ret = 0;
 
  cleanup:
-    if (obj)
-        virNWFilterObjUnlock(obj);
+    virNWFilterObjEndAPI(&obj);
 
     virNWFilterCallbackDriversUnlock();
     virNWFilterUnlockFilterUpdates();
@@ -601,7 +602,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 6758200..608cfbc 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -316,7 +316,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;
 
@@ -426,8 +426,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStatePtr driver,
     if (ret < 0)
         virNWFilterInstReset(inst);
     virNWFilterHashTableFree(tmpvars);
-    if (obj)
-        virNWFilterObjUnlock(obj);
+    virNWFilterObjEndAPI(&obj);
     return ret;
 }
 
@@ -545,7 +544,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
             /* create a temporary hashmap for depth-first tree traversal */
             if (!(tmpvars = virNWFilterCreateVarsFrom(inc->params, vars))) {
                 rc = -1;
-                virNWFilterObjUnlock(obj);
+                virNWFilterObjEndAPI(&obj);
                 break;
             }
 
@@ -569,7 +568,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
 
             virNWFilterHashTableFree(tmpvars);
 
-            virNWFilterObjUnlock(obj);
+            virNWFilterObjEndAPI(&obj);
             if (rc < 0)
                 break;
         }
@@ -843,7 +842,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