[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v4 08/21] blockjobs: add ABORTING state
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [RFC v4 08/21] blockjobs: add ABORTING state |
Date: |
Wed, 28 Feb 2018 14:26:00 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 02/28/2018 09:54 AM, Kevin Wolf wrote:
> Am 24.02.2018 um 00:51 hat John Snow geschrieben:
>> Add a new state ABORTING.
>>
>> This makes transitions from normative states to error states explicit
>> in the STM, and serves as a disambiguation for which states may complete
>> normally when normal end-states (CONCLUDED) are added in future commits.
>>
>> Notably, Paused/Standby jobs do not transition directly to aborting,
>> as they must wake up first and cooperate in their cancellation.
>>
>> Transitions:
>> Running -> Aborting: can be cancelled or encounter an error
>> Ready -> Aborting: can be cancelled or encounter an error
>>
>> Verbs:
>> None. The job must finish cleaning itself up and report its final status.
>>
>> +---------+
>> |UNDEFINED|
>> +--+------+
>> |
>> +--v----+
>> |CREATED|
>> +--+----+
>> |
>> +--v----+ +------+
>> +---------+RUNNING<----->PAUSED|
>> | +--+----+ +------+
>> | |
>> | +--v--+ +-------+
>> +---------+READY<------->STANDBY|
>> | +-----+ +-------+
>> |
>> +--v-----+
>> |ABORTING|
>> +--------+
>>
>> Signed-off-by: John Snow <address@hidden>
>
>> @@ -383,6 +384,10 @@ static void block_job_completed_single(BlockJob *job)
>> {
>> assert(job->completed);
>>
>> + if (job->ret || block_job_is_cancelled(job)) {
>> + block_job_state_transition(job, BLOCK_JOB_STATUS_ABORTING);
>> + }
>> +
>> if (!job->ret) {
>> if (job->driver->commit) {
>> job->driver->commit(job);
>
> Reviewed-by: Kevin Wolf <address@hidden>
>
> But I do have a question about the code below the new lines: I wonder
> where job->ret is set to an error value? I can clearly see that the job
> is marked as cancelled, so your added code should be working, but
> looking at the block job code, it looks to me as if the jobs were
> completing with job->cancelled = true and job->ret = 0.
>
> For block_job_completed_single(), this means that we would call the
> .commit callback and .cb with a success return code, while sending a
> CANCELLED event on QMP.
>
> Of course, the only job type that supports transactions is the backup
> job, and that one seems to compensate for this by explicitly checking
> block_job_is_cancelled() in its .commit implementation, but is that
> really how things should be working?
>
> Kevin
>
You re-discovered the same nonsensical thing that I discovered, which
led to patch 12.
- Re: [Qemu-devel] [RFC v4 16/21] blockjobs: add waiting status, (continued)
[Qemu-devel] [RFC v4 12/21] blockjobs: ensure abort is called for cancelled jobs, John Snow, 2018/02/23
[Qemu-devel] [RFC v4 08/21] blockjobs: add ABORTING state, John Snow, 2018/02/23
[Qemu-devel] [RFC v4 20/21] iotests: test manual job dismissal, John Snow, 2018/02/23
[Qemu-devel] [RFC v4 10/21] blockjobs: add NULL state, John Snow, 2018/02/23
[Qemu-devel] [RFC v4 01/21] blockjobs: fix set-speed kick, John Snow, 2018/02/23