snapshot: allow reuse of existing files in disk snapshot

When disk snapshots were first implemented, libvirt blindly refused
to allow an external snapshot destination that already exists, since
qemu will blindly overwrite the contents of that file during the
snapshot_blkdev monitor command, and we don't like a default of
data loss by default.  But VDSM has a scenario where NFS permissions
are intentionally set so that the destination file can only be
created by the management machine, and not the machine where the
guest is running, so that libvirt will necessarily see the destination
file already existing; adding a flag will allow VDSM to force the file
reuse without libvirt complaining of possible data loss.

https://bugzilla.redhat.com/show_bug.cgi?id=767104

* include/libvirt/libvirt.h.in (virDomainSnapshotCreateFlags): Add
VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT.
* src/libvirt.c (virDomainSnapshotCreateXML): Document it.  Add
note about partial failure.
* tools/virsh.c (cmdSnapshotCreate, cmdSnapshotCreateAs): Add new
flag.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
it.
* src/qemu/qemu_driver.c (qemuDomainSnapshotDiskPrepare)
(qemuDomainSnapshotCreateXML): Implement the new flag.

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index ad6fcced6e2d33461ed81d5dacc1131d9326bfb4..0b564cf8f9db7ee969eb9022647d27d818311141 100644 (file)
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2990,6 +2990,8 @@ typedef enum {
                                                           after snapshot */
     VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY   = (1 << 4), /* disk snapshot, not
                                                           system checkpoint */
+    VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT   = (1 << 5), /* reuse any existing
+                                                          external files */
 } virDomainSnapshotCreateFlags;
 
 /* Take a snapshot of the current VM state */

diff --git a/src/libvirt.c b/src/libvirt.c
index 96eb6412b5d396afea2ea937f1bbf5a441042ad8..dbf02963caa3fe9f46c47ad4c37bde7e16fbd02d 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -16517,7 +16517,20 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot)
  * inconsistent (as if power had been pulled), and specifying this
  * with the VIR_DOMAIN_SNAPSHOT_CREATE_HALT flag risks data loss.
  *
+ * By default, if the snapshot involves external files, and any of the
+ * destination files already exist as a regular file, the snapshot is
+ * rejected to avoid losing contents of those files.  However, if
+ * @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, then existing
+ * destination files are instead truncated and reused.
+ *
  * Returns an (opaque) virDomainSnapshotPtr on success, NULL on failure.
+ * Be aware that although libvirt prefers to report errors up front with
+ * no other effect, there are certain types of failures where a failure
+ * can occur even though the guest configuration was changed (for
+ * example, if a disk snapshot request over two disks only fails on the
+ * second disk, leaving the first disk altered); so after getting a NULL
+ * return, it can be wise to use virDomainGetXMLDesc() to determine if
+ * any partial changes occurred.
  */
 virDomainSnapshotPtr
 virDomainSnapshotCreateXML(virDomainPtr domain,

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 383b2be3ff9497a7214724efff6e100de89bbb42..c535ebf86f2258539d0ac37a0ebd6cbac3bb7f79 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9693,7 +9693,8 @@ endjob:
 }
 
 static int
-qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def)
+qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
+                              bool allow_reuse)
 {
     int ret = -1;
     int i;
@@ -9746,7 +9747,7 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def)
                                          disk->name, disk->file);
                     goto cleanup;
                 }
