[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [RFC PATCHv2 1/n] snapshot: add revert-and-create branching of external snapshots



Right now, libvirt refuses to revert to the state at the time
of an external snapshot, because doing so would reset things so
that the next time the domain boots, we are using the backing
file; but modifying the backing file invalidates all qcow2
files that are based on top of it.  There are three possibilities
for lifting this restriction:
1. delete all snapshot metadata and qcow2 files that are
invalidated by the revert (losing changes since the snapshot)
2. perform a block commit (such as with qemu-img commit) to
merge the qcow2 file back into the backing file (keeping the
changes since the snapshot, but losing the qcow2 files)
3. create NEW qcow2 files that wrap the same base file as
the original snapshot (keeping the changes since the original
snapshot)

This patch documents the API for option 3, by adding a new flag
to virDomainSnapshotCreateXML (the only snapshot-related function
that takes XML, which is required to pass in new file names
during the branch), and wires up virsh to expose it.  Later
patches will enhance virDomainRevertToSnapshot to cover options
1 and 2.

API wise, an application wishing to do the revert-and-create
operation must add a <branch>name</branch> element to their
<domainsnapshot> XML, and pass VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH
in the flags argument.  In virsh, snapshot-create gains a new
boolean flag --branch that must match the XML passed in, and
snapshot-create-as gains a new --branch=name argument along
with a --current boolean for creating such XML.

* include/libvirt/libvirt.h.in
(VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH): New flag.
* src/libvirt.c (virDomainSnapshotCreateXML): Document it, and
enforce some mutual exclusion.
* docs/formatsnapshot.html.in: Document the new <branch> element.
* docs/schemas/domainsnapshot.rng: Allow new element.
* tools/virsh-snapshot.c (cmdSnapshotCreate): Add --branch option.
(cmdSnapshotCreateAs): Likewise, also add --current.
* tools/virsh.pod (snapshot-create, snapshot-create-as): Document
new usage.
---
 docs/formatsnapshot.html.in     | 16 +++++++++++++---
 docs/schemas/domainsnapshot.rng |  5 +++++
 include/libvirt/libvirt.h.in    |  3 +++
 src/libvirt.c                   | 27 ++++++++++++++++++++++-----
 tools/virsh-snapshot.c          | 40 ++++++++++++++++++++++++++++++++--------
 tools/virsh.pod                 | 16 ++++++++++++++--
 6 files changed, 89 insertions(+), 18 deletions(-)

diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
index 8fcc04c..b92295f 100644
--- a/docs/formatsnapshot.html.in
+++ b/docs/formatsnapshot.html.in
@@ -188,9 +188,19 @@
       </dd>
       <dt><code>parent</code></dt>
       <dd>The parent of this snapshot.  If present, this element
-        contains exactly one child element, name.  This specifies the
-        name of the parent snapshot of this snapshot, and is used to
-        represent trees of snapshots.  Readonly.
+        contains exactly one child element, <code>name</code>.  This
+        specifies the name of the parent snapshot of this snapshot,
+        and is used to represent trees of snapshots.  Readonly.
+      </dd>
+      <dt><code>branch</code></dt>
+      <dd>The name of an existing external snapshot that forms the
+        branch point for this snapshot.  Required when creating a
+        snapshot with
+        the <code>VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH</code> flag, and
+        not present on output.  When creating a branch snapshot, the
+        same set of <code>disks</code> must be external,
+        and <code>memory</code> is copied from the branch
+        point.  <span class="since">since 1.0.1</span>.
       </dd>
       <dt><code>domain</code></dt>
       <dd>The domain that this snapshot was taken against.  Older
diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng
index 45d55b5..7ec01e1 100644
--- a/docs/schemas/domainsnapshot.rng
+++ b/docs/schemas/domainsnapshot.rng
@@ -85,6 +85,11 @@
             </element>
           </element>
         </optional>
+        <optional>
+          <element name='branch'>
+            <text/>
+          </element>
+        </optional>
       </interleave>
     </element>
   </define>
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 84dcde1..be16629 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3777,6 +3777,9 @@ typedef enum {
     VIR_DOMAIN_SNAPSHOT_CREATE_LIVE        = (1 << 8), /* create the snapshot
                                                           while the guest is
                                                           running */
+    VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH      = (1 << 9), /* specify an existing
+                                                          external snapshot as
+                                                          the branching point */
 } virDomainSnapshotCreateFlags;

 /* Take a snapshot of the current VM state */
diff --git a/src/libvirt.c b/src/libvirt.c
index 4af6089..3229133 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -17746,9 +17746,7 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot)
  * normally fails if snapshot metadata still remains on the source
  * machine).  When redefining snapshot metadata, the current snapshot
  * will not be altered unless the VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT
