[libvirt] [PATCH v4 05/20] backup: Introduce virDomainCheckpointPtr

[Eric Blake]

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