Rework internal pool lookup code to avoid printing the raw UUID buffer in the case a storage pool can't be found: $ virsh pool-name e012ace0-0460-5810-39ef-1bce5fa5a4dd error: failed to get pool 'e012ace0-0460-5810-39ef-1bce5fa5a4dd' error: Storage pool not found: no storage pool with matching uuid à¬à`X9ï_¥¤Ý The rework is mostly done by switching the lookup code to the newly introduced helper virStoragePoolObjFromStoragePoo Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1104993 --- src/storage/storage_driver.c | 263 +++++++++++++++---------------------------- 1 file changed, 90 insertions(+), 173 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 4f51517..c9916ff 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -275,9 +275,11 @@ storagePoolLookupByUUID(virConnectPtr conn, storageDriverUnlock(driver); if (!pool) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), uuid); - goto cleanup; + _("no storage pool with matching uuid '%s'"), uuidstr); + return NULL; } if (virStoragePoolLookupByUUIDEnsureACL(conn, pool->def) < 0) @@ -287,8 +289,7 @@ storagePoolLookupByUUID(virConnectPtr conn, NULL, NULL); cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -307,7 +308,7 @@ storagePoolLookupByName(virConnectPtr conn, if (!pool) { virReportError(VIR_ERR_NO_STORAGE_POOL, _("no storage pool with matching name '%s'"), name); - goto cleanup; + return NULL; } if (virStoragePoolLookupByNameEnsureACL(conn, pool->def) < 0) @@ -317,8 +318,7 @@ storagePoolLookupByName(virConnectPtr conn, NULL, NULL); cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -336,7 +336,7 @@ storagePoolLookupByVolume(virStorageVolPtr vol) if (!pool) { virReportError(VIR_ERR_NO_STORAGE_POOL, _("no storage pool with matching name '%s'"), vol->pool); - goto cleanup; + return NULL; } if (virStoragePoolLookupByVolumeEnsureACL(vol->conn, pool->def) < 0) @@ -346,8 +346,7 @@ storagePoolLookupByVolume(virStorageVolPtr vol) NULL, NULL); cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -535,19 +534,33 @@ storageConnectFindStoragePoolSources(virConnectPtr conn, } -static int storagePoolIsActive(virStoragePoolPtr pool) +static virStoragePoolObjPtr +virStoragePoolObjFromStoragePool(virStoragePoolPtr pool) { virStorageDriverStatePtr driver = pool->conn->storagePrivateData; - virStoragePoolObjPtr obj; - int ret = -1; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virStoragePoolObjPtr ret; storageDriverLock(driver); - obj = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid); - storageDriverUnlock(driver); - if (!obj) { - virReportError(VIR_ERR_NO_STORAGE_POOL, NULL); - goto cleanup; + if (!(ret = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid))) { + virUUIDFormat(pool->uuid, uuidstr); + virReportError(VIR_ERR_NO_STORAGE_POOL, + _("no storage pool with matching uuid '%s' (%s)"), + uuidstr, pool->name); } + storageDriverUnlock(driver); + + return ret; +} + + +static int storagePoolIsActive(virStoragePoolPtr pool) +{ + virStoragePoolObjPtr obj; + int ret = -1; + + if (!(obj = virStoragePoolObjFromStoragePool(pool))) + return -1; if (virStoragePoolIsActiveEnsureACL(pool->conn, obj->def) < 0) goto cleanup; @@ -555,24 +568,17 @@ static int storagePoolIsActive(virStoragePoolPtr pool) ret = virStoragePoolObjIsActive(obj); cleanup: - if (obj) - virStoragePoolObjUnlock(obj); + virStoragePoolObjUnlock(obj); return ret; } static int storagePoolIsPersistent(virStoragePoolPtr pool) { - virStorageDriverStatePtr driver = pool->conn->storagePrivateData; virStoragePoolObjPtr obj; int ret = -1; - storageDriverLock(driver); - obj = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid); - storageDriverUnlock(driver); - if (!obj) { - virReportError(VIR_ERR_NO_STORAGE_POOL, NULL); - goto cleanup; - } + if (!(obj = virStoragePoolObjFromStoragePool(pool))) + return -1; if (virStoragePoolIsPersistentEnsureACL(pool->conn, obj->def) < 0) goto cleanup; @@ -580,8 +586,7 @@ static int storagePoolIsPersistent(virStoragePoolPtr pool) ret = obj->configFile ? 1 : 0; cleanup: - if (obj) - virStoragePoolObjUnlock(obj); + virStoragePoolObjUnlock(obj); return ret; } @@ -705,10 +710,12 @@ storagePoolUndefine(virStoragePoolPtr obj) int ret = -1; storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - if (!pool) { + if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); + _("no storage pool with matching uuid '%s' (%s)"), + uuidstr, obj->name); goto cleanup; } @@ -757,22 +764,14 @@ static int storagePoolCreate(virStoragePoolPtr obj, unsigned int flags) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; virStorageBackendPtr backend; int ret = -1; virCheckFlags(0, -1); - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return -1; if (virStoragePoolCreateEnsureACL(obj->conn, pool->def) < 0) goto cleanup; @@ -801,8 +800,7 @@ storagePoolCreate(virStoragePoolPtr obj, ret = 0; cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -810,20 +808,12 @@ static int storagePoolBuild(virStoragePoolPtr obj, unsigned int flags) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; virStorageBackendPtr backend; int ret = -1; - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return -1; if (virStoragePoolBuildEnsureACL(obj->conn, pool->def) < 0) goto cleanup; @@ -844,8 +834,7 @@ storagePoolBuild(virStoragePoolPtr obj, ret = 0; cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -859,11 +848,12 @@ storagePoolDestroy(virStoragePoolPtr obj) int ret = -1; storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - - if (!pool) { + if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); + _("no storage pool with matching uuid '%s' (%s)"), + uuidstr, obj->name); goto cleanup; } @@ -916,20 +906,12 @@ static int storagePoolDelete(virStoragePoolPtr obj, unsigned int flags) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; virStorageBackendPtr backend; int ret = -1; - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return -1; if (virStoragePoolDeleteEnsureACL(obj->conn, pool->def) < 0) goto cleanup; @@ -962,8 +944,7 @@ storagePoolDelete(virStoragePoolPtr obj, ret = 0; cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -980,11 +961,12 @@ storagePoolRefresh(virStoragePoolPtr obj, virCheckFlags(0, -1); storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - - if (!pool) { + if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); + _("no storage pool with matching uuid '%s' (%s)"), + uuidstr, obj->name); goto cleanup; } @@ -1034,19 +1016,11 @@ static int storagePoolGetInfo(virStoragePoolPtr obj, virStoragePoolInfoPtr info) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; int ret = -1; - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return -1; if (virStoragePoolGetInfoEnsureACL(obj->conn, pool->def) < 0) goto cleanup; @@ -1065,8 +1039,7 @@ storagePoolGetInfo(virStoragePoolPtr obj, ret = 0; cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -1074,22 +1047,14 @@ static char * storagePoolGetXMLDesc(virStoragePoolPtr obj, unsigned int flags) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; virStoragePoolDefPtr def; char *ret = NULL; virCheckFlags(VIR_STORAGE_XML_INACTIVE, NULL); - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return NULL; if (virStoragePoolGetXMLDescEnsureACL(obj->conn, pool->def) < 0) goto cleanup; @@ -1102,8 +1067,7 @@ storagePoolGetXMLDesc(virStoragePoolPtr obj, ret = virStoragePoolDefFormat(def); cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -1111,19 +1075,11 @@ static int storagePoolGetAutostart(virStoragePoolPtr obj, int *autostart) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; int ret = -1; - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return -1; if (virStoragePoolGetAutostartEnsureACL(obj->conn, pool->def) < 0) goto cleanup; @@ -1136,8 +1092,7 @@ storagePoolGetAutostart(virStoragePoolPtr obj, ret = 0; cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -1153,8 +1108,11 @@ storagePoolSetAutostart(virStoragePoolPtr obj, pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); if (!pool) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); + _("no storage pool with matching uuid '%s' (%s)"), + uuidstr, obj->name); goto cleanup; } @@ -1208,20 +1166,12 @@ storagePoolSetAutostart(virStoragePoolPtr obj, static int storagePoolNumOfVolumes(virStoragePoolPtr obj) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; int ret = -1; size_t i; - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return -1; if (virStoragePoolNumOfVolumesEnsureACL(obj->conn, pool->def) < 0) goto cleanup; @@ -1239,8 +1189,7 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj) } cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -1249,22 +1198,14 @@ storagePoolListVolumes(virStoragePoolPtr obj, char **const names, int maxnames) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; size_t i; int n = 0; memset(names, 0, maxnames * sizeof(*names)); - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return -1; if (virStoragePoolListVolumesEnsureACL(obj->conn, pool->def) < 0) goto cleanup; @@ -1287,8 +1228,7 @@ storagePoolListVolumes(virStoragePoolPtr obj, return n; cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); for (n = 0; n < maxnames; n++) VIR_FREE(names[n]); @@ -1301,7 +1241,6 @@ storagePoolListAllVolumes(virStoragePoolPtr pool, virStorageVolPtr **vols, unsigned int flags) { - virStorageDriverStatePtr driver = pool->conn->storagePrivateData; virStoragePoolObjPtr obj; size_t i; virStorageVolPtr *tmp_vols = NULL; @@ -1311,16 +1250,8 @@ storagePoolListAllVolumes(virStoragePoolPtr pool, virCheckFlags(0, -1); - storageDriverLock(driver); - obj = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid); - storageDriverUnlock(driver); - - if (!obj) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), - pool->uuid); - goto cleanup; - } + if (!(obj = virStoragePoolObjFromStoragePool(pool))) + return -1; if (virStoragePoolListAllVolumesEnsureACL(pool->conn, obj->def) < 0) goto cleanup; @@ -1365,8 +1296,7 @@ storagePoolListAllVolumes(virStoragePoolPtr pool, VIR_FREE(tmp_vols); } - if (obj) - virStoragePoolObjUnlock(obj); + virStoragePoolObjUnlock(obj); return ret; } @@ -1375,20 +1305,12 @@ static virStorageVolPtr storageVolLookupByName(virStoragePoolPtr obj, const char *name) { - virStorageDriverStatePtr driver = obj->conn->storagePrivateData; virStoragePoolObjPtr pool; virStorageVolDefPtr vol; virStorageVolPtr ret = NULL; - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return NULL; if (!virStoragePoolObjIsActive(pool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -1412,8 +1334,7 @@ storageVolLookupByName(virStoragePoolPtr obj, NULL, NULL); cleanup: - if (pool) - virStoragePoolObjUnlock(pool); + virStoragePoolObjUnlock(pool); return ret; } @@ -1681,15 +1602,8 @@ storageVolCreateXML(virStoragePoolPtr obj, virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); - storageDriverLock(driver); - pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid); - storageDriverUnlock(driver); - - if (!pool) { - virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); - goto cleanup; - } + if (!(pool = virStoragePoolObjFromStoragePool(obj))) + return NULL; if (!virStoragePoolObjIsActive(pool)) { virReportError(VIR_ERR_OPERATION_INVALID, @@ -1821,8 +1735,11 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj, } storageDriverUnlock(driver); if (!pool) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virReportError(VIR_ERR_NO_STORAGE_POOL, - _("no storage pool with matching uuid %s"), obj->uuid); + _("no storage pool with matching uuid '%s' (%s)"), + uuidstr, obj->name); goto cleanup; } -- 1.9.3 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list