From 4d5533ca87e542e22a80bbab6ffe013742b117e6 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 16 Mar 2012 15:23:00 -0600 Subject: [PATCH] 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. --- include/libvirt/libvirt.h.in | 2 ++ src/libvirt.c | 21 ++++++++++++++------- tools/virsh.c | 6 ++++++ tools/virsh.pod | 16 ++++++++++++++-- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8bb5c5754c..499dcd4514 100644 --- 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 586729d81f..ae8c9fddec 100644 --- 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 96bea39028..ee6db4c2ef 100644 --- 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 f0c58706df..c7d5bbd35e 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2286,7 +2286,7 @@ used to represent properties of snapshots. =item B I [I] {[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 with the properties specified in I. 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 must be used to +see whether any partial changes occurred. + Existence of snapshot metadata will prevent attempts to B 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 or by internal guest action). =item B I {[I<--print-xml>] | [I<--no-metadata>] [I<--halt>] [I<--reuse-existing>]} [I] -[I] [I<--disk-only> [I<--quiesce>] +[I] [I<--disk-only> [I<--quiesce>] [I<--atomic>] [[I<--diskspec>] B]...] Create a snapshot for domain I with the given and @@ -2380,6 +2386,12 @@ treat the snapshot as current, and cannot revert to the snapshot unless B 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 must be used to +see whether any partial changes occurred. + =item B I {[I<--name>] | [I<--security-info>] | [I]}