Re: [Qemu-devel] job_pause_point

[John Snow]

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