[libvirt] [PATCH 1/2] storage: pool: Fix handling of errors on pool lookup failure
Peter Krempa
pkrempa at redhat.com
Thu Jun 5 11:52:33 UTC 2014
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
More information about the libvir-list
mailing list