Re: [libvirt] [PATCH 16/16] backup: Introduce virDomainCheckpointPtr
On 3/20/19 1:41 AM, 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. Thus, it shares the common
virDomainMoment base class created in the previous patches.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
include/libvirt/virterror.h | 6 +++++-
src/util/virerror.c | 12 ++++++++++-
include/libvirt/libvirt.h | 6 +++++-
src/datatypes.h | 43 +++++++++++++++++++++++++++++++++++++
src/datatypes.c | 22 +++++++++++++++++++
src/libvirt_private.syms | 2 ++
6 files changed, 88 insertions(+), 3 deletions(-)
You noted in a response a need to rework this due to removing patch 2&3,
which well probably isn't entirely necessary, but if you do, then it's
fine. I've seen this in previous reviews as well and it generally looks
fine to me.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
[one nit below]
diff --git a/include/libvirt/virterror.h
b/include/libvirt/virterror.h
index 3c19ff5e2e..bccf3c731e 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 */
You could add a space between ,/ like you did below even though the
columnar formatting changed.
# ifdef VIR_ENUM_SENTINELS
VIR_ERR_DOMAIN_LAST
@@ -322,6 +323,9 @@ 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
[...]