From 7efe930ec3c81feefeea8ec9b7ad9c8a55f8f53d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 24 Jul 2019 00:17:23 -0500 Subject: [PATCH] backup: Prevent snapshots and checkpoints at same time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Daniel P. Berrangé --- docs/formatcheckpoint.html.in | 4 +++- docs/formatsnapshot.html.in | 4 +++- src/qemu/qemu_driver.c | 37 +++++++++++++++++++++++++++++++++++ src/test/test_driver.c | 12 ++++++++++++ tests/virsh-checkpoint | 10 ++++++++++ tests/virsh-snapshot | 10 ++++++++++ tools/virsh.pod | 16 +++++++++++++++ 7 files changed, 91 insertions(+), 2 deletions(-) diff --git a/docs/formatcheckpoint.html.in b/docs/formatcheckpoint.html.in index 1dcbf7fd3c..044bbfe4b0 100644 --- 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 virDomainCheckpointCreateXML() since - 5.6.0. + 5.6.0. Likewise, the creation of checkpoints when + external snapshots exist is currently forbidden, although future + work will make it possible to integrate these two concepts.

Attributes of libvirt checkpoints are stored as child elements diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 5081765178..d640deb86d 100644 --- 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.

The top-level domainsnapshot element may contain diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 691e27fd9f..4ca3eb7bde 100644 --- 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 = ¶ms[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 ab0f8b06d6..e439b35d2f 100755 --- 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 4fe111f5b7..75bdc293be 100755 --- 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 < 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 1034140cde..20ff966a51 100755 --- 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 < 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 f58578e3b3..af739bec24 100644 --- 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 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 I {[I<--print-xml>] [I<--no-metadata>] [I<--halt>] [I<--reuse-external>]} [I] [I] [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 I {[I<--name>] | [I<--security-info>] | [I]} @@ -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 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 I [I<--print-xml>] [I] [I] [I<--quiesce>] [I<--diskspec>] B]... @@ -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 I I Edit the XML configuration file for I of a domain.