[libvirt] [PATCH v2 4/9] backup: Introduce virDomainCheckpointPtr

[Eric Blake]

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 at 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