[libvirt] [PATCH 22/24] maint: improve VIR_ERR_INVALID_DOMAIN_SNAPSHOT usage

The existing check of domain snapshots validated that they point to a domain, but did not validate that the domain points to a connection, even though any errors blindly assume the connection is valid. For consistency with previous patches, continue the trend of using a common macro. For now, we don't need virCheckDomainSnapshotGoto(). * src/datatypes.h (virCheckDomainSnapshotReturn): New macro. (VIR_IS_SNAPSHOT, VIR_IS_DOMAIN_SNAPSHOT): Drop unused macros. * src/libvirt.c: Use macro throughout. (virLibDomainSnapshotError): Drop unused macro. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/datatypes.h | 18 +++++++-- src/libvirt.c | 118 +++++++++++--------------------------------------------- 2 files changed, 36 insertions(+), 100 deletions(-) diff --git a/src/datatypes.h b/src/datatypes.h index 7932a6c..10eeebb 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -232,10 +232,20 @@ extern virClassPtr virStoragePoolClass; } \ } while (0) -# define VIR_IS_SNAPSHOT(obj) \ - (virObjectIsClass((obj), virDomainSnapshotClass)) -# define VIR_IS_DOMAIN_SNAPSHOT(obj) \ - (VIR_IS_SNAPSHOT(obj) && virObjectIsClass((obj)->domain, virDomainClass)) +# define virCheckDomainSnapshotReturn(obj, retval) \ + do { \ + virDomainSnapshotPtr _snap = (obj); \ + if (!virObjectIsClass(_snap, virNWFilterClass) || \ + !virObjectIsClass(_snap->domain, virDomainClass) || \ + !virObjectIsClass(_snap->domain->conn, virConnectClass)) { \ + virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, \ + VIR_ERR_INVALID_DOMAIN_SNAPSHOT, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + virDispatchError(NULL); \ + return retval; \ + } \ + } while (0) /* Helper macros to implement VIR_DOMAIN_DEBUG using just C99. This diff --git a/src/libvirt.c b/src/libvirt.c index 9fb69ed..a74bfc7 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 virLibDomainSnapshotError(code, ...) \ - virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, code, __FILE__, \ - __FUNCTION__, __LINE__, __VA_ARGS__) /** @@ -17940,12 +17937,8 @@ virDomainSnapshotGetName(virDomainSnapshotPtr snapshot) virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return NULL; - } + virCheckDomainSnapshotReturn(snapshot, NULL); + return snapshot->name; } @@ -17970,12 +17963,8 @@ virDomainSnapshotGetDomain(virDomainSnapshotPtr snapshot) virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return NULL; - } + virCheckDomainSnapshotReturn(snapshot, NULL); + return snapshot->domain; } @@ -18000,12 +17989,8 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return NULL; - } + virCheckDomainSnapshotReturn(snapshot, NULL); + return snapshot->domain->conn; } @@ -18185,13 +18170,7 @@ virDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return NULL; - } - + virCheckDomainSnapshotReturn(snapshot, NULL); conn = snapshot->domain->conn; if ((conn->flags & VIR_CONNECT_RO) && (flags & VIR_DOMAIN_XML_SECURE)) { @@ -18500,14 +18479,9 @@ virDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, unsigned int flags) virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckDomainSnapshotReturn(snapshot, -1); conn = snapshot->domain->conn; + if (conn->driver->domainSnapshotNumChildren) { int ret = conn->driver->domainSnapshotNumChildren(snapshot, flags); if (ret < 0) @@ -18591,13 +18565,7 @@ virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckDomainSnapshotReturn(snapshot, -1); conn = snapshot->domain->conn; virCheckNonNullArgGoto(names, error); @@ -18685,13 +18653,7 @@ virDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, if (snaps) *snaps = NULL; - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckDomainSnapshotReturn(snapshot, -1); conn = snapshot->domain->conn; if (conn->driver->domainSnapshotListAllChildren) { @@ -18847,13 +18809,7 @@ virDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return NULL; - } - + virCheckDomainSnapshotReturn(snapshot, NULL); conn = snapshot->domain->conn; if (conn->driver->domainSnapshotGetParent) { @@ -18891,13 +18847,7 @@ virDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckDomainSnapshotReturn(snapshot, -1); conn = snapshot->domain->conn; if (conn->driver->domainSnapshotIsCurrent) { @@ -18936,13 +18886,7 @@ virDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckDomainSnapshotReturn(snapshot, -1); conn = snapshot->domain->conn; if (conn->driver->domainSnapshotHasMetadata) { @@ -19013,14 +18957,9 @@ virDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckDomainSnapshotReturn(snapshot, -1); conn = snapshot->domain->conn; + virCheckReadOnlyGoto(conn->flags, error); if ((flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) && @@ -19079,14 +19018,9 @@ virDomainSnapshotDelete(virDomainSnapshotPtr snapshot, virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return -1; - } - + virCheckDomainSnapshotReturn(snapshot, -1); conn = snapshot->domain->conn; + virCheckReadOnlyGoto(conn->flags, error); if ((flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) && @@ -19137,12 +19071,8 @@ virDomainSnapshotRef(virDomainSnapshotPtr snapshot) virResetLastError(); - if ((!VIR_IS_DOMAIN_SNAPSHOT(snapshot))) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckDomainSnapshotReturn(snapshot, -1); + virObjectRef(snapshot); return 0; } @@ -19164,12 +19094,8 @@ virDomainSnapshotFree(virDomainSnapshotPtr snapshot) virResetLastError(); - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, - __FUNCTION__); - virDispatchError(NULL); - return -1; - } + virCheckDomainSnapshotReturn(snapshot, -1); + virObjectUnref(snapshot); return 0; } -- 1.8.4.2