- * flag is also present.  It is an error to request the
- * VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flag without
- * VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE.  On some hypervisors,
+ * flag is also present.  On some hypervisors,
  * redefining an existing snapshot can be used to alter host-specific
  * portions of the domain XML to be used during revert (such as
  * backing filenames associated with disk devices), but must not alter
@@ -17787,6 +17785,15 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot)
  * is not present, an error is thrown. Moreover, this flag requires
  * VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY to be passed as well.
  *
+ * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH, then @xmlDesc
+ * must describe an existing external snapshot as well as actions to
+ * create a new snapshot from the point where the existing snapshot was
+ * created rather than the current state of the guest.  In this mode,
+ * the new snapshot branch is created but not activated unless the
+ * VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flag is also present.  It is an
+ * error to request VIR_DOMAIN_SNAPSHOT_CREATE_LIVE or
+ * VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE in this mode.
+ *
  * By default, if the snapshot involves external files, and any of the
  * destination files already exist as a non-empty regular file, the
  * snapshot is rejected to avoid losing contents of those files.
@@ -17843,9 +17850,11 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
     }

     if ((flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT) &&
-        !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) {
+        !(flags & (VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
+                   VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH))) {
         virReportInvalidArg(flags,
-                            _("use of 'current' flag in %s requires 'redefine' flag"),
+                            _("use of 'current' flag in %s requires 'redefine' "
+                              "or 'branch' flag"),
                             __FUNCTION__);
         goto error;
     }
@@ -17863,6 +17872,14 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
                             __FUNCTION__);
         goto error;
     }
