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@xxxxxxxxxx>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
> 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
