On 06/02/2017 12:25 PM, John Ferlan wrote:
Create a common API to handle the instantiation path filter lookup.
Signed-off-by: John Ferlan <jferlan@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))) {
No, please move the function call onto the same line as if(!(obj =. 83 chars long line a not EOW (as we know it)
+ rc = -1; + break; + }
- /* create a temporary hashmap for depth-first tree traversal */ - virNWFilterHashTablePtr tmpvars = - virNWFilterCreateVarsFrom(inc->params, - vars);
Nor 84 chars long line.
- if (!tmpvars) { - rc = -1; - virNWFilterObjUnlock(obj); - break; - }
ACK if you fix it. Michal