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

Eric Blake posted 8 patches 7 years ago
[libvirt] [PATCH 3/8] backup: Introduce virDomainCheckpointPtr
Posted by Eric Blake 7 years ago
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@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;
diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
index 36f6d60775..26887a40e7 100644
--- a/include/libvirt/libvirt.h
+++ b/include/libvirt/libvirt.h
@@ -36,6 +36,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;
 # include <libvirt/libvirt-domain-snapshot.h>
 # include <libvirt/libvirt-event.h>
 # include <libvirt/libvirt-interface.h>
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 5e58b6a3f9..87ac16be0b 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-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
@@ -133,6 +133,7 @@ typedef enum {
     VIR_FROM_PERF = 65,         /* Error from perf */
     VIR_FROM_LIBSSH = 66,       /* Error from libssh connection transport */
     VIR_FROM_RESCTRL = 67,      /* Error from resource control */
+    VIR_FROM_DOMAIN_CHECKPOINT = 68,/* Error from domain checkpoint */

 # ifdef VIR_ENUM_SENTINELS
     VIR_ERR_DOMAIN_LAST
@@ -321,6 +322,8 @@ typedef enum {
                                            to guest-sync command (DEPRECATED)*/
     VIR_ERR_LIBSSH = 98,                /* error in libssh transport driver */
     VIR_ERR_DEVICE_MISSING = 99,        /* fail to find the desired device */
+    VIR_ERR_INVALID_DOMAIN_CHECKPOINT = 100,/* invalid domain checkpoint */
+    VIR_ERR_NO_DOMAIN_CHECKPOINT = 101, /* domain checkpoint not found */
 } virErrorNumber;

 /**
diff --git a/src/datatypes.c b/src/datatypes.c
index 09b8eea5a2..3c9069c938 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -1,7 +1,7 @@
 /*
  * datatypes.c: 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
@@ -36,6 +36,7 @@ VIR_LOG_INIT("datatypes");
 virClassPtr virConnectClass;
 virClassPtr virConnectCloseCallbackDataClass;
 virClassPtr virDomainClass;
+virClassPtr virDomainCheckpointClass;
 virClassPtr virDomainSnapshotClass;
 virClassPtr virInterfaceClass;
 virClassPtr virNetworkClass;
@@ -49,6 +50,7 @@ virClassPtr virStoragePoolClass;
 static void virConnectDispose(void *obj);
 static void virConnectCloseCallbackDataDispose(void *obj);
 static void virDomainDispose(void *obj);
+static void virDomainCheckpointDispose(void *obj);
 static void virDomainSnapshotDispose(void *obj);
 static void virInterfaceDispose(void *obj);
 static void virNetworkDispose(void *obj);
@@ -84,6 +86,7 @@ virDataTypesOnceInit(void)
     DECLARE_CLASS_LOCKABLE(virConnect);
     DECLARE_CLASS_LOCKABLE(virConnectCloseCallbackData);
     DECLARE_CLASS(virDomain);
+    DECLARE_CLASS(virDomainCheckpoint);
     DECLARE_CLASS(virDomainSnapshot);
     DECLARE_CLASS(virInterface);
     DECLARE_CLASS(virNetwork);
@@ -887,6 +890,63 @@ virDomainSnapshotDispose(void *obj)
 }


+/**
+ * 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)
+{
+    virDomainCheckpointPtr ret = NULL;
+
+    if (virDataTypesInitialize() < 0)
+        return NULL;
+
+    virCheckDomainGoto(domain, error);
+    virCheckNonNullArgGoto(name, error);
+
+    if (!(ret = virObjectNew(virDomainCheckpointClass)))
+        goto error;
+    if (VIR_STRDUP(ret->name, name) < 0)
+        goto error;
+
+    ret->domain = virObjectRef(domain);
+
+    return ret;
+
+ error:
+    virObjectUnref(ret);
+    return NULL;
+}
+
+
+/**
+ * virDomainCheckpointDispose:
+ * @obj: the domain checkpoint to release
+ *
+ * Unconditionally release all memory associated with a checkpoint.
+ * The checkpoint object must not be used once this method returns.
+ *
+ * It will also unreference the associated connection object,
+ * which may also be released if its ref count hits zero.
+ */
+static void
+virDomainCheckpointDispose(void *obj)
+{
+    virDomainCheckpointPtr checkpoint = obj;
+    VIR_DEBUG("release checkpoint %p %s", checkpoint, checkpoint->name);
+
+    VIR_FREE(checkpoint->name);
+    virObjectUnref(checkpoint->domain);
+}
+
+
 virAdmConnectPtr
 virAdmConnectNew(void)
 {
diff --git a/src/datatypes.h b/src/datatypes.h
index 192c86be80..fbe842d105 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;
@@ -292,6 +293,21 @@ extern virClassPtr virAdmClientClass;
         } \
     } while (0)

