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

[John Snow]

On 05/25/2018 04:00 AM, Kevin Wolf wrote:
> Am 24.05.2018 um 19:42 hat John Snow geschrieben:
>>
>>
>> On 05/24/2018 04:24 AM, Kevin Wolf wrote:
>>> Am 24.05.2018 um 01:18 hat John Snow geschrieben:
>>>>> diff --git a/include/qemu/job.h b/include/qemu/job.h
>>>>> index 3e817beee9..2648c74281 100644
>>>>> --- a/include/qemu/job.h
>>>>> +++ b/include/qemu/job.h
>>>>> @@ -97,6 +97,12 @@ typedef struct Job {
>>>>>       */
>>>>>      bool cancelled;
>>>>>  
>>>>> +    /**
>>>>> +     * Set to true if the job should abort immediately without waiting
>>>>> +     * for data to be in sync.
>>>>> +     */
>>>>> +    bool force_cancel;
>>>>> +
>>>>
>>>> Does this comment need an update now, though?
>>>>
>>>> Actually, in terms of "new jobs" API, it'd be really nice if cancel
>>>> *always meant cancel*.
>>>>
>>>> I think "cancel" should never be used to mean "successful completion,
>>>> but different from the one we'd get if we used job_complete."
>>>>
>>>> i.e., either we need a job setting:
>>>>
>>>> job-set completion-mode=[pivot|no-pivot]
>>>>
>>>> or optional parameters to pass to job-complete:
>>>>
>>>> job-complete mode=[understood-by-job-type]
>>>>
>>>> or some other mechanism that accomplishes the same type of behavior. It
>>>> would be nice if it did not have to be determined at job creation time
>>>> but instead could be determined later.
>>>
>>> I agree. We already made sure that job-cancel really means cancel on the
>>> QAPI level, so we're free to do that. We just need to keep supporting
>>> block-job-cancel with the old semantics, so what I have is the easy
>>> conversion. We can change the internal implementation when we actually
>>> implement the selection of a completion mode.
>>>
>>> Kevin
>>>
>>
>> We need this before 3.0 though, yeah? unless we make job-cancel
>> x-job-cancel or some other warning that the way it works might change, yeah?
>>
>> Or do I misunderstand our leeway to change this at a later point in time?
>>
>> (can job-cancel apply to block jobs created with the legacy
>> infrastructure? My reading was "yes.")
> 
> 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.
> 
> 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.
> 
> Kevin
> 

Oho, I _was_ missing something. I hadn't realized cancel takes on the
newer semantics. Thanks.