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

[John Snow]

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.")

--js