+    if (!!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) +
+        !!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_LIVE) +
+        !!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH) > 1) {
+        virReportInvalidArg(flags,
+                            _("'redefine', 'live', and 'branch' flags in %s are mutually exclusive"),
+                            __FUNCTION__);
+        goto error;
+    }

     if (conn->driver->domainSnapshotCreateXML) {
         virDomainSnapshotPtr ret;
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 398730c..6232ec2 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -121,7 +121,8 @@ static const vshCmdOptDef opts_snapshot_create[] = {
     {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
     {"xmlfile", VSH_OT_DATA, 0, N_("domain snapshot XML")},
     {"redefine", VSH_OT_BOOL, 0, N_("redefine metadata for existing snapshot")},
-    {"current", VSH_OT_BOOL, 0, N_("with redefine, set current snapshot")},
+    {"current", VSH_OT_BOOL, 0,
+     N_("with redefine or branch, set current snapshot")},
     {"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")},
@@ -129,6 +130,7 @@ static const vshCmdOptDef opts_snapshot_create[] = {
     {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")},
     {"atomic", VSH_OT_BOOL, 0, N_("require atomic operation")},
     {"live", VSH_OT_BOOL, 0, N_("take a live snapshot")},
+    {"branch", VSH_OT_BOOL, 0, N_("create a branch snapshot")},
     {NULL, 0, 0, NULL}
 };

@@ -159,6 +161,8 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
     if (vshCommandOptBool(cmd, "live"))
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE;
+    if (vshCommandOptBool(cmd, "branch"))
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH;

     dom = vshCommandOptDomain(ctl, cmd, NULL);
     if (dom == NULL)
@@ -307,6 +311,10 @@ static const vshCmdOptDef opts_snapshot_create_as[] = {
     {"quiesce", VSH_OT_BOOL, 0, N_("quiesce guest's file systems")},
     {"atomic", VSH_OT_BOOL, 0, N_("require atomic operation")},
     {"live", VSH_OT_BOOL, 0, N_("take a live snapshot")},
+    {"branch", VSH_OT_DATA, VSH_OFLAG_REQ_OPT,
+     N_("name of external snapshot to use as a branch point")},
+    {"current", VSH_OT_BOOL, 0,
+     N_("with branch, make the new snapshot current")},
     {"memspec", VSH_OT_DATA, VSH_OFLAG_REQ_OPT,
      N_("memory attributes: [file=]name[,snapshot=type]")},
     {"diskspec", VSH_OT_ARGV, 0,
@@ -323,6 +331,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
     const char *name = NULL;
     const char *desc = NULL;
     const char *memspec = NULL;
+    const char *branch = NULL;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     unsigned int flags = 0;
     const vshCmdOpt *opt = NULL;
@@ -341,22 +350,37 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
     if (vshCommandOptBool(cmd, "live"))
         flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE;
+    if (vshCommandOptBool(cmd, "branch")) {
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_BRANCH;
+        if (vshCommandOptString(cmd, "branch", &branch) < 0) {
+            vshError(ctl, _("branch argument must not be empty"));
+            goto cleanup;
+        }
+        if (vshCommandOptBool(cmd, "memspec")) {
+            vshError(ctl, _("--branch and --memspec are mutually exclusive"));
+            goto cleanup;
+        }
+    }
+    if (vshCommandOptBool(cmd, "current"))
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT;

     dom = vshCommandOptDomain(ctl, cmd, NULL);
     if (dom == NULL)
         goto cleanup;

-    if (vshCommandOptString(cmd, "name", &name) < 0 ||
-        vshCommandOptString(cmd, "description", &desc) < 0) {
-        vshError(ctl, _("argument must not be empty"));
+    if (vshCommandOptString(cmd, "name", &name) < 0) {
+        vshError(ctl, _("name argument must not be empty"));
+        goto cleanup;
+    }
+    if (vshCommandOptString(cmd, "description", &desc) < 0) {
+        vshError(ctl, _("description argument must not be empty"));
         goto cleanup;
     }

     virBufferAddLit(&buf, "<domainsnapshot>\n");
-    if (name)
-        virBufferEscapeString(&buf, "  <name>%s</name>\n", name);
-    if (desc)
-        virBufferEscapeString(&buf, "  <description>%s</description>\n", desc);
+    virBufferEscapeString(&buf, "  <name>%s</name>\n", name);
+    virBufferEscapeString(&buf, "  <description>%s</description>\n", desc);
+    virBufferEscapeString(&buf, "  <branch>%s</branch>\n", branch);

     if (vshCommandOptString(cmd, "memspec", &memspec) < 0 ||
         vshParseSnapshotMemspec(ctl, &buf, memspec) < 0) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index c4d505f..38c691e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2594,7 +2594,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<--atomic>] [I<--live>]}
+[I<--quiesce>] [I<--atomic>] [I<--live>] [I<--branch>]}

 Create a snapshot for domain I<domain> with the properties specified in
 I<xmlfile>.  Normally, the only properties settable for a domain snapshot
@@ -2651,6 +2651,12 @@ If I<--live> is specified, libvirt takes the snapshot while the guest is
 running. This increases the size of the memory image of the external
 checkpoint. This is currently supported only for external checkpoints.

+If I<--branch> is specified, then I<xmlfile> must contain a B<branch>
+element naming an existing external snapshot which the new snapshot
+will branch from.  In this usage, the new snapshot must also be
+external, and will not be made current unless I<--current> is also
+requested.
+
 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
@@ -2659,7 +2665,8 @@ 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-external>]} [I<name>]
 [I<description>] [I<--disk-only> [I<--quiesce>]] [I<--atomic>]
-[[I<--live>] [I<--memspec> B<memspec>]] [I<--diskspec>] B<diskspec>]...
+[[I<--live>] [I<--memspec> B<memspec>]] [I<--branch> B<branch> [I<--current>]]
+[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
@@ -2675,6 +2682,11 @@ by a B<memspec> of the form B<[file=]name[,snapshot=type]>, where
 type can be B<none>, B<internal>, or B<external>.  To include a literal
 comma in B<file=name>, escape it with a second comma.

+The I<--branch> option can be used to create an external snapshot that
+branches from an existing snapshot named B<branch>.  In this usage, the
+new snapshot is not made current unless I<--current> is also present.
+Specifying a branch point is incompatible with using I<--memspec>.
+
 The I<--diskspec> option can be used to control how I<--disk-only> and
 external checkpoints create external files.  This option can occur
 multiple times, according to the number of <disk> elements in the domain
-- 
1.7.11.7


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]