Re: [Qemu-block] [Qemu-devel] [PATCH v2 21/40] job: Convert block_job_cancel_async() to Job

[Kashyap Chamarthy]

On Tue, May 29, 2018 at 02:30:47PM +0200, Max Reitz wrote:
> On 2018-05-29 13:59, Kashyap Chamarthy wrote:
> > On Fri, May 25, 2018 at 10:00:35AM +0200, Kevin Wolf wrote:

[...]

> >> It can, and it already has its final semantics, so nothing has to change
> >> before 3.0. job-cancel is equivalent to block-job-cancel with fixed
> >> force=true. If you want the complete-by-cancel behaviour of mirror, you
> >> have to use block-job-cancel for now, because job-cancel doesn't provide
> >> that functionality.
> > 
> > I think by "complete-by-cancel" you are referring to this[*] magic
> > behaviour, mentioned in the last sentence here:
> > 
> >     When you cancel an in-progress ‘mirror’ job before the source and
> >     target are synchronized, block-job-cancel will emit the event
> >     BLOCK_JOB_CANCELLED. However, note that if you cancel a ‘mirror’ job
> >     after it has indicated (via the event BLOCK_JOB_READY) that the
> >     source and target have reached synchronization, then the event
> >     emitted by block-job-cancel changes to BLOCK_JOB_COMPLETED.
> > 
> > 
> > [*] 
> > https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/live-block-operations.rst#l515
> > 
> >> So what we can change later is adding a way to initiate this secondary
> >> completion mode with a job-* command (probably with a new option for
> >> job-complete). But we wouldn't change the semantics of exisiting
> >> commands.
> > 
> > Ah, so if I'm understanding it correctly, the existing subtle magic of
> > "complete-by-cancel" will be rectified by separting the two distinct
> > operations: 'job-cancel' and 'job-complete'.
> 
> Not really, because we already had those two operations for block jobs.
> The thing is that block-job-complete (and thus job-complete) will pivot
> to the target disk, but block-job-cancel doesn't.

Indeed; from the doc linked earlier: "Issuing the command
``block-job-complete`` (after it emits the event
``BLOCK_JOB_COMPLETED``) will adjust the guest device (i.e. live QEMU)
to point to the target image, [E], causing all the new writes from this
point on to happen there."

> The special behavior is that you can use block-job-cancel after
> BLOCK_JOB_READY to complete the job, but not pivot to it.  I don't think
> we have a real plan on how to represent that with the generic job
> commands, we just know that we don't want to use job-cancel.

Ah, thanks for clarifying.  Yes, what you say makes sense —  not using
'job-cancel' to represent completion.

> (Maybe we can add a flag to job-complete (which to me does not sound
> like a good idea), or you could set flags on jobs while they are
> running, so you can set a do-not-pivot flag on the mirror job before you
> complete it.)

Yeah, spelling that out, 'do-not-pivot' or something along those lines,
as a flag makes it clearer.  "Implicit is better than explicit".

-- 
/kashyap