30.01.2019 22:58, John Snow wrote:
>
>
> On 1/30/19 8:58 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Hi!
>>
>> I have a question about job_pause_point. Now a good job of moving generic
>> things about
>> block-jobs to job.c is done..
>>
>> job_pause_point do visible job-state change, as changing job->status and
>> job->paused.
>>
>> Is it assumed, that job don't do anything after that state change? Iotests
>> are written
>> exactly with this assumption. On the other hand, code don't guarantee, that
>> all in-flight
>> requests are finished to this point.
>>
>
> Oh, right. The job being paused only certifies that the coroutine
> driving the job isn't running. It has no real idea about mirror's
> asynchronous children.
>
>> Moreover, only the job itself knows about it's in-flight requests, therefore
>> we can't
>> handle it in generic job_pause_point(). So, should not job do something like
>>
>> if (job_should_pause()) {
>> don't create more requests
>> wait all in-flight requests to complete
>> }
>> job_pause_point()
>>
>> Or didn't I miss these things already discussed?
>>
>
> Well, mirror's pause callback does this:
>
> static void coroutine_fn mirror_pause(Job *job)
> {
> MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
>
> mirror_wait_for_all_io(s);
> }
>
> That's invoked BEFORE we change our status. That ought to be sufficient,
> is it not?
>
> --js
>
Oh, yes, seems that it is what I missed, thank you.
--
Best regards,
Vladimir
