From dd0e04d9d089c516a83f0f7702c1d1d2a1fab873 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_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. On the other hand, as mentioned in commit 6e130ddc, any valid domain is already tied to a valid connection, and VIR_IS_SNAPSHOT vs. VIR_IS_DOMAIN_SNAPSHOT makes no real difference; it's best to just validate the chain of all three. 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 --- 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 92e0810c1d..9621c556c0 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, virDomainSnapshotClass) || \ + !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 8fd0d8ac4e..c6d8a25501 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 virLibDomainSnapshotError(code, ...) \ - virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, code, __FILE__, \ - __FUNCTION__, __LINE__, __VA_ARGS__) /** @@ -17975,12 +17972,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; } @@ -18005,12 +17998,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; } @@ -18035,12 +18024,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; } @@ -18223,13 +18208,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)) { @@ -18538,14 +18517,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) @@ -18629,13 +18603,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); @@ -18723,13 +18691,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) { @@ -18885,13 +18847,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) { @@ -18929,13 +18885,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) { @@ -18974,13 +18924,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) { @@ -19051,14 +18995,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) && @@ -19118,14 +19057,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) && @@ -19176,12 +19110,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; } @@ -19203,12 +19133,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; } -- GitLab