On 01/29/2018 12:17 PM, Kevin Wolf wrote:
> 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.
Whoops, Yes, I mean to say that Y->P->R is not possible.
That is, a paused state can only return to where it came from, but the
STM doesn't show this restriction. Really, this hints at there being
*two* paused states, but I didn't want to go through the trouble of
adding a new one.
>
>> ready: May transition to paused.
I swear my script used to add blank lines for me here. *shrug*
>> Signed-off-by: John Snow <js...@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.
>
*cough* oops.
> 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.
>
Hmm. OK, I can look at doing it in this way. I will probably also have
it omit the events in this case too, though.
>> + job->status = s1;
>> +}
>> +
>
> Kevin
>
Thanks!
