[libvirt] [PATCH v2 00/13] Multiple cleanups within nwfilterobj and nwfilter drivers
v1: https://www.redhat.com/archives/libvir-list/2017-April/msg01074.html Changes since v1: * Patch 2 -> Do not add the local @def when just dereference obj->def * Patch 13 & 15 dropped Most patches were ACK'd, but patch 2's impact was felt in a few other patches so I just resent the whole thing. In particular patch 5 for nwfilterUndefine and nwfilterGetXMLDesc as well as patch 7 for the _virNWFilterObjListDefLoopDetect call w/ _virNWFilterObjListDefLoopDetect John Ferlan (13): nwfilter: Use consistent naming for variables nwfilter: Use virNWFilterDefPtr rather than deref virNWFilterObjPtr nwfilter: Remove unused 'active' in virNWFilterObj nwfilter: Convert wantRemoved to bool nwfilter: Make _virNWFilterObjPtr private nwfilter: Introduce virNWFilterObjNew nwfilter: Rename some virNWFilterObj* API's nwfilter: Make _virNWFilterObjList private nwfilter: Make a common UUID lookup function from driver nwfilter: Replace virNWFilterConfigFile with virFileBuildPath nwfilter: Replace virNWFilterSaveDef with virNWFilterSaveConfig nwfilter: Move creation of configDir to driver initialization nwfilter: Move save of config until after successful assign src/conf/nwfilter_conf.c | 57 +------ src/conf/nwfilter_conf.h | 8 - src/conf/virnwfilterobj.c | 270 +++++++++++++++++++++------------ src/conf/virnwfilterobj.h | 70 ++++----- src/libvirt_private.syms | 20 ++- src/nwfilter/nwfilter_driver.c | 138 ++++++++++------- src/nwfilter/nwfilter_gentech_driver.c | 42 ++--- 7 files changed, 320 insertions(+), 285 deletions(-) -- 2.9.3 FWIW: The diffs between this series and the previous taking away patches 13/15: diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 62206b8..8c9da13 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -92,16 +92,11 @@ virNWFilterObjWantRemoved(virNWFilterObjPtr obj) static void virNWFilterObjFree(virNWFilterObjPtr obj) { - virNWFilterDefPtr def; - virNWFilterDefPtr newDef; - if (!obj) return; - def = obj->def; - newDef = obj->newDef; - virNWFilterDefFree(def); - virNWFilterDefFree(newDef); + virNWFilterDefFree(obj->def); + virNWFilterDefFree(obj->newDef); virMutexDestroy(&obj->lock); @@ -204,7 +199,6 @@ _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters, size_t i; virNWFilterEntryPtr entry; virNWFilterObjPtr obj; - virNWFilterDefPtr objdef; if (!def) return 0; @@ -221,8 +215,7 @@ _virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters, obj = virNWFilterObjListFindByName(nwfilters, entry->include->filterref); if (obj) { - objdef = obj->def; - rc = _virNWFilterObjListDefLoopDetect(nwfilters, objdef, + rc = _virNWFilterObjListDefLoopDetect(nwfilters, obj->def, filtername); virNWFilterObjUnlock(obj); if (rc < 0) @@ -386,13 +379,11 @@ virNWFilterObjListNumOfNWFilters(virNWFilterObjListPtr nwfilters, { size_t i; int nfilters = 0; - virNWFilterDefPtr def; for (i = 0; i < nwfilters->count; i++) { virNWFilterObjPtr obj = nwfilters->objs[i]; virNWFilterObjLock(obj); - def = obj->def; - if (!aclfilter || aclfilter(conn, def)) + if (!aclfilter || aclfilter(conn, obj->def)) nfilters++; virNWFilterObjUnlock(obj); }
When processing a virNWFilterPtr use 'nwfilter' as a variable name. When processing a virNWFilterObjPtr use 'obj' as a variable name. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 92 +++++++++++++++++++++--------------------- src/conf/virnwfilterobj.h | 7 ++-- src/nwfilter/nwfilter_driver.c | 78 +++++++++++++++++------------------ 3 files changed, 89 insertions(+), 88 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 34d843c..cb697f9 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -62,15 +62,15 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) void virNWFilterObjRemove(virNWFilterObjListPtr nwfilters, - virNWFilterObjPtr nwfilter) + virNWFilterObjPtr obj) { size_t i; - virNWFilterObjUnlock(nwfilter); + virNWFilterObjUnlock(obj); for (i = 0; i < nwfilters->count; i++) { virNWFilterObjLock(nwfilters->objs[i]); - if (nwfilters->objs[i] == nwfilter) { + if (nwfilters->objs[i] == obj) { virNWFilterObjUnlock(nwfilters->objs[i]); virNWFilterObjFree(nwfilters->objs[i]); @@ -174,16 +174,16 @@ virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters, int -virNWFilterObjTestUnassignDef(virNWFilterObjPtr nwfilter) +virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj) { int rc = 0; - nwfilter->wantRemoved = 1; + obj->wantRemoved = 1; /* trigger the update on VMs referencing the filter */ if (virNWFilterTriggerVMFilterRebuild()) rc = -1; - nwfilter->wantRemoved = 0; + obj->wantRemoved = 0; return rc; } @@ -225,29 +225,29 @@ virNWFilterObjPtr virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, virNWFilterDefPtr def) { - virNWFilterObjPtr nwfilter; + virNWFilterObjPtr obj; - nwfilter = virNWFilterObjFindByUUID(nwfilters, def->uuid); + obj = virNWFilterObjFindByUUID(nwfilters, def->uuid); - if (nwfilter) { - if (STRNEQ(def->name, nwfilter->def->name)) { + if (obj) { + if (STRNEQ(def->name, obj->def->name)) { virReportError(VIR_ERR_OPERATION_FAILED, _("filter with same UUID but different name " "('%s') already exists"), - nwfilter->def->name); - virNWFilterObjUnlock(nwfilter); + obj->def->name); + virNWFilterObjUnlock(obj); return NULL; } - virNWFilterObjUnlock(nwfilter); + virNWFilterObjUnlock(obj); } else { - nwfilter = virNWFilterObjFindByName(nwfilters, def->name); - if (nwfilter) { + obj = virNWFilterObjFindByName(nwfilters, def->name); + if (obj) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(nwfilter->def->uuid, uuidstr); + virUUIDFormat(obj->def->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, _("filter '%s' already exists with uuid %s"), def->name, uuidstr); - virNWFilterObjUnlock(nwfilter); + virNWFilterObjUnlock(obj); return NULL; } } @@ -259,49 +259,49 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, } - if ((nwfilter = virNWFilterObjFindByName(nwfilters, def->name))) { + if ((obj = virNWFilterObjFindByName(nwfilters, def->name))) { - if (virNWFilterDefEqual(def, nwfilter->def, false)) { - virNWFilterDefFree(nwfilter->def); - nwfilter->def = def; - return nwfilter; + if (virNWFilterDefEqual(def, obj->def, false)) { + virNWFilterDefFree(obj->def); + obj->def = def; + return obj; } - nwfilter->newDef = def; + obj->newDef = def; /* trigger the update on VMs referencing the filter */ if (virNWFilterTriggerVMFilterRebuild()) { - nwfilter->newDef = NULL; - virNWFilterObjUnlock(nwfilter); + obj->newDef = NULL; + virNWFilterObjUnlock(obj); return NULL; } - virNWFilterDefFree(nwfilter->def); - nwfilter->def = def; - nwfilter->newDef = NULL; - return nwfilter; + virNWFilterDefFree(obj->def); + obj->def = def; + obj->newDef = NULL; + return obj; } - if (VIR_ALLOC(nwfilter) < 0) + if (VIR_ALLOC(obj) < 0) return NULL; - if (virMutexInitRecursive(&nwfilter->lock) < 0) { + if (virMutexInitRecursive(&obj->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize mutex")); - VIR_FREE(nwfilter); + VIR_FREE(obj); return NULL; } - virNWFilterObjLock(nwfilter); - nwfilter->active = 0; + virNWFilterObjLock(obj); + obj->active = 0; if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, - nwfilters->count, nwfilter) < 0) { - virNWFilterObjUnlock(nwfilter); - virNWFilterObjFree(nwfilter); + nwfilters->count, obj) < 0) { + virNWFilterObjUnlock(obj); + virNWFilterObjFree(obj); return NULL; } - nwfilter->def = def; + obj->def = def; - return nwfilter; + return obj; } @@ -414,7 +414,7 @@ virNWFilterObjLoadConfig(virNWFilterObjListPtr nwfilters, const char *name) { virNWFilterDefPtr def = NULL; - virNWFilterObjPtr nwfilter; + virNWFilterObjPtr obj; char *configFile = NULL; if (!(configFile = virFileBuildPath(configDir, name, ".xml"))) @@ -436,11 +436,11 @@ virNWFilterObjLoadConfig(virNWFilterObjListPtr nwfilters, virNWFilterSaveConfig(configDir, def) < 0) goto error; - if (!(nwfilter = virNWFilterObjAssignDef(nwfilters, def))) + if (!(obj = virNWFilterObjAssignDef(nwfilters, def))) goto error; VIR_FREE(configFile); - return nwfilter; + return obj; error: VIR_FREE(configFile); @@ -462,14 +462,14 @@ virNWFilterObjLoadAllConfigs(virNWFilterObjListPtr nwfilters, return rc; while ((ret = virDirRead(dir, &entry, configDir)) > 0) { - virNWFilterObjPtr nwfilter; + virNWFilterObjPtr obj; if (!virFileStripSuffix(entry->d_name, ".xml")) continue; - nwfilter = virNWFilterObjLoadConfig(nwfilters, configDir, entry->d_name); - if (nwfilter) - virNWFilterObjUnlock(nwfilter); + obj = virNWFilterObjLoadConfig(nwfilters, configDir, entry->d_name); + if (obj) + virNWFilterObjUnlock(obj); } VIR_DIR_CLOSE(dir); diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 49b1170..2adffd9 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -62,9 +62,10 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters); void virNWFilterObjRemove(virNWFilterObjListPtr nwfilters, - virNWFilterObjPtr nwfilter); + virNWFilterObjPtr obj); -void virNWFilterObjFree(virNWFilterObjPtr obj); +void +virNWFilterObjFree(virNWFilterObjPtr obj); virNWFilterObjPtr virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters, @@ -79,7 +80,7 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, virNWFilterDefPtr def); int -virNWFilterObjTestUnassignDef(virNWFilterObjPtr nwfilter); +virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj); typedef bool (*virNWFilterObjListFilter)(virConnectPtr conn, diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index f6c419c..72b9b8e 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -357,27 +357,27 @@ static virNWFilterPtr nwfilterLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - virNWFilterObjPtr nwfilter; + virNWFilterObjPtr obj; virNWFilterPtr ret = NULL; nwfilterDriverLock(); - nwfilter = virNWFilterObjFindByUUID(&driver->nwfilters, uuid); + obj = virNWFilterObjFindByUUID(&driver->nwfilters, uuid); nwfilterDriverUnlock(); - if (!nwfilter) { + if (!obj) { virReportError(VIR_ERR_NO_NWFILTER, "%s", _("no nwfilter with matching uuid")); goto cleanup; } - if (virNWFilterLookupByUUIDEnsureACL(conn, nwfilter->def) < 0) + if (virNWFilterLookupByUUIDEnsureACL(conn, obj->def) < 0) goto cleanup; - ret = virGetNWFilter(conn, nwfilter->def->name, nwfilter->def->uuid); + ret = virGetNWFilter(conn, obj->def->name, obj->def->uuid); cleanup: - if (nwfilter) - virNWFilterObjUnlock(nwfilter); + if (obj) + virNWFilterObjUnlock(obj); return ret; } @@ -386,27 +386,27 @@ static virNWFilterPtr nwfilterLookupByName(virConnectPtr conn, const char *name) { - virNWFilterObjPtr nwfilter; + virNWFilterObjPtr obj; virNWFilterPtr ret = NULL; nwfilterDriverLock(); - nwfilter = virNWFilterObjFindByName(&driver->nwfilters, name); + obj = virNWFilterObjFindByName(&driver->nwfilters, name); nwfilterDriverUnlock(); - if (!nwfilter) { + if (!obj) { virReportError(VIR_ERR_NO_NWFILTER, _("no nwfilter with matching name '%s'"), name); goto cleanup; } - if (virNWFilterLookupByNameEnsureACL(conn, nwfilter->def) < 0) + if (virNWFilterLookupByNameEnsureACL(conn, obj->def) < 0) goto cleanup; - ret = virGetNWFilter(conn, nwfilter->def->name, nwfilter->def->uuid); + ret = virGetNWFilter(conn, obj->def->name, obj->def->uuid); cleanup: - if (nwfilter) - virNWFilterObjUnlock(nwfilter); + if (obj) + virNWFilterObjUnlock(obj); return ret; } @@ -466,7 +466,7 @@ nwfilterDefineXML(virConnectPtr conn, const char *xml) { virNWFilterDefPtr def; - virNWFilterObjPtr nwfilter = NULL; + virNWFilterObjPtr obj = NULL; virNWFilterPtr ret = NULL; if (!driver->privileged) { @@ -485,22 +485,22 @@ nwfilterDefineXML(virConnectPtr conn, if (virNWFilterDefineXMLEnsureACL(conn, def) < 0) goto cleanup; - if (!(nwfilter = virNWFilterObjAssignDef(&driver->nwfilters, def))) + if (!(obj = virNWFilterObjAssignDef(&driver->nwfilters, def))) goto cleanup; if (virNWFilterSaveDef(driver->configDir, def) < 0) { - virNWFilterObjRemove(&driver->nwfilters, nwfilter); + virNWFilterObjRemove(&driver->nwfilters, obj); def = NULL; goto cleanup; } def = NULL; - ret = virGetNWFilter(conn, nwfilter->def->name, nwfilter->def->uuid); + ret = virGetNWFilter(conn, obj->def->name, obj->def->uuid); cleanup: virNWFilterDefFree(def); - if (nwfilter) - virNWFilterObjUnlock(nwfilter); + if (obj) + virNWFilterObjUnlock(obj); virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); @@ -510,42 +510,42 @@ nwfilterDefineXML(virConnectPtr conn, static int -nwfilterUndefine(virNWFilterPtr obj) +nwfilterUndefine(virNWFilterPtr nwfilter) { - virNWFilterObjPtr nwfilter; + virNWFilterObjPtr obj; int ret = -1; nwfilterDriverLock(); virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); - nwfilter = virNWFilterObjFindByUUID(&driver->nwfilters, obj->uuid); - if (!nwfilter) { + obj = virNWFilterObjFindByUUID(&driver->nwfilters, nwfilter->uuid); + if (!obj) { virReportError(VIR_ERR_NO_NWFILTER, "%s", _("no nwfilter with matching uuid")); goto cleanup; } - if (virNWFilterUndefineEnsureACL(obj->conn, nwfilter->def) < 0) + if (virNWFilterUndefineEnsureACL(nwfilter->conn, obj->def) < 0) goto cleanup; - if (virNWFilterObjTestUnassignDef(nwfilter) < 0) { + if (virNWFilterObjTestUnassignDef(obj) < 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("nwfilter is in use")); goto cleanup; } - if (virNWFilterDeleteDef(driver->configDir, nwfilter->def) < 0) + if (virNWFilterDeleteDef(driver->configDir, obj->def) < 0) goto cleanup; - virNWFilterObjRemove(&driver->nwfilters, nwfilter); - nwfilter = NULL; + virNWFilterObjRemove(&driver->nwfilters, obj); + obj = NULL; ret = 0; cleanup: - if (nwfilter) - virNWFilterObjUnlock(nwfilter); + if (obj) + virNWFilterObjUnlock(obj); virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); @@ -555,32 +555,32 @@ nwfilterUndefine(virNWFilterPtr obj) static char * -nwfilterGetXMLDesc(virNWFilterPtr obj, +nwfilterGetXMLDesc(virNWFilterPtr nwfilter, unsigned int flags) { - virNWFilterObjPtr nwfilter; + virNWFilterObjPtr obj; char *ret = NULL; virCheckFlags(0, NULL); nwfilterDriverLock(); - nwfilter = virNWFilterObjFindByUUID(&driver->nwfilters, obj->uuid); + obj = virNWFilterObjFindByUUID(&driver->nwfilters, nwfilter->uuid); nwfilterDriverUnlock(); - if (!nwfilter) { + if (!obj) { virReportError(VIR_ERR_NO_NWFILTER, "%s", _("no nwfilter with matching uuid")); goto cleanup; } - if (virNWFilterGetXMLDescEnsureACL(obj->conn, nwfilter->def) < 0) + if (virNWFilterGetXMLDescEnsureACL(nwfilter->conn, obj->def) < 0) goto cleanup; - ret = virNWFilterDefFormat(nwfilter->def); + ret = virNWFilterDefFormat(obj->def); cleanup: - if (nwfilter) - virNWFilterObjUnlock(nwfilter); + if (obj) + virNWFilterObjUnlock(obj); return ret; } -- 2.9.3
Rather than dereferencing obj->def->XXX or nwfilters->objs[i]->X create local virNWFilterObjPtr and virNWFilterDefPtr variables. Future adjustments will be privatizing the object more, so this just prepares the code for that reality. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 65 +++++++++++++++++++++++++----------------- src/nwfilter/nwfilter_driver.c | 21 ++++++++------ 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index cb697f9..5570c72 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -87,12 +87,16 @@ virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters, const unsigned char *uuid) { size_t i; + virNWFilterObjPtr obj; + virNWFilterDefPtr def; for (i = 0; i < nwfilters->count; i++) { - virNWFilterObjLock(nwfilters->objs[i]); - if (!memcmp(nwfilters->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) - return nwfilters->objs[i]; - virNWFilterObjUnlock(nwfilters->objs[i]); + obj = nwfilters->objs[i]; + virNWFilterObjLock(obj); + def = obj->def; + if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN)) + return obj; + virNWFilterObjUnlock(obj); } return NULL; @@ -104,12 +108,16 @@ virNWFilterObjFindByName(virNWFilterObjListPtr nwfilters, const char *name) { size_t i; + virNWFilterObjPtr obj; + virNWFilterDefPtr def; for (i = 0; i < nwfilters->count; i++) { - virNWFilterObjLock(nwfilters->objs[i]); - if (STREQ_NULLABLE(nwfilters->objs[i]->def->name, name)) - return nwfilters->objs[i]; - virNWFilterObjUnlock(nwfilters->objs[i]); + obj = nwfilters->objs[i]; + virNWFilterObjLock(obj); + def = obj->def; + if (STREQ_NULLABLE(def->name, name)) + return obj; + virNWFilterObjUnlock(obj); } return NULL; @@ -141,9 +149,8 @@ _virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters, obj = virNWFilterObjFindByName(nwfilters, entry->include->filterref); if (obj) { - rc = _virNWFilterObjDefLoopDetect(nwfilters, - obj->def, filtername); - + rc = _virNWFilterObjDefLoopDetect(nwfilters, obj->def, + filtername); virNWFilterObjUnlock(obj); if (rc < 0) break; @@ -226,24 +233,26 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, virNWFilterDefPtr def) { virNWFilterObjPtr obj; + virNWFilterDefPtr objdef; - obj = virNWFilterObjFindByUUID(nwfilters, def->uuid); + if ((obj = virNWFilterObjFindByUUID(nwfilters, def->uuid))) { + objdef = obj->def; - if (obj) { - if (STRNEQ(def->name, obj->def->name)) { + if (STRNEQ(def->name, objdef->name)) { virReportError(VIR_ERR_OPERATION_FAILED, _("filter with same UUID but different name " "('%s') already exists"), - obj->def->name); + objdef->name); virNWFilterObjUnlock(obj); return NULL; } virNWFilterObjUnlock(obj); } else { - obj = virNWFilterObjFindByName(nwfilters, def->name); - if (obj) { + if ((obj = virNWFilterObjFindByName(nwfilters, def->name))) { char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(obj->def->uuid, uuidstr); + + objdef = obj->def; + virUUIDFormat(objdef->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, _("filter '%s' already exists with uuid %s"), def->name, uuidstr); @@ -261,8 +270,9 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, if ((obj = virNWFilterObjFindByName(nwfilters, def->name))) { - if (virNWFilterDefEqual(def, obj->def, false)) { - virNWFilterDefFree(obj->def); + objdef = obj->def; + if (virNWFilterDefEqual(def, objdef, false)) { + virNWFilterDefFree(objdef); obj->def = def; return obj; } @@ -275,7 +285,7 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, return NULL; } - virNWFilterDefFree(obj->def); + virNWFilterDefFree(objdef); obj->def = def; obj->newDef = NULL; return obj; @@ -334,12 +344,14 @@ virNWFilterObjGetNames(virNWFilterObjListPtr nwfilters, { int nnames = 0; size_t i; + virNWFilterDefPtr def; for (i = 0; i < nwfilters->count && nnames < maxnames; i++) { virNWFilterObjPtr obj = nwfilters->objs[i]; virNWFilterObjLock(obj); - if (!aclfilter || aclfilter(conn, obj->def)) { - if (VIR_STRDUP(names[nnames], obj->def->name) < 0) { + def = obj->def; + if (!aclfilter || aclfilter(conn, def)) { + if (VIR_STRDUP(names[nnames], def->name) < 0) { virNWFilterObjUnlock(obj); goto failure; } @@ -368,6 +380,7 @@ virNWFilterObjListExport(virConnectPtr conn, int nfilters = 0; virNWFilterPtr filter = NULL; virNWFilterObjPtr obj = NULL; + virNWFilterDefPtr def; size_t i; int ret = -1; @@ -382,9 +395,9 @@ virNWFilterObjListExport(virConnectPtr conn, for (i = 0; i < nwfilters->count; i++) { obj = nwfilters->objs[i]; virNWFilterObjLock(obj); - if (!aclfilter || aclfilter(conn, obj->def)) { - if (!(filter = virGetNWFilter(conn, obj->def->name, - obj->def->uuid))) { + def = obj->def; + if (!aclfilter || aclfilter(conn, def)) { + if (!(filter = virGetNWFilter(conn, def->name, def->uuid))) { virNWFilterObjUnlock(obj); goto cleanup; } diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 72b9b8e..f3f75a3 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -358,6 +358,7 @@ nwfilterLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { virNWFilterObjPtr obj; + virNWFilterDefPtr def; virNWFilterPtr ret = NULL; nwfilterDriverLock(); @@ -369,11 +370,12 @@ nwfilterLookupByUUID(virConnectPtr conn, "%s", _("no nwfilter with matching uuid")); goto cleanup; } + def = obj->def; - if (virNWFilterLookupByUUIDEnsureACL(conn, obj->def) < 0) + if (virNWFilterLookupByUUIDEnsureACL(conn, def) < 0) goto cleanup; - ret = virGetNWFilter(conn, obj->def->name, obj->def->uuid); + ret = virGetNWFilter(conn, def->name, def->uuid); cleanup: if (obj) @@ -387,6 +389,7 @@ nwfilterLookupByName(virConnectPtr conn, const char *name) { virNWFilterObjPtr obj; + virNWFilterDefPtr def; virNWFilterPtr ret = NULL; nwfilterDriverLock(); @@ -398,11 +401,12 @@ nwfilterLookupByName(virConnectPtr conn, _("no nwfilter with matching name '%s'"), name); goto cleanup; } + def = obj->def; - if (virNWFilterLookupByNameEnsureACL(conn, obj->def) < 0) + if (virNWFilterLookupByNameEnsureACL(conn, def) < 0) goto cleanup; - ret = virGetNWFilter(conn, obj->def->name, obj->def->uuid); + ret = virGetNWFilter(conn, def->name, def->uuid); cleanup: if (obj) @@ -467,6 +471,7 @@ nwfilterDefineXML(virConnectPtr conn, { virNWFilterDefPtr def; virNWFilterObjPtr obj = NULL; + virNWFilterDefPtr objdef; virNWFilterPtr ret = NULL; if (!driver->privileged) { @@ -487,15 +492,15 @@ nwfilterDefineXML(virConnectPtr conn, if (!(obj = virNWFilterObjAssignDef(&driver->nwfilters, def))) goto cleanup; + def = NULL; + objdef = obj->def; - if (virNWFilterSaveDef(driver->configDir, def) < 0) { + if (virNWFilterSaveDef(driver->configDir, objdef) < 0) { virNWFilterObjRemove(&driver->nwfilters, obj); - def = NULL; goto cleanup; } - def = NULL; - ret = virGetNWFilter(conn, obj->def->name, obj->def->uuid); + ret = virGetNWFilter(conn, objdef->name, objdef->uuid); cleanup: virNWFilterDefFree(def); -- 2.9.3
It was only ever set to false, which is ironically the default Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 1 - src/conf/virnwfilterobj.h | 1 - 2 files changed, 2 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 5570c72..6606d3b 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -301,7 +301,6 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, return NULL; } virNWFilterObjLock(obj); - obj->active = 0; if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, nwfilters->count, obj) < 0) { diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 2adffd9..7a2addf 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -29,7 +29,6 @@ typedef virNWFilterObj *virNWFilterObjPtr; struct _virNWFilterObj { virMutex lock; - int active; int wantRemoved; virNWFilterDefPtr def; -- 2.9.3
It is what it is anyway, so let's describe it that way too. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 4 ++-- src/conf/virnwfilterobj.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 6606d3b..d7c4a13 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -185,12 +185,12 @@ virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj) { int rc = 0; - obj->wantRemoved = 1; + obj->wantRemoved = true; /* trigger the update on VMs referencing the filter */ if (virNWFilterTriggerVMFilterRebuild()) rc = -1; - obj->wantRemoved = 0; + obj->wantRemoved = false; return rc; } diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 7a2addf..f31938d 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -29,7 +29,7 @@ typedef virNWFilterObj *virNWFilterObjPtr; struct _virNWFilterObj { virMutex lock; - int wantRemoved; + bool wantRemoved; virNWFilterDefPtr def; virNWFilterDefPtr newDef; -- 2.9.3
Move the structure to virnwfilterobj.c and create necessary accessor API's for the various fields. Also make virNWFilterObjFree static since there's no external callers. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 32 +++++++++++++++++++++++++++++++- src/conf/virnwfilterobj.h | 22 +++++++++------------- src/libvirt_private.syms | 3 +++ src/nwfilter/nwfilter_driver.c | 18 +++++++++++------- src/nwfilter/nwfilter_gentech_driver.c | 30 ++++++++++++++++++------------ 5 files changed, 72 insertions(+), 33 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index d7c4a13..77d5c1e 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -33,8 +33,38 @@ VIR_LOG_INIT("conf.virnwfilterobj"); +struct _virNWFilterObj { + virMutex lock; -void + bool wantRemoved; + + virNWFilterDefPtr def; + virNWFilterDefPtr newDef; +}; + + +virNWFilterDefPtr +virNWFilterObjGetDef(virNWFilterObjPtr obj) +{ + return obj->def; +} + + +virNWFilterDefPtr +virNWFilterObjGetNewDef(virNWFilterObjPtr obj) +{ + return obj->newDef; +} + + +bool +virNWFilterObjWantRemoved(virNWFilterObjPtr obj) +{ + return obj->wantRemoved; +} + + +static void virNWFilterObjFree(virNWFilterObjPtr obj) { if (!obj) diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index f31938d..b02dcfa 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -26,16 +26,6 @@ typedef struct _virNWFilterObj virNWFilterObj; typedef virNWFilterObj *virNWFilterObjPtr; -struct _virNWFilterObj { - virMutex lock; - - bool wantRemoved; - - virNWFilterDefPtr def; - virNWFilterDefPtr newDef; -}; - - typedef struct _virNWFilterObjList virNWFilterObjList; typedef virNWFilterObjList *virNWFilterObjListPtr; struct _virNWFilterObjList { @@ -56,6 +46,15 @@ struct _virNWFilterDriverState { bool watchingFirewallD; }; +virNWFilterDefPtr +virNWFilterObjGetDef(virNWFilterObjPtr obj); + +virNWFilterDefPtr +virNWFilterObjGetNewDef(virNWFilterObjPtr obj); + +bool +virNWFilterObjWantRemoved(virNWFilterObjPtr obj); + void virNWFilterObjListFree(virNWFilterObjListPtr nwfilters); @@ -63,9 +62,6 @@ void virNWFilterObjRemove(virNWFilterObjListPtr nwfilters, virNWFilterObjPtr obj); -void -virNWFilterObjFree(virNWFilterObjPtr obj); - virNWFilterObjPtr virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters, const unsigned char *uuid); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 83e979a..dd6cb98 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -965,7 +965,9 @@ virNodeDeviceObjUnlock; virNWFilterObjAssignDef; virNWFilterObjFindByName; virNWFilterObjFindByUUID; +virNWFilterObjGetDef; virNWFilterObjGetNames; +virNWFilterObjGetNewDef; virNWFilterObjListExport; virNWFilterObjListFree; virNWFilterObjLoadAllConfigs; @@ -974,6 +976,7 @@ virNWFilterObjNumOfNWFilters; virNWFilterObjRemove; virNWFilterObjTestUnassignDef; virNWFilterObjUnlock; +virNWFilterObjWantRemoved; # conf/virsecretobj.h diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index f3f75a3..dd3645a 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -370,7 +370,7 @@ nwfilterLookupByUUID(virConnectPtr conn, "%s", _("no nwfilter with matching uuid")); goto cleanup; } - def = obj->def; + def = virNWFilterObjGetDef(obj); if (virNWFilterLookupByUUIDEnsureACL(conn, def) < 0) goto cleanup; @@ -401,7 +401,7 @@ nwfilterLookupByName(virConnectPtr conn, _("no nwfilter with matching name '%s'"), name); goto cleanup; } - def = obj->def; + def = virNWFilterObjGetDef(obj); if (virNWFilterLookupByNameEnsureACL(conn, def) < 0) goto cleanup; @@ -493,7 +493,7 @@ nwfilterDefineXML(virConnectPtr conn, if (!(obj = virNWFilterObjAssignDef(&driver->nwfilters, def))) goto cleanup; def = NULL; - objdef = obj->def; + objdef = virNWFilterObjGetDef(obj); if (virNWFilterSaveDef(driver->configDir, objdef) < 0) { virNWFilterObjRemove(&driver->nwfilters, obj); @@ -518,6 +518,7 @@ static int nwfilterUndefine(virNWFilterPtr nwfilter) { virNWFilterObjPtr obj; + virNWFilterDefPtr def; int ret = -1; nwfilterDriverLock(); @@ -530,8 +531,9 @@ nwfilterUndefine(virNWFilterPtr nwfilter) "%s", _("no nwfilter with matching uuid")); goto cleanup; } + def = virNWFilterObjGetDef(obj); - if (virNWFilterUndefineEnsureACL(nwfilter->conn, obj->def) < 0) + if (virNWFilterUndefineEnsureACL(nwfilter->conn, def) < 0) goto cleanup; if (virNWFilterObjTestUnassignDef(obj) < 0) { @@ -541,7 +543,7 @@ nwfilterUndefine(virNWFilterPtr nwfilter) goto cleanup; } - if (virNWFilterDeleteDef(driver->configDir, obj->def) < 0) + if (virNWFilterDeleteDef(driver->configDir, def) < 0) goto cleanup; virNWFilterObjRemove(&driver->nwfilters, obj); @@ -564,6 +566,7 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, unsigned int flags) { virNWFilterObjPtr obj; + virNWFilterDefPtr def; char *ret = NULL; virCheckFlags(0, NULL); @@ -577,11 +580,12 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, "%s", _("no nwfilter with matching uuid")); goto cleanup; } + def = virNWFilterObjGetDef(obj); - if (virNWFilterGetXMLDescEnsureACL(nwfilter->conn, obj->def) < 0) + if (virNWFilterGetXMLDescEnsureACL(nwfilter->conn, def) < 0) goto cleanup; - ret = virNWFilterDefFormat(obj->def); + ret = virNWFilterDefFormat(def); cleanup: if (obj) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 761a01b..b356d87 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -379,6 +379,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStatePtr driver, virNWFilterObjPtr obj; virNWFilterHashTablePtr tmpvars = NULL; virNWFilterDefPtr childdef; + virNWFilterDefPtr newChilddef; int ret = -1; VIR_DEBUG("Instantiating filter %s", inc->filterref); @@ -390,7 +391,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStatePtr driver, inc->filterref); goto cleanup; } - if (obj->wantRemoved) { + if (virNWFilterObjWantRemoved(obj)) { virReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), inc->filterref); @@ -402,12 +403,13 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStatePtr driver, vars))) goto cleanup; - childdef = obj->def; + childdef = virNWFilterObjGetDef(obj); switch (useNewFilter) { case INSTANTIATE_FOLLOW_NEWFILTER: - if (obj->newDef) { - childdef = obj->newDef; + newChilddef = virNWFilterObjGetNewDef(obj); + if (newChilddef) { + childdef = newChilddef; *foundNewFilter = true; } break; @@ -505,6 +507,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, int rc = 0; size_t i, j; virNWFilterDefPtr next_filter; + virNWFilterDefPtr newNext_filter; virNWFilterVarValuePtr val; for (i = 0; i < filter->nentries; i++) { @@ -545,7 +548,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, obj = virNWFilterObjFindByName(&driver->nwfilters, inc->filterref); if (obj) { - if (obj->wantRemoved) { + if (virNWFilterObjWantRemoved(obj)) { virReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), inc->filterref); @@ -564,12 +567,13 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, break; } - next_filter = obj->def; + next_filter = virNWFilterObjGetDef(obj); switch (useNewFilter) { case INSTANTIATE_FOLLOW_NEWFILTER: - if (obj->newDef) - next_filter = obj->newDef; + newNext_filter = virNWFilterObjGetNewDef(obj); + if (newNext_filter) + next_filter = newNext_filter; break; case INSTANTIATE_ALWAYS: break; @@ -790,6 +794,7 @@ __virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, virNWFilterObjPtr obj; virNWFilterHashTablePtr vars, vars1; virNWFilterDefPtr filter; + virNWFilterDefPtr newFilter; char vmmacaddr[VIR_MAC_STRING_BUFLEN] = {0}; char *str_macaddr = NULL; virNWFilterVarValuePtr ipaddr; @@ -815,7 +820,7 @@ __virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, return -1; } - if (obj->wantRemoved) { + if (virNWFilterObjWantRemoved(obj)) { virReportError(VIR_ERR_NO_NWFILTER, _("Filter '%s' is in use."), filtername); @@ -847,12 +852,13 @@ __virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, goto err_exit_vars1; } - filter = obj->def; + filter = virNWFilterObjGetDef(obj); switch (useNewFilter) { case INSTANTIATE_FOLLOW_NEWFILTER: - if (obj->newDef) { - filter = obj->newDef; + newFilter = virNWFilterObjGetNewDef(obj); + if (newFilter) { + filter = newFilter; *foundNewFilter = true; } break; -- 2.9.3
Perform the object initialization in a helper rather than inline Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 77d5c1e..7c21327 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -43,6 +43,26 @@ struct _virNWFilterObj { }; +static virNWFilterObjPtr +virNWFilterObjNew(void) +{ + virNWFilterObjPtr obj; + + if (VIR_ALLOC(obj) < 0) + return NULL; + + if (virMutexInitRecursive(&obj->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot initialize mutex")); + VIR_FREE(obj); + return NULL; + } + + virNWFilterObjLock(obj); + return obj; +} + + virNWFilterDefPtr virNWFilterObjGetDef(virNWFilterObjPtr obj) { @@ -321,16 +341,8 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, return obj; } - if (VIR_ALLOC(obj) < 0) - return NULL; - - if (virMutexInitRecursive(&obj->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot initialize mutex")); - VIR_FREE(obj); + if (!(obj = virNWFilterObjNew())) return NULL; - } - virNWFilterObjLock(obj); if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs, nwfilters->count, obj) < 0) { -- 2.9.3
Prefix should have been virNWFilterObjList since the API is operating on the list of filters Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 76 +++++++++++++++++----------------- src/conf/virnwfilterobj.h | 36 ++++++++-------- src/libvirt_private.syms | 14 +++---- src/nwfilter/nwfilter_driver.c | 22 +++++----- src/nwfilter/nwfilter_gentech_driver.c | 12 +++--- 5 files changed, 80 insertions(+), 80 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 7c21327..28425ba 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -111,8 +111,8 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) void -virNWFilterObjRemove(virNWFilterObjListPtr nwfilters, - virNWFilterObjPtr obj) +virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters, + virNWFilterObjPtr obj) { size_t i; @@ -133,8 +133,8 @@ virNWFilterObjRemove(virNWFilterObjListPtr nwfilters, virNWFilterObjPtr -virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters, - const unsigned char *uuid) +virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters, + const unsigned char *uuid) { size_t i; virNWFilterObjPtr obj; @@ -154,8 +154,8 @@ virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters, virNWFilterObjPtr -virNWFilterObjFindByName(virNWFilterObjListPtr nwfilters, - const char *name) +virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters, + const char *name) { size_t i; virNWFilterObjPtr obj; @@ -175,9 +175,9 @@ virNWFilterObjFindByName(virNWFilterObjListPtr nwfilters, static int -_virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters, - virNWFilterDefPtr def, - const char *filtername) +_virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters, + virNWFilterDefPtr def, + const char *filtername) { int rc = 0; size_t i; @@ -196,11 +196,11 @@ _virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters, break; } - obj = virNWFilterObjFindByName(nwfilters, - entry->include->filterref); + obj = virNWFilterObjListFindByName(nwfilters, + entry->include->filterref); if (obj) { - rc = _virNWFilterObjDefLoopDetect(nwfilters, obj->def, - filtername); + rc = _virNWFilterObjListDefLoopDetect(nwfilters, obj->def, + filtername); virNWFilterObjUnlock(obj); if (rc < 0) break; @@ -213,7 +213,7 @@ _virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters, /* - * virNWFilterObjDefLoopDetect: + * virNWFilterObjListDefLoopDetect: * @nwfilters : the nwfilters to search * @def : the filter definition that may add a loop and is to be tested * @@ -223,10 +223,10 @@ _virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters, * Returns 0 in case no loop was detected, -1 otherwise. */ static int -virNWFilterObjDefLoopDetect(virNWFilterObjListPtr nwfilters, - virNWFilterDefPtr def) +virNWFilterObjListDefLoopDetect(virNWFilterObjListPtr nwfilters, + virNWFilterDefPtr def) { - return _virNWFilterObjDefLoopDetect(nwfilters, def, def->name); + return _virNWFilterObjListDefLoopDetect(nwfilters, def, def->name); } @@ -279,13 +279,13 @@ virNWFilterDefEqual(const virNWFilterDef *def1, virNWFilterObjPtr -virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, - virNWFilterDefPtr def) +virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, + virNWFilterDefPtr def) { virNWFilterObjPtr obj; virNWFilterDefPtr objdef; - if ((obj = virNWFilterObjFindByUUID(nwfilters, def->uuid))) { + if ((obj = virNWFilterObjListFindByUUID(nwfilters, def->uuid))) { objdef = obj->def; if (STRNEQ(def->name, objdef->name)) { @@ -298,7 +298,7 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, } virNWFilterObjUnlock(obj); } else { - if ((obj = virNWFilterObjFindByName(nwfilters, def->name))) { + if ((obj = virNWFilterObjListFindByName(nwfilters, def->name))) { char uuidstr[VIR_UUID_STRING_BUFLEN]; objdef = obj->def; @@ -311,14 +311,14 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, } } - if (virNWFilterObjDefLoopDetect(nwfilters, def) < 0) { + if (virNWFilterObjListDefLoopDetect(nwfilters, def) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("filter would introduce a loop")); return NULL; } - if ((obj = virNWFilterObjFindByName(nwfilters, def->name))) { + if ((obj = virNWFilterObjListFindByName(nwfilters, def->name))) { objdef = obj->def; if (virNWFilterDefEqual(def, objdef, false)) { @@ -357,9 +357,9 @@ virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, int -virNWFilterObjNumOfNWFilters(virNWFilterObjListPtr nwfilters, - virConnectPtr conn, - virNWFilterObjListFilter aclfilter) +virNWFilterObjListNumOfNWFilters(virNWFilterObjListPtr nwfilters, + virConnectPtr conn, + virNWFilterObjListFilter aclfilter) { size_t i; int nfilters = 0; @@ -377,11 +377,11 @@ virNWFilterObjNumOfNWFilters(virNWFilterObjListPtr nwfilters, int -virNWFilterObjGetNames(virNWFilterObjListPtr nwfilters, - virConnectPtr conn, - virNWFilterObjListFilter aclfilter, - char **const names, - int maxnames) +virNWFilterObjListGetNames(virNWFilterObjListPtr nwfilters, + virConnectPtr conn, + virNWFilterObjListFilter aclfilter, + char **const names, + int maxnames) { int nnames = 0; size_t i; @@ -463,9 +463,9 @@ virNWFilterObjListExport(virConnectPtr conn, static virNWFilterObjPtr -virNWFilterObjLoadConfig(virNWFilterObjListPtr nwfilters, - const char *configDir, - const char *name) +virNWFilterObjListLoadConfig(virNWFilterObjListPtr nwfilters, + const char *configDir, + const char *name) { virNWFilterDefPtr def = NULL; virNWFilterObjPtr obj; @@ -490,7 +490,7 @@ virNWFilterObjLoadConfig(virNWFilterObjListPtr nwfilters, virNWFilterSaveConfig(configDir, def) < 0) goto error; - if (!(obj = virNWFilterObjAssignDef(nwfilters, def))) + if (!(obj = virNWFilterObjListAssignDef(nwfilters, def))) goto error; VIR_FREE(configFile); @@ -504,8 +504,8 @@ virNWFilterObjLoadConfig(virNWFilterObjListPtr nwfilters, int -virNWFilterObjLoadAllConfigs(virNWFilterObjListPtr nwfilters, - const char *configDir) +virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters, + const char *configDir) { DIR *dir; struct dirent *entry; @@ -521,7 +521,7 @@ virNWFilterObjLoadAllConfigs(virNWFilterObjListPtr nwfilters, if (!virFileStripSuffix(entry->d_name, ".xml")) continue; - obj = virNWFilterObjLoadConfig(nwfilters, configDir, entry->d_name); + obj = virNWFilterObjListLoadConfig(nwfilters, configDir, entry->d_name); if (obj) virNWFilterObjUnlock(obj); } diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index b02dcfa..4c19223 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -59,20 +59,20 @@ void virNWFilterObjListFree(virNWFilterObjListPtr nwfilters); void -virNWFilterObjRemove(virNWFilterObjListPtr nwfilters, - virNWFilterObjPtr obj); +virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters, + virNWFilterObjPtr obj); virNWFilterObjPtr -virNWFilterObjFindByUUID(virNWFilterObjListPtr nwfilters, - const unsigned char *uuid); +virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters, + const unsigned char *uuid); virNWFilterObjPtr -virNWFilterObjFindByName(virNWFilterObjListPtr nwfilters, - const char *name); +virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters, + const char *name); virNWFilterObjPtr -virNWFilterObjAssignDef(virNWFilterObjListPtr nwfilters, - virNWFilterDefPtr def); +virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters, + virNWFilterDefPtr def); int virNWFilterObjTestUnassignDef(virNWFilterObjPtr obj); @@ -82,16 +82,16 @@ typedef bool virNWFilterDefPtr def); int -virNWFilterObjNumOfNWFilters(virNWFilterObjListPtr nwfilters, - virConnectPtr conn, - virNWFilterObjListFilter aclfilter); +virNWFilterObjListNumOfNWFilters(virNWFilterObjListPtr nwfilters, + virConnectPtr conn, + virNWFilterObjListFilter aclfilter); int -virNWFilterObjGetNames(virNWFilterObjListPtr nwfilters, - virConnectPtr conn, - virNWFilterObjListFilter aclfilter, - char **const names, - int maxnames); +virNWFilterObjListGetNames(virNWFilterObjListPtr nwfilters, + virConnectPtr conn, + virNWFilterObjListFilter aclfilter, + char **const names, + int maxnames); int virNWFilterObjListExport(virConnectPtr conn, @@ -100,8 +100,8 @@ virNWFilterObjListExport(virConnectPtr conn, virNWFilterObjListFilter aclfilter); int -virNWFilterObjLoadAllConfigs(virNWFilterObjListPtr nwfilters, - const char *configDir); +virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters, + const char *configDir); void virNWFilterObjLock(virNWFilterObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dd6cb98..453d0fe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -962,18 +962,18 @@ virNodeDeviceObjUnlock; # conf/virnwfilterobj.h -virNWFilterObjAssignDef; -virNWFilterObjFindByName; -virNWFilterObjFindByUUID; virNWFilterObjGetDef; -virNWFilterObjGetNames; virNWFilterObjGetNewDef; +virNWFilterObjListAssignDef; virNWFilterObjListExport; +virNWFilterObjListFindByName; +virNWFilterObjListFindByUUID; virNWFilterObjListFree; -virNWFilterObjLoadAllConfigs; +virNWFilterObjListGetNames; +virNWFilterObjListLoadAllConfigs; +virNWFilterObjListNumOfNWFilters; +virNWFilterObjListRemove; virNWFilterObjLock; -virNWFilterObjNumOfNWFilters; -virNWFilterObjRemove; virNWFilterObjTestUnassignDef; virNWFilterObjUnlock; virNWFilterObjWantRemoved; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index dd3645a..6516053 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -237,7 +237,7 @@ nwfilterStateInitialize(bool privileged, VIR_FREE(base); - if (virNWFilterObjLoadAllConfigs(&driver->nwfilters, driver->configDir) < 0) + if (virNWFilterObjListLoadAllConfigs(&driver->nwfilters, driver->configDir) < 0) goto error; nwfilterDriverUnlock(); @@ -289,7 +289,7 @@ nwfilterStateReload(void) virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); - virNWFilterObjLoadAllConfigs(&driver->nwfilters, driver->configDir); + virNWFilterObjListLoadAllConfigs(&driver->nwfilters, driver->configDir); virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); @@ -362,7 +362,7 @@ nwfilterLookupByUUID(virConnectPtr conn, virNWFilterPtr ret = NULL; nwfilterDriverLock(); - obj = virNWFilterObjFindByUUID(&driver->nwfilters, uuid); + obj = virNWFilterObjListFindByUUID(&driver->nwfilters, uuid); nwfilterDriverUnlock(); if (!obj) { @@ -393,7 +393,7 @@ nwfilterLookupByName(virConnectPtr conn, virNWFilterPtr ret = NULL; nwfilterDriverLock(); - obj = virNWFilterObjFindByName(&driver->nwfilters, name); + obj = virNWFilterObjListFindByName(&driver->nwfilters, name); nwfilterDriverUnlock(); if (!obj) { @@ -421,7 +421,7 @@ nwfilterConnectNumOfNWFilters(virConnectPtr conn) if (virConnectNumOfNWFiltersEnsureACL(conn) < 0) return -1; - return virNWFilterObjNumOfNWFilters(&driver->nwfilters, conn, + return virNWFilterObjListNumOfNWFilters(&driver->nwfilters, conn, virConnectNumOfNWFiltersCheckACL); } @@ -437,7 +437,7 @@ nwfilterConnectListNWFilters(virConnectPtr conn, return -1; nwfilterDriverLock(); - nnames = virNWFilterObjGetNames(&driver->nwfilters, conn, + nnames = virNWFilterObjListGetNames(&driver->nwfilters, conn, virConnectListNWFiltersCheckACL, names, maxnames); nwfilterDriverUnlock(); @@ -490,13 +490,13 @@ nwfilterDefineXML(virConnectPtr conn, if (virNWFilterDefineXMLEnsureACL(conn, def) < 0) goto cleanup; - if (!(obj = virNWFilterObjAssignDef(&driver->nwfilters, def))) + if (!(obj = virNWFilterObjListAssignDef(&driver->nwfilters, def))) goto cleanup; def = NULL; objdef = virNWFilterObjGetDef(obj); if (virNWFilterSaveDef(driver->configDir, objdef) < 0) { - virNWFilterObjRemove(&driver->nwfilters, obj); + virNWFilterObjListRemove(&driver->nwfilters, obj); goto cleanup; } @@ -525,7 +525,7 @@ nwfilterUndefine(virNWFilterPtr nwfilter) virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); - obj = virNWFilterObjFindByUUID(&driver->nwfilters, nwfilter->uuid); + obj = virNWFilterObjListFindByUUID(&driver->nwfilters, nwfilter->uuid); if (!obj) { virReportError(VIR_ERR_NO_NWFILTER, "%s", _("no nwfilter with matching uuid")); @@ -546,7 +546,7 @@ nwfilterUndefine(virNWFilterPtr nwfilter) if (virNWFilterDeleteDef(driver->configDir, def) < 0) goto cleanup; - virNWFilterObjRemove(&driver->nwfilters, obj); + virNWFilterObjListRemove(&driver->nwfilters, obj); obj = NULL; ret = 0; @@ -572,7 +572,7 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, virCheckFlags(0, NULL); nwfilterDriverLock(); - obj = virNWFilterObjFindByUUID(&driver->nwfilters, nwfilter->uuid); + obj = virNWFilterObjListFindByUUID(&driver->nwfilters, nwfilter->uuid); nwfilterDriverUnlock(); if (!obj) { diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index b356d87..23f1999 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 __virNWFilterInstantiateFilter * will hold a lock on a virNWFilterObjPtr. This in turn invokes * virNWFilterInstantiate which invokes virNWFilterDetermineMissingVarsRec - * which invokes virNWFilterObjFindByName. This iterates over every single + * 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 * __virNWFilterInstantiateFilter which will cause the other thread - * to deadlock in virNWFilterObjFindByName. + * to deadlock in virNWFilterObjListFindByName. * * XXX better long term solution is to make virNWFilterObjList use a * hash table as is done for virDomainObjList. You can then get @@ -383,8 +383,8 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStatePtr driver, int ret = -1; VIR_DEBUG("Instantiating filter %s", inc->filterref); - obj = virNWFilterObjFindByName(&driver->nwfilters, - inc->filterref); + obj = virNWFilterObjListFindByName(&driver->nwfilters, + inc->filterref); if (!obj) { virReportError(VIR_ERR_INTERNAL_ERROR, _("referenced filter '%s' is missing"), @@ -545,7 +545,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, break; } else if (inc) { VIR_DEBUG("Following filter %s", inc->filterref); - obj = virNWFilterObjFindByName(&driver->nwfilters, inc->filterref); + obj = virNWFilterObjListFindByName(&driver->nwfilters, inc->filterref); if (obj) { if (virNWFilterObjWantRemoved(obj)) { @@ -812,7 +812,7 @@ __virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, VIR_DEBUG("filter name: %s", filtername); - obj = virNWFilterObjFindByName(&driver->nwfilters, filtername); + obj = virNWFilterObjListFindByName(&driver->nwfilters, filtername); if (!obj) { virReportError(VIR_ERR_NO_NWFILTER, _("Could not find filter '%s'"), -- 2.9.3
Move from virnwfilterobj.h to virnwfilterobj.c. Create the virNWFilterObjListNew() API in order to allocate Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 16 ++++++++++++++++ src/conf/virnwfilterobj.h | 10 ++++------ src/libvirt_private.syms | 1 + src/nwfilter/nwfilter_driver.c | 29 ++++++++++++++++------------- src/nwfilter/nwfilter_gentech_driver.c | 6 +++--- 5 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 28425ba..c1d2042 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -42,6 +42,11 @@ struct _virNWFilterObj { virNWFilterDefPtr newDef; }; +struct _virNWFilterObjList { + size_t count; + virNWFilterObjPtr *objs; +}; + static virNWFilterObjPtr virNWFilterObjNew(void) @@ -110,6 +115,17 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters) } +virNWFilterObjListPtr +virNWFilterObjListNew(void) +{ + virNWFilterObjListPtr nwfilters; + + if (VIR_ALLOC(nwfilters) < 0) + return NULL; + return nwfilters; +} + + void virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters, virNWFilterObjPtr obj) diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h index 4c19223..29d9086 100644 --- a/src/conf/virnwfilterobj.h +++ b/src/conf/virnwfilterobj.h @@ -28,11 +28,6 @@ typedef virNWFilterObj *virNWFilterObjPtr; typedef struct _virNWFilterObjList virNWFilterObjList; typedef virNWFilterObjList *virNWFilterObjListPtr; -struct _virNWFilterObjList { - size_t count; - virNWFilterObjPtr *objs; -}; - typedef struct _virNWFilterDriverState virNWFilterDriverState; typedef virNWFilterDriverState *virNWFilterDriverStatePtr; @@ -40,7 +35,7 @@ struct _virNWFilterDriverState { virMutex lock; bool privileged; - virNWFilterObjList nwfilters; + virNWFilterObjListPtr nwfilters; char *configDir; bool watchingFirewallD; @@ -55,6 +50,9 @@ virNWFilterObjGetNewDef(virNWFilterObjPtr obj); bool virNWFilterObjWantRemoved(virNWFilterObjPtr obj); +virNWFilterObjListPtr +virNWFilterObjListNew(void); + void virNWFilterObjListFree(virNWFilterObjListPtr nwfilters); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 453d0fe..13f2ab9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -971,6 +971,7 @@ virNWFilterObjListFindByUUID; virNWFilterObjListFree; virNWFilterObjListGetNames; virNWFilterObjListLoadAllConfigs; +virNWFilterObjListNew; virNWFilterObjListNumOfNWFilters; virNWFilterObjListRemove; virNWFilterObjLock; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 6516053..650c528 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -237,7 +237,10 @@ nwfilterStateInitialize(bool privileged, VIR_FREE(base); - if (virNWFilterObjListLoadAllConfigs(&driver->nwfilters, driver->configDir) < 0) + if (!(driver->nwfilters = virNWFilterObjListNew())) + goto error; + + if (virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir) < 0) goto error; nwfilterDriverUnlock(); @@ -289,7 +292,7 @@ nwfilterStateReload(void) virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); - virNWFilterObjListLoadAllConfigs(&driver->nwfilters, driver->configDir); + virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir); virNWFilterCallbackDriversUnlock(); virNWFilterUnlockFilterUpdates(); @@ -340,7 +343,7 @@ nwfilterStateCleanup(void) nwfilterDriverRemoveDBusMatches(); /* free inactive nwfilters */ - virNWFilterObjListFree(&driver->nwfilters); + virNWFilterObjListFree(driver->nwfilters); VIR_FREE(driver->configDir); nwfilterDriverUnlock(); @@ -362,7 +365,7 @@ nwfilterLookupByUUID(virConnectPtr conn, virNWFilterPtr ret = NULL; nwfilterDriverLock(); - obj = virNWFilterObjListFindByUUID(&driver->nwfilters, uuid); + obj = virNWFilterObjListFindByUUID(driver->nwfilters, uuid); nwfilterDriverUnlock(); if (!obj) { @@ -393,7 +396,7 @@ nwfilterLookupByName(virConnectPtr conn, virNWFilterPtr ret = NULL; nwfilterDriverLock(); - obj = virNWFilterObjListFindByName(&driver->nwfilters, name); + obj = virNWFilterObjListFindByName(driver->nwfilters, name); nwfilterDriverUnlock(); if (!obj) { @@ -421,7 +424,7 @@ nwfilterConnectNumOfNWFilters(virConnectPtr conn) if (virConnectNumOfNWFiltersEnsureACL(conn) < 0) return -1; - return virNWFilterObjListNumOfNWFilters(&driver->nwfilters, conn, + return virNWFilterObjListNumOfNWFilters(driver->nwfilters, conn, virConnectNumOfNWFiltersCheckACL); } @@ -437,7 +440,7 @@ nwfilterConnectListNWFilters(virConnectPtr conn, return -1; nwfilterDriverLock(); - nnames = virNWFilterObjListGetNames(&driver->nwfilters, conn, + nnames = virNWFilterObjListGetNames(driver->nwfilters, conn, virConnectListNWFiltersCheckACL, names, maxnames); nwfilterDriverUnlock(); @@ -458,7 +461,7 @@ nwfilterConnectListAllNWFilters(virConnectPtr conn, return -1; nwfilterDriverLock(); - ret = virNWFilterObjListExport(conn, &driver->nwfilters, filters, + ret = virNWFilterObjListExport(conn, driver->nwfilters, filters, virConnectListAllNWFiltersCheckACL); nwfilterDriverUnlock(); @@ -490,13 +493,13 @@ nwfilterDefineXML(virConnectPtr conn, if (virNWFilterDefineXMLEnsureACL(conn, def) < 0) goto cleanup; - if (!(obj = virNWFilterObjListAssignDef(&driver->nwfilters, def))) + if (!(obj = virNWFilterObjListAssignDef(driver->nwfilters, def))) goto cleanup; def = NULL; objdef = virNWFilterObjGetDef(obj); if (virNWFilterSaveDef(driver->configDir, objdef) < 0) { - virNWFilterObjListRemove(&driver->nwfilters, obj); + virNWFilterObjListRemove(driver->nwfilters, obj); goto cleanup; } @@ -525,7 +528,7 @@ nwfilterUndefine(virNWFilterPtr nwfilter) virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); - obj = virNWFilterObjListFindByUUID(&driver->nwfilters, nwfilter->uuid); + obj = virNWFilterObjListFindByUUID(driver->nwfilters, nwfilter->uuid); if (!obj) { virReportError(VIR_ERR_NO_NWFILTER, "%s", _("no nwfilter with matching uuid")); @@ -546,7 +549,7 @@ nwfilterUndefine(virNWFilterPtr nwfilter) if (virNWFilterDeleteDef(driver->configDir, def) < 0) goto cleanup; - virNWFilterObjListRemove(&driver->nwfilters, obj); + virNWFilterObjListRemove(driver->nwfilters, obj); obj = NULL; ret = 0; @@ -572,7 +575,7 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, virCheckFlags(0, NULL); nwfilterDriverLock(); - obj = virNWFilterObjListFindByUUID(&driver->nwfilters, nwfilter->uuid); + obj = virNWFilterObjListFindByUUID(driver->nwfilters, nwfilter->uuid); nwfilterDriverUnlock(); if (!obj) { diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 23f1999..82e20de 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -383,7 +383,7 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverStatePtr driver, int ret = -1; VIR_DEBUG("Instantiating filter %s", inc->filterref); - obj = virNWFilterObjListFindByName(&driver->nwfilters, + obj = virNWFilterObjListFindByName(driver->nwfilters, inc->filterref); if (!obj) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -545,7 +545,7 @@ virNWFilterDetermineMissingVarsRec(virNWFilterDefPtr filter, break; } else if (inc) { VIR_DEBUG("Following filter %s", inc->filterref); - obj = virNWFilterObjListFindByName(&driver->nwfilters, inc->filterref); + obj = virNWFilterObjListFindByName(driver->nwfilters, inc->filterref); if (obj) { if (virNWFilterObjWantRemoved(obj)) { @@ -812,7 +812,7 @@ __virNWFilterInstantiateFilter(virNWFilterDriverStatePtr driver, VIR_DEBUG("filter name: %s", filtername); - obj = virNWFilterObjListFindByName(&driver->nwfilters, filtername); + obj = virNWFilterObjListFindByName(driver->nwfilters, filtername); if (!obj) { virReportError(VIR_ERR_NO_NWFILTER, _("Could not find filter '%s'"), -- 2.9.3
Rather than separate calls, use a common call and generate a better error message which includes the incorrect uuidstr Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/nwfilter/nwfilter_driver.c | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 650c528..781a7a0 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -356,6 +356,21 @@ nwfilterStateCleanup(void) } +static virNWFilterObjPtr +nwfilterObjFromNWFilter(const unsigned char *uuid) +{ + virNWFilterObjPtr obj; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (!(obj = virNWFilterObjListFindByUUID(driver->nwfilters, uuid))) { + virUUIDFormat(uuid, uuidstr); + virReportError(VIR_ERR_NO_NWFILTER, + _("no nwfilter with matching uuid '%s'"), uuidstr); + } + return obj; +} + + static virNWFilterPtr nwfilterLookupByUUID(virConnectPtr conn, const unsigned char *uuid) @@ -365,14 +380,11 @@ nwfilterLookupByUUID(virConnectPtr conn, virNWFilterPtr ret = NULL; nwfilterDriverLock(); - obj = virNWFilterObjListFindByUUID(driver->nwfilters, uuid); + obj = nwfilterObjFromNWFilter(uuid); nwfilterDriverUnlock(); - if (!obj) { - virReportError(VIR_ERR_NO_NWFILTER, - "%s", _("no nwfilter with matching uuid")); + if (!obj) goto cleanup; - } def = virNWFilterObjGetDef(obj); if (virNWFilterLookupByUUIDEnsureACL(conn, def) < 0) @@ -528,12 +540,8 @@ nwfilterUndefine(virNWFilterPtr nwfilter) virNWFilterWriteLockFilterUpdates(); virNWFilterCallbackDriversLock(); - obj = virNWFilterObjListFindByUUID(driver->nwfilters, nwfilter->uuid); - if (!obj) { - virReportError(VIR_ERR_NO_NWFILTER, - "%s", _("no nwfilter with matching uuid")); + if (!(obj = nwfilterObjFromNWFilter(nwfilter->uuid))) goto cleanup; - } def = virNWFilterObjGetDef(obj); if (virNWFilterUndefineEnsureACL(nwfilter->conn, def) < 0) @@ -575,14 +583,11 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter, virCheckFlags(0, NULL); nwfilterDriverLock(); - obj = virNWFilterObjListFindByUUID(driver->nwfilters, nwfilter->uuid); + obj = nwfilterObjFromNWFilter(nwfilter->uuid); nwfilterDriverUnlock(); - if (!obj) { - virReportError(VIR_ERR_NO_NWFILTER, - "%s", _("no nwfilter with matching uuid")); + if (!obj) goto cleanup; - } def = virNWFilterObjGetDef(obj); if (virNWFilterGetXMLDescEnsureACL(nwfilter->conn, def) < 0) -- 2.9.3
Remove open coded helper. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/nwfilter_conf.c | 13 +------------ src/conf/nwfilter_conf.h | 4 ---- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index b69d9db..2352e60 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2775,7 +2775,7 @@ virNWFilterSaveXML(const char *configDir, char *configFile = NULL; int ret = -1; - if ((configFile = virNWFilterConfigFile(configDir, def->name)) == NULL) + if (!(configFile = virFileBuildPath(configDir, def->name, ".xml"))) goto cleanup; if (virFileMakePath(configDir) < 0) { @@ -3236,17 +3236,6 @@ virNWFilterDefFormat(const virNWFilterDef *def) } -char * -virNWFilterConfigFile(const char *dir, - const char *name) -{ - char *ret = NULL; - - ignore_value(virAsprintf(&ret, "%s/%s.xml", dir, name)); - return ret; -} - - int virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB, void *opaque) diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index d8a5daf..5bf9c3d 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -593,10 +593,6 @@ int virNWFilterSaveConfig(const char *configDir, virNWFilterDefPtr def); -char * -virNWFilterConfigFile(const char *dir, - const char *name); - virNWFilterDefPtr virNWFilterDefParseString(const char *xml); -- 2.9.3
Essentially virNWFilterSaveDef executed in a different order the same sequence of calls, so let's just make one point of reference. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/nwfilter_conf.c | 37 ------------------------------------- src/conf/nwfilter_conf.h | 4 ---- src/libvirt_private.syms | 2 +- src/nwfilter/nwfilter_driver.c | 2 +- 4 files changed, 2 insertions(+), 43 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 2352e60..752d4e1 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2932,43 +2932,6 @@ virNWFilterTriggerVMFilterRebuild(void) int -virNWFilterSaveDef(const char *configDir, - virNWFilterDefPtr def) -{ - char uuidstr[VIR_UUID_STRING_BUFLEN]; - char *xml; - int ret = -1; - char *configFile = NULL; - - if (virFileMakePath(configDir) < 0) { - virReportSystemError(errno, - _("cannot create config directory %s"), - configDir); - goto error; - } - - if (!(configFile = virFileBuildPath(configDir, def->name, ".xml"))) - goto error; - - if (!(xml = virNWFilterDefFormat(def))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to generate XML")); - goto error; - } - - virUUIDFormat(def->uuid, uuidstr); - ret = virXMLSaveFile(configFile, - virXMLPickShellSafeComment(def->name, uuidstr), - "nwfilter-edit", xml); - VIR_FREE(xml); - - error: - VIR_FREE(configFile); - return ret; -} - - -int virNWFilterDeleteDef(const char *configDir, virNWFilterDefPtr def) { diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index 5bf9c3d..5cac260 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -570,10 +570,6 @@ int virNWFilterTriggerVMFilterRebuild(void); int -virNWFilterSaveDef(const char *configDir, - virNWFilterDefPtr def); - -int virNWFilterDeleteDef(const char *configDir, virNWFilterDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 13f2ab9..170ecce 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -740,7 +740,7 @@ virNWFilterRuleIsProtocolEthernet; virNWFilterRuleIsProtocolIPv4; virNWFilterRuleIsProtocolIPv6; virNWFilterRuleProtocolTypeToString; -virNWFilterSaveDef; +virNWFilterSaveConfig; virNWFilterTriggerVMFilterRebuild; virNWFilterUnlockFilterUpdates; virNWFilterUnRegisterCallbackDriver; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 781a7a0..faa4fe8 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -510,7 +510,7 @@ nwfilterDefineXML(virConnectPtr conn, def = NULL; objdef = virNWFilterObjGetDef(obj); - if (virNWFilterSaveDef(driver->configDir, objdef) < 0) { + if (virNWFilterSaveConfig(driver->configDir, objdef) < 0) { virNWFilterObjListRemove(driver->nwfilters, obj); goto cleanup; } -- 2.9.3
Rather than "wait" for the first config file to be created, force creation of the configDir during driver state initialization. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/nwfilter_conf.c | 7 ------- src/nwfilter/nwfilter_driver.c | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 752d4e1..032700c 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2778,13 +2778,6 @@ virNWFilterSaveXML(const char *configDir, if (!(configFile = virFileBuildPath(configDir, def->name, ".xml"))) goto cleanup; - if (virFileMakePath(configDir) < 0) { - virReportSystemError(errno, - _("cannot create config directory '%s'"), - configDir); - goto cleanup; - } - virUUIDFormat(def->uuid, uuidstr); ret = virXMLSaveFile(configFile, virXMLPickShellSafeComment(def->name, uuidstr), diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index faa4fe8..5e62023 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -40,6 +40,7 @@ #include "nwfilter_driver.h" #include "nwfilter_gentech_driver.h" #include "configmake.h" +#include "virfile.h" #include "virstring.h" #include "viraccessapicheck.h" @@ -237,6 +238,12 @@ nwfilterStateInitialize(bool privileged, VIR_FREE(base); + if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) { + virReportSystemError(errno, _("cannot create config directory '%s'"), + driver->configDir); + goto error; + } + if (!(driver->nwfilters = virNWFilterObjListNew())) goto error; -- 2.9.3
Only save the config when using a generated UUID if we were able to create an object for the def. There could have been "other reasons" for the assignment to fail, so saving the config could be incorrect. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virnwfilterobj.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index c1d2042..8c9da13 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -501,14 +501,14 @@ virNWFilterObjListLoadConfig(virNWFilterObjListPtr nwfilters, goto error; } + if (!(obj = virNWFilterObjListAssignDef(nwfilters, def))) + goto error; + /* We generated a UUID, make it permanent by saving the config to disk */ if (!def->uuid_specified && virNWFilterSaveConfig(configDir, def) < 0) goto error; - if (!(obj = virNWFilterObjListAssignDef(nwfilters, def))) - goto error; - VIR_FREE(configFile); return obj; -- 2.9.3
On Tue, Apr 25, 2017 at 03:30:20PM -0400, John Ferlan wrote:
v1: https://www.redhat.com/archives/libvir-list/2017-April/msg01074.html
Changes since v1:
* Patch 2 -> Do not add the local @def when just dereference obj->def
* Patch 13 & 15 dropped
Most patches were ACK'd, but patch 2's impact was felt in a few other patches so I just resent the whole thing. In particular patch 5 for nwfilterUndefine and nwfilterGetXMLDesc as well as patch 7 for the _virNWFilterObjListDefLoopDetect call w/ _virNWFilterObjListDefLoopDetect
One small nit for some of the commit messages, you are inconsistent with ending a sentence with a period. Just something to keep in mind, you don't have to fix it. ACK series. Pavel
participants (2)
-
John Ferlan -
Pavel Hrdina