[PATCH] qemu: blockjob: Avoid spurious log errors when cancelling a shallow copy with reused images

[Kashyap Chamarthy]

On Wed, Feb 23, 2022 at 02:33:04PM +0100, Peter Krempa wrote:
> On Wed, Feb 23, 2022 at 14:17:40 +0100, Kashyap Chamarthy wrote:
> > On Tue, Feb 22, 2022 at 05:55:38PM +0100, Peter Krempa wrote:

[...]

> > 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
> 
> If you are working on a persistent domain/vm use --transient-job to
> avoid the need to undefine it.

Yeah, makes sense.

> Also note that '--finish' is equivalent to aborting the job after it
> reaches synchronized phase ...

Damn, I thought I _removed_ finish, but still forgot it despite making a
mental note.  Thanks for catching htat.

> > (5) And then pivot the job, so that live QEMU now points to copy
> > 
> >     $ virsh blockjob --pivot
> 
> 
> ... so this will actually say that there is no blockjob, because the
> previous step actually cancelled it.

Yeah, indeed.

> > 
> > 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?
> 
> Now the bug is that when you _cancel_ the job we attempt to blockdev-del
> some images which were not blockdev-added, and thus create some spurious
> log entries, but the rest of the code behaves correctly.

Yep, understood.

> So you'll only see the failure this is fixing in the logs.

Noted.  Thanks for the review.

-- 
/kashyap