[libvirt] [PATCH 3/8] backup: Introduce virDomainCheckpointPtr

[John Ferlan]

On 06/13/2018 12:42 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>
> ---
>  include/libvirt/libvirt-domain-snapshot.h |  8 ++--
>  include/libvirt/libvirt.h                 |  2 +
>  include/libvirt/virterror.h               |  5 ++-
>  src/datatypes.c                           | 62 ++++++++++++++++++++++++++++++-
>  src/datatypes.h                           | 31 +++++++++++++++-
>  src/libvirt_private.syms                  |  2 +
>  src/util/virerror.c                       | 15 +++++++-
>  7 files changed, 118 insertions(+), 7 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain-snapshot.h b/include/libvirt/libvirt-domain-snapshot.h
> index e5a893a767..ff1e890cfc 100644
> --- a/include/libvirt/libvirt-domain-snapshot.h
> +++ b/include/libvirt/libvirt-domain-snapshot.h
> @@ -31,15 +31,17 @@
>  /**
>   * virDomainSnapshot:
>   *
> - * a virDomainSnapshot is a private structure representing a snapshot of
> - * a domain.
> + * A virDomainSnapshot is a private structure representing a snapshot of
> + * a domain.  A snapshot captures the state of the domain at a point in
> + * time, with the intent that the guest can be reverted back to that
> + * state at a later time.
>   */
>  typedef struct _virDomainSnapshot virDomainSnapshot;
> 
>  /**
>   * virDomainSnapshotPtr:
>   *
> - * a virDomainSnapshotPtr is pointer to a virDomainSnapshot private structure,
> + * A virDomainSnapshotPtr is pointer to a virDomainSnapshot private structure,
>   * and is the type used to reference a domain snapshot in the API.
>   */
>  typedef virDomainSnapshot *virDomainSnapshotPtr;

The above hunk is separable (and push-able) as it's own patch, so you
can consider it :

Reviewed-by: John Ferlan <jferlan at redhat.com>


Naming scheme aside, the rest had one minor nit:

[...]

> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index 93632dbdf7..1e6fd77abf 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
> @@ -140,6 +140,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
>                "Perf", /* 65 */
>                "Libssh transport layer",
>                "Resource control",
> +              "Domain Checkpoint",
>      )
> 
> 
> @@ -1494,6 +1495,18 @@ virErrorMsg(virErrorNumber error, const char *info)
>              else
>                  errmsg = _("device not found: %s");
>              break;
> +        case VIR_ERR_INVALID_DOMAIN_CHECKPOINT:
> +            if (info == NULL)
> +                errmsg = _("Invalid checkpoint");
> +            else
> +                errmsg = _("Invalid checkpoint: %s");
> +            break;
> +        case VIR_ERR_NO_DOMAIN_CHECKPOINT:
> +            if (info == NULL)
> +                errmsg = _("Domain snapshot not found");
> +            else
> +                errmsg = _("Domain snapshot not found: %s");

checkpoint

> +            break;
>      }
>      return errmsg;
>  }
> 

John