From 8add79a991e6ebc8b5440497e2ada2d1804aec59 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 27 Dec 2013 18:12:05 -0700 Subject: [PATCH] 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. Furthermore, just as in commit 6e130ddc, the difference between VIR_IS_STORAGE_POOL and VIR_IS_CONNECTED_STORAGE_POOL is moot (due to reference counting, any valid pool must be tied to a valid connection). 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 --- 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 ce64577947..a441cf6a23 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 acbadbade4..ffd4f8e2a8 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -528,9 +528,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__) @@ -12183,11 +12180,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; } @@ -12732,12 +12726,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) { @@ -12772,12 +12763,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) { @@ -12814,12 +12802,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) { @@ -12858,13 +12843,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) { @@ -12903,13 +12884,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) { @@ -12944,11 +12921,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; @@ -12979,11 +12953,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; } @@ -13009,13 +12980,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) { @@ -13049,11 +13016,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; } @@ -13075,11 +13039,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); @@ -13109,11 +13069,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); @@ -13144,11 +13100,7 @@ virStoragePoolGetInfo(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(info, error); memset(info, 0, sizeof(virStoragePoolInfo)); @@ -13191,12 +13143,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) { @@ -13234,11 +13181,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; @@ -13277,16 +13220,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); @@ -13330,11 +13268,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) { @@ -13368,11 +13302,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; @@ -13412,12 +13342,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); @@ -13485,12 +13410,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) { @@ -13660,14 +13580,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) { @@ -13716,11 +13630,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__); @@ -16748,11 +16658,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); @@ -16784,11 +16691,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); -- GitLab