5:25 a.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 | 13 +++---
src/nwfilter/nwfilter_gentech_driver.c | 11 +++--
5 files changed, 80 insertions(+), 39 deletions(-)
diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index b21b570..99be59c 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;
@@ -67,11 +75,24 @@ virNWFilterObjNew(virNWFilterDefPtr def)
virNWFilterObjLock(obj);
obj->def = def;
+ virAtomicIntSet(&obj->refs, 1);
return obj;
}
+void
+virNWFilterObjEndAPI(virNWFilterObjPtr *obj)
+{
+ if (!*obj)
+ return;
+
+ virNWFilterObjUnlock(*obj);
+ virNWFilterObjUnref(*obj);
+ *obj = NULL;
+}
+
+
virNWFilterDefPtr
virNWFilterObjGetDef(virNWFilterObjPtr obj)
{
@@ -109,12 +130,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);
}
@@ -143,7 +185,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;
@@ -166,7 +208,7 @@ virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters,
virNWFilterObjLock(obj);
def = obj->def;
if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN))
- return obj;
+ return virNWFilterObjRef(obj);
virNWFilterObjUnlock(obj);
}
@@ -187,7 +229,7 @@ virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters,
virNWFilterObjLock(obj);
def = obj->def;
if (STREQ_NULLABLE(def->name, name))
- return obj;
+ return virNWFilterObjRef(obj);
virNWFilterObjUnlock(obj);
}
@@ -210,7 +252,7 @@ virNWFilterObjListFindInstantiateFilter(virNWFilterObjListPtr
nwfilters,
if (virNWFilterObjWantRemoved(obj)) {
virReportError(VIR_ERR_NO_NWFILTER,
_("Filter '%s' is in use."), filtername);
- virNWFilterObjUnlock(obj);
+ virNWFilterObjEndAPI(&obj);
return NULL;
}
@@ -245,7 +287,7 @@ _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters,
if (obj) {
rc = _virNWFilterObjListDefLoopDetect(nwfilters, obj->def,
filtername);
- virNWFilterObjUnlock(obj);
+ virNWFilterObjEndAPI(&obj);
if (rc < 0)
break;
}
@@ -338,10 +380,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];
@@ -351,7 +393,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;
}
}
@@ -362,7 +404,6 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
return NULL;
}
-
if ((obj = virNWFilterObjListFindByName(nwfilters, def->name))) {
objdef = obj->def;
@@ -376,7 +417,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
/* trigger the update on VMs referencing the filter */
if (virNWFilterTriggerVMFilterRebuild() < 0) {
obj->newDef = NULL;
- virNWFilterObjUnlock(obj);
+ virNWFilterObjEndAPI(&obj);
return NULL;
}
@@ -397,12 +438,12 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, nwfilters->count, obj) < 0)
goto error;
- return obj;
+ return virNWFilterObjRef(obj);
error:
obj->def = NULL;
virNWFilterObjUnlock(obj);
- virNWFilterObjFree(obj);
+ virNWFilterObjUnref(obj);
return NULL;
}
@@ -600,8 +641,8 @@ virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters,
continue;
obj = virNWFilterObjListLoadConfig(nwfilters, configDir, entry->d_name);
- if (obj)
- virNWFilterObjUnlock(obj);
+
+ virNWFilterObjEndAPI(&obj);
}
VIR_DIR_CLOSE(dir);
@@ -609,14 +650,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 85c8ead..31aa345 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);
@@ -109,10 +118,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 ee19cb9..ac0507e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -965,6 +965,7 @@ virNodeDeviceObjUnlock;
# conf/virnwfilterobj.h
+virNWFilterObjEndAPI;
virNWFilterObjGetDef;
virNWFilterObjGetNewDef;
virNWFilterObjListAssignDef;
@@ -978,10 +979,10 @@ virNWFilterObjListLoadAllConfigs;
virNWFilterObjListNew;
virNWFilterObjListNumOfNWFilters;
virNWFilterObjListRemove;
-virNWFilterObjLock;
+virNWFilterObjRef;
virNWFilterObjSaveConfig;
virNWFilterObjTestUnassignDef;
-virNWFilterObjUnlock;
+virNWFilterObjUnref;
virNWFilterObjWantRemoved;
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 7db4f87..b6e11e6 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;
}
@@ -525,8 +525,7 @@ nwfilterDefineXML(virConnectPtr conn,
cleanup:
virNWFilterDefFree(def);
- if (obj)
- virNWFilterObjUnlock(obj);
+ virNWFilterObjEndAPI(&obj);
virNWFilterCallbackDriversUnlock();
virNWFilterUnlockFilterUpdates();
@@ -564,12 +563,10 @@ nwfilterUndefine(virNWFilterPtr nwfilter)
goto cleanup;
virNWFilterObjListRemove(driver->nwfilters, obj);
- obj = NULL;
ret = 0;
cleanup:
- if (obj)
- virNWFilterObjUnlock(obj);
+ virNWFilterObjEndAPI(&obj);
virNWFilterCallbackDriversUnlock();
virNWFilterUnlockFilterUpdates();
@@ -602,7 +599,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 68a2ddb..4c228ea 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -315,7 +315,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;
@@ -425,8 +425,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStatePtr driver,
if (ret < 0)
virNWFilterInstReset(inst);
virNWFilterHashTableFree(tmpvars);
- if (obj)
- virNWFilterObjUnlock(obj);
+ virNWFilterObjEndAPI(&obj);
return ret;
}
@@ -547,7 +546,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
vars);
if (!tmpvars) {
rc = -1;
- virNWFilterObjUnlock(obj);
+ virNWFilterObjEndAPI(&obj);
break;
}
@@ -571,7 +570,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter,
virNWFilterHashTableFree(tmpvars);
- virNWFilterObjUnlock(obj);
+ virNWFilterObjEndAPI(&obj);
if (rc < 0)
break;
}
@@ -845,7 +844,7 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
virNWFilterHashTableFree(vars1);
err_exit:
- virNWFilterObjUnlock(obj);
+ virNWFilterObjEndAPI(&obj);
VIR_FREE(str_ipaddr);
VIR_FREE(str_macaddr);
--
2.9.4