[libvirt] [PATCH 1/4] snapshot: new query APIs

[Osier Yang]

On 2012年05月25日 11:33, Eric Blake wrote:
> Right now, starting from just a virDomainSnapshotPtr, and wanting to
> know if it is the current snapshot for its respective domain, you have
> to use virDomainSnapshotGetDomain(), then virDomainSnapshotCurrent(),
> then compare the two names returned by virDomainSnapshotGetName().
> It is a bit easier if we can directly query this information from the
> snapshot itself.
>
> Right now, it is possible to filter a snapshot listing based on
> whether snapshots have metadata that would prevent domain deletion,
> but the only way to learn if an individual snapshot has metadata is
> to see if that snapshot appears in the list returned by a listing.
> Additionally, I hope to expand the qemu driver in a future patch to
> use qemu-img to reconstruct snapshot XML corresponding to internal
> qcow2 snapshot names not otherwise tracked by libvirt (in part, so
> that libvirt can guarantee that new snapshots are not created with
> a name that would silently corrupt the existing portion of the qcow2
> file); if I ever get that in, then it would no longer be an all-or-none
> decision on whether snapshots have metadata, and becomes all the more
> important to be able to directly determine that information from a
> particular snapshot.
>
> Other query functions (such as virDomainIsActive) do not have a flags
> argument, but since virDomainHasCurrentSnapshot takes a flags argument,
> I figured it was safer to provide a flags argument here as well.
>
> * include/libvirt/libvirt.h.in (virDomainSnapshotIsCurrent)
> (virDomainSnapshotHasMetadata): New declarations.
> * src/libvirt.c (virDomainSnapshotIsCurrent)
> (virDomainSnapshotHasMetadata): New functions.
> * src/libvirt_public.syms (LIBVIRT_0.9.13): Export them.
> * src/driver.h (virDrvDomainSnapshotIsCurrent)
> (virDrvDomainSnapshotHasMetadata): New driver callbacks.
> ---
>   include/libvirt/libvirt.h.in |    9 ++++
>   src/driver.h                 |   10 +++++
>   src/libvirt.c                |   85 ++++++++++++++++++++++++++++++++++++++++++
>   src/libvirt_public.syms      |    6 +++
>   4 files changed, 110 insertions(+), 0 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index da3ce29..dde5c4c 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -3401,6 +3401,15 @@ virDomainSnapshotPtr virDomainSnapshotCurrent(virDomainPtr domain,
>   virDomainSnapshotPtr virDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
>                                                   unsigned int flags);
>
> +/* Determine if a snapshot is the current snapshot of its domain.  */
> +int virDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot,
> +                               unsigned int flags);
> +
> +/* Determine if a snapshot has associated libvirt metadata that would
> + * prevent the deletion of its domain.  */
> +int virDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot,
> +                                 unsigned int flags);
> +
>   typedef enum {
>       VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1<<  0, /* Run after revert */
>       VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED  = 1<<  1, /* Pause after revert */
> diff --git a/src/driver.h b/src/driver.h
> index aa7a377..94cc851 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -650,6 +650,14 @@ typedef virDomainSnapshotPtr
>                                      unsigned int flags);
>
>   typedef int
> +    (*virDrvDomainSnapshotIsCurrent)(virDomainSnapshotPtr snapshot,
> +                                     unsigned int flags);
> +
> +typedef int
> +    (*virDrvDomainSnapshotHasMetadata)(virDomainSnapshotPtr snapshot,
> +                                       unsigned int flags);
> +
> +typedef int
>       (*virDrvDomainRevertToSnapshot)(virDomainSnapshotPtr snapshot,
>                                       unsigned int flags);
>
> @@ -986,6 +994,8 @@ struct _virDriver {
>       virDrvDomainHasCurrentSnapshot domainHasCurrentSnapshot;
>       virDrvDomainSnapshotGetParent domainSnapshotGetParent;
>       virDrvDomainSnapshotCurrent domainSnapshotCurrent;
> +    virDrvDomainSnapshotIsCurrent domainSnapshotIsCurrent;
> +    virDrvDomainSnapshotHasMetadata domainSnapshotHasMetadata;
>       virDrvDomainRevertToSnapshot domainRevertToSnapshot;
>       virDrvDomainSnapshotDelete domainSnapshotDelete;
>       virDrvDomainQemuMonitorCommand qemuDomainMonitorCommand;
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 8900011..41ca840 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -17621,6 +17621,91 @@ error:
>   }
>
>   /**
> + * virDomainSnapshotIsCurrent:
> + * @snapshot: a snapshot object
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * Determine if the given snapshot is the domain's current snapshot.  See
> + * also virDomainHasCurrentSnapshot().
> + *
> + * Returns 1 if current, 0 if not current, or -1 on error.
> + */
> +int virDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot,
> +                               unsigned int flags)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DEBUG("snapshot=%p, flags=%x", snapshot, flags);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) {
> +        virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT,
> +                                  __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }
> +
> +    conn = snapshot->domain->conn;
> +
> +    if (conn->driver->domainSnapshotIsCurrent) {
> +        int ret;
> +        ret = conn->driver->domainSnapshotIsCurrent(snapshot, flags);
> +        if (ret<  0)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +error:
> +    virDispatchError(conn);
> +    return -1;
> +}
> +
> +/**
> + * virDomainSnapshotHasMetadata:
> + * @snapshot: a snapshot object
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * Determine if the given snapshot is associated with libvirt metadata
> + * that would prevent the deletion of the domain.
> + *
> + * Returns 1 if the snapshot has metadata, 0 if the snapshot exists without
> + * help from libvirt, or -1 on error.
> + */
> +int virDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot,
> +                                 unsigned int flags)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DEBUG("snapshot=%p, flags=%x", snapshot, flags);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) {
> +        virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT,
> +                                  __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }
> +
> +    conn = snapshot->domain->conn;
> +
> +    if (conn->driver->domainSnapshotHasMetadata) {
> +        int ret;
> +        ret = conn->driver->domainSnapshotHasMetadata(snapshot, flags);
> +        if (ret<  0)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +error:
> +    virDispatchError(conn);
> +    return -1;
> +}
> +
> +/**
>    * virDomainRevertToSnapshot:
>    * @snapshot: a domain snapshot object
>    * @flags: bitwise-OR of virDomainSnapshotRevertFlags
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 46c13fb..ebcbd94 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -534,4 +534,10 @@ LIBVIRT_0.9.11 {
>           virDomainPMWakeup;
>   } LIBVIRT_0.9.10;
>
> +LIBVIRT_0.9.13 {
> +    global:
> +        virDomainSnapshotHasMetadata;
> +        virDomainSnapshotIsCurrent;
> +} LIBVIRT_0.9.11;
> +
>   # .... define new API here using predicted next version number ....

I'm not hep in the overall snapshot design, but the APIs looks
quite straightforward, so ACK.

Osier