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@xxxxxxxxxx>
>>
>> ---
>> 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@xxxxxxxxxx>
>
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