+# define virCheckDomainCheckpointReturn(obj, retval) \
+    do { \
+        virDomainCheckpointPtr _check = (obj); \
+        if (!virObjectIsClass(_check, virDomainCheckpointClass) || \
+            !virObjectIsClass(_check->domain, virDomainClass) || \
+            !virObjectIsClass(_check->domain->conn, virConnectClass)) { \
+            virReportErrorHelper(VIR_FROM_DOMAIN_CHECKPOINT, \
+                                 VIR_ERR_INVALID_DOMAIN_CHECKPOINT, \
+                                 __FILE__, __FUNCTION__, __LINE__, \
+                                 __FUNCTION__); \
+            virDispatchError(NULL); \
+            return retval; \
+        } \
+    } while (0)
+

 /* Helper macros to implement VIR_DOMAIN_DEBUG using just C99.  This
  * assumes you pass fewer than 15 arguments to VIR_DOMAIN_DEBUG, but
@@ -652,6 +668,17 @@ struct _virStream {
     void *privateData;
 };

+/**
+ * _virDomainCheckpoint
+ *
+ * Internal structure associated with a domain checkpoint
+ */
+struct _virDomainCheckpoint {
+    virObject parent;
+    char *name;
+    virDomainPtr domain;
+};
+
 /**
  * _virDomainSnapshot
  *
@@ -712,6 +739,8 @@ virStreamPtr virGetStream(virConnectPtr conn);
 virNWFilterPtr virGetNWFilter(virConnectPtr conn,
                               const char *name,
                               const unsigned char *uuid);
+virDomainCheckpointPtr virGetDomainCheckpoint(virDomainPtr domain,
+                                              const char *name);
 virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
                                           const char *name);

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ea24f2847c..4686c775a5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1183,10 +1183,12 @@ virConnectCloseCallbackDataClass;
 virConnectCloseCallbackDataGetCallback;
 virConnectCloseCallbackDataRegister;
 virConnectCloseCallbackDataUnregister;
+virDomainCheckpointClass;
 virDomainClass;
 virDomainSnapshotClass;
 virGetConnect;
 virGetDomain;
+virGetDomainCheckpoint;
 virGetDomainSnapshot;
 virGetInterface;
 virGetNetwork;
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");
+            break;
     }
     return errmsg;
 }
-- 
2.14.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] backup: Introduce virDomainCheckpointPtr
Posted by John Ferlan 7 years ago

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] backup: Introduce virDomainCheckpointPtr
Posted by Nir Soffer 7 years ago
On Wed, Jun 13, 2018 at 7:42 PM Eric Blake <eblake@redhat.com> 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@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.
>

The extra context is very nice...


>   */
>  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.
>   */
>

But I think users of this API would like to find it here, explaining the
public
type.


