Re: [Qemu-block] [Qemu-devel] [RFC v4 05/21] blockjobs: add state transition table

[Eric Blake]

On 02/23/2018 05:51 PM, John Snow wrote:
> The state transition table has mostly been implied. We're about to make
> it a bit more complex, so let's make the STM explicit instead.
>
> Perform state transitions with a function that for now just asserts the
> transition is appropriate.
>
> Transitions:
> Undefined -> Created: During job initialization.

Unless we use Created as the default state 0 for g_new0().

> Created   -> Running: Once the job is started.
>                        Jobs cannot transition from "Created" to "Paused"
>                        directly, but will instead synchronously transition
>                        to running to paused immediately.
> Running   -> Paused:  Normal workflow for pauses.
> Running   -> Ready:   Normal workflow for jobs reaching their sync point.
>                        (e.g. mirror)
> Ready     -> Standby: Normal workflow for pausing ready jobs.
> Paused    -> Running: Normal resume.
> Standby   -> Ready:   Resume of a Standby job.
>
>
> +---------+
> |UNDEFINED|
> +--+------+
>     |
> +--v----+
> |CREATED|
> +--+----+
>     |
> +--v----+     +------+
> |RUNNING<----->PAUSED|
> +--+----+     +------+
>     |
> +--v--+       +-------+
> |READY<------->STANDBY|
> +-----+       +-------+
>
>
> Notably, there is no state presently defined as of this commit that
> deals with a job after the "running" or "ready" states, so this table
> will be adjusted alongside the commits that introduce those states.

The ascii-art tables help in this and other patches.  Thanks for 
producing them.

> Signed-off-by: John Snow <address@hidden>
> ---
>   block/trace-events |  3 +++
>   blockjob.c         | 42 ++++++++++++++++++++++++++++++++++++------
>   2 files changed, 39 insertions(+), 6 deletions(-)

> +static void block_job_state_transition(BlockJob *job, BlockJobStatus s1)
> +{
> +    BlockJobStatus s0 = job->status;
> +    if (s0 == s1) {
> +        return;
> +    }
> +    assert(s1 >= 0 && s1 <= BLOCK_JOB_STATUS__MAX);

Or, if you keep the zero state distinct from valid states, this could be 
'assert(s1 > 0 && ...)'

> @@ -320,7 +349,7 @@ void block_job_start(BlockJob *job)
>       job->pause_count--;
>       job->busy = true;
>       job->paused = false;
> -    job->status = BLOCK_JOB_STATUS_RUNNING;
> +    block_job_state_transition(job, BLOCK_JOB_STATUS_RUNNING);
>       bdrv_coroutine_enter(blk_bs(job->blk), job->co);
>   }
>   
> @@ -704,6 +733,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
>       job->refcnt        = 1;
>       job->manual        = (flags & BLOCK_JOB_MANUAL);
>       job->status        = BLOCK_JOB_STATUS_CREATED;
> +    block_job_state_transition(job, BLOCK_JOB_STATUS_CREATED);

Oops, missing a deletion of the job->status assignment line.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org