[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 26/42] job: Convert block_job_cancel_async() to
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH 26/42] job: Convert block_job_cancel_async() to Job |
Date: |
Mon, 14 May 2018 21:00:20 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
On 2018-05-09 18:26, Kevin Wolf wrote:
> block_job_cancel_async() did two things that were still block job
> specific:
>
> * Setting job->force. This field makes sense on the Job level, so we can
> just move it. While at it, rename it to job->force_cancel to make its
> purpose more obvious.
>
> * Resetting the I/O status. This can't be moved because generic Jobs
> don't have an I/O status. What the function really implements is a
> user resume, except without entering the coroutine. Consequently, it
> makes sense to call the .user_resume driver callback here which
> already resets the I/O status.
>
> The old block_job_cancel_async() has two separate if statements that
> check job->iostatus != BLOCK_DEVICE_IO_STATUS_OK and job->user_paused.
> However, the former condition always implies the latter (as is
> asserted in block_job_iostatus_reset()), so changing the explicit call
> of block_job_iostatus_reset() on the former condition with the
> .user_resume callback on the latter condition is equivalent and
> doesn't need to access any BlockJob specific state.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> include/block/blockjob.h | 6 ------
> include/qemu/job.h | 6 ++++++
> block/mirror.c | 4 ++--
> blockjob.c | 25 +++++++++++++------------
> 4 files changed, 21 insertions(+), 20 deletions(-)
I'm not quite sure why you keep this function in blockjob.c, when you've
previously moved such static functions over to job.c and made them
temporarily public (e.g. job_state_transition()).
But I don't really care either way, in fact keeping the function in the
same file makes reviewing easier for me, so:
Reviewed-by: Max Reitz <address@hidden>
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [PATCH 24/42] job: Add job_event_*(), (continued)
- [Qemu-block] [PATCH 22/42] job: Move BlockJobCreateFlags to Job, Kevin Wolf, 2018/05/09
- [Qemu-block] [PATCH 25/42] job: Move single job finalisation to Job, Kevin Wolf, 2018/05/09
- [Qemu-block] [PATCH 27/42] job: Add job_drain(), Kevin Wolf, 2018/05/09
- [Qemu-block] [PATCH 26/42] job: Convert block_job_cancel_async() to Job, Kevin Wolf, 2018/05/09
- Re: [Qemu-block] [PATCH 26/42] job: Convert block_job_cancel_async() to Job,
Max Reitz <=
- [Qemu-block] [PATCH 29/42] job: Move job_finish_sync() to Job, Kevin Wolf, 2018/05/09
- [Qemu-block] [PATCH 28/42] job: Move .complete callback to Job, Kevin Wolf, 2018/05/09
- [Qemu-block] [PATCH 30/42] job: Switch transactions to JobTxn, Kevin Wolf, 2018/05/09
- [Qemu-block] [PATCH 31/42] job: Move transactions to Job, Kevin Wolf, 2018/05/09
- [Qemu-block] [PATCH 34/42] job: Add job_dismiss(), Kevin Wolf, 2018/05/09