Am 24.02.2018 um 00:51 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. > > Transitions: > Undefined -> Created: During job initialization. > 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. > > Signed-off-by: John Snow <js...@redhat.com> > --- > block/trace-events | 3 +++ > blockjob.c | 42 ++++++++++++++++++++++++++++++++++++------ > 2 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/block/trace-events b/block/trace-events > index 02dd80ff0c..b75a0c8409 100644 > --- a/block/trace-events > +++ b/block/trace-events > @@ -4,6 +4,9 @@ > bdrv_open_common(void *bs, const char *filename, int flags, const char > *format_name) "bs %p filename \"%s\" flags 0x%x format_name \"%s\"" > bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d" > > +# blockjob.c > +block_job_state_transition(void *job, int ret, const char *legal, const > char *s0, const char *s1) "job %p (ret: %d) attempting %s transition > (%s-->%s)" > + > # block/block-backend.c > blk_co_preadv(void *blk, void *bs, int64_t offset, unsigned int bytes, int > flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x" > blk_co_pwritev(void *blk, void *bs, int64_t offset, unsigned int bytes, int > flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x" > diff --git a/blockjob.c b/blockjob.c > index 1be9c20cff..d745b3bb69 100644 > --- a/blockjob.c > +++ b/blockjob.c > @@ -28,6 +28,7 @@ > #include "block/block.h" > #include "block/blockjob_int.h" > #include "block/block_int.h" > +#include "block/trace.h" > #include "sysemu/block-backend.h" > #include "qapi/error.h" > #include "qapi/qmp/qerror.h" > @@ -41,6 +42,34 @@ > * 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, S */ > + /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0},
Even at the end of the series, this is the only use of BLOCK_JOB_STATUS_UNDEFINED. > + /* C: */ [BLOCK_JOB_STATUS_CREATED] = {0, 0, 1, 0, 0, 0}, > + /* R: */ [BLOCK_JOB_STATUS_RUNNING] = {0, 0, 0, 1, 1, 0}, > + /* P: */ [BLOCK_JOB_STATUS_PAUSED] = {0, 0, 1, 0, 0, 0}, > + /* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1}, > + /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 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); > + trace_block_job_state_transition(job, job->ret, BlockJobSTT[s0][s1] ? > + "allowed" : "disallowed", > + qapi_enum_lookup(&BlockJobStatus_lookup, > + s0), > + qapi_enum_lookup(&BlockJobStatus_lookup, > + s1)); > + assert(BlockJobSTT[s0][s1]); > + job->status = s1; > +} > + > static void block_job_lock(void) > { > qemu_mutex_lock(&block_job_mutex); > @@ -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); So did you intend to start with BLOCK_JOB_STATUS_UNDEFINED and then transition to BLOCK_JOB_STATUS_CREATED? Or should we completely remove BLOCK_JOB_STATUS_UNDEFINED, keep the initialisation and not call block_job_state_transition() here? Kevin