[libvirt] [PATCH 16/24] maint: improve VIR_ERR_INVALID_STORAGE_POOL usage

virStoragePoolBuild reported an invalid pool as if it were an invalid network. Likewise, we weren't consistent on whether to use VIR_FROM_NONE or VIR_FROM_STORAGE. Similar to previous patches, use a common macro to make it nicer. For now, we don't need virCheckStoragePoolGoto(). * src/datatypes.h (virCheckStoragePoolReturn): New macro. (VIR_IS_STORAGE_POOL, VIR_IS_CONNECTED_STORAGE_POOL): Drop unused macros. * src/libvirt.c: Use macro throughout. (virLibStoragePoolError): Drop unused macro. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/datatypes.h | 17 ++++-- src/libvirt.c | 172 +++++++++++++------------------------------------------- 2 files changed, 51 insertions(+), 138 deletions(-) diff --git a/src/datatypes.h b/src/datatypes.h index 304e9b5..0be271d 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -124,10 +124,19 @@ extern virClassPtr virStoragePoolClass; } \ } while (0) -# define VIR_IS_STORAGE_POOL(obj) \ - (virObjectIsClass((obj), virStoragePoolClass)) -# define VIR_IS_CONNECTED_STORAGE_POOL(obj) \ - (VIR_IS_STORAGE_POOL(obj) && virObjectIsClass((obj)->conn, virConnectClass)) +# define virCheckStoragePoolReturn(obj, retval) \ + do { \ + virStoragePoolPtr _pool = (obj); \ + if (!virObjectIsClass(_pool, virStoragePoolClass) || \ + !virObjectIsClass(_pool->conn, virConnectClass)) { \ + virReportErrorHelper(VIR_FROM_STORAGE, \ + VIR_ERR_INVALID_STORAGE_POOL, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + virDispatchError(NULL); \ + return retval; \ + } \ + } while (0) # define VIR_IS_STORAGE_VOL(obj) \ (virObjectIsClass((obj), virStorageVolClass)) diff --git a/src/libvirt.c b/src/libvirt.c index 55c35d5..65dc364 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -518,9 +518,6 @@ DllMain(HINSTANCE instance ATTRIBUTE_UNUSED, #define virLibDomainError(code, ...) \ virReportErrorHelper(VIR_FROM_DOM, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) -#define virLibStoragePoolError(code, ...) \ - virReportErrorHelper(VIR_FROM_STORAGE, code, __FILE__, \ - __FUNCTION__, __LINE__, __VA_ARGS__) #define virLibStorageVolError(code, ...) \ virReportErrorHelper(VIR_FROM_STORAGE, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) @@ -12164,11 +12161,8 @@ virStoragePoolGetConnect(virStoragePoolPtr pool) virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return NULL; - } + virCheckStoragePoolReturn(pool, NULL); + return pool->conn; } @@ -12713,12 +12707,9 @@ virStoragePoolBuild(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_NETWORK, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); conn = pool->conn; + virCheckReadOnlyGoto(conn->flags, error); if (conn->storageDriver && conn->storageDriver->storagePoolBuild) { @@ -12753,12 +12744,9 @@ virStoragePoolUndefine(virStoragePoolPtr pool) virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_NETWORK, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); conn = pool->conn; + virCheckReadOnlyGoto(conn->flags, error); if (conn->storageDriver && conn->storageDriver->storagePoolUndefine) { @@ -12795,12 +12783,9 @@ virStoragePoolCreate(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); conn = pool->conn; + virCheckReadOnlyGoto(conn->flags, error); if (conn->storageDriver && conn->storageDriver->storagePoolCreate) { @@ -12839,13 +12824,9 @@ virStoragePoolDestroy(virStoragePoolPtr pool) virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckStoragePoolReturn(pool, -1); conn = pool->conn; + virCheckReadOnlyGoto(conn->flags, error); if (conn->storageDriver && conn->storageDriver->storagePoolDestroy) { @@ -12884,13 +12865,9 @@ virStoragePoolDelete(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckStoragePoolReturn(pool, -1); conn = pool->conn; + virCheckReadOnlyGoto(conn->flags, error); if (conn->storageDriver && conn->storageDriver->storagePoolDelete) { @@ -12925,11 +12902,8 @@ virStoragePoolFree(virStoragePoolPtr pool) virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); + virObjectUnref(pool); return 0; @@ -12960,11 +12934,8 @@ virStoragePoolRef(virStoragePoolPtr pool) virResetLastError(); - if ((!VIR_IS_CONNECTED_STORAGE_POOL(pool))) { - virLibConnError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); + virObjectRef(pool); return 0; } @@ -12990,13 +12961,9 @@ virStoragePoolRefresh(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckStoragePoolReturn(pool, -1); conn = pool->conn; + virCheckReadOnlyGoto(conn->flags, error); if (conn->storageDriver && conn->storageDriver->storagePoolRefresh) { @@ -13030,11 +12997,8 @@ virStoragePoolGetName(virStoragePoolPtr pool) virResetLastError(); - if (!VIR_IS_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return NULL; - } + virCheckStoragePoolReturn(pool, NULL); + return pool->name; } @@ -13056,11 +13020,7 @@ virStoragePoolGetUUID(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); virCheckNonNullArgGoto(uuid, error); memcpy(uuid, &pool->uuid[0], VIR_UUID_BUFLEN); @@ -13090,11 +13050,7 @@ virStoragePoolGetUUIDString(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); virCheckNonNullArgGoto(buf, error); virUUIDFormat(pool->uuid, buf); @@ -13127,11 +13083,7 @@ virStoragePoolGetInfo(virStoragePoolPtr pool, memset(info, 0, sizeof(virStoragePoolInfo)); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); virCheckNonNullArgGoto(info, error); conn = pool->conn; @@ -13172,12 +13124,7 @@ virStoragePoolGetXMLDesc(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return NULL; - } - + virCheckStoragePoolReturn(pool, NULL); conn = pool->conn; if (conn->storageDriver && conn->storageDriver->storagePoolGetXMLDesc) { @@ -13215,11 +13162,7 @@ virStoragePoolGetAutostart(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); virCheckNonNullArgGoto(autostart, error); conn = pool->conn; @@ -13258,16 +13201,11 @@ virStoragePoolSetAutostart(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibStoragePoolError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } - - virCheckReadOnlyGoto(pool->conn->flags, error); - + virCheckStoragePoolReturn(pool, -1); conn = pool->conn; + virCheckReadOnlyGoto(conn->flags, error); + if (conn->storageDriver && conn->storageDriver->storagePoolSetAutostart) { int ret; ret = conn->storageDriver->storagePoolSetAutostart(pool, autostart); @@ -13311,11 +13249,7 @@ virStoragePoolListAllVolumes(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); if (pool->conn->storageDriver && pool->conn->storageDriver->storagePoolListAllVolumes) { @@ -13349,11 +13283,7 @@ virStoragePoolNumOfVolumes(virStoragePoolPtr pool) virResetLastError(); - if (!VIR_IS_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); if (pool->conn->storageDriver && pool->conn->storageDriver->storagePoolNumOfVolumes) { int ret; @@ -13393,12 +13323,7 @@ virStoragePoolListVolumes(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckStoragePoolReturn(pool, -1); virCheckNonNullArgGoto(names, error); virCheckNonNegativeArgGoto(maxnames, error); @@ -13466,12 +13391,7 @@ virStorageVolLookupByName(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); - virDispatchError(NULL); - return NULL; - } - + virCheckStoragePoolReturn(pool, NULL); virCheckNonNullArgGoto(name, error); if (pool->conn->storageDriver && pool->conn->storageDriver->storageVolLookupByName) { @@ -13641,14 +13561,8 @@ virStorageVolCreateXML(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return NULL; - } - + virCheckStoragePoolReturn(pool, NULL); virCheckNonNullArgGoto(xmlDesc, error); - virCheckReadOnlyGoto(pool->conn->flags, error); if (pool->conn->storageDriver && pool->conn->storageDriver->storageVolCreateXML) { @@ -13697,11 +13611,7 @@ virStorageVolCreateXMLFrom(virStoragePoolPtr pool, virResetLastError(); - if (!VIR_IS_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__); - virDispatchError(NULL); - return NULL; - } + virCheckStoragePoolReturn(pool, NULL); if (!VIR_IS_STORAGE_VOL(clonevol)) { virLibConnError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__); @@ -16719,11 +16629,8 @@ virStoragePoolIsActive(virStoragePoolPtr pool) virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); + if (pool->conn->storageDriver->storagePoolIsActive) { int ret; ret = pool->conn->storageDriver->storagePoolIsActive(pool); @@ -16755,11 +16662,8 @@ virStoragePoolIsPersistent(virStoragePoolPtr pool) virResetLastError(); - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckStoragePoolReturn(pool, -1); + if (pool->conn->storageDriver->storagePoolIsPersistent) { int ret; ret = pool->conn->storageDriver->storagePoolIsPersistent(pool); -- 1.8.4.2