3:57 p.m.
"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(a)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