[libvirt] [PATCHv3 13/43] snapshot: prevent stranding snapshot data on domain destruction

[Daniel P. Berrange]

On Wed, Aug 24, 2011 at 09:22:30AM -0600, Eric Blake wrote:
> Just as leaving managed save metadata behind can cause problems
> when creating a new domain that happens to collide with the name
> of the just-deleted domain, the same is true of leaving any
> snapshot metadata behind.  For safety sake, extend the semantic
> change of commit b26a9fa9 to also cover snapshot metadata as a
> reason to reject losing the last reference to a domain (undefine
> on an inactive, or shutdown/destroy on a transient).  The caller
> must first take care of snapshots, possible via the existing
> virDomainSnapshotDelete.
> 
> This also documents some new flags that hypervisors can choose to
> support to shortcuts taking care of the metadata as part of the
> shutdown process; however, nontrivial driver support for these flags
> will be deferred to future patches.
> 
> Note that ESX and VBox can never be transient; therefore, they
> do not have to affect shutdown/destroy (the persistent domain
> still remains); likewise they never store snapshot metadata,
> so one of the two flags is trivial.  The bulk of the nontrivial
> work remaining is thus in the qemu driver.
> 
> * include/libvirt/libvirt.h.in (VIR_DOMAIN_UNDEFINE_SNAPSHOTS)
> (VIR_DOMAIN_DESTROY_SNAPSHOTS): New flags.
> * src/libvirt.c (virDomainUndefine, virDomainUndefineFlags)
> (virDomainDestroy, virDomainDestroyFlags, virDomainShutdown):
> Document new limitations and flags.
> * src/esx/esx_driver.c (esxDomainUndefineFlags): Enforce the
> limitations.
> * src/vbox/vbox_tmpl.c (vboxDomainUndefineFlags): Likewise.
> * src/qemu/qemu_driver.c (qemuDomainUndefineFlags)
> (qemuDomainShutdown, qemuDomainDestroyFlags): Likewise.
> ---
>  include/libvirt/libvirt.h.in |   25 ++++++++++++++++++---
>  src/esx/esx_driver.c         |   11 ++++++++-
>  src/libvirt.c                |   47 +++++++++++++++++++++++++++++++++++------
>  src/qemu/qemu_driver.c       |   27 ++++++++++++++++++++++++
>  src/vbox/vbox_tmpl.c         |   11 ++++++++-
>  5 files changed, 106 insertions(+), 15 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 20fdbdf..36f1b34 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -919,10 +919,20 @@ virConnectPtr           virDomainGetConnect     (virDomainPtr domain);
>   * Domain creation and destruction
>   */
> 
> -/*
> - * typedef enum {
> - * } virDomainDestroyFlagsValues;
> +
> +/* Counterparts to virDomainUndefineFlagsValues, but note that running
> + * domains have no managed save data, so no flag is provided for that.
>   */
> +typedef enum {
> +    /* VIR_DOMAIN_DESTROY_MANAGED_SAVE = (1 << 0), */ /* Reserved */
> +    VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA = (1 << 1), /* If last use of domain,
> +                                                         then also remove any
> +                                                         snapshot metadata */
> +    VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL     = (1 << 2), /* If last use of domain,
> +                                                         then also remove any
> +                                                         snapshot data */
> +} virDomainDestroyFlagsValues;
> +
>  virDomainPtr            virDomainCreateXML      (virConnectPtr conn,
>                                                   const char *xmlDesc,
>                                                   unsigned int flags);
> @@ -1240,7 +1250,14 @@ virDomainPtr            virDomainDefineXML      (virConnectPtr conn,
>  int                     virDomainUndefine       (virDomainPtr domain);
> 
>  typedef enum {
> -    VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = (1 << 0),
> +    VIR_DOMAIN_UNDEFINE_MANAGED_SAVE       = (1 << 0), /* Also remove any
> +                                                          managed save */
> +    VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA = (1 << 1), /* If last use of domain,
> +                                                          then also remove any
> +                                                          snapshot metadata */
> +    VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL     = (1 << 2), /* If last use of domain,
> +                                                          then also remove any
> +                                                          snapshot data */
> 
>      /* Future undefine control flags should come here. */
>  } virDomainUndefineFlagsValues;

This feels a little bit wrong to me, for the same reasons we
discussed wrt deleteing disks on undefine, particularly once
we start storing snapshots in external files across arbitrary
storage, instead of just inside qcow2 images

  https://www.redhat.com/archives/libvir-list/2011-July/msg01548.html

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|