Re: [PATCH v4 05/20] backup: Introduce virDomainCheckpointPtr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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.

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

>  # include <libvirt/libvirt-domain-snapshot.h>
>  # include <libvirt/libvirt-event.h>
>  # include <libvirt/libvirt-interface.h>
> 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.

haha 2018 - I know when you last touched this ;-)

>   *
>   * 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) || \
> +            !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
> @@ -667,6 +683,17 @@ struct _virStream {
>      void *privateData;
>  };
> 
> +/**
> + * _virDomainCheckpoint
> + *
> + * Internal structure associated with a domain checkpoint
> + */
> +struct _virDomainCheckpoint {
> +    virObject parent;
> +    char *name;
> +    virDomainPtr domain;
> +};
> +
>  /**
>   * _virDomainSnapshot
>   *
> @@ -743,6 +770,8 @@ virNWFilterPtr virGetNWFilter(virConnectPtr conn,
>  virNWFilterBindingPtr virGetNWFilterBinding(virConnectPtr conn,
>                                              const char *portdev,
>                                              const char *filtername);
> +virDomainCheckpointPtr virGetDomainCheckpoint(virDomainPtr domain,
> +                                              const char *name);
>  virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
>                                            const char *name);
> 
> diff --git a/src/datatypes.c b/src/datatypes.c
> index 09f63d9e15..68f7d86661 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;
> @@ -50,6 +51,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);
> @@ -86,6 +88,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);
> @@ -955,6 +958,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)

More recently we or I have been trying to enforce one line per argument
for new code or changed methods.

> +{
> +    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>

John

> +    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/libvirt_private.syms b/src/libvirt_private.syms
> index 28a2ef707e..5e22acb059 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1219,10 +1219,12 @@ virConnectCloseCallbackDataClass;
>  virConnectCloseCallbackDataGetCallback;
>  virConnectCloseCallbackDataRegister;
>  virConnectCloseCallbackDataUnregister;
> +virDomainCheckpointClass;
>  virDomainClass;
>  virDomainSnapshotClass;
>  virGetConnect;
>  virGetDomain;
> +virGetDomainCheckpoint;
>  virGetDomainSnapshot;
>  virGetInterface;
>  virGetNetwork;
> 


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux