backup: Prevent snapshots and checkpoints at same time

Earlier patches mentioned that the initial implementation will prevent
snapshots and checkpoints from being used on the same domain at once.
However, the actual restriction is done in this separate patch to make
it easier to lift that restriction via a revert, when we are finally
ready to tackle that integration in the future.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/docs/formatcheckpoint.html.in b/docs/formatcheckpoint.html.in
index 1dcbf7fd3cff97171c828c1920f31d33358f4908..044bbfe4b01a25faf3f769eb506bebc2592ba48c 100644 (file)
--- a/docs/formatcheckpoint.html.in
+++ b/docs/formatcheckpoint.html.in
@@ -36,7 +36,9 @@
       now, libvirt exposes enough support to create disk checkpoints
       independently from a backup operation
       via <code>virDomainCheckpointCreateXML()</code> <span class="since">since
-      5.6.0</span>.
+      5.6.0</span>.  Likewise, the creation of checkpoints when
+      external snapshots exist is currently forbidden, although future
+      work will make it possible to integrate these two concepts.
     </p>
     <p>
       Attributes of libvirt checkpoints are stored as child elements

diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
index 508176517893076f9e88bf318f332d24936d35c7..d640deb86d82c8a90a345e3e1687e4a90a9d6aa9 100644 (file)
--- a/docs/formatsnapshot.html.in
+++ b/docs/formatsnapshot.html.in
@@ -93,7 +93,9 @@
       sets that snapshot as current, and the prior current snapshot is
       the parent of the new snapshot.  Branches in the hierarchy can
       be formed by reverting to a snapshot with a child, then creating
-      another snapshot.
+      another snapshot.  For now, the creation of external snapshots
+      when checkpoints exist is forbidden, although future work will
+      make it possible to integrate these two concepts.
     </p>
     <p>
       The top-level <code>domainsnapshot</code> element may contain

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 691e27fd9fe5036e2e45f08d03e7bec21740f744..4ca3eb7bdea8c0ef43b8294428dc5112ec089bc5 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15777,6 +15777,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
     if (!(vm = qemuDomObjFromDomain(domain)))
         goto cleanup;
 
+    if (virDomainListCheckpoints(vm->checkpoints, NULL, domain, NULL, 0) > 0) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot create snapshot while checkpoint exists"));
+        goto cleanup;
+    }
+
     cfg = virQEMUDriverGetConfig(driver);
 
     if (virDomainSnapshotCreateXMLEnsureACL(domain->conn, vm->def, flags) < 0)
@@ -18515,6 +18521,12 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
     if (virDomainBlockRebaseEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
+    if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, 0) > 0) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot perform block rebase while checkpoint exists"));
+        goto cleanup;
+    }
+
     /* For normal rebase (enhanced blockpull), the common code handles
      * everything, including vm cleanup. */
     if (!(flags & VIR_DOMAIN_BLOCK_REBASE_COPY))
@@ -18599,6 +18611,12 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml,
     if (virDomainBlockCopyEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
+    if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, 0) > 0) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot perform block copy while checkpoint exists"));
+        goto cleanup;
+    }
+
     for (i = 0; i < nparams; i++) {
         virTypedParameterPtr param = &params[i];
 
@@ -18661,6 +18679,13 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth,
         return -1;
     }
 
+    if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, 0) > 0) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot perform block pull while checkpoint exists"));
+        virDomainObjEndAPI(&vm);
+        return -1;
+    }
+
     return qemuDomainBlockPullCommon(dom->conn->privateData,
                                      vm, path, NULL, bandwidth, flags);
 }
@@ -18711,6 +18736,12 @@ qemuDomainBlockCommit(virDomainPtr dom,
     if (virDomainBlockCommitEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
+    if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, 0) > 0) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot perform block commit while checkpoint exists"));
+        goto cleanup;
+    }
+
     if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
         goto cleanup;
 
@@ -22431,6 +22462,12 @@ static int qemuDomainRename(virDomainPtr dom,
         goto endjob;
     }
 
+    if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, flags) > 0) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("cannot rename domain with checkpoints"));
+        goto endjob;
+    }
+
     if (virDomainObjListRename(driver->domains, vm, new_name, flags,
                                qemuDomainRenameCallback, driver) < 0)
         goto endjob;

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ab0f8b06d61d36a080f60db702c9c4e92f4850f1..e439b35d2fd457eed740a33eb896ad3d325a7181 100755 (executable)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -7698,6 +7698,12 @@ testDomainSnapshotCreateXML(virDomainPtr domain,
     if (!(vm = testDomObjFromDomain(domain)))
         goto cleanup;
 
+    if (virDomainListCheckpoints(vm->checkpoints, NULL, domain, NULL, 0) > 0) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot create snapshot while checkpoint exists"));
+        goto cleanup;
+    }
+
     if (!vm->persistent && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("cannot halt after transient domain snapshot"));
