Re: [libvirt] [PATCH v4 05/20] backup: Introduce virDomainCheckpointPtr
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
>>
>> @@ -1214,6 +1215,15 @@ const virErrorMsgTuple
>> virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
>> [VIR_ERR_NO_NWFILTER_BINDING] = {
>> N_("Network filter binding not found"),
>> N_("Network filter binding not found: %s") },
>> +[VIR_ERR_INVALID_DOMAIN_CHECKPOINT] = {
>> +N_("Invalid checkpoint"),
>> +N_("Invalid checkpoint: %s") },
>
> Invalid domain checkpoint
Copy and paste from VIR_ERR_INVALID_DOMAIN_SNAPSHOT; will do separate
patch to fix that wording.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 05/20] backup: Introduce virDomainCheckpointPtr
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
>>
>> ---
>> 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
>> # include
>> # include
>> +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
>
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 05/20] backup: Introduce virDomainCheckpointPtr
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
>
> ---
> 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.
> 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 ;-)
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -132,6 +132,7 @@ typedef enum {
> VIR_FROM_LIBSSH = 66, /* Error from libssh connection transport */
> VIR_FROM_RESCTRL = 67, /* Error from resource control */
> VIR_FROM_FIREWALLD = 68,/* Error from firewalld */
> +VIR_FROM_DOMAIN_CHECKPOINT = 69,/* Error from domain checkpoint */
>
> # ifdef VIR_ENUM_SENTINELS
> VIR_ERR_DOMAIN_LAST
> @@ -322,11 +323,13 @@ typedef enum {
> VIR_ERR_DEVICE_MISSING = 99,/* fail to find the desired device */
> VIR_ERR_INVALID_NWFILTER_BINDING = 100, /* invalid nwfilter binding */
> VIR_ERR_NO_NWFILTER_BINDING = 101, /* no nwfilter binding */
> +VIR_ERR_INVALID_DOMAIN_CHECKPOINT = 102,/* invalid domain checkpoint */
> +VIR_ERR_NO_DOMAIN_CHECKPOINT = 103, /* domain checkpoint not found */
> +VIR_ERR_NO_DOMAIN_BACKUP = 104, /* domain backup job id not found */
>
> # ifdef VIR_ENUM_SENTINELS
> VIR_ERR_NUMBER_LAST
> # endif
> -
> } virErrorNumber;
>
> /**
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index 63de0cb278..325e8df346 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -1,7 +1,7 @@
> /*
> * virerror.c: error handling and reporting code for libvirt
> *
> - * Copyright (C) 2006, 2008-2016 Red Hat, Inc.
> + * Copyright (C) 2006, 2008-2018 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -139,6 +139,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
>"Libssh transport layer",
>"Resource control",
>"FirewallD",
> + "Domain Checkpoint",
> );
>
>
> @@ -1214,6 +1215,15 @@ const virErrorMsgTuple
> virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
> [VIR_ERR_NO_NWFILTER_BINDING] = {
> N_("Network filter binding not found"),
> N_("Network filter binding not found: %s") },
> +[VIR_ERR_INVALID_DOMAIN_CHECKPOINT] = {
> +N_("Invalid checkpoint"),
> +N_("Invalid checkpoint: %s") },
Invalid domain checkpoint
> +[VIR_ERR_NO_DOMAIN_CHECKPOINT] = {
> +N_("Domain checkpoint not found"),
> +N_("Domain checkpoint not found: %s") },
> +[VIR_ERR_NO_DOMAIN_BACKUP] = {
> +N_("Domain backup job id not found"),
> +N_("Domain backup job id not found: %s") },
> };
>
>
> diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
> index 20e5d276a7..b7238bd96e 100644
> --- a/include/libvirt/libvirt.h
> +++ b/include/libvirt/libvirt.h
> @@ -34,6 +34,8 @@ extern "C" {
> # include
> # include
> # include
> +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.
> # include
> # include
> # include
> diff --git a/src/datatypes.h b/src/datatypes.h
> index 529b340587..f42ad5f989 100644
> --- a/src/datatypes.h
> +++ b/src/datatypes.h
> @@ -1,7 +1,7 @@
> /*
> * datatypes.h: management of structs for public data types
> *
> - * Copyright (C) 2006-2015 Red Hat, Inc.
[libvirt] [PATCH v4 05/20] backup: Introduce virDomainCheckpointPtr
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
---
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(-)
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.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -132,6 +132,7 @@ typedef enum {
VIR_FROM_LIBSSH = 66, /* Error from libssh connection transport */
VIR_FROM_RESCTRL = 67, /* Error from resource control */
VIR_FROM_FIREWALLD = 68,/* Error from firewalld */
+VIR_FROM_DOMAIN_CHECKPOINT = 69,/* Error from domain checkpoint */
# ifdef VIR_ENUM_SENTINELS
VIR_ERR_DOMAIN_LAST
@@ -322,11 +323,13 @@ typedef enum {
VIR_ERR_DEVICE_MISSING = 99,/* fail to find the desired device */
VIR_ERR_INVALID_NWFILTER_BINDING = 100, /* invalid nwfilter binding */
VIR_ERR_NO_NWFILTER_BINDING = 101, /* no nwfilter binding */
+VIR_ERR_INVALID_DOMAIN_CHECKPOINT = 102,/* invalid domain checkpoint */
+VIR_ERR_NO_DOMAIN_CHECKPOINT = 103, /* domain checkpoint not found */
+VIR_ERR_NO_DOMAIN_BACKUP = 104, /* domain backup job id not found */
# ifdef VIR_ENUM_SENTINELS
VIR_ERR_NUMBER_LAST
# endif
-
} virErrorNumber;
/**
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 63de0cb278..325e8df346 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1,7 +1,7 @@
/*
* virerror.c: error handling and reporting code for libvirt
*
- * Copyright (C) 2006, 2008-2016 Red Hat, Inc.
+ * Copyright (C) 2006, 2008-2018 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -139,6 +139,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
"Libssh transport layer",
"Resource control",
"FirewallD",
+ "Domain Checkpoint",
);
@@ -1214,6 +1215,15 @@ const virErrorMsgTuple
virErrorMsgStrings[VIR_ERR_NUMBER_LAST] = {
[VIR_ERR_NO_NWFILTER_BINDING] = {
N_("Network filter binding not found"),
N_("Network filter binding not found: %s") },
+[VIR_ERR_INVALID_DOMAIN_CHECKPOINT] = {
+N_("Invalid checkpoint"),
+N_("Invalid checkpoint: %s") },
+[VIR_ERR_NO_DOMAIN_CHECKPOINT] = {
+N_("Domain checkpoint not found"),
+N_("Domain checkpoint not found: %s") },
+[VIR_ERR_NO_DOMAIN_BACKUP] = {
+N_("Domain backup job id not found"),
+N_("Domain backup job id not found: %s") },
};
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index 20e5d276a7..b7238bd96e 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -34,6 +34,8 @@ extern "C" {
# include
# include
# include
+typedef struct _virDomainCheckpoint virDomainCheckpoint;
+typedef virDomainCheckpoint *virDomainCheckpointPtr;
# include
# include
# include
diff --git a/src/datatypes.h b/src/datatypes.h
index 529b340587..f42ad5f989 100644
--- a/src/datatypes.h
+++ 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.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -31,6 +31,7 @@
extern virClassPtr virConnectClass;
extern virClassPtr virDomainClass;
+extern virClassPtr virDomainCheckpointClass;
extern virClassPtr virDomainSnapshotClass;
extern virClassPtr virInterfaceClass;
extern virClassPtr virNetworkClass;
@@ -307,6 +308,21 @@ extern virClassPtr virAdmClientClass;
} \
} while (0)
+# define virCheckDomainCheckpointReturn(obj, retval) \
+do { \
+virDomainCheckpointPtr _check = (obj); \
+if (!virObjectIsClass(_check, virDomainCheckpointClass) || \
