snapshot: add atomic create flag

Right now, it is appallingly easy to cause qemu disk snapshots
to alter a domain then fail; for example, by requesting a two-disk
snapshot where the second disk name resides on read-only storage.
In this failure scenario, libvirt reports failure, but modifies
the live domain XML in-place to record that the first disk snapshot
was taken; and places a difficult burden on the management app
to grab the XML and reparse it to see which disks, if any, were
altered by the partial snapshot.

This patch adds a new flag where implementations can request that
the hypervisor make snapshots atomically; either no changes to
XML occur, or all disks were altered as a group.  If you request
the flag, you either get outright failure up front, or you take
advantage of hypervisor abilities to make an atomic snapshot. Of
course, drivers should prefer the atomic means even without the
flag explicitly requested.

There's no way to make snapshots 100% bulletproof - even if the
hypervisor does it perfectly atomic, we could run out of memory
during the followup tasks of updating our in-memory XML, and report
a failure.  However, these sorts of catastrophic failures are rare
and unlikely, and it is still nicer to know that either all
snapshots happened or none of them, as that is an easier state to
recover from.

* include/libvirt/libvirt.h.in
(VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC): New flag.
* src/libvirt.c (virDomainSnapshotCreateXML): Document it.
* tools/virsh.c (cmdSnapshotCreate, cmdSnapshotCreateAs): Expose it.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
it.

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 8bb5c5754c480ee2756aae3643362f91250092b1..499dcd4514bf793b531e53496c56237fb055e1ba 100644 (file)
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3301,6 +3301,8 @@ typedef enum {
                                                           quiesce all mounted
                                                           file systems within
                                                           the domain */
+    VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC      = (1 << 7), /* atomically avoid
+                                                          partial changes */
 } virDomainSnapshotCreateFlags;
 
 /* Take a snapshot of the current VM state */

diff --git a/src/libvirt.c b/src/libvirt.c
index 586729d81ffdf5dc64d9d463efcaf97c46571b7d..ae8c9fddecab28cb6e096e0f86f908518495f26d 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17107,14 +17107,21 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot)
  * @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.
+ * no other effect, some hypervisors have certain types of failures where
+ * the overall command can easily fail even though the guest configuration
+ * was partially altered (for example, if a disk snapshot request for two
+ * disks fails on the second disk, but the first disk alteration cannot be
+ * rolled back).  If this API call fails, it is therefore normally
+ * necessary to follow up with virDomainGetXMLDesc() and check each disk
+ * to determine if any partial changes occurred.  However, if @flags
+ * contains VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC, then libvirt guarantees
+ * that this command will not alter any disks unless the entire set of
+ * changes can be done atomically, making failure recovery simpler (note
+ * that it is still possible to fail after disks have changed, but only
+ * in the much rarer cases of running out of memory or disk space).
+ *
+ * Returns an (opaque) virDomainSnapshotPtr on success, NULL on failure.
  */
 virDomainSnapshotPtr
 virDomainSnapshotCreateXML(virDomainPtr domain,

diff --git a/tools/virsh.c b/tools/virsh.c
index 96bea39028c3a2e8ede2d40e3891104dc36dfd5e..ee6db4c2ef42ec14eebc65764cdd5690edc32b8f 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -15715,6 +15715,7 @@ static const vshCmdOptDef opts_snapshot_create[] = {
     {"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")},
     {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")},
+    {"atomic", VSH_OT_BOOL, 0, N_("require atomic operation")},
     {NULL, 0, 0, NULL}
 };
 
@@ -15741,6 +15742,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT;
     if (vshCommandOptBool(cmd, "quiesce"))
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE;
+    if (vshCommandOptBool(cmd, "atomic"))
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         goto cleanup;
@@ -15851,6 +15854,7 @@ static const vshCmdOptDef opts_snapshot_create_as[] = {
     {"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")},
     {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")},
+    {"atomic", VSH_OT_BOOL, 0, N_("require atomic operation")},
     {"diskspec", VSH_OT_ARGV, 0,
      N_("disk attributes: disk[,snapshot=type][,driver=type][,file=name]")},
     {NULL, 0, 0, NULL}
@@ -15878,6 +15882,8 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT;
     if (vshCommandOptBool(cmd, "quiesce"))
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE;
+    if (vshCommandOptBool(cmd, "atomic"))
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
 
     if (!vshConnectionUsability(ctl, ctl->conn))
         goto cleanup;

diff --git a/tools/virsh.pod b/tools/virsh.pod
index f0c58706df92602ce8d366f902f64d7d730d9a52..c7d5bbd35eb965349649c60f31fe516dfd5f82da 100644 (file)
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2286,7 +2286,7 @@ used to represent properties of snapshots.
 
 =item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]]
 | [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external>]
-[I<--quiesce>]}
+[I<--quiesce>] [I<--atomic>]}
 
 Create a snapshot for domain I<domain> with the properties specified in
 I<xmlfile>.  Normally, the only properties settable for a domain snapshot
@@ -2333,6 +2333,12 @@ to freeze and unfreeze domain's mounted file systems. However,
 if domain has no guest agent, snapshot creation will fail.
 Currently, this requires I<--disk-only> to be passed as well.
 
+If I<--atomic> is specified, libvirt will guarantee that the snapshot
+either succeeds, or fails with no changes; not all hypervisors support
+this.  If this flag is not specified, then some hypervisors may fail
+after partially performing the action, and B<dumpxml> must be used to
+see whether any partial changes occurred.
+
 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
@@ -2340,7 +2346,7 @@ 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<--reuse-existing>]} [I<name>]
-[I<description>] [I<--disk-only> [I<--quiesce>]
+[I<description>] [I<--disk-only> [I<--quiesce>] [I<--atomic>]
 [[I<--diskspec>] B<diskspec>]...]
 
 Create a snapshot for domain I<domain> with the given <name> and
@@ -2380,6 +2386,12 @@ treat the snapshot as current, and cannot revert to the snapshot
 unless B<snapshot-create> is later used to teach libvirt about the
 metadata again).  This flag is incompatible with I<--print-xml>.
 
+If I<--atomic> is specified, libvirt will guarantee that the snapshot
+either succeeds, or fails with no changes; not all hypervisors support
+this.  If this flag is not specified, then some hypervisors may fail
+after partially performing the action, and B<dumpxml> must be used to
+see whether any partial changes occurred.
+
 =item B<snapshot-current> I<domain> {[I<--name>] | [I<--security-info>]
 | [I<snapshotname>]}