@@ -8158,6 +8164,12 @@ testDomainCheckpointCreateXML(virDomainPtr domain,
     if (!(vm = testDomObjFromDomain(domain)))
         goto cleanup;
 
+    if (virDomainSnapshotObjListNum(vm->snapshots, NULL, 0) > 0) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("cannot create checkpoint while snapshot exists"));
+        goto cleanup;
+    }
+
     if (!virDomainObjIsActive(vm)) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("cannot create checkpoint for inactive domain"));

diff --git a/tests/virsh-checkpoint b/tests/virsh-checkpoint
index 4fe111f5b76daa059bd80225f79b9f5bbae85197..75bdc293be97c76fe0ce558efb82e3a928c2eccc 100755 (executable)
--- a/tests/virsh-checkpoint
+++ b/tests/virsh-checkpoint
@@ -38,6 +38,10 @@ $abs_top_builddir/tools/virsh --connect test:///default >out 2>err '
   checkpoint-create-as test c1
   checkpoint-create-as test c3
   checkpoint-create-as test c2
+  # snapshots cannot be created while checkpoints exist
+  echo --err marker
+  snapshot-create-as test s1
+  echo --err marker
   # Checking tree view (siblings sorted alphabetically)
   checkpoint-list test --tree
   # Demonstrate list filtering
@@ -77,6 +81,9 @@ Domain checkpoint c1 created
 
 Domain checkpoint c3 created
 Domain checkpoint c2 created
+
+
+
 c1
   |
   +- c3
@@ -126,6 +133,9 @@ compare exp out.cooked || fail=1
 
 cat <<EOF > exp || fail=1
 error: operation failed: domain moment c1 already exists
+error: marker
+error: Operation not supported: cannot create snapshot while checkpoint exists
+error: marker
 EOF
 compare exp err || fail=1
 

diff --git a/tests/virsh-snapshot b/tests/virsh-snapshot
index 1034140cde1a1aba955969e73031666e2de1fc61..20ff966a51c589858c9f4069229825d8581457a4 100755 (executable)
--- a/tests/virsh-snapshot
+++ b/tests/virsh-snapshot
@@ -45,6 +45,10 @@ $abs_top_builddir/tools/virsh --connect test:///default >out 2>err '
   snapshot-revert test s1
   snapshot-create-as test s7
   snapshot-create-as test s8
+  # checkpoints cannot be created while snapshots exist
+  echo --err marker
+  checkpoint-create-as test c1
+  echo --err marker
   # Checking tree view (siblings sorted alphabetically)
   snapshot-list test --tree
   # Current was last one created, but we can change that
@@ -102,6 +106,9 @@ Domain snapshot s4 created
 
 Domain snapshot s7 created
 Domain snapshot s8 created
+
+
+
 s1
   |
   +- s3
@@ -175,6 +182,9 @@ compare exp out.cooked || fail=1
 cat <<EOF > exp || fail=1
 error: operation failed: domain moment s1 already exists
 error: marker
+error: Operation not supported: cannot create checkpoint while snapshot exists
+error: marker
+error: marker
 error: domain 'test' has no current snapshot
 error: marker
 EOF

diff --git a/tools/virsh.pod b/tools/virsh.pod
index f58578e3b376e6b06b79da1e14b5c13dbab0a64c..af739bec24d73ec6a560ac922722302ce39bd922 100644 (file)
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -4669,6 +4669,10 @@ 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).
 
+For now, it is not possible to create snapshots in a domain that has
+checkpoints, although this restriction will be lifted in a future
+release.
+
 =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>]
@@ -4735,6 +4739,10 @@ 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
 snapshot. This is currently supported only for external full system snapshots.
 
+For now, it is not possible to create snapshots in a domain that has
+checkpoints, although this restriction will be lifted in a future
+release.
+
 =item B<snapshot-current> I<domain> {[I<--name>] | [I<--security-info>]
 | [I<snapshotname>]}
 
@@ -4943,6 +4951,10 @@ a persistent domain.  However, for transient domains, checkpoint
 metadata is silently lost when the domain quits running (whether
 by command such as B<destroy> or by internal guest action).
 
+For now, it is not possible to create checkpoints in a domain that has
+snapshots, although this restriction will be lifted in a future
+release.
+
 =item B<checkpoint-create-as> I<domain> [I<--print-xml>]
 [I<name>] [I<description>] [I<--quiesce>] [I<--diskspec>] B<diskspec>]...
 
@@ -4966,6 +4978,10 @@ If I<--quiesce> is specified, libvirt will try to use guest agent
 to freeze and unfreeze domain's mounted file systems. However,
 if domain has no guest agent, checkpoint creation will fail.
 
+For now, it is not possible to create checkpoints in a domain that has
+snapshots, although this restriction will be lifted in a future
+release.
+
 =item B<checkpoint-edit> I<domain> I<checkpointname>
 
 Edit the XML configuration file for I<checkpointname> of a domain.