Re: [Qemu-devel] [RFC v3 03/14] blockjobs: add state transition table

[Kevin Wolf <kwolf@redhat.com>]

Am 27.01.2018 um 03:05 hat John Snow geschrieben:
> 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.
> 
> undefined: May only transition to 'created'.
> created: May only transition to 'running'.
>          It cannot transition to pause directly, but if a created job
>          is requested to pause prior to starting, it will transition
>          synchronously to 'running' and then to 'paused'.
> running: May transition either to 'paused' or 'ready'.
> paused: May transition to either 'running' or 'ready', but only in
>         terms of returning to that prior state.
>         p->r->y is not possible, but this is not encapsulated by the
>         STM table.

Do you mean y->p->r->y here? If not, I don't understand.

> ready: May transition to paused.
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  blockjob.c | 39 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index 6eb783a354..d084a1e318 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -42,6 +42,35 @@
>   * block_job_enter. */
>  static QemuMutex block_job_mutex;
>  
> +/* BlockJob State Transition Table */
> +bool BlockJobSTT[BLOCK_JOB_STATUS__MAX][BLOCK_JOB_STATUS__MAX] = {
> +                                          /* U, C, R, P, Y */
> +    /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0},
> +    /* C: */ [BLOCK_JOB_STATUS_CREATED]   = {0, 0, 1, 0, 0},
> +    /* R: */ [BLOCK_JOB_STATUS_RUNNING]   = {0, 0, 0, 1, 1},
> +    /* P: */ [BLOCK_JOB_STATUS_PAUSED]    = {0, 0, 1, 0, 1},
> +    /* Y: */ [BLOCK_JOB_STATUS_READY]     = {0, 0, 0, 1, 0},
> +};
> +
> +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);
> +    assert(BlockJobSTT[s0][s1]);
> +    switch (s1) {
> +    case BLOCK_JOB_STATUS_WAITING:
> +    case BLOCK_JOB_STATUS_PENDING:
> +    case BLOCK_JOB_STATUS_CONCLUDED:
> +        assert(job->manual);
> +    default:
> +        break;
> +    }

This doesn't compile because the constants don't exist yet.

Apart from that, I think the assertion is misguided, too. Which states a
job goes through shouldn't depend on whether the client wants to
complete the job manually or have it completed automatically. The
difference should only be which state transitions are automatic.

In other words, WAITING/PENDING/CONCLUDED may never be visible in
query-block-job for automatically completed jobs, but the jobs should
still (synchronously) go through all of these states.

> +    job->status = s1;
> +}
> +

Kevin