Create a common API to handle the instantiation path filter lookup. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list