Re: [libvirt] [PATCH v2 4/9] backup: Introduce virDomainCheckpointPtr
On 10/12/18 3:59 AM, Peter Krempa wrote:
On Fri, Oct 12, 2018 at 00:10:06 -0500, 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(a)redhat.com>
>
All the error stuff should be in a separate commit. You don't even
mention it in the commit message.
Will split.
> }
> diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
> index 36f6d60775..26887a40e7 100644
> --- a/include/libvirt/libvirt.h
> +++ b/include/libvirt/libvirt.h
> @@ -36,6 +36,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;
Ewwww. Don't mix includes with definitions.
Goes away in 6/9 when I create the public
<libvirt/libvirt-domain-checkpoint.h>. I should clearly document this in
the commit message and with a comment, if I can't find a way to reorder
the patches so that the hack isn't even needed.
> # include <libvirt/libvirt-domain-snapshot.h>
> # include <libvirt/libvirt-event.h>
> # include <libvirt/libvirt-interface.h>
> diff --git a/src/datatypes.h b/src/datatypes.h
> index e1b38706dc..c27ac98437 100644
> --- a/src/datatypes.h
> +++ b/src/datatypes.h
[...]
> @@ -667,6 +683,17 @@ struct _virStream {
> void *privateData;
> };
>
> +/**
> + * _virDomainCheckpoint
> + *
> + * Internal structure associated with a domain checkpoint
> + */
> +struct _virDomainCheckpoint {
> + virObject parent;
> + char *name;
I was thinking whether an UUID should not be used as well. But on the
other hand most management tools will stash an UUID into the name
anyways so it should be fine.
And, it matches that snapshots also use a name.
In fact, I'm half-heartedly thinking of refactoring into:
struct _virDomainAffiliate {
virObject parent;
char *name;
};
struct _virDomainSnapshot {
_virDomainAffiliate base;
};
struct _virDomainCheckpoint {
_virDomainAffliate base;
};
because there were several places where I could reuse more code between
snapshots and checkpoints if they had a common base type.
> +++ b/src/datatypes.c
> @@ -1,7 +1,7 @@
> /*
> * datatypes.c: management of structs for public data types
> *
> - * Copyright (C) 2006-2015 Red Hat, Inc.
> + * Copyright (C) 2006-2018 Red Hat, Inc.
AFAIK we established that bumping these is pointless churn. The dates
can be re-establised from git history.
My emacs has a hook that does it automatically. I can turn the hook off
for libvirt, if you don't like the churn.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org