>  typedef virDomainSnapshot *virDomainSnapshotPtr;
> diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
> index 36f6d60775..26887a40e7 100644
> --- a/include/libvirt/libvirt.h
> +++ b/include/libvirt/libvirt.h
> @@ -36,6 +36,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;
>  # include <libvirt/libvirt-domain-snapshot.h>
>  # include <libvirt/libvirt-event.h>
>  # include <libvirt/libvirt-interface.h>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 5e58b6a3f9..87ac16be0b 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-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
> @@ -133,6 +133,7 @@ typedef enum {
>      VIR_FROM_PERF = 65,         /* Error from perf */
>      VIR_FROM_LIBSSH = 66,       /* Error from libssh connection transport
> */
>      VIR_FROM_RESCTRL = 67,      /* Error from resource control */
> +    VIR_FROM_DOMAIN_CHECKPOINT = 68,/* Error from domain checkpoint */
>
>  # ifdef VIR_ENUM_SENTINELS
>      VIR_ERR_DOMAIN_LAST
> @@ -321,6 +322,8 @@ typedef enum {
>                                             to guest-sync command
> (DEPRECATED)*/
>      VIR_ERR_LIBSSH = 98,                /* error in libssh transport
> driver */
>      VIR_ERR_DEVICE_MISSING = 99,        /* fail to find the desired
> device */
> +    VIR_ERR_INVALID_DOMAIN_CHECKPOINT = 100,/* invalid domain checkpoint
> */
>

What is invalid checkpoint? It would be nice if there would not be
such thing.

Also the comment does not add anything.

+    VIR_ERR_NO_DOMAIN_CHECKPOINT = 101, /* domain checkpoint not found */

 } virErrorNumber;
>
>  /**
> diff --git a/src/datatypes.c b/src/datatypes.c
> index 09b8eea5a2..3c9069c938 100644
> --- a/src/datatypes.c
> +++ b/src/datatypes.c
> @@ -1,7 +1,7 @@
>  /*
>   * datatypes.c: 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
> @@ -36,6 +36,7 @@ VIR_LOG_INIT("datatypes");
>  virClassPtr virConnectClass;
>  virClassPtr virConnectCloseCallbackDataClass;
>  virClassPtr virDomainClass;
> +virClassPtr virDomainCheckpointClass;
>  virClassPtr virDomainSnapshotClass;
>  virClassPtr virInterfaceClass;
>  virClassPtr virNetworkClass;
> @@ -49,6 +50,7 @@ virClassPtr virStoragePoolClass;
>  static void virConnectDispose(void *obj);
>  static void virConnectCloseCallbackDataDispose(void *obj);
>  static void virDomainDispose(void *obj);
> +static void virDomainCheckpointDispose(void *obj);
>  static void virDomainSnapshotDispose(void *obj);
>  static void virInterfaceDispose(void *obj);
>  static void virNetworkDispose(void *obj);
> @@ -84,6 +86,7 @@ virDataTypesOnceInit(void)
>      DECLARE_CLASS_LOCKABLE(virConnect);
>      DECLARE_CLASS_LOCKABLE(virConnectCloseCallbackData);
>      DECLARE_CLASS(virDomain);
> +    DECLARE_CLASS(virDomainCheckpoint);
>      DECLARE_CLASS(virDomainSnapshot);
>      DECLARE_CLASS(virInterface);
>      DECLARE_CLASS(virNetwork);
> @@ -887,6 +890,63 @@ virDomainSnapshotDispose(void *obj)
>  }
>
>
> +/**
> + * 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)
> +{
> +    virDomainCheckpointPtr ret = NULL;
> +
> +    if (virDataTypesInitialize() < 0)
> +        return NULL;
> +
> +    virCheckDomainGoto(domain, error);
> +    virCheckNonNullArgGoto(name, error);
>
+
> +    if (!(ret = virObjectNew(virDomainCheckpointClass)))
> +        goto error;
> +    if (VIR_STRDUP(ret->name, name) < 0)
> +        goto error;
> +
> +    ret->domain = virObjectRef(domain);
> +
> +    return ret;
> +
> + error:
> +    virObjectUnref(ret);
> +    return NULL;
> +}
> +
> +
> +/**
> + * virDomainCheckpointDispose:
> + * @obj: the domain checkpoint to release
> + *
> + * Unconditionally release all memory associated with a checkpoint.
> + * The checkpoint object must not be used once this method returns.
> + *
> + * It will also unreference the associated connection object,
> + * which may also be released if its ref count hits zero.
> + */
> +static void
> +virDomainCheckpointDispose(void *obj)
> +{
> +    virDomainCheckpointPtr checkpoint = obj;
> +    VIR_DEBUG("release checkpoint %p %s", checkpoint, checkpoint->name);
> +
> +    VIR_FREE(checkpoint->name);
> +    virObjectUnref(checkpoint->domain);
> +}
> +
> +
>  virAdmConnectPtr
>  virAdmConnectNew(void)
>  {
> diff --git a/src/datatypes.h b/src/datatypes.h
> index 192c86be80..fbe842d105 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;
> @@ -292,6 +293,21 @@ extern virClassPtr virAdmClientClass;
>          } \
>      } while (0)
>
> +# define virCheckDomainCheckpointReturn(obj, retval) \
> +    do { \
> +        virDomainCheckpointPtr _check = (obj); \
> +        if (!virObjectIsClass(_check, virDomainCheckpointClass) || \
> +            !virObjectIsClass(_check->domain, virDomainClass) || \
> +            !virObjectIsClass(_check->domain->conn, virConnectClass)) { \
> +            virReportErrorHelper(VIR_FROM_DOMAIN_CHECKPOINT, \
> +                                 VIR_ERR_INVALID_DOMAIN_CHECKPOINT, \
> +                                 __FILE__, __FUNCTION__, __LINE__, \
> +                                 __FUNCTION__); \
>

I guess that this is invalid domain checkpoint. Isn't this a generic
error, providing a pointer of the wrong type?


> +            virDispatchError(NULL); \
> +            return retval; \
> +        } \
> +    } while (0)
> +
>
>  /* Helper macros to implement VIR_DOMAIN_DEBUG using just C99.  This
>   * assumes you pass fewer than 15 arguments to VIR_DOMAIN_DEBUG, but
> @@ -652,6 +668,17 @@ struct _virStream {
>      void *privateData;
>  };
>
> +/**
> + * _virDomainCheckpoint
> + *
> + * Internal structure associated with a domain checkpoint
> + */
> +struct _virDomainCheckpoint {
> +    virObject parent;
> +    char *name;
> +    virDomainPtr domain;
> +};
> +
>  /**
>   * _virDomainSnapshot
>   *
> @@ -712,6 +739,8 @@ virStreamPtr virGetStream(virConnectPtr conn);
>  virNWFilterPtr virGetNWFilter(virConnectPtr conn,
>                                const char *name,
>                                const unsigned char *uuid);
> +virDomainCheckpointPtr virGetDomainCheckpoint(virDomainPtr domain,
> +                                              const char *name);
>

I guess this implemented and documented elsewhere.



>  virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
>                                            const char *name);
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ea24f2847c..4686c775a5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1183,10 +1183,12 @@ virConnectCloseCallbackDataClass;
>  virConnectCloseCallbackDataGetCallback;
>  virConnectCloseCallbackDataRegister;
>  virConnectCloseCallbackDataUnregister;
> +virDomainCheckpointClass;
>  virDomainClass;
>  virDomainSnapshotClass;
>  virGetConnect;
>  virGetDomain;
> +virGetDomainCheckpoint;
>  virGetDomainSnapshot;
>  virGetInterface;
>  virGetNetwork;
> 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");
> +            break;
>      }
>      return errmsg;
>  }
> --
> 2.14.4
>
>
Nir
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/8] backup: Introduce virDomainCheckpointPtr
Posted by Eric Blake 7 years ago
On 06/26/2018 02:03 PM, Nir Soffer wrote:
> On Wed, Jun 13, 2018 at 7:42 PM Eric Blake <eblake@redhat.com> 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@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.
>>
> 
> The extra context is very nice...
> 
> 
>>    */
>>   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.
>>    */
>>
> 
> But I think users of this API would like to find it here, explaining the
> public
> type.

