On 02/28/2018 10:42 AM, Kevin Wolf wrote: > Am 24.02.2018 um 00:51 hat John Snow geschrieben: >> Add a new state that specifically demarcates when we begin to permanently >> demolish a job after it has performed all work. This makes the transition >> explicit in the STM table and highlights conditions under which a job may >> be demolished. >> >> >> Transitions: >> Created -> Null: Early failure event before the job is started >> Concluded -> Null: Standard transition. >> >> Verbs: >> None. This should not ever be visible to the monitor. >> >> +---------+ >> |UNDEFINED| >> +--+------+ >> | >> +--v----+ >> |CREATED+------------------+ >> +--+----+ | >> | | >> +--v----+ +------+ | >> +---------+RUNNING<----->PAUSED| | >> | +--+-+--+ +------+ | >> | | | | >> | | +------------------+ | >> | | | | >> | +--v--+ +-------+ | | >> +---------+READY<------->STANDBY| | | >> | +--+--+ +-------+ | | >> | | | | >> +--v-----+ +--v------+ | | >> |ABORTING+--->CONCLUDED<-------------+ | >> +--------+ +--+------+ | >> | | >> +--v-+ | >> |NULL<---------------------+ >> +----+ >> >> Signed-off-by: John Snow <js...@redhat.com> > > The commit message does not match the code: > >> @@ -423,6 +424,7 @@ static void block_job_completed_single(BlockJob *job) >> QLIST_REMOVE(job, txn_list); >> block_job_txn_unref(job->txn); >> block_job_event_concluded(job); >> + block_job_state_transition(job, BLOCK_JOB_STATUS_NULL); >> block_job_unref(job); >> } >> >> @@ -734,9 +736,6 @@ static void block_job_event_completed(BlockJob *job, >> const char *msg) >> >> static void block_job_event_concluded(BlockJob *job) >> { >> - if (block_job_is_internal(job) || !job->manual) { >> - return; >> - } >> block_job_state_transition(job, BLOCK_JOB_STATUS_CONCLUDED); >> } > > Any job that transition to NULL goes first through CONCLUDED. There is > no way to reach NULL directly from CREATED. >
Ah, that's true... I was trying to think of the case in which we do an early uninit, but actually we just dismantle the job without having it go to "null" first. I guess I found that edge case and then never did anything about it except acknowledge it in the graph. Something to fix... > The second hunk addresses my comment in the previous patch, so it should > probably be squashed in there. > > Kevin >