On 2/9/19 9:48 AM, John Ferlan wrote:
On 2/6/19 2:18 PM, Eric Blake 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@redhat.com>
--- v2: fix copy-and-paste issue in virerror.c [John] --- include/libvirt/virterror.h | 7 +++-- src/util/virerror.c | 12 ++++++- include/libvirt/libvirt.h | 2 ++ src/datatypes.h | 31 ++++++++++++++++++- src/datatypes.c | 62 ++++++++++++++++++++++++++++++++++++- src/libvirt_private.syms | 2 ++ 6 files changed, 111 insertions(+), 5 deletions(-)
Naturally something changed recently in datatypes.h... so you will have a merge conflict here.
Yeah, a long-running issue. v5 will be rebased yet again.
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 3c19ff5e2e..acbf03d0ea 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -4,7 +4,7 @@ * Description: Provides the interfaces of the libvirt library to handle * errors raised while using the library. * - * Copyright (C) 2006-2016 Red Hat, Inc. + * Copyright (C) 2006-2019 Red Hat, Inc.
I'll let someone else complain about your emacs macro ;-)
--- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -34,6 +34,8 @@ extern "C" { # include <libvirt/libvirt-common.h> # include <libvirt/libvirt-host.h> # include <libvirt/libvirt-domain.h> +typedef struct _virDomainCheckpoint virDomainCheckpoint; +typedef virDomainCheckpoint *virDomainCheckpointPtr;
This is a weird construct for this file...
I think this should be in a libvirt-domain-checkpoint.h type file which I see is generated a few patches later and these lines moved. In the long run I guess as long as it's in a file it doesn't matter.
I'll add a comment here making it obvious that this is a temporary hack to avoid even more invasive changes in this patch, and that it gets cleaned up later.
+++ b/src/datatypes.h @@ -1,7 +1,7 @@ /* * datatypes.h: management of structs for public data types * - * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006-2018 Red Hat, Inc.
haha 2018 - I know when you last touched this ;-)
D'oh - my emacs macro may be nice, but it isn't consistent unless I retouch the file. :-)
+/** + * virGetDomainCheckpoint: + * @domain: the domain to checkpoint + * @name: pointer to the domain checkpoint name + * + * Allocates a new domain checkpoint object. When the object is no longer needed, + * virObjectUnref() must be called in order to not leak data. + * + * Returns a pointer to the domain checkpoint object, or NULL on error. + */ +virDomainCheckpointPtr +virGetDomainCheckpoint(virDomainPtr domain, const char *name)
More recently we or I have been trying to enforce one line per argument for new code or changed methods.
Can do. Old habits die hard, but I can make the effort to avoid them in favor of one-argument-per-line.
+{ + virDomainCheckpointPtr ret = NULL; + + if (virDataTypesInitialize() < 0) + return NULL; + + virCheckDomainGoto(domain, error); + virCheckNonNullArgGoto(name, error); + + if (!(ret = virObjectNew(virDomainCheckpointClass))) + goto error;
Could return NULL too, but it is a copy of virGetDomainSnapshot
Reviewed-by: John Ferlan <jferlan@redhat.com>
And as you've seen in my other recent patches, I've been cleaning up some snapshot oddities; any cleanups I do there will also need to be folded into v5 here. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org