Am 29.01.2018 um 20:07 hat John Snow geschrieben:
>
>
> 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.
Hm, yes... It would probably be cleaner to have two states. If a client
queries the state and just sees PAUSED, it doesn't know whether the bulk
phase has already finished.
> >
> >> 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.
Actually, while there is probably no use for the events, I don't think
they would hurt if omitting them isn't completely trivial. Clients
always need to be prepared to see new unknown events.
Kevin