-            } else if (!S_ISBLK(st.st_mode)) {
+            } else if (!(S_ISBLK(st.st_mode) || allow_reuse)) {
                 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                 _("external snapshot file for disk %s already "
                                   "exists and is not a block device: %s"),
@@ -10026,7 +10027,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                   VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT |
                   VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA |
                   VIR_DOMAIN_SNAPSHOT_CREATE_HALT |
-                  VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY, NULL);
+                  VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY |
+                  VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT, NULL);
 
     if (((flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) &&
          !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) ||
@@ -10162,11 +10164,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
             goto cleanup;
 
         if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
+            bool allow_reuse;
+
+            allow_reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0;
             if (virDomainSnapshotAlignDisks(def,
                                             VIR_DOMAIN_DISK_SNAPSHOT_EXTERNAL,
                                             false) < 0)
                 goto cleanup;
-            if (qemuDomainSnapshotDiskPrepare(vm, def) < 0)
+            if (qemuDomainSnapshotDiskPrepare(vm, def, allow_reuse) < 0)
                 goto cleanup;
             def->state = VIR_DOMAIN_DISK_SNAPSHOT;
         } else {

diff --git a/tools/virsh.c b/tools/virsh.c
index e4b812e92a73f098fd6f4db71917fc9c6b1972b9..04c1f6eaac2f50f3ccd30c0e0755d1974b8226f6 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1,7 +1,7 @@
 /*
  * virsh.c: a shell to exercise the libvirt API
  *
- * Copyright (C) 2005, 2007-2011 Red Hat, Inc.
+ * Copyright (C) 2005, 2007-2012 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -14462,6 +14462,7 @@ static const vshCmdOptDef opts_snapshot_create[] = {
     {"no-metadata", VSH_OT_BOOL, 0, N_("take snapshot but create no metadata")},
     {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")},
     {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")},
+    {"reuse-external", VSH_OT_BOOL, 0, N_("reuse any existing external files")},
     {NULL, 0, 0, NULL}
 };
 
@@ -14484,6 +14485,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_HALT;
     if (vshCommandOptBool(cmd, "disk-only"))
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
+    if (vshCommandOptBool(cmd, "reuse-external"))
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT;
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         goto cleanup;
@@ -14592,6 +14595,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = {
     {"no-metadata", VSH_OT_BOOL, 0, N_("take snapshot but create no metadata")},
     {"halt", VSH_OT_BOOL, 0, N_("halt domain after snapshot is created")},
     {"disk-only", VSH_OT_BOOL, 0, N_("capture disk state but not vm state")},
+    {"reuse-external", VSH_OT_BOOL, 0, N_("reuse any existing external files")},
     {"diskspec", VSH_OT_ARGV, 0,
      N_("disk attributes: disk[,snapshot=type][,driver=type][,file=name]")},
     {NULL, 0, 0, NULL}
@@ -14615,6 +14619,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_HALT;
     if (vshCommandOptBool(cmd, "disk-only"))
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
+    if (vshCommandOptBool(cmd, "reuse-external"))
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT;
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         goto cleanup;

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 138f88692cea8f2bfb4ec67520243575ee8761d3..1abf4489672e2c2a3a0932d4b1284a9f9018259d 100644 (file)
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2024,7 +2024,7 @@ used to represent properties of snapshots.
 =over 4
 
 =item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]]
-| [I<--no-metadata>] [I<--halt>] [I<--disk-only>]}
+| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external]}
 
 Create a snapshot for domain I<domain> with the properties specified in
 I<xmlfile>.  Normally, the only properties settable for a domain snapshot
@@ -2061,14 +2061,19 @@ treat the snapshot as current, and cannot revert to the snapshot
 unless I<--redefine> is later used to teach libvirt about the
 metadata again).
 
+If I<--reuse-external> is specified, and the snapshot XML requests an
+external snapshot with a destination of an existing file, then the
+existing file is truncated and reused; otherwise, a snapshot is refused
+to avoid losing contents of the existing files.
+
 Existence of snapshot metadata will prevent attempts to B<undefine>
 a persistent domain.  However, for transient domains, snapshot
 metadata is silently lost when the domain quits running (whether
 by command such as B<destroy> or by internal guest action).
 
 =item B<snapshot-create-as> I<domain> {[I<--print-xml>]
-| [I<--no-metadata>] [I<--halt>]} [I<name>] [I<description>]
-[I<--disk-only> [[I<--diskspec>] B<diskspec>]...
+| [I<--no-metadata>] [I<--halt>] [I<--reuse-existing>]} [I<name>]
+[I<description>] [I<--disk-only> [[I<--diskspec>] B<diskspec>]...]
 
 Create a snapshot for domain I<domain> with the given <name> and
 <description>; if either value is omitted, libvirt will choose a
@@ -2091,6 +2096,11 @@ results in the following XML:
     <source file='/path/to,new'/>
   </disk>
 
+If I<--reuse-external> is specified, and the domain XML or I<diskspec>
+option requests an external snapshot with a destination of an existing
+file, then the existing file is truncated and reused; otherwise, a
+snapshot is refused to avoid losing contents of the existing files.
+
 If I<--no-metadata> is specified, then the snapshot data is created,
 but any metadata is immediately discarded (that is, libvirt does not
 treat the snapshot as current, and cannot revert to the snapshot