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 <kw...@redhat.com>
Reviewed-by: Max Reitz <mre...@redhat.com>
Reviewed-by: John Snow <js...@redhat.com>
---
@@ -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 <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org