Re: [Qemu-devel] [RFC v4 10/21] blockjobs: add NULL state

[John Snow <jsnow@redhat.com>]

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 <jsnow@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
>