On Tue, Feb 22, 2022 at 05:55:38PM +0100, Peter Krempa wrote:
In case when a user starts a block copy operation with
VIR_DOMAIN_BLOCK_COPY_SHALLOW and VIR_DOMAIN_BLOCK_COPY_REUSE_EXT and
both the reused image and the original disk have a backing image libvirt
specifically does not insert the backing image until after the job is
asked to be completed via virBlockJobAbort with
VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT.
Thanks for the quick patch!
First, to refresh my memory (and for others reading the thread), these
are the two main uses of combining '--reuse-external' and '--shallow'
flags with "blockcopy":
(a) You can control what the backing file is, as long as the top of that
backing file chain has identical guest-visible contents to what the
original chain had in its backing file.
(b) The new backing file can have a _different_ backing chain depth or
even different format.
* * *
Now to construct a reproducer with `virsh`. Peter, tell me what I got
wrong :-)
(1) Let's start with this image chain (overlays are always of qcow2
format):
base.raw <-- overlay1 <-- overlay2 (live QEMU).
With the goal of copying the above into the below chain (note, here
we're flattening both base.raw and overlay1 into a single file,
"flat-o-b1")
flat-o-b1.raw <-- copy (live QEMU)
(2) Make a *raw* variant of "base <-- overlay1", call it
"flat-b-o1.raw"
(i.e. flattened version of combined base and overlay1):
$ qemu-img convert -f qcow2 -O raw overlay1.qcow2 \
flat-b-o1.raw
(3) Then, create an empty QCOW2 file to create "flat-b-o1 <-- copy (empty)":
$ qemu-img create -f qcow2 \
-o backing_file=flat-b-o1.raw,backing_fmt=raw copy.qcow2
(4) *Then* perform the `blockcopy --reuse-external --shallow`:
$ virsh blockcopy \
--domain vm1 vda ./copy.qcow2 \
--wait --verbose --reuse-external --shallow \
--finish
(5) And then pivot the job, so that live QEMU now points to copy
$ virsh blockjob --pivot
And the final result, we get the "goal" chain in step (1).
src: base <-- overlay1 <-- overlay2
== ==
dst: flat-o-b1 <-- copy (live QEMU)
Am I missing anything else?
This is so that management applications can copy the backing image
on
the background.
Now when a user aborts the block job instead of cancelling it we'd
ignore the fact that we didn't insert the backing image yet and the
cancellation would result into a 'blockdev-del' of a invalid node name
and thus an 'error' severity entry in the log.
To solve this issue we use the same conditions when the backing image
addition is avoided to remove the internal state for them prior to the
call to unplug the mirror destination.
Reported-by: Kashyap Chamarthy <kchamart(a)redhat.com>
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 726df95067..2442865b9b 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1406,6 +1406,8 @@ qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriver *driver,
qemuBlockJobData *job,
qemuDomainAsyncJob asyncJob)
{
+ qemuDomainObjPrivate *priv = vm->privateData;
+
VIR_DEBUG("copy job '%s' on VM '%s' aborted",
job->name, vm->def->name);
/* mirror may be NULL for copy job corresponding to migration */
@@ -1413,6 +1415,21 @@ qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriver *driver,
!job->disk->mirror)
return;
+ if (!job->jobflagsmissing) {
+ bool shallow = job->jobflags & VIR_DOMAIN_BLOCK_COPY_SHALLOW;
+ bool reuse = job->jobflags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT;
+
+ /* In the special case of a shallow copy with reused image we don't
+ * hotplug the full chain when QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY
+ * is supported. Attempting to delete it would thus result in spurious
+ * errors as we'd attempt to blockdev-del images which were not added
+ * yet */
+ if (reuse && shallow &&
+ virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY) &&
+ virStorageSourceHasBacking(job->disk->mirror))
+ g_clear_pointer(&job->disk->mirror->backingStore,
virObjectUnref);
+ }
+
/* activeWrite bitmap is removed automatically here */
qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob,
job->disk->mirror);
g_clear_pointer(&job->disk->mirror, virObjectUnref);
--
2.35.1
--
/kashyap