[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2 08/40] job: Move state transitions to Job
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v2 08/40] job: Move state transitions to Job |
Date: |
Fri, 18 May 2018 09:36:57 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
On 05/18/2018 08:20 AM, Kevin Wolf wrote:
This moves BlockJob.status and the closely related functions
(block_)job_state_transition() and (block_)job_apply_verb to Job. The
two QAPI enums are renamed to JobStatus and JobVerb.
Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Reviewed-by: John Snow <address@hidden>
---
@@ -1077,14 +1021,14 @@ void coroutine_fn block_job_pause_point(BlockJob *job)
}
if (block_job_should_pause(job) && !block_job_is_cancelled(job)) {
- BlockJobStatus status = job->status;
- block_job_state_transition(job, status == BLOCK_JOB_STATUS_READY ? \
- BLOCK_JOB_STATUS_STANDBY : \
- BLOCK_JOB_STATUS_PAUSED);
+ JobStatus status = job->job.status;
+ job_state_transition(&job->job, status == JOB_STATUS_READY
+ ? JOB_STATUS_STANDBY
+ : JOB_STATUS_PAUSED);
Nice that you are getting rid of the pointless \. Here, you split the
?: operator by wrapping the ? onto the start of the next line...
+++ b/job.c
+int job_apply_verb(Job *job, JobVerb verb, Error **errp)
+{
+ assert(verb >= 0 && verb <= JOB_VERB__MAX);
+ trace_job_apply_verb(job, JobStatus_str(job->status), JobVerb_str(verb),
+ JobVerbTable[verb][job->status] ?
+ "allowed" : "prohibited");
while here, you put it at the end of the previous line. We have a
slight preference for one style over the other (there are also false
positives in both of these greps):
$ git grep '?$' | wc -c
27251
$ git grep '^ *?' | wc -c
9619
But I don't care which style you use, other than pointing out that a
consistent style within the patch doesn't hurt. :)
So, whether or not you make a whitespace-only tweak to one of those two
spots,
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- Re: [Qemu-block] [Qemu-devel] [PATCH v2 02/40] blockjob: Improve BlockJobInfo.offset/len documentation, (continued)
- [Qemu-block] [PATCH v2 01/40] blockjob: Update block-job-pause/resume documentation, Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 04/40] job: Rename BlockJobType into JobType, Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 03/40] job: Create Job, JobDriver and job_create(), Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 05/40] job: Add JobDriver.job_type, Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 06/40] job: Add job_delete(), Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 07/40] job: Maintain a list of all jobs, Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 08/40] job: Move state transitions to Job, Kevin Wolf, 2018/05/18
- Re: [Qemu-block] [PATCH v2 08/40] job: Move state transitions to Job,
Eric Blake <=
- [Qemu-block] [PATCH v2 11/40] job: Add Job.aio_context, Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 09/40] job: Add reference counting, Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 10/40] job: Move cancelled to Job, Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 14/40] job: Add job_sleep_ns(), Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 12/40] job: Move defer_to_main_loop to Job, Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 15/40] job: Move pause/resume functions to Job, Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 16/40] job: Replace BlockJob.completed with job_is_completed(), Kevin Wolf, 2018/05/18
- [Qemu-block] [PATCH v2 13/40] job: Move coroutine and related code to Job, Kevin Wolf, 2018/05/18