[libvirt] [PATCH 13/17] nwfilter: Introduce virNWFilterObjListFindInstantiateFilter

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


Create a common API to handle the instantiation path filter lookup.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/virnwfilterobj.c              |  23 +++++++
 src/conf/virnwfilterobj.h              |   4 ++
 src/libvirt_private.syms               |   1 +
 src/nwfilter/nwfilter_gentech_driver.c | 119 ++++++++++++---------------------
 4 files changed, 70 insertions(+), 77 deletions(-)

diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index c86b1a9..b21b570 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -195,6 +195,29 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters,
 }
 
 
+virNWFilterObjPtr
+virNWFilterObjListFindInstantiateFilter(virNWFilterObjListPtr nwfilters,
+                                        const char *filtername)
+{
+    virNWFilterObjPtr obj;
+
+    if (!(obj = virNWFilterObjListFindByName(nwfilters, filtername))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("referenced filter '%s' is missing"), filtername);
+        return NULL;
+    }
+
+    if (virNWFilterObjWantRemoved(obj)) {
+        virReportError(VIR_ERR_NO_NWFILTER,
+                       _("Filter '%s' is in use."), filtername);
+        virNWFilterObjUnlock(obj);
+        return NULL;
+    }
+
+    return obj;
+}
+
+
 static int
 _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters,
                                  virNWFilterDefPtr def,
diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index e4dadda..85c8ead 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -69,6 +69,10 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters,
                              const char *name);
 
 virNWFilterObjPtr
+virNWFilterObjListFindInstantiateFilter(virNWFilterObjListPtr nwfilters,
+                                        const char *filtername);
+
+virNWFilterObjPtr
 virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
                             virNWFilterDefPtr def,
                             const char *configDir);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cda5f92..ee19cb9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -971,6 +971,7 @@ virNWFilterObjListAssignDef;
 virNWFilterObjListExport;
 virNWFilterObjListFindByName;
 virNWFilterObjListFindByUUID;
+virNWFilterObjListFindInstantiateFilter;
 virNWFilterObjListFree;
 virNWFilterObjListGetNames;
 virNWFilterObjListLoadAllConfigs;
diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index 5798371..68a2ddb 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -60,11 +60,11 @@ static virNWFilterTechDriverPtr filter_tech_drivers[] = {
  * to avoid lock ordering deadlocks. eg virNWFilterInstantiateFilterUpdate
  * will hold a lock on a virNWFilterObjPtr. This in turn invokes
  * virNWFilterDoInstantiate which invokes virNWFilterDetermineMissingVarsRec
- * which invokes virNWFilterObjListFindByName. This iterates over every single
- * virNWFilterObjPtr in the list. So if 2 threads try to instantiate a
- * filter in parallel, they'll both hold 1 lock at the top level in
- * virNWFilterInstantiateFilterUpdate which will cause the other thread
- * to deadlock in virNWFilterObjListFindByName.
+ * which invokes virNWFilterObjListFindInstantiateFilter. This iterates over
+ * every single virNWFilterObjPtr in the list. So if 2 threads try to
+ * instantiate a filter in parallel, they'll both hold 1 lock at the top level
+ * in virNWFilterInstantiateFilterUpdate which will cause the other thread
+ * to deadlock in virNWFilterObjListFindInstantiateFilter.
  *
  * XXX better long term solution is to make virNWFilterObjList use a
  * hash table as is done for virDomainObjList. You can then get
@@ -383,20 +383,9 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStatePtr driver,
     int ret = -1;
 
     VIR_DEBUG("Instantiating filter %s", inc->filterref);
-    obj = virNWFilterObjListFindByName(driver->nwfilters,
-                                       inc->filterref);
-    if (!obj) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("referenced filter '%s' is missing"),
-                       inc->filterref);
-        goto cleanup;
-    }
-    if (virNWFilterObjWantRemoved(obj)) {
-        virReportError(VIR_ERR_NO_NWFILTER,
-                       _("Filter '%s' is in use."),
-                       inc->filterref);
+    if (!(obj = virNWFilterObjListFindInstantiateFilter(driver->nwfilters,
+                                                        inc->filterref)))
         goto cleanup;
-    }
 
     /* create a temporary hashmap for depth-first tree traversal */
     if (!(tmpvars = virNWFilterCreateVarsFrom(inc->params,
@@ -545,58 +534,46 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
                 break;
         } else if (inc) {
             VIR_DEBUG("Following filter %s", inc->filterref);
-            obj = virNWFilterObjListFindByName(driver->nwfilters, inc->filterref);
-            if (obj) {
-
-                if (virNWFilterObjWantRemoved(obj)) {
-                    virReportError(VIR_ERR_NO_NWFILTER,
-                                   _("Filter '%s' is in use."),
-                                   inc->filterref);
-                    rc = -1;
-                    virNWFilterObjUnlock(obj);
-                    break;
-                }
+            if (!(obj =
+                  virNWFilterObjListFindInstantiateFilter(driver->nwfilters,
+                                                          inc->filterref))) {
+                rc = -1;
+                break;
+            }
 
-                /* create a temporary hashmap for depth-first tree traversal */
-                virNWFilterHashTablePtr tmpvars =
-                                      virNWFilterCreateVarsFrom(inc->params,
-                                                                vars);
-                if (!tmpvars) {
-                    rc = -1;
-                    virNWFilterObjUnlock(obj);
-                    break;
-                }
+            /* create a temporary hashmap for depth-first tree traversal */
+            virNWFilterHashTablePtr tmpvars =
+                                  virNWFilterCreateVarsFrom(inc->params,
+                                                            vars);
+            if (!tmpvars) {
+                rc = -1;
+                virNWFilterObjUnlock(obj);
+                break;
+            }
 
-                next_filter = virNWFilterObjGetDef(obj);
+            next_filter = virNWFilterObjGetDef(obj);
 
-                switch (useNewFilter) {
-                case INSTANTIATE_FOLLOW_NEWFILTER:
-                    newNext_filter = virNWFilterObjGetNewDef(obj);
-                    if (newNext_filter)
-                        next_filter = newNext_filter;
-                    break;
-                case INSTANTIATE_ALWAYS:
-                    break;
-                }
+            switch (useNewFilter) {
+            case INSTANTIATE_FOLLOW_NEWFILTER:
+                newNext_filter = virNWFilterObjGetNewDef(obj);
+                if (newNext_filter)
+                    next_filter = newNext_filter;
+                break;
+            case INSTANTIATE_ALWAYS:
+                break;
+            }
 
-                rc = virNWFilterDetermineMissingVarsRec(next_filter,
-                                                        tmpvars,
-                                                        missing_vars,
-                                                        useNewFilter,
-                                                        driver);
+            rc = virNWFilterDetermineMissingVarsRec(next_filter,
+                                                    tmpvars,
+                                                    missing_vars,
+                                                    useNewFilter,
+                                                    driver);
 
-                virNWFilterHashTableFree(tmpvars);
+            virNWFilterHashTableFree(tmpvars);
 
-                virNWFilterObjUnlock(obj);
-                if (rc < 0)
-                    break;
-            } else {
-                virReportError(VIR_ERR_INTERNAL_ERROR,
-                               _("referenced filter '%s' is missing"),
-                               inc->filterref);
-                rc = -1;
+            virNWFilterObjUnlock(obj);
+            if (rc < 0)
                 break;
-            }
         }
     }
     return rc;
@@ -813,21 +790,9 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
 
     VIR_DEBUG("filter name: %s", filtername);
 
-    obj = virNWFilterObjListFindByName(driver->nwfilters, filtername);
-    if (!obj) {
-        virReportError(VIR_ERR_NO_NWFILTER,
-                       _("Could not find filter '%s'"),
-                       filtername);
+    if (!(obj = virNWFilterObjListFindInstantiateFilter(driver->nwfilters,
+                                                        filtername)))
         return -1;
-    }
-
-    if (virNWFilterObjWantRemoved(obj)) {
-        virReportError(VIR_ERR_NO_NWFILTER,
-                       _("Filter '%s' is in use."),
-                       filtername);
-        rc = -1;
-        goto err_exit;
-    }
 
     virMacAddrFormat(macaddr, vmmacaddr);
     if (VIR_STRDUP(str_macaddr, vmmacaddr) < 0) {
-- 
2.9.4




More information about the libvir-list mailing list