[libvirt] [PATCH REPOST 0/6] Privatize _virStoragePoolObj and _virStorageVolDefList (Batch #1)
Since the previous series (19 patches): https://www.redhat.com/archives/libvir-list/2017-September/msg00594.html didn't garner any attention, let me try with smaller patch piles with the hope that forward progress will be made. The only "new" one in this series from the other is patch2 - something that Coverity let me know about in an error scenario. Essentially the bulk of these patches remove direct obj->def referencing in favor of a virStoragePoolObjGetDef call. John Ferlan (6): conf: Fix prototype/definition for virStoragePoolObj get functions tests: Fix possible NULL deref storage: Use virStoragePoolObjGetDef accessor for driver test: Rename @vol to @volDef in testOpenVolumesForPool test: Create local virStoragePoolObjPtr VolLookup APIs test: Use virStoragePoolObjGetDef accessor src/conf/virstorageobj.c | 4 +- src/conf/virstorageobj.h | 4 +- src/storage/storage_driver.c | 411 +++++++++++++++++++++++------------------ src/test/test_driver.c | 203 +++++++++++--------- tests/storagevolxml2argvtest.c | 3 +- 5 files changed, 352 insertions(+), 273 deletions(-) -- 2.13.6
Modify virStoragePoolObjGetAutostartLink and virStoragePoolObjGetConfigFile to return "const char *" since that's how both are used and to ensure no one tries to VIR_FREE the result. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/virstorageobj.c | 4 ++-- src/conf/virstorageobj.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 1364bddd15..ff04c9efe4 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -89,7 +89,7 @@ virStoragePoolObjDefUseNewDef(virStoragePoolObjPtr obj) } -char * +const char * virStoragePoolObjGetConfigFile(virStoragePoolObjPtr obj) { return obj->configFile; @@ -105,7 +105,7 @@ virStoragePoolObjSetConfigFile(virStoragePoolObjPtr obj, } -char * +const char * virStoragePoolObjGetAutostartLink(virStoragePoolObjPtr obj) { return obj->autostartLink; diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index b65b16019c..cf7ee06cd1 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -86,14 +86,14 @@ virStoragePoolObjGetNewDef(virStoragePoolObjPtr obj); void virStoragePoolObjDefUseNewDef(virStoragePoolObjPtr obj); -char * +const char * virStoragePoolObjGetConfigFile(virStoragePoolObjPtr obj); void virStoragePoolObjSetConfigFile(virStoragePoolObjPtr obj, char *configFile); -char * +const char * virStoragePoolObjGetAutostartLink(virStoragePoolObjPtr obj); bool -- 2.13.6
Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/storagevolxml2argvtest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 1b3003216d..73d9c4bc6b 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -110,7 +110,8 @@ testCompareXMLToArgvFiles(bool shouldFail, virStorageVolDefFree(inputvol); virCommandFree(cmd); VIR_FREE(actualCmdline); - virStoragePoolObjUnlock(obj); + if (obj) + virStoragePoolObjUnlock(obj); virStoragePoolObjFree(obj); virObjectUnref(conn); return ret; -- 2.13.6
In preparation for privatizing the object, use the accessor to fetch the obj->def instead of the direct reference. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_driver.c | 411 ++++++++++++++++++++++++------------------- 1 file changed, 234 insertions(+), 177 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index d08dedd879..b0edf9f885 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -104,17 +104,17 @@ virStoragePoolUpdateInactive(virStoragePoolObjPtr *objptr) static void storagePoolUpdateState(virStoragePoolObjPtr obj) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); bool active = false; virStorageBackendPtr backend; char *stateFile; - if (!(stateFile = virFileBuildPath(driver->stateDir, - obj->def->name, ".xml"))) + if (!(stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"))) goto cleanup; - if ((backend = virStorageBackendForType(obj->def->type)) == NULL) { + if ((backend = virStorageBackendForType(def->type)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing backend %d"), obj->def->type); + _("Missing backend %d"), def->type); goto cleanup; } @@ -124,7 +124,7 @@ storagePoolUpdateState(virStoragePoolObjPtr obj) backend->checkPool(obj, &active) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to initialize storage pool '%s': %s"), - obj->def->name, virGetLastErrorMessage()); + def->name, virGetLastErrorMessage()); active = false; } @@ -139,7 +139,7 @@ storagePoolUpdateState(virStoragePoolObjPtr obj) backend->stopPool(NULL, obj); virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to restart storage pool '%s': %s"), - obj->def->name, virGetLastErrorMessage()); + def->name, virGetLastErrorMessage()); active = false; } } @@ -186,11 +186,12 @@ storageDriverAutostart(void) for (i = 0; i < driver->pools.count; i++) { virStoragePoolObjPtr obj = driver->pools.objs[i]; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); virStorageBackendPtr backend; bool started = false; virStoragePoolObjLock(obj); - if ((backend = virStorageBackendForType(obj->def->type)) == NULL) { + if ((backend = virStorageBackendForType(def->type)) == NULL) { virStoragePoolObjUnlock(obj); continue; } @@ -201,7 +202,7 @@ storageDriverAutostart(void) backend->startPool(conn, obj) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to autostart storage pool '%s': %s"), - obj->def->name, virGetLastErrorMessage()); + def->name, virGetLastErrorMessage()); virStoragePoolObjUnlock(obj); continue; } @@ -212,10 +213,9 @@ storageDriverAutostart(void) char *stateFile; virStoragePoolObjClearVols(obj); - stateFile = virFileBuildPath(driver->stateDir, - obj->def->name, ".xml"); + stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"); if (!stateFile || - virStoragePoolSaveState(stateFile, obj->def) < 0 || + virStoragePoolSaveState(stateFile, def) < 0 || backend->refreshPool(conn, obj) < 0) { if (stateFile) unlink(stateFile); @@ -223,7 +223,7 @@ storageDriverAutostart(void) backend->stopPool(conn, obj); virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to autostart storage pool '%s': %s"), - obj->def->name, virGetLastErrorMessage()); + def->name, virGetLastErrorMessage()); } else { virStoragePoolObjSetActive(obj, true); } @@ -442,6 +442,7 @@ storagePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; storageDriverLock(); @@ -449,11 +450,12 @@ storagePoolLookupByUUID(virConnectPtr conn, storageDriverUnlock(); if (!obj) return NULL; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolLookupByUUIDEnsureACL(conn, obj->def) < 0) + if (virStoragePoolLookupByUUIDEnsureACL(conn, def) < 0) goto cleanup; - pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, NULL, NULL); + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); cleanup: virStoragePoolObjUnlock(obj); @@ -465,15 +467,17 @@ storagePoolLookupByName(virConnectPtr conn, const char *name) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; if (!(obj = storagePoolObjFindByName(name))) return NULL; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolLookupByNameEnsureACL(conn, obj->def) < 0) + if (virStoragePoolLookupByNameEnsureACL(conn, def) < 0) goto cleanup; - pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, NULL, NULL); + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); cleanup: virStoragePoolObjUnlock(obj); @@ -484,16 +488,17 @@ static virStoragePoolPtr storagePoolLookupByVolume(virStorageVolPtr vol) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; if (!(obj = storagePoolObjFindByName(vol->pool))) return NULL; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolLookupByVolumeEnsureACL(vol->conn, obj->def) < 0) + if (virStoragePoolLookupByVolumeEnsureACL(vol->conn, def) < 0) goto cleanup; - pool = virGetStoragePool(vol->conn, obj->def->name, obj->def->uuid, - NULL, NULL); + pool = virGetStoragePool(vol->conn, def->name, def->uuid, NULL, NULL); cleanup: virStoragePoolObjUnlock(obj); @@ -614,12 +619,14 @@ static int storagePoolIsActive(virStoragePoolPtr pool) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; int ret = -1; if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolIsActiveEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolIsActiveEnsureACL(pool->conn, def) < 0) goto cleanup; ret = virStoragePoolObjIsActive(obj); @@ -634,12 +641,14 @@ static int storagePoolIsPersistent(virStoragePoolPtr pool) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; int ret = -1; if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolIsPersistentEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolIsPersistentEnsureACL(pool->conn, def) < 0) goto cleanup; ret = virStoragePoolObjGetConfigFile(obj) ? 1 : 0; @@ -655,8 +664,9 @@ storagePoolCreateXML(virConnectPtr conn, const char *xml, unsigned int flags) { - virStoragePoolDefPtr def; + virStoragePoolDefPtr newDef; virStoragePoolObjPtr obj = NULL; + virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; virStorageBackendPtr backend; virObjectEventPtr event = NULL; @@ -671,24 +681,25 @@ storagePoolCreateXML(virConnectPtr conn, VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, NULL); storageDriverLock(); - if (!(def = virStoragePoolDefParseString(xml))) + if (!(newDef = virStoragePoolDefParseString(xml))) goto cleanup; - if (virStoragePoolCreateXMLEnsureACL(conn, def) < 0) + if (virStoragePoolCreateXMLEnsureACL(conn, newDef) < 0) goto cleanup; - if (virStoragePoolObjIsDuplicate(&driver->pools, def, 1) < 0) + if (virStoragePoolObjIsDuplicate(&driver->pools, newDef, 1) < 0) goto cleanup; - if (virStoragePoolObjSourceFindDuplicate(conn, &driver->pools, def) < 0) + if (virStoragePoolObjSourceFindDuplicate(conn, &driver->pools, newDef) < 0) goto cleanup; - if ((backend = virStorageBackendForType(def->type)) == NULL) + if ((backend = virStorageBackendForType(newDef->type)) == NULL) goto cleanup; - if (!(obj = virStoragePoolObjAssignDef(&driver->pools, def))) + if (!(obj = virStoragePoolObjAssignDef(&driver->pools, newDef))) goto cleanup; - def = NULL; + newDef = NULL; + def = virStoragePoolObjGetDef(obj); if (backend->buildPool) { if (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE) @@ -713,11 +724,10 @@ storagePoolCreateXML(virConnectPtr conn, goto cleanup; } - stateFile = virFileBuildPath(driver->stateDir, - obj->def->name, ".xml"); + stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"); virStoragePoolObjClearVols(obj); - if (!stateFile || virStoragePoolSaveState(stateFile, obj->def) < 0 || + if (!stateFile || virStoragePoolSaveState(stateFile, def) < 0 || backend->refreshPool(conn, obj) < 0) { if (stateFile) unlink(stateFile); @@ -728,19 +738,19 @@ storagePoolCreateXML(virConnectPtr conn, goto cleanup; } - event = virStoragePoolEventLifecycleNew(obj->def->name, - obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, + def->uuid, VIR_STORAGE_POOL_EVENT_STARTED, 0); - VIR_INFO("Creating storage pool '%s'", obj->def->name); + VIR_INFO("Creating storage pool '%s'", def->name); virStoragePoolObjSetActive(obj, true); - pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, NULL, NULL); + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); cleanup: VIR_FREE(stateFile); - virStoragePoolDefFree(def); + virStoragePoolDefFree(newDef); if (event) virObjectEventStateQueue(driver->storageEventState, event); if (obj) @@ -754,38 +764,40 @@ storagePoolDefineXML(virConnectPtr conn, const char *xml, unsigned int flags) { - virStoragePoolDefPtr def; + virStoragePoolDefPtr newDef; virStoragePoolObjPtr obj = NULL; + virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; virObjectEventPtr event = NULL; virCheckFlags(0, NULL); storageDriverLock(); - if (!(def = virStoragePoolDefParseString(xml))) + if (!(newDef = virStoragePoolDefParseString(xml))) goto cleanup; - if (virXMLCheckIllegalChars("name", def->name, "\n") < 0) + if (virXMLCheckIllegalChars("name", newDef->name, "\n") < 0) goto cleanup; - if (virStoragePoolDefineXMLEnsureACL(conn, def) < 0) + if (virStoragePoolDefineXMLEnsureACL(conn, newDef) < 0) goto cleanup; - if (virStoragePoolObjIsDuplicate(&driver->pools, def, 0) < 0) + if (virStoragePoolObjIsDuplicate(&driver->pools, newDef, 0) < 0) goto cleanup; - if (virStoragePoolObjSourceFindDuplicate(conn, &driver->pools, def) < 0) + if (virStoragePoolObjSourceFindDuplicate(conn, &driver->pools, newDef) < 0) goto cleanup; - if (virStorageBackendForType(def->type) == NULL) + if (virStorageBackendForType(newDef->type) == NULL) goto cleanup; - if (!(obj = virStoragePoolObjAssignDef(&driver->pools, def))) + if (!(obj = virStoragePoolObjAssignDef(&driver->pools, newDef))) goto cleanup; + newDef = NULL; + def = virStoragePoolObjGetDef(obj); if (virStoragePoolObjSaveDef(driver, obj, def) < 0) { virStoragePoolObjRemove(&driver->pools, obj); - def = NULL; obj = NULL; goto cleanup; } @@ -794,15 +806,13 @@ storagePoolDefineXML(virConnectPtr conn, VIR_STORAGE_POOL_EVENT_DEFINED, 0); - def = NULL; - - VIR_INFO("Defining storage pool '%s'", obj->def->name); - pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, NULL, NULL); + VIR_INFO("Defining storage pool '%s'", def->name); + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); cleanup: if (event) virObjectEventStateQueue(driver->storageEventState, event); - virStoragePoolDefFree(def); + virStoragePoolDefFree(newDef); if (obj) virStoragePoolObjUnlock(obj); storageDriverUnlock(); @@ -813,6 +823,7 @@ static int storagePoolUndefine(virStoragePoolPtr pool) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; const char *autostartLink; virObjectEventPtr event = NULL; int ret = -1; @@ -820,21 +831,22 @@ storagePoolUndefine(virStoragePoolPtr pool) storageDriverLock(); if (!(obj = storagePoolObjFindByUUID(pool->uuid, pool->name))) goto cleanup; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolUndefineEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolUndefineEnsureACL(pool->conn, def) < 0) goto cleanup; if (virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, _("storage pool '%s' is still active"), - obj->def->name); + def->name); goto cleanup; } if (virStoragePoolObjGetAsyncjobs(obj) > 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("pool '%s' has asynchronous jobs running."), - obj->def->name); + def->name); goto cleanup; } @@ -849,13 +861,12 @@ storagePoolUndefine(virStoragePoolPtr pool) autostartLink, virStrerror(errno, ebuf, sizeof(ebuf))); } - - event = virStoragePoolEventLifecycleNew(obj->def->name, - obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, + def->uuid, VIR_STORAGE_POOL_EVENT_UNDEFINED, 0); - VIR_INFO("Undefining storage pool '%s'", obj->def->name); + VIR_INFO("Undefining storage pool '%s'", def->name); virStoragePoolObjRemove(&driver->pools, obj); obj = NULL; ret = 0; @@ -874,6 +885,7 @@ storagePoolCreate(virStoragePoolPtr pool, unsigned int flags) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageBackendPtr backend; virObjectEventPtr event = NULL; int ret = -1; @@ -889,17 +901,18 @@ storagePoolCreate(virStoragePoolPtr pool, if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolCreateEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolCreateEnsureACL(pool->conn, def) < 0) goto cleanup; - if ((backend = virStorageBackendForType(obj->def->type)) == NULL) + if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; if (virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, _("storage pool '%s' is already active"), - obj->def->name); + def->name); goto cleanup; } @@ -916,16 +929,15 @@ storagePoolCreate(virStoragePoolPtr pool, } } - VIR_INFO("Starting up storage pool '%s'", obj->def->name); + VIR_INFO("Starting up storage pool '%s'", def->name); if (backend->startPool && backend->startPool(pool->conn, obj) < 0) goto cleanup; - stateFile = virFileBuildPath(driver->stateDir, - obj->def->name, ".xml"); + stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"); virStoragePoolObjClearVols(obj); - if (!stateFile || virStoragePoolSaveState(stateFile, obj->def) < 0 || + if (!stateFile || virStoragePoolSaveState(stateFile, def) < 0 || backend->refreshPool(pool->conn, obj) < 0) { if (stateFile) unlink(stateFile); @@ -934,8 +946,8 @@ storagePoolCreate(virStoragePoolPtr pool, goto cleanup; } - event = virStoragePoolEventLifecycleNew(obj->def->name, - obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, + def->uuid, VIR_STORAGE_POOL_EVENT_STARTED, 0); @@ -956,23 +968,25 @@ storagePoolBuild(virStoragePoolPtr pool, unsigned int flags) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageBackendPtr backend; virObjectEventPtr event = NULL; int ret = -1; if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolBuildEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolBuildEnsureACL(pool->conn, def) < 0) goto cleanup; - if ((backend = virStorageBackendForType(obj->def->type)) == NULL) + if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; if (virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, _("storage pool '%s' is already active"), - obj->def->name); + def->name); goto cleanup; } @@ -999,6 +1013,7 @@ static int storagePoolDestroy(virStoragePoolPtr pool) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageBackendPtr backend; virObjectEventPtr event = NULL; char *stateFile = NULL; @@ -1007,31 +1022,30 @@ storagePoolDestroy(virStoragePoolPtr pool) storageDriverLock(); if (!(obj = storagePoolObjFindByUUID(pool->uuid, pool->name))) goto cleanup; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolDestroyEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolDestroyEnsureACL(pool->conn, def) < 0) goto cleanup; - if ((backend = virStorageBackendForType(obj->def->type)) == NULL) + if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; - VIR_INFO("Destroying storage pool '%s'", obj->def->name); + VIR_INFO("Destroying storage pool '%s'", def->name); if (!virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, - _("storage pool '%s' is not active"), obj->def->name); + _("storage pool '%s' is not active"), def->name); goto cleanup; } if (virStoragePoolObjGetAsyncjobs(obj) > 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("pool '%s' has asynchronous jobs running."), - obj->def->name); + def->name); goto cleanup; } - if (!(stateFile = virFileBuildPath(driver->stateDir, - obj->def->name, - ".xml"))) + if (!(stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"))) goto cleanup; unlink(stateFile); @@ -1043,8 +1057,8 @@ storagePoolDestroy(virStoragePoolPtr pool) virStoragePoolObjClearVols(obj); - event = virStoragePoolEventLifecycleNew(obj->def->name, - obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, + def->uuid, VIR_STORAGE_POOL_EVENT_STOPPED, 0); @@ -1068,6 +1082,7 @@ storagePoolDelete(virStoragePoolPtr pool, unsigned int flags) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageBackendPtr backend; virObjectEventPtr event = NULL; char *stateFile = NULL; @@ -1075,32 +1090,31 @@ storagePoolDelete(virStoragePoolPtr pool, if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolDeleteEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolDeleteEnsureACL(pool->conn, def) < 0) goto cleanup; - if ((backend = virStorageBackendForType(obj->def->type)) == NULL) + if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; - VIR_INFO("Deleting storage pool '%s'", obj->def->name); + VIR_INFO("Deleting storage pool '%s'", def->name); if (virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, _("storage pool '%s' is still active"), - obj->def->name); + def->name); goto cleanup; } if (virStoragePoolObjGetAsyncjobs(obj) > 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("pool '%s' has asynchronous jobs running."), - obj->def->name); + def->name); goto cleanup; } - if (!(stateFile = virFileBuildPath(driver->stateDir, - obj->def->name, - ".xml"))) + if (!(stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"))) goto cleanup; unlink(stateFile); @@ -1134,6 +1148,7 @@ storagePoolRefresh(virStoragePoolPtr pool, unsigned int flags) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageBackendPtr backend; int ret = -1; virObjectEventPtr event = NULL; @@ -1143,23 +1158,24 @@ storagePoolRefresh(virStoragePoolPtr pool, storageDriverLock(); if (!(obj = storagePoolObjFindByUUID(pool->uuid, pool->name))) goto cleanup; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolRefreshEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolRefreshEnsureACL(pool->conn, def) < 0) goto cleanup; - if ((backend = virStorageBackendForType(obj->def->type)) == NULL) + if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; if (!virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, - _("storage pool '%s' is not active"), obj->def->name); + _("storage pool '%s' is not active"), def->name); goto cleanup; } if (virStoragePoolObjGetAsyncjobs(obj) > 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("pool '%s' has asynchronous jobs running."), - obj->def->name); + def->name); goto cleanup; } @@ -1168,8 +1184,8 @@ storagePoolRefresh(virStoragePoolPtr pool, if (backend->stopPool) backend->stopPool(pool->conn, obj); - event = virStoragePoolEventLifecycleNew(obj->def->name, - obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, + def->uuid, VIR_STORAGE_POOL_EVENT_STOPPED, 0); virStoragePoolObjSetActive(obj, false); @@ -1179,8 +1195,8 @@ storagePoolRefresh(virStoragePoolPtr pool, goto cleanup; } - event = virStoragePoolEventRefreshNew(obj->def->name, - obj->def->uuid); + event = virStoragePoolEventRefreshNew(def->name, + def->uuid); ret = 0; cleanup: @@ -1198,15 +1214,17 @@ storagePoolGetInfo(virStoragePoolPtr pool, virStoragePoolInfoPtr info) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; int ret = -1; if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolGetInfoEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolGetInfoEnsureACL(pool->conn, def) < 0) goto cleanup; - if (virStorageBackendForType(obj->def->type) == NULL) + if (virStorageBackendForType(def->type) == NULL) goto cleanup; memset(info, 0, sizeof(virStoragePoolInfo)); @@ -1214,9 +1232,9 @@ storagePoolGetInfo(virStoragePoolPtr pool, info->state = VIR_STORAGE_POOL_RUNNING; else info->state = VIR_STORAGE_POOL_INACTIVE; - info->capacity = obj->def->capacity; - info->allocation = obj->def->allocation; - info->available = obj->def->available; + info->capacity = def->capacity; + info->allocation = def->allocation; + info->available = def->available; ret = 0; cleanup: @@ -1230,22 +1248,26 @@ storagePoolGetXMLDesc(virStoragePoolPtr pool, { virStoragePoolObjPtr obj; virStoragePoolDefPtr def; + virStoragePoolDefPtr newDef; + virStoragePoolDefPtr curDef; char *ret = NULL; virCheckFlags(VIR_STORAGE_XML_INACTIVE, NULL); if (!(obj = virStoragePoolObjFromStoragePool(pool))) return NULL; + def = virStoragePoolObjGetDef(obj); + newDef = virStoragePoolObjGetNewDef(obj); - if (virStoragePoolGetXMLDescEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolGetXMLDescEnsureACL(pool->conn, def) < 0) goto cleanup; - if ((flags & VIR_STORAGE_XML_INACTIVE) && obj->newDef) - def = obj->newDef; + if ((flags & VIR_STORAGE_XML_INACTIVE) && newDef) + curDef = newDef; else - def = obj->def; + curDef = def; - ret = virStoragePoolDefFormat(def); + ret = virStoragePoolDefFormat(curDef); cleanup: virStoragePoolObjUnlock(obj); @@ -1262,7 +1284,8 @@ storagePoolGetAutostart(virStoragePoolPtr pool, if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; - if (virStoragePoolGetAutostartEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolGetAutostartEnsureACL(pool->conn, + virStoragePoolObjGetDef(obj)) < 0) goto cleanup; *autostart = virStoragePoolObjIsAutostart(obj) ? 1 : 0; @@ -1289,7 +1312,8 @@ storagePoolSetAutostart(virStoragePoolPtr pool, if (!(obj = storagePoolObjFindByUUID(pool->uuid, pool->name))) goto cleanup; - if (virStoragePoolSetAutostartEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolSetAutostartEnsureACL(pool->conn, + virStoragePoolObjGetDef(obj)) < 0) goto cleanup; if (!(configFile = virStoragePoolObjGetConfigFile(obj))) { @@ -1343,17 +1367,19 @@ static int storagePoolNumOfVolumes(virStoragePoolPtr pool) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; int ret = -1; if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolNumOfVolumesEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolNumOfVolumesEnsureACL(pool->conn, def) < 0) goto cleanup; if (!virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, - _("storage pool '%s' is not active"), obj->def->name); + _("storage pool '%s' is not active"), def->name); goto cleanup; } @@ -1372,17 +1398,19 @@ storagePoolListVolumes(virStoragePoolPtr pool, int maxnames) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; int n = -1; if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolListVolumesEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolListVolumesEnsureACL(pool->conn, def) < 0) goto cleanup; if (!virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, - _("storage pool '%s' is not active"), obj->def->name); + _("storage pool '%s' is not active"), def->name); goto cleanup; } @@ -1401,19 +1429,21 @@ storagePoolListAllVolumes(virStoragePoolPtr pool, unsigned int flags) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; int ret = -1; virCheckFlags(0, -1); if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStoragePoolListAllVolumesEnsureACL(pool->conn, obj->def) < 0) + if (virStoragePoolListAllVolumesEnsureACL(pool->conn, def) < 0) goto cleanup; if (!virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, - _("storage pool '%s' is not active"), obj->def->name); + _("storage pool '%s' is not active"), def->name); goto cleanup; } @@ -1432,15 +1462,17 @@ storageVolLookupByName(virStoragePoolPtr pool, const char *name) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageVolDefPtr voldef; virStorageVolPtr vol = NULL; if (!(obj = virStoragePoolObjFromStoragePool(pool))) return NULL; + def = virStoragePoolObjGetDef(obj); if (!virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, - _("storage pool '%s' is not active"), obj->def->name); + _("storage pool '%s' is not active"), def->name); goto cleanup; } @@ -1453,10 +1485,10 @@ storageVolLookupByName(virStoragePoolPtr pool, goto cleanup; } - if (virStorageVolLookupByNameEnsureACL(pool->conn, obj->def, voldef) < 0) + if (virStorageVolLookupByNameEnsureACL(pool->conn, def, voldef) < 0) goto cleanup; - vol = virGetStorageVol(pool->conn, obj->def->name, voldef->name, + vol = virGetStorageVol(pool->conn, def->name, voldef->name, voldef->key, NULL, NULL); cleanup: @@ -1474,26 +1506,26 @@ storageVolLookupByKey(virConnectPtr conn, storageDriverLock(); for (i = 0; i < driver->pools.count && !vol; i++) { - virStoragePoolObjLock(driver->pools.objs[i]); - if (virStoragePoolObjIsActive(driver->pools.objs[i])) { - virStorageVolDefPtr voldef = - virStorageVolDefFindByKey(driver->pools.objs[i], key); + virStoragePoolObjPtr obj = driver->pools.objs[i]; + virStoragePoolDefPtr def; + + virStoragePoolObjLock(obj); + def = virStoragePoolObjGetDef(obj); + if (virStoragePoolObjIsActive(obj)) { + virStorageVolDefPtr voldef = virStorageVolDefFindByKey(obj, key); if (voldef) { - virStoragePoolDefPtr def = driver->pools.objs[i]->def; if (virStorageVolLookupByKeyEnsureACL(conn, def, voldef) < 0) { - virStoragePoolObjUnlock(driver->pools.objs[i]); + virStoragePoolObjUnlock(obj); goto cleanup; } - vol = virGetStorageVol(conn, - def->name, - voldef->name, - voldef->key, + vol = virGetStorageVol(conn, def->name, + voldef->name, voldef->key, NULL, NULL); } } - virStoragePoolObjUnlock(driver->pools.objs[i]); + virStoragePoolObjUnlock(obj); } if (!vol) @@ -1520,17 +1552,19 @@ storageVolLookupByPath(virConnectPtr conn, storageDriverLock(); for (i = 0; i < driver->pools.count && !vol; i++) { virStoragePoolObjPtr obj = driver->pools.objs[i]; + virStoragePoolDefPtr def; virStorageVolDefPtr voldef; char *stable_path = NULL; virStoragePoolObjLock(obj); + def = virStoragePoolObjGetDef(obj); if (!virStoragePoolObjIsActive(obj)) { virStoragePoolObjUnlock(obj); continue; } - switch ((virStoragePoolType) obj->def->type) { + switch ((virStoragePoolType) def->type) { case VIR_STORAGE_POOL_DIR: case VIR_STORAGE_POOL_FS: case VIR_STORAGE_POOL_NETFS: @@ -1548,7 +1582,7 @@ storageVolLookupByPath(virConnectPtr conn, * getting the stable path for some of the pools. */ VIR_WARN("Failed to get stable path for pool '%s'", - obj->def->name); + def->name); virStoragePoolObjUnlock(obj); continue; } @@ -1570,12 +1604,12 @@ storageVolLookupByPath(virConnectPtr conn, VIR_FREE(stable_path); if (voldef) { - if (virStorageVolLookupByPathEnsureACL(conn, obj->def, voldef) < 0) { + if (virStorageVolLookupByPathEnsureACL(conn, def, voldef) < 0) { virStoragePoolObjUnlock(obj); goto cleanup; } - vol = virGetStorageVol(conn, obj->def->name, + vol = virGetStorageVol(conn, def->name, voldef->name, voldef->key, NULL, NULL); } @@ -1615,18 +1649,18 @@ storagePoolLookupByTargetPath(virConnectPtr conn, storageDriverLock(); for (i = 0; i < driver->pools.count && !pool; i++) { virStoragePoolObjPtr obj = driver->pools.objs[i]; + virStoragePoolDefPtr def; virStoragePoolObjLock(obj); + def = virStoragePoolObjGetDef(obj); if (!virStoragePoolObjIsActive(obj)) { virStoragePoolObjUnlock(obj); continue; } - if (STREQ(path, obj->def->target.path)) { - pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, - NULL, NULL); - } + if (STREQ(path, def->target.path)) + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); virStoragePoolObjUnlock(obj); } @@ -1651,6 +1685,7 @@ storageVolDeleteInternal(virStorageVolPtr vol, unsigned int flags, bool updateMeta) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); int ret = -1; if (!backend->deleteVol) { @@ -1668,9 +1703,9 @@ storageVolDeleteInternal(virStorageVolPtr vol, * Ignore the disk backend since it updates the pool values. */ if (updateMeta) { - if (obj->def->type != VIR_STORAGE_POOL_DISK) { - obj->def->allocation -= voldef->target.allocation; - obj->def->available += voldef->target.allocation; + if (def->type != VIR_STORAGE_POOL_DISK) { + def->allocation -= voldef->target.allocation; + def->available += voldef->target.allocation; } } @@ -1688,14 +1723,16 @@ virStorageVolDefFromVol(virStorageVolPtr vol, virStorageBackendPtr *backend) { virStorageVolDefPtr voldef = NULL; + virStoragePoolDefPtr def; if (!(*obj = storagePoolObjFindByName(vol->pool))) return NULL; + def = virStoragePoolObjGetDef(*obj); if (!virStoragePoolObjIsActive(*obj)) { virReportError(VIR_ERR_OPERATION_INVALID, _("storage pool '%s' is not active"), - (*obj)->def->name); + def->name); goto error; } @@ -1707,7 +1744,7 @@ virStorageVolDefFromVol(virStorageVolPtr vol, } if (backend) { - if (!(*backend = virStorageBackendForType((*obj)->def->type))) + if (!(*backend = virStorageBackendForType(def->type))) goto error; } @@ -1733,7 +1770,8 @@ storageVolDelete(virStorageVolPtr vol, if (!(voldef = virStorageVolDefFromVol(vol, &obj, &backend))) return -1; - if (virStorageVolDeleteEnsureACL(vol->conn, obj->def, voldef) < 0) + if (virStorageVolDeleteEnsureACL(vol->conn, virStoragePoolObjGetDef(obj), + voldef) < 0) goto cleanup; if (voldef->in_use) { @@ -1767,6 +1805,7 @@ storageVolCreateXML(virStoragePoolPtr pool, unsigned int flags) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageBackendPtr backend; virStorageVolDefPtr voldef = NULL; virStorageVolPtr vol = NULL, newvol = NULL; @@ -1775,17 +1814,18 @@ storageVolCreateXML(virStoragePoolPtr pool, if (!(obj = virStoragePoolObjFromStoragePool(pool))) return NULL; + def = virStoragePoolObjGetDef(obj); if (!virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, - _("storage pool '%s' is not active"), obj->def->name); + _("storage pool '%s' is not active"), def->name); goto cleanup; } - if ((backend = virStorageBackendForType(obj->def->type)) == NULL) + if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; - voldef = virStorageVolDefParseString(obj->def, xmldesc, + voldef = virStorageVolDefParseString(def, xmldesc, VIR_VOL_XML_PARSE_OPT_CAPACITY); if (voldef == NULL) goto cleanup; @@ -1797,7 +1837,7 @@ storageVolCreateXML(virStoragePoolPtr pool, goto cleanup; } - if (virStorageVolCreateXMLEnsureACL(pool->conn, obj->def, voldef) < 0) + if (virStorageVolCreateXMLEnsureACL(pool->conn, def, voldef) < 0) goto cleanup; if (virStorageVolDefFindByName(obj, voldef->name)) { @@ -1819,7 +1859,7 @@ storageVolCreateXML(virStoragePoolPtr pool, if (backend->createVol(pool->conn, obj, voldef) < 0) goto cleanup; - if (!(newvol = virGetStorageVol(pool->conn, obj->def->name, voldef->name, + if (!(newvol = virGetStorageVol(pool->conn, def->name, voldef->name, voldef->key, NULL, NULL))) goto cleanup; @@ -1878,13 +1918,13 @@ storageVolCreateXML(virStoragePoolPtr pool, /* Update pool metadata ignoring the disk backend since * it updates the pool values. */ - if (obj->def->type != VIR_STORAGE_POOL_DISK) { - obj->def->allocation += voldef->target.allocation; - obj->def->available -= voldef->target.allocation; + if (def->type != VIR_STORAGE_POOL_DISK) { + def->allocation += voldef->target.allocation; + def->available -= voldef->target.allocation; } VIR_INFO("Creating volume '%s' in storage pool '%s'", - newvol->name, obj->def->name); + newvol->name, def->name); vol = newvol; newvol = NULL; voldef = NULL; @@ -1904,6 +1944,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, unsigned int flags) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStoragePoolObjPtr objsrc = NULL; virStorageBackendPtr backend; virStorageVolDefPtr voldefsrc = NULL; @@ -1933,6 +1974,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, uuidstr, pool->name); goto cleanup; } + def = virStoragePoolObjGetDef(obj); if (STRNEQ(pool->name, volsrc->pool) && !objsrc) { virReportError(VIR_ERR_NO_STORAGE_POOL, @@ -1943,18 +1985,19 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, if (!virStoragePoolObjIsActive(obj)) { virReportError(VIR_ERR_OPERATION_INVALID, - _("storage pool '%s' is not active"), obj->def->name); + _("storage pool '%s' is not active"), def->name); goto cleanup; } if (objsrc && !virStoragePoolObjIsActive(objsrc)) { + virStoragePoolDefPtr objsrcdef = virStoragePoolObjGetDef(objsrc); virReportError(VIR_ERR_OPERATION_INVALID, _("storage pool '%s' is not active"), - objsrc->def->name); + objsrcdef->name); goto cleanup; } - if ((backend = virStorageBackendForType(obj->def->type)) == NULL) + if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; voldefsrc = virStorageVolDefFindByName(objsrc ? @@ -1966,12 +2009,12 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, goto cleanup; } - voldef = virStorageVolDefParseString(obj->def, xmldesc, + voldef = virStorageVolDefParseString(def, xmldesc, VIR_VOL_XML_PARSE_NO_CAPACITY); if (voldef == NULL) goto cleanup; - if (virStorageVolCreateXMLFromEnsureACL(pool->conn, obj->def, voldef) < 0) + if (virStorageVolCreateXMLFromEnsureACL(pool->conn, def, voldef) < 0) goto cleanup; if (virStorageVolDefFindByName(obj, voldef->name)) { @@ -2027,7 +2070,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, memcpy(shadowvol, voldef, sizeof(*voldef)); - if (!(newvol = virGetStorageVol(pool->conn, obj->def->name, voldef->name, + if (!(newvol = virGetStorageVol(pool->conn, def->name, voldef->name, voldef->key, NULL, NULL))) goto cleanup; @@ -2075,13 +2118,13 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, /* Updating pool metadata ignoring the disk backend since * it updates the pool values */ - if (obj->def->type != VIR_STORAGE_POOL_DISK) { - obj->def->allocation += voldef->target.allocation; - obj->def->available -= voldef->target.allocation; + if (def->type != VIR_STORAGE_POOL_DISK) { + def->allocation += voldef->target.allocation; + def->available -= voldef->target.allocation; } VIR_INFO("Creating volume '%s' in storage pool '%s'", - newvol->name, obj->def->name); + newvol->name, def->name); vol = newvol; newvol = NULL; voldef = NULL; @@ -2115,7 +2158,8 @@ storageVolDownload(virStorageVolPtr vol, if (!(voldef = virStorageVolDefFromVol(vol, &obj, &backend))) return -1; - if (virStorageVolDownloadEnsureACL(vol->conn, obj->def, voldef) < 0) + if (virStorageVolDownloadEnsureACL(vol->conn, virStoragePoolObjGetDef(obj), + voldef) < 0) goto cleanup; if (voldef->building) { @@ -2209,6 +2253,7 @@ virStorageVolPoolRefreshThread(void *opaque) virStorageVolStreamInfoPtr cbdata = opaque; virStoragePoolObjPtr obj = NULL; + virStoragePoolDefPtr def; virStorageBackendPtr backend; virObjectEventPtr event = NULL; @@ -2220,16 +2265,16 @@ virStorageVolPoolRefreshThread(void *opaque) if (!(obj = virStoragePoolObjFindByName(&driver->pools, cbdata->pool_name))) goto cleanup; + def = virStoragePoolObjGetDef(obj); - if (!(backend = virStorageBackendForType(obj->def->type))) + if (!(backend = virStorageBackendForType(def->type))) goto cleanup; virStoragePoolObjClearVols(obj); if (backend->refreshPool(NULL, obj) < 0) VIR_DEBUG("Failed to refresh storage pool"); - event = virStoragePoolEventRefreshNew(obj->def->name, - obj->def->uuid); + event = virStoragePoolEventRefreshNew(def->name, def->uuid); cleanup: if (event) @@ -2274,6 +2319,7 @@ storageVolUpload(virStorageVolPtr vol, { virStorageBackendPtr backend; virStoragePoolObjPtr obj = NULL; + virStoragePoolDefPtr def; virStorageVolDefPtr voldef = NULL; virStorageVolStreamInfoPtr cbdata = NULL; int ret = -1; @@ -2282,8 +2328,9 @@ storageVolUpload(virStorageVolPtr vol, if (!(voldef = virStorageVolDefFromVol(vol, &obj, &backend))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStorageVolUploadEnsureACL(vol->conn, obj->def, voldef) < 0) + if (virStorageVolUploadEnsureACL(vol->conn, def, voldef) < 0) goto cleanup; if (voldef->in_use) { @@ -2313,7 +2360,7 @@ storageVolUpload(virStorageVolPtr vol, * routine in order to call the refresh API. */ if (VIR_ALLOC(cbdata) < 0 || - VIR_STRDUP(cbdata->pool_name, obj->def->name) < 0) + VIR_STRDUP(cbdata->pool_name, def->name) < 0) goto cleanup; if (voldef->type == VIR_STORAGE_VOL_PLOOP && VIR_STRDUP(cbdata->vol_path, voldef->target.path) < 0) @@ -2346,6 +2393,7 @@ storageVolResize(virStorageVolPtr vol, { virStorageBackendPtr backend; virStoragePoolObjPtr obj = NULL; + virStoragePoolDefPtr def; virStorageVolDefPtr voldef = NULL; unsigned long long abs_capacity, delta = 0; int ret = -1; @@ -2356,8 +2404,9 @@ storageVolResize(virStorageVolPtr vol, if (!(voldef = virStorageVolDefFromVol(vol, &obj, &backend))) return -1; + def = virStoragePoolObjGetDef(obj); - if (virStorageVolResizeEnsureACL(vol->conn, obj->def, voldef) < 0) + if (virStorageVolResizeEnsureACL(vol->conn, def, voldef) < 0) goto cleanup; if (voldef->in_use) { @@ -2402,7 +2451,7 @@ storageVolResize(virStorageVolPtr vol, if (flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE) delta = abs_capacity - voldef->target.allocation; - if (delta > obj->def->available) { + if (delta > def->available) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Not enough space left in storage pool")); goto cleanup; @@ -2425,8 +2474,8 @@ storageVolResize(virStorageVolPtr vol, */ if (flags & VIR_STORAGE_VOL_RESIZE_ALLOCATE) { voldef->target.allocation = abs_capacity; - obj->def->allocation += delta; - obj->def->available -= delta; + def->allocation += delta; + def->available -= delta; } ret = 0; @@ -2460,7 +2509,9 @@ storageVolWipePattern(virStorageVolPtr vol, if (!(voldef = virStorageVolDefFromVol(vol, &obj, &backend))) return -1; - if (virStorageVolWipePatternEnsureACL(vol->conn, obj->def, voldef) < 0) + if (virStorageVolWipePatternEnsureACL(vol->conn, + virStoragePoolObjGetDef(obj), + voldef) < 0) goto cleanup; if (voldef->in_use) { @@ -2524,7 +2575,9 @@ storageVolGetInfoFlags(virStorageVolPtr vol, if (!(voldef = virStorageVolDefFromVol(vol, &obj, &backend))) return -1; - if (virStorageVolGetInfoFlagsEnsureACL(vol->conn, obj->def, voldef) < 0) + if (virStorageVolGetInfoFlagsEnsureACL(vol->conn, + virStoragePoolObjGetDef(obj), + voldef) < 0) goto cleanup; if (backend->refreshVol && @@ -2559,6 +2612,7 @@ storageVolGetXMLDesc(virStorageVolPtr vol, unsigned int flags) { virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageBackendPtr backend; virStorageVolDefPtr voldef; char *ret = NULL; @@ -2567,15 +2621,16 @@ storageVolGetXMLDesc(virStorageVolPtr vol, if (!(voldef = virStorageVolDefFromVol(vol, &obj, &backend))) return NULL; + def = virStoragePoolObjGetDef(obj); - if (virStorageVolGetXMLDescEnsureACL(vol->conn, obj->def, voldef) < 0) + if (virStorageVolGetXMLDescEnsureACL(vol->conn, def, voldef) < 0) goto cleanup; if (backend->refreshVol && backend->refreshVol(vol->conn, obj, voldef) < 0) goto cleanup; - ret = virStorageVolDefFormat(obj->def, voldef); + ret = virStorageVolDefFormat(def, voldef); cleanup: virStoragePoolObjUnlock(obj); @@ -2593,7 +2648,8 @@ storageVolGetPath(virStorageVolPtr vol) if (!(voldef = virStorageVolDefFromVol(vol, &obj, NULL))) return NULL; - if (virStorageVolGetPathEnsureACL(vol->conn, obj->def, voldef) < 0) + if (virStorageVolGetPathEnsureACL(vol->conn, virStoragePoolObjGetDef(obj), + voldef) < 0) goto cleanup; ignore_value(VIR_STRDUP(ret, voldef->target.path)); @@ -3043,9 +3099,10 @@ virStoragePoolObjBuildTempFilePath(virStoragePoolObjPtr obj, virStorageVolDefPtr voldef) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); char *tmp = NULL; ignore_value(virAsprintf(&tmp, "%s/%s.%s.secret.XXXXXX", - driver->stateDir, obj->def->name, voldef->name)); + driver->stateDir, def->name, voldef->name)); return tmp; } -- 2.13.6
Make it more obvious as we're about to need to change how obj->def gets referenced. Perform a couple of minor cleanups along the way too. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/test/test_driver.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e92768a975..a1a74b8bd1 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1041,7 +1041,7 @@ testOpenVolumesForPool(const char *file, size_t i; int num, ret = -1; xmlNodePtr *nodes = NULL; - virStorageVolDefPtr def = NULL; + virStorageVolDefPtr volDef = NULL; /* Find storage volumes */ if (virAsprintf(&vol_xpath, "/node/pool[%d]/volume", objidx) < 0) @@ -1058,30 +1058,29 @@ testOpenVolumesForPool(const char *file, if (!node) goto error; - def = virStorageVolDefParseNode(obj->def, ctxt->doc, node, 0); - if (!def) + if (!(volDef = virStorageVolDefParseNode(obj->def, ctxt->doc, node, 0))) goto error; - if (def->target.path == NULL) { - if (virAsprintf(&def->target.path, "%s/%s", - obj->def->target.path, def->name) < 0) + if (!volDef->target.path) { + if (virAsprintf(&volDef->target.path, "%s/%s", + obj->def->target.path, volDef->name) < 0) goto error; } - if (!def->key && VIR_STRDUP(def->key, def->target.path) < 0) + if (!volDef->key && VIR_STRDUP(volDef->key, volDef->target.path) < 0) goto error; - if (virStoragePoolObjAddVol(obj, def) < 0) + if (virStoragePoolObjAddVol(obj, volDef) < 0) goto error; - obj->def->allocation += def->target.allocation; + obj->def->allocation += volDef->target.allocation; obj->def->available = (obj->def->capacity - obj->def->allocation); - def = NULL; + volDef = NULL; } ret = 0; error: - virStorageVolDefFree(def); + virStorageVolDefFree(volDef); VIR_FREE(nodes); return ret; } -- 2.13.6
Rather than accessing privconn->pools.objs[i] in the for loop, let's use an @obj variable to make it easier to read the code. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/test/test_driver.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a1a74b8bd1..e84acf3228 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4905,27 +4905,28 @@ testStorageVolLookupByKey(virConnectPtr conn, const char *key) { testDriverPtr privconn = conn->privateData; + virStoragePoolObjPtr obj; size_t i; virStorageVolPtr ret = NULL; testDriverLock(privconn); for (i = 0; i < privconn->pools.count; i++) { - virStoragePoolObjLock(privconn->pools.objs[i]); - if (virStoragePoolObjIsActive(privconn->pools.objs[i])) { - virStorageVolDefPtr privvol = - virStorageVolDefFindByKey(privconn->pools.objs[i], key); + obj = privconn->pools.objs[i]; + virStoragePoolObjLock(obj); + if (virStoragePoolObjIsActive(obj)) { + virStorageVolDefPtr privvol = virStorageVolDefFindByKey(obj, key); if (privvol) { ret = virGetStorageVol(conn, - privconn->pools.objs[i]->def->name, + obj->def->name, privvol->name, privvol->key, NULL, NULL); - virStoragePoolObjUnlock(privconn->pools.objs[i]); + virStoragePoolObjUnlock(obj); break; } } - virStoragePoolObjUnlock(privconn->pools.objs[i]); + virStoragePoolObjUnlock(obj); } testDriverUnlock(privconn); @@ -4942,27 +4943,28 @@ testStorageVolLookupByPath(virConnectPtr conn, const char *path) { testDriverPtr privconn = conn->privateData; + virStoragePoolObjPtr obj; size_t i; virStorageVolPtr ret = NULL; testDriverLock(privconn); for (i = 0; i < privconn->pools.count; i++) { - virStoragePoolObjLock(privconn->pools.objs[i]); - if (virStoragePoolObjIsActive(privconn->pools.objs[i])) { - virStorageVolDefPtr privvol = - virStorageVolDefFindByPath(privconn->pools.objs[i], path); + obj = privconn->pools.objs[i]; + virStoragePoolObjLock(obj); + if (virStoragePoolObjIsActive(obj)) { + virStorageVolDefPtr privvol = virStorageVolDefFindByPath(obj, path); if (privvol) { ret = virGetStorageVol(conn, - privconn->pools.objs[i]->def->name, + obj->def->name, privvol->name, privvol->key, NULL, NULL); - virStoragePoolObjUnlock(privconn->pools.objs[i]); + virStoragePoolObjUnlock(obj); break; } } - virStoragePoolObjUnlock(privconn->pools.objs[i]); + virStoragePoolObjUnlock(obj); } testDriverUnlock(privconn); -- 2.13.6
On 10/05/2017 10:23 PM, John Ferlan wrote:
Rather than accessing privconn->pools.objs[i] in the for loop, let's use an @obj variable to make it easier to read the code.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/test/test_driver.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a1a74b8bd1..e84acf3228 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4905,27 +4905,28 @@ testStorageVolLookupByKey(virConnectPtr conn, const char *key) { testDriverPtr privconn = conn->privateData; + virStoragePoolObjPtr obj;
How about s/obj/poolObj/ or something so that one know just from looking whether obj refers to pool or volume. But I don't care that much. Michal
On 10/06/2017 05:34 AM, Michal Privoznik wrote:
On 10/05/2017 10:23 PM, John Ferlan wrote:
Rather than accessing privconn->pools.objs[i] in the for loop, let's use an @obj variable to make it easier to read the code.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/test/test_driver.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a1a74b8bd1..e84acf3228 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4905,27 +4905,28 @@ testStorageVolLookupByKey(virConnectPtr conn, const char *key) { testDriverPtr privconn = conn->privateData; + virStoragePoolObjPtr obj;
How about s/obj/poolObj/ or something so that one know just from looking whether obj refers to pool or volume. But I don't care that much.
Michal
I think I've now gone with just obj for everything for so long that it's been more common. Also, there's no such thing as a virStorageVolObjPtr yet, so I think I'd rather just stick with obj unless the all the storage pool tests change to use poolObj. I can do that, but other similar variable name change only patches have been widely disliked throughout this effort so I've tried hard not to add them in these more recent patches. In my original local branches - all there was a patch for each of the storage backends which changed virStoragePoolObjPtr pool to be obj, but I doubt those will ever see the list ;-) I'm going to keep it as obj - I can do a follow up to make the naming consistent throughout, but I kind of doubt it'll get much support. Tks for the quick look - John
In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/test/test_driver.c | 162 +++++++++++++++++++++++++++---------------------- 1 file changed, 91 insertions(+), 71 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e84acf3228..1c48347994 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1037,6 +1037,7 @@ testOpenVolumesForPool(const char *file, virStoragePoolObjPtr obj, int objidx) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); char *vol_xpath; size_t i; int num, ret = -1; @@ -1058,12 +1059,12 @@ testOpenVolumesForPool(const char *file, if (!node) goto error; - if (!(volDef = virStorageVolDefParseNode(obj->def, ctxt->doc, node, 0))) + if (!(volDef = virStorageVolDefParseNode(def, ctxt->doc, node, 0))) goto error; if (!volDef->target.path) { if (virAsprintf(&volDef->target.path, "%s/%s", - obj->def->target.path, volDef->name) < 0) + def->target.path, volDef->name) < 0) goto error; } @@ -1073,8 +1074,8 @@ testOpenVolumesForPool(const char *file, if (virStoragePoolObjAddVol(obj, volDef) < 0) goto error; - obj->def->allocation += volDef->target.allocation; - obj->def->available = (obj->def->capacity - obj->def->allocation); + def->allocation += volDef->target.allocation; + def->available = (def->capacity - def->allocation); volDef = NULL; } @@ -4056,10 +4057,11 @@ static int testStoragePoolObjSetDefaults(virStoragePoolObjPtr obj) { char *configFile; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); - obj->def->capacity = defaultPoolCap; - obj->def->allocation = defaultPoolAlloc; - obj->def->available = defaultPoolCap - defaultPoolAlloc; + def->capacity = defaultPoolCap; + def->allocation = defaultPoolAlloc; + def->available = defaultPoolCap - defaultPoolAlloc; if (VIR_STRDUP(configFile, "") < 0) return -1; @@ -4156,13 +4158,14 @@ testStoragePoolLookupByUUID(virConnectPtr conn, { testDriverPtr privconn = conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; if (!(obj = testStoragePoolObjFindByUUID(privconn, uuid))) return NULL; + def = virStoragePoolObjGetDef(obj); - pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, - NULL, NULL); + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); virStoragePoolObjUnlock(obj); return pool; @@ -4175,13 +4178,14 @@ testStoragePoolLookupByName(virConnectPtr conn, { testDriverPtr privconn = conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; if (!(obj = testStoragePoolObjFindByName(privconn, name))) return NULL; + def = virStoragePoolObjGetDef(obj); - pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, - NULL, NULL); + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); virStoragePoolObjUnlock(obj); return pool; @@ -4426,38 +4430,40 @@ testStoragePoolCreateXML(virConnectPtr conn, unsigned int flags) { testDriverPtr privconn = conn->privateData; - virStoragePoolDefPtr def; + virStoragePoolDefPtr newDef; virStoragePoolObjPtr obj = NULL; + virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; virObjectEventPtr event = NULL; virCheckFlags(0, NULL); testDriverLock(privconn); - if (!(def = virStoragePoolDefParseString(xml))) + if (!(newDef = virStoragePoolDefParseString(xml))) goto cleanup; - obj = virStoragePoolObjFindByUUID(&privconn->pools, def->uuid); + obj = virStoragePoolObjFindByUUID(&privconn->pools, newDef->uuid); if (!obj) - obj = virStoragePoolObjFindByName(&privconn->pools, def->name); + obj = virStoragePoolObjFindByName(&privconn->pools, newDef->name); if (obj) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("storage pool already exists")); goto cleanup; } - if (!(obj = virStoragePoolObjAssignDef(&privconn->pools, def))) + if (!(obj = virStoragePoolObjAssignDef(&privconn->pools, newDef))) goto cleanup; - def = NULL; + newDef = NULL; + def = virStoragePoolObjGetDef(obj); - if (obj->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { /* In the real code, we'd call virVHBAManageVport followed by * find_new_device, but we cannot do that here since we're not * mocking udev. The mock routine will copy an existing vHBA and * rename a few fields to mock that. */ if (testCreateVport(privconn, - obj->def->source.adapter.data.fchost.wwnn, - obj->def->source.adapter.data.fchost.wwpn) < 0) { + def->source.adapter.data.fchost.wwnn, + def->source.adapter.data.fchost.wwpn) < 0) { virStoragePoolObjRemove(&privconn->pools, obj); obj = NULL; goto cleanup; @@ -4477,14 +4483,14 @@ testStoragePoolCreateXML(virConnectPtr conn, virStoragePoolObjSetActive(obj, true); - event = virStoragePoolEventLifecycleNew(obj->def->name, obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, def->uuid, VIR_STORAGE_POOL_EVENT_STARTED, 0); - pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, NULL, NULL); + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); cleanup: - virStoragePoolDefFree(def); + virStoragePoolDefFree(newDef); testObjectEventQueue(privconn, event); if (obj) virStoragePoolObjUnlock(obj); @@ -4499,26 +4505,28 @@ testStoragePoolDefineXML(virConnectPtr conn, unsigned int flags) { testDriverPtr privconn = conn->privateData; - virStoragePoolDefPtr def; + virStoragePoolDefPtr newDef; virStoragePoolObjPtr obj = NULL; + virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; virObjectEventPtr event = NULL; virCheckFlags(0, NULL); testDriverLock(privconn); - if (!(def = virStoragePoolDefParseString(xml))) + if (!(newDef = virStoragePoolDefParseString(xml))) goto cleanup; - def->capacity = defaultPoolCap; - def->allocation = defaultPoolAlloc; - def->available = defaultPoolCap - defaultPoolAlloc; + newDef->capacity = defaultPoolCap; + newDef->allocation = defaultPoolAlloc; + newDef->available = defaultPoolCap - defaultPoolAlloc; - if (!(obj = virStoragePoolObjAssignDef(&privconn->pools, def))) + if (!(obj = virStoragePoolObjAssignDef(&privconn->pools, newDef))) goto cleanup; - def = NULL; + newDef = NULL; + def = virStoragePoolObjGetDef(obj); - event = virStoragePoolEventLifecycleNew(obj->def->name, obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, def->uuid, VIR_STORAGE_POOL_EVENT_DEFINED, 0); @@ -4528,10 +4536,10 @@ testStoragePoolDefineXML(virConnectPtr conn, goto cleanup; } - pool = virGetStoragePool(conn, obj->def->name, obj->def->uuid, NULL, NULL); + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); cleanup: - virStoragePoolDefFree(def); + virStoragePoolDefFree(newDef); testObjectEventQueue(privconn, event); if (obj) virStoragePoolObjUnlock(obj); @@ -4624,24 +4632,25 @@ testStoragePoolDestroy(virStoragePoolPtr pool) { testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; int ret = -1; virObjectEventPtr event = NULL; if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name))) return -1; + def = virStoragePoolObjGetDef(obj); virStoragePoolObjSetActive(obj, false); - if (obj->def->source.adapter.type == - VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { if (testDestroyVport(privconn, - obj->def->source.adapter.data.fchost.wwnn, - obj->def->source.adapter.data.fchost.wwpn) < 0) + def->source.adapter.data.fchost.wwnn, + def->source.adapter.data.fchost.wwpn) < 0) goto cleanup; } - event = virStoragePoolEventLifecycleNew(obj->def->name, - obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, + def->uuid, VIR_STORAGE_POOL_EVENT_STOPPED, 0); @@ -4710,18 +4719,20 @@ testStoragePoolGetInfo(virStoragePoolPtr pool, { testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; if (!(obj = testStoragePoolObjFindByName(privconn, pool->name))) return -1; + def = virStoragePoolObjGetDef(obj); memset(info, 0, sizeof(virStoragePoolInfo)); if (virStoragePoolObjIsActive(obj)) info->state = VIR_STORAGE_POOL_RUNNING; else info->state = VIR_STORAGE_POOL_INACTIVE; - info->capacity = obj->def->capacity; - info->allocation = obj->def->allocation; - info->available = obj->def->available; + info->capacity = def->capacity; + info->allocation = def->allocation; + info->available = def->available; virStoragePoolObjUnlock(obj); return 0; @@ -4741,7 +4752,7 @@ testStoragePoolGetXMLDesc(virStoragePoolPtr pool, if (!(obj = testStoragePoolObjFindByName(privconn, pool->name))) return NULL; - ret = virStoragePoolDefFormat(obj->def); + ret = virStoragePoolDefFormat(virStoragePoolObjGetDef(obj)); virStoragePoolObjUnlock(obj); return ret; @@ -4881,16 +4892,18 @@ testStorageVolLookupByName(virStoragePoolPtr pool, { testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageVolDefPtr privvol; virStorageVolPtr ret = NULL; if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name))) return NULL; + def = virStoragePoolObjGetDef(obj); if (!(privvol = testStorageVolDefFindByName(obj, name))) goto cleanup; - ret = virGetStorageVol(pool->conn, obj->def->name, + ret = virGetStorageVol(pool->conn, def->name, privvol->name, privvol->key, NULL, NULL); @@ -4906,6 +4919,7 @@ testStorageVolLookupByKey(virConnectPtr conn, { testDriverPtr privconn = conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; size_t i; virStorageVolPtr ret = NULL; @@ -4913,14 +4927,13 @@ testStorageVolLookupByKey(virConnectPtr conn, for (i = 0; i < privconn->pools.count; i++) { obj = privconn->pools.objs[i]; virStoragePoolObjLock(obj); + def = virStoragePoolObjGetDef(obj); if (virStoragePoolObjIsActive(obj)) { virStorageVolDefPtr privvol = virStorageVolDefFindByKey(obj, key); if (privvol) { - ret = virGetStorageVol(conn, - obj->def->name, - privvol->name, - privvol->key, + ret = virGetStorageVol(conn, def->name, + privvol->name, privvol->key, NULL, NULL); virStoragePoolObjUnlock(obj); break; @@ -4944,6 +4957,7 @@ testStorageVolLookupByPath(virConnectPtr conn, { testDriverPtr privconn = conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; size_t i; virStorageVolPtr ret = NULL; @@ -4951,14 +4965,13 @@ testStorageVolLookupByPath(virConnectPtr conn, for (i = 0; i < privconn->pools.count; i++) { obj = privconn->pools.objs[i]; virStoragePoolObjLock(obj); + def = virStoragePoolObjGetDef(obj); if (virStoragePoolObjIsActive(obj)) { virStorageVolDefPtr privvol = virStorageVolDefFindByPath(obj, path); if (privvol) { - ret = virGetStorageVol(conn, - obj->def->name, - privvol->name, - privvol->key, + ret = virGetStorageVol(conn, def->name, + privvol->name, privvol->key, NULL, NULL); virStoragePoolObjUnlock(obj); break; @@ -4983,6 +4996,7 @@ testStorageVolCreateXML(virStoragePoolPtr pool, { testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageVolDefPtr privvol = NULL; virStorageVolPtr ret = NULL; @@ -4990,8 +5004,9 @@ testStorageVolCreateXML(virStoragePoolPtr pool, if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name))) return NULL; + def = virStoragePoolObjGetDef(obj); - privvol = virStorageVolDefParseString(obj->def, xmldesc, 0); + privvol = virStorageVolDefParseString(def, xmldesc, 0); if (privvol == NULL) goto cleanup; @@ -5002,8 +5017,8 @@ testStorageVolCreateXML(virStoragePoolPtr pool, } /* Make sure enough space */ - if ((obj->def->allocation + privvol->target.allocation) > - obj->def->capacity) { + if ((def->allocation + privvol->target.allocation) > + def->capacity) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Not enough free space in pool for volume '%s'"), privvol->name); @@ -5011,17 +5026,17 @@ testStorageVolCreateXML(virStoragePoolPtr pool, } if (virAsprintf(&privvol->target.path, "%s/%s", - obj->def->target.path, privvol->name) < 0) + def->target.path, privvol->name) < 0) goto cleanup; if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 || virStoragePoolObjAddVol(obj, privvol) < 0) goto cleanup; - obj->def->allocation += privvol->target.allocation; - obj->def->available = (obj->def->capacity - obj->def->allocation); + def->allocation += privvol->target.allocation; + def->available = (def->capacity - def->allocation); - ret = virGetStorageVol(pool->conn, obj->def->name, + ret = virGetStorageVol(pool->conn, def->name, privvol->name, privvol->key, NULL, NULL); privvol = NULL; @@ -5041,6 +5056,7 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, { testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageVolDefPtr privvol = NULL, origvol = NULL; virStorageVolPtr ret = NULL; @@ -5048,8 +5064,9 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, if (!(obj = testStoragePoolObjFindActiveByName(privconn, pool->name))) return NULL; + def = virStoragePoolObjGetDef(obj); - privvol = virStorageVolDefParseString(obj->def, xmldesc, 0); + privvol = virStorageVolDefParseString(def, xmldesc, 0); if (privvol == NULL) goto cleanup; @@ -5068,27 +5085,26 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, } /* Make sure enough space */ - if ((obj->def->allocation + privvol->target.allocation) > - obj->def->capacity) { + if ((def->allocation + privvol->target.allocation) > def->capacity) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Not enough free space in pool for volume '%s'"), privvol->name); goto cleanup; } - obj->def->available = (obj->def->capacity - obj->def->allocation); + def->available = (def->capacity - def->allocation); if (virAsprintf(&privvol->target.path, "%s/%s", - obj->def->target.path, privvol->name) < 0) + def->target.path, privvol->name) < 0) goto cleanup; if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 || virStoragePoolObjAddVol(obj, privvol) < 0) goto cleanup; - obj->def->allocation += privvol->target.allocation; - obj->def->available = (obj->def->capacity - obj->def->allocation); + def->allocation += privvol->target.allocation; + def->available = (def->capacity - def->allocation); - ret = virGetStorageVol(pool->conn, obj->def->name, + ret = virGetStorageVol(pool->conn, def->name, privvol->name, privvol->key, NULL, NULL); privvol = NULL; @@ -5106,6 +5122,7 @@ testStorageVolDelete(virStorageVolPtr vol, { testDriverPtr privconn = vol->conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageVolDefPtr privvol; int ret = -1; @@ -5113,12 +5130,13 @@ testStorageVolDelete(virStorageVolPtr vol, if (!(obj = testStoragePoolObjFindActiveByName(privconn, vol->pool))) return -1; + def = virStoragePoolObjGetDef(obj); if (!(privvol = testStorageVolDefFindByName(obj, vol->name))) goto cleanup; - obj->def->allocation -= privvol->target.allocation; - obj->def->available = (obj->def->capacity - obj->def->allocation); + def->allocation -= privvol->target.allocation; + def->available = (def->capacity - def->allocation); virStoragePoolObjRemoveVol(obj, privvol); @@ -5151,17 +5169,19 @@ testStorageVolGetInfo(virStorageVolPtr vol, { testDriverPtr privconn = vol->conn->privateData; virStoragePoolObjPtr obj; + virStoragePoolDefPtr def; virStorageVolDefPtr privvol; int ret = -1; if (!(obj = testStoragePoolObjFindActiveByName(privconn, vol->pool))) return -1; + def = virStoragePoolObjGetDef(obj); if (!(privvol = testStorageVolDefFindByName(obj, vol->name))) goto cleanup; memset(info, 0, sizeof(*info)); - info->type = testStorageVolumeTypeForPool(obj->def->type); + info->type = testStorageVolumeTypeForPool(def->type); info->capacity = privvol->target.capacity; info->allocation = privvol->target.allocation; ret = 0; @@ -5189,7 +5209,7 @@ testStorageVolGetXMLDesc(virStorageVolPtr vol, if (!(privvol = testStorageVolDefFindByName(obj, vol->name))) goto cleanup; - ret = virStorageVolDefFormat(obj->def, privvol); + ret = virStorageVolDefFormat(virStoragePoolObjGetDef(obj), privvol); cleanup: virStoragePoolObjUnlock(obj); -- 2.13.6
On 10/05/2017 10:23 PM, John Ferlan wrote:
Since the previous series (19 patches):
https://www.redhat.com/archives/libvir-list/2017-September/msg00594.html
didn't garner any attention, let me try with smaller patch piles with the hope that forward progress will be made.
The only "new" one in this series from the other is patch2 - something that Coverity let me know about in an error scenario.
Essentially the bulk of these patches remove direct obj->def referencing in favor of a virStoragePoolObjGetDef call.
John Ferlan (6): conf: Fix prototype/definition for virStoragePoolObj get functions tests: Fix possible NULL deref storage: Use virStoragePoolObjGetDef accessor for driver test: Rename @vol to @volDef in testOpenVolumesForPool test: Create local virStoragePoolObjPtr VolLookup APIs test: Use virStoragePoolObjGetDef accessor
src/conf/virstorageobj.c | 4 +- src/conf/virstorageobj.h | 4 +- src/storage/storage_driver.c | 411 +++++++++++++++++++++++------------------ src/test/test_driver.c | 203 +++++++++++--------- tests/storagevolxml2argvtest.c | 3 +- 5 files changed, 352 insertions(+), 273 deletions(-)
ACK series. Michal
participants (2)
-
John Ferlan -
Michal Privoznik