Re: [PATCH 04/11] job: add .cancel handler for the driver

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

21.01.2021 02:17, Eric Blake wrote:
> On 11/18/20 12:04 PM, Vladimir Sementsov-Ogievskiy wrote:
>> To be used in mirror in the following commit to cancel in-flight io on
>> target to not waste the time.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   include/qemu/job.h | 5 +++++
>>   job.c              | 3 +++
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/include/qemu/job.h b/include/qemu/job.h
>> index 32aabb1c60..efc6fa7544 100644
>> --- a/include/qemu/job.h
>> +++ b/include/qemu/job.h
>> @@ -251,6 +251,11 @@ struct JobDriver {
>>        */
>>       void (*clean)(Job *job);
>>   
>> +    /**
>> +     * If the callback is not NULL, it will be invoked in job_cancel_async
>> +     */
>> +    void (*cancel)(Job *job);
>> +
> 
> Does the call need to be re-entrant or even worry about being invoked
> more than once on the same BDS?  Or worded differently,
> 
>> +++ b/job.c
>> @@ -712,6 +712,9 @@ static int job_finalize_single(Job *job)
>>   
>>   static void job_cancel_async(Job *job, bool force)
>>   {
>> +    if (job->driver->cancel) {
>> +        job->driver->cancel(job);
>> +    }
>>       if (job->user_paused) {
> 
> can job_cancel_async be reached more than once on the same BDS?

what do you mean by same BDS? On same job?

I don't think that job_cancel_async should be called several times during one "cancel" operation. But if it is, it's still safe enough with only .cancel implementation in [05], which just calls bdrv_cancel_in_flight() (which of course should be safe to call several times).

> 
>>           /* Do not call job_enter here, the caller will handle it.  */
>>           if (job->driver->user_resume) {
>>
> 


-- 
Best regards,
Vladimir