On Thu, Mar 16, 2017 at 05:23:49PM -0400, John Snow wrote:
> The purpose of this shim is to allow us to pause pre-started jobs.
> The purpose of *that* is to allow us to buffer a pause request that
> will be able to take effect before the job ever does any work, allowing
> us to create jobs during a quiescent state (under which they will be
> automatically paused), then resuming the jobs after the critical section
> in any order, either:
>
> (1) -block_job_start
> -block_job_resume (via e.g. drained_end)
>
> (2) -block_job_resume (via e.g. drained_end)
> -block_job_start
>
> The problem that requires a startup wrapper is the idea that a job must
> start in the busy=true state only its first time-- all subsequent entries
> require busy to be false, and the toggling of this state is otherwise
> handled during existing pause and yield points.
>
> The wrapper simply allows us to mandate that a job can "start," set busy
> to true, then immediately pause only if necessary. We could avoid
> requiring a wrapper, but all jobs would need to do it, so it's been
> factored out here.
I think this makes sense. So when this happens:
* block_job_create
* block_job_pause
* block_job_resume <-- only effects pause_count, rest is noop
* block_job_start
The block_job_resume is mostly a no-op, only affecting the pause_count but
since there is no job coroutine created yet, the block_job_enter does
nothing.
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> blockjob.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/blockjob.c b/blockjob.c
> index 69126af..69b4ec6 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -250,16 +250,28 @@ static bool block_job_started(BlockJob *job)
> return job->co;
> }
>
> +/**
> + * All jobs must allow a pause point before entering their job proper. This
> + * ensures that jobs can be paused prior to being started, then resumed
> later.
> + */
> +static void coroutine_fn block_job_co_entry(void *opaque)
> +{
> + BlockJob *job = opaque;
> +
> + assert(job && job->driver && job->driver->start);
> + block_job_pause_point(job);
> + job->driver->start(job);
> +}
> +
> void block_job_start(BlockJob *job)
> {
> assert(job && !block_job_started(job) && job->paused &&
> - !job->busy && job->driver->start);
> - job->co = qemu_coroutine_create(job->driver->start, job);
> - if (--job->pause_count == 0) {
> - job->paused = false;
> - job->busy = true;
> - qemu_coroutine_enter(job->co);
> - }
> + job->driver && job->driver->start);
> + job->co = qemu_coroutine_create(block_job_co_entry, job);
> + job->pause_count--;
> + job->busy = true;
> + job->paused = false;
> + qemu_coroutine_enter(job->co);
> }
>
> void block_job_ref(BlockJob *job)
> --
> 2.9.3
>
