Re: [PATCH for-6.1? 4/6] job: Add job_cancel_requested()

From: Max Reitz <mreitz@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>,
	qemu-devel@nongnu.org
Subject: Re: [PATCH for-6.1? 4/6] job: Add job_cancel_requested()
Date: Tue, 27 Jul 2021 17:30:46 +0200	[thread overview]
Message-ID: <dd9d521a-24f2-650e-843e-e30a40a58d5b@redhat.com> (raw)
In-Reply-To: <bd4d225d-0bcd-9a38-9813-af229d8fbe4f@virtuozzo.com>

On 27.07.21 16:47, Vladimir Sementsov-Ogievskiy wrote:
> 26.07.2021 10:09, Max Reitz wrote:
>>
>>>>           job->ret = -ECANCELED;
>>>>       }
>>>>       if (job->ret) {
>>>> @@ -704,7 +709,7 @@ static int job_finalize_single(Job *job)
>>>>         /* Emit events only if we actually started */
>>>>       if (job_started(job)) {
>>>> -        if (job_is_cancelled(job)) {
>>>> +        if (job_cancel_requested(job)) {
>>>>               job_event_cancelled(job);
>>>
>>> Same question here.. Shouldn't mirror report COMPLETED event in case 
>>> of not-force cancelled in READY state?
>>
>> Same here, I thought this is user-visible, nothing internal, so I 
>> should leave it as-is.
>>
>> Now I see that cancelling mirror post-READY indeed should result in a 
>> COMPLETED event.  So I’m actually not exactly sure how mirror does 
>> that, despite this code here
>
>
> Hmm. Now looking at mirror code, I see that it does 
> "s->common.job.cancelled = false"

*lol*, that’s nice.

So since we’ve missed the rc1 boat now, I think this is 6.2 material.  
I’ll look into whether we can drop that then, that would be nice.

Max