That's a pre-existing documentation issue (probably worth a separate 
cleanup patch to a lot of files, if it really does render better to tie 
the details to the 'Ptr' typedef rather than the opaque typedef).

>> @@ -321,6 +322,8 @@ typedef enum {
>>                                              to guest-sync command
>> (DEPRECATED)*/
>>       VIR_ERR_LIBSSH = 98,                /* error in libssh transport
>> driver */
>>       VIR_ERR_DEVICE_MISSING = 99,        /* fail to find the desired
>> device */
>> +    VIR_ERR_INVALID_DOMAIN_CHECKPOINT = 100,/* invalid domain checkpoint
>> */
>>
> 
> What is invalid checkpoint? It would be nice if there would not be
> such thing.

Copied from the existing VIR_ERR_INVALID_DOMAIN_SNAPSHOT. Sadly, there 
MUST be such a thing - it exists to (try and) catch bugs such as:

void *ptr = virAPI1() (which returns virDomainPtr)
virDomainCheckpointAPI2(ptr, ...) (which expects virDomainCheckpointPtr)

where you are passing in the wrong type, or such as:

virConnectPtr conn = virAPI1()
virDomainCheckpointPtr chk = virAPI2(conn)
virConnectClose(conn)
virDomainCheckpointAPI3(chk)

where you are passing in the right type but wrong order because the 
checkpoint depends on a connection that you have closed

> 
> Also the comment does not add anything.

Such is the life of copy-and-paste.  My excuse is that the code I copied 
from has the same sort of poor comment.

>> @@ -292,6 +293,21 @@ extern virClassPtr virAdmClientClass;
>>           } \
>>       } while (0)
>>
>> +# define virCheckDomainCheckpointReturn(obj, retval) \
>> +    do { \
>> +        virDomainCheckpointPtr _check = (obj); \
>> +        if (!virObjectIsClass(_check, virDomainCheckpointClass) || \
>> +            !virObjectIsClass(_check->domain, virDomainClass) || \
>> +            !virObjectIsClass(_check->domain->conn, virConnectClass)) { \
>> +            virReportErrorHelper(VIR_FROM_DOMAIN_CHECKPOINT, \
>> +                                 VIR_ERR_INVALID_DOMAIN_CHECKPOINT, \
>> +                                 __FILE__, __FUNCTION__, __LINE__, \
>> +                                 __FUNCTION__); \
>>
> 
> I guess that this is invalid domain checkpoint. Isn't this a generic
> error, providing a pointer of the wrong type?

Yes, except that libvirt already has the practice of distinguishing 
error messages according to which type was expected.

>> @@ -712,6 +739,8 @@ virStreamPtr virGetStream(virConnectPtr conn);
>>   virNWFilterPtr virGetNWFilter(virConnectPtr conn,
>>                                 const char *name,
>>                                 const unsigned char *uuid);
>> +virDomainCheckpointPtr virGetDomainCheckpoint(virDomainPtr domain,
>> +                                              const char *name);
>>
> 
> I guess this implemented and documented elsewhere.

This is a function for internal use only; it is not exported as a public 
function, but exists to mirror...

> 
> 
> 
>>   virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
>>                                             const char *name);

...this existing snapshot function with the exact same amount of zero 
comments.

It was implemented in this patch, in src/datatypes.c.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list