On 06/26/2018 02:03 PM, Nir Soffer wrote:
On Wed, Jun 13, 2018 at 7:42 PM Eric Blake <eblake@xxxxxxxxxx> wrote:
Prepare for introducing a bunch of new public APIs related to
backup checkpoints by first introducing a new internal type
and errors associated with that type. Checkpoints are modeled
heavily after virDomainSnapshotPtr (both represent a point in
time of the guest), although a snapshot exists with the intent
of rolling back to that state, while a checkpoint exists to
make it possible to create an incremental backup at a later
time.
Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
include/libvirt/libvirt-domain-snapshot.h | 8 ++--
include/libvirt/libvirt.h | 2 +
include/libvirt/virterror.h | 5 ++-
src/datatypes.c | 62
++++++++++++++++++++++++++++++-
src/datatypes.h | 31 +++++++++++++++-
src/libvirt_private.syms | 2 +
src/util/virerror.c | 15 +++++++-
7 files changed, 118 insertions(+), 7 deletions(-)
diff --git a/include/libvirt/libvirt-domain-snapshot.h
b/include/libvirt/libvirt-domain-snapshot.h
index e5a893a767..ff1e890cfc 100644
--- a/include/libvirt/libvirt-domain-snapshot.h
+++ b/include/libvirt/libvirt-domain-snapshot.h
@@ -31,15 +31,17 @@
/**
* virDomainSnapshot:
*
- * a virDomainSnapshot is a private structure representing a snapshot of
- * a domain.
+ * A virDomainSnapshot is a private structure representing a snapshot of
+ * a domain. A snapshot captures the state of the domain at a point in
+ * time, with the intent that the guest can be reverted back to that
+ * state at a later time.
The extra context is very nice...
*/
typedef struct _virDomainSnapshot virDomainSnapshot;
/**
* virDomainSnapshotPtr:
*
- * a virDomainSnapshotPtr is pointer to a virDomainSnapshot private
structure,
+ * A virDomainSnapshotPtr is pointer to a virDomainSnapshot private
structure,
* and is the type used to reference a domain snapshot in the API.
*/
But I think users of this API would like to find it here, explaining the
public
type.
That's a pre-existing documentation issue (probably worth a separate
cleanup patch to a lot of files, if it really does render better to tie
the details to the 'Ptr' typedef rather than the opaque typedef).
@@ -321,6 +322,8 @@ typedef enum {
to guest-sync command
(DEPRECATED)*/
VIR_ERR_LIBSSH = 98, /* error in libssh transport
driver */
VIR_ERR_DEVICE_MISSING = 99, /* fail to find the desired
device */
+ VIR_ERR_INVALID_DOMAIN_CHECKPOINT = 100,/* invalid domain checkpoint
*/
What is invalid checkpoint? It would be nice if there would not be
such thing.
Copied from the existing VIR_ERR_INVALID_DOMAIN_SNAPSHOT. Sadly, there
MUST be such a thing - it exists to (try and) catch bugs such as:
void *ptr = virAPI1() (which returns virDomainPtr)
virDomainCheckpointAPI2(ptr, ...) (which expects virDomainCheckpointPtr)
where you are passing in the wrong type, or such as:
virConnectPtr conn = virAPI1()
virDomainCheckpointPtr chk = virAPI2(conn)
virConnectClose(conn)
virDomainCheckpointAPI3(chk)
where you are passing in the right type but wrong order because the
checkpoint depends on a connection that you have closed
Also the comment does not add anything.
Such is the life of copy-and-paste. My excuse is that the code I copied
from has the same sort of poor comment.
@@ -292,6 +293,21 @@ extern virClassPtr virAdmClientClass;
} \
} while (0)
+# define virCheckDomainCheckpointReturn(obj, retval) \
+ do { \
+ virDomainCheckpointPtr _check = (obj); \
+ if (!virObjectIsClass(_check, virDomainCheckpointClass) || \
+ !virObjectIsClass(_check->domain, virDomainClass) || \
+ !virObjectIsClass(_check->domain->conn, virConnectClass)) { \
+ virReportErrorHelper(VIR_FROM_DOMAIN_CHECKPOINT, \
+ VIR_ERR_INVALID_DOMAIN_CHECKPOINT, \
+ __FILE__, __FUNCTION__, __LINE__, \
+ __FUNCTION__); \
I guess that this is invalid domain checkpoint. Isn't this a generic
error, providing a pointer of the wrong type?
Yes, except that libvirt already has the practice of distinguishing
error messages according to which type was expected.
@@ -712,6 +739,8 @@ virStreamPtr virGetStream(virConnectPtr conn);
virNWFilterPtr virGetNWFilter(virConnectPtr conn,
const char *name,
const unsigned char *uuid);
+virDomainCheckpointPtr virGetDomainCheckpoint(virDomainPtr domain,
+ const char *name);
I guess this implemented and documented elsewhere.
This is a function for internal use only; it is not exported as a public
function, but exists to mirror...
virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
const char *name);
...this existing snapshot function with the exact same amount of zero
comments.
It was implemented in this patch, in src/datatypes.c.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
