[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v5 10/21] blockjobs: add NULL state
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH v5 10/21] blockjobs: add NULL state |
Date: |
Mon, 12 Mar 2018 16:28:35 +0100 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 10.03.2018 um 09:27 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.
>
> Alongside this state, add a new helper command "block_job_decommission",
> which transitions to the NULL state and puts down our implicit reference.
> This separates instances in the code for "block_job_unref" which merely
> undo a matching "block_job_ref" with instances intended to initiate the
> full destruction of the object.
>
> This decommission action also sets a number of fields to make sure that
> block internals or external users that are holding a reference to a job
> to see when it "finishes" are convinced that the job object is "done."
> This is necessary, for instance, to do a block_job_cancel_sync on a
> created object which will not make any progress.
>
> Now, all jobs must go through block_job_decommission prior to being
> freed, giving us start-to-finish state machine coverage for jobs.
>
>
> 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 <address@hidden>
> +static void block_job_decommission(BlockJob *job)
> +{
> + assert(job);
> + job->completed = true;
> + job->busy = false;
> + job->paused = false;
> + job->deferred_to_main_loop = true;
Why do we set all of these fields now? I don't see the use of it, and
overwriting fields here potentially makes debugging harder.
Especially for deferred_to_main_loop I might expect an assert() that it
already is true, but shouldn't setting it always be done while actually
deferring to the main loop?
Can we turn all of these assignments into asserts or are there some that
actually aren't already guaranteed, but that we want anyway?
> + block_job_state_transition(job, BLOCK_JOB_STATUS_NULL);
> + block_job_unref(job);
> +}
Kevin
- [Qemu-block] [PATCH v5 14/21] blockjobs: add block_job_txn_apply function, (continued)
- [Qemu-block] [PATCH v5 14/21] blockjobs: add block_job_txn_apply function, John Snow, 2018/03/10
- [Qemu-block] [PATCH v5 09/21] blockjobs: add CONCLUDED state, John Snow, 2018/03/10
- [Qemu-block] [PATCH v5 01/21] blockjobs: fix set-speed kick, John Snow, 2018/03/10
- [Qemu-block] [PATCH v5 03/21] Blockjobs: documentation touchup, John Snow, 2018/03/10
- [Qemu-block] [PATCH v5 06/21] iotests: add pause_wait, John Snow, 2018/03/10
- [Qemu-block] [PATCH v5 12/21] blockjobs: ensure abort is called for cancelled jobs, John Snow, 2018/03/10
- [Qemu-block] [PATCH v5 08/21] blockjobs: add ABORTING state, John Snow, 2018/03/10
- [Qemu-block] [PATCH v5 15/21] blockjobs: add prepare callback, John Snow, 2018/03/10
- [Qemu-block] [PATCH v5 10/21] blockjobs: add NULL state, John Snow, 2018/03/10
- Re: [Qemu-block] [PATCH v5 10/21] blockjobs: add NULL state,
Kevin Wolf <=
[Qemu-block] [PATCH v5 11/21] blockjobs: add block_job_dismiss, John Snow, 2018/03/10
[Qemu-block] [PATCH v5 13/21] blockjobs: add commit, abort, clean helpers, John Snow, 2018/03/10
[Qemu-block] [PATCH v5 05/21] blockjobs: add state transition table, John Snow, 2018/03/10
[Qemu-block] [PATCH v5 02/21] blockjobs: model single jobs as transactions, John Snow, 2018/03/10
[Qemu-block] [PATCH v5 16/21] blockjobs: add waiting status, John Snow, 2018/03/10
[Qemu-block] [PATCH v5 18/21] blockjobs: add block-job-finalize, John Snow, 2018/03/10
[Qemu-block] [PATCH v5 04/21] blockjobs: add status enum, John Snow, 2018/03/10