Re: [Qemu-devel] [PATCH 22/42] job: Move BlockJobCreateFlags to Job

[Eric Blake]

On 05/09/2018 11:26 AM, Kevin Wolf wrote:
> This renames the BlockJobCreateFlags constants, moves a few JOB_INTERNAL
> checks to job_create() and the auto_{finalize,dismiss} fields from
> BlockJob to Job.

Do we still want to allow auto-finalize on all jobs, or should we keep 
it just for legacy support on block jobs?  Even more so for auto-dismiss 
(if you use the legacy interface, that's what you expect to happen; but 
other than for legacy block jobs, any sane management app is going to 
request auto-dismiss be false, so why expose it to generic jobs?)

Of course, it may still be easiest to plumb up auto- actions in the 
internal code (where it has to work to keep legacy block jobs from 
breaking, but no new callers have to enable it), so my argument may not 
apply until you actually expose a QMP interface for generic jobs.

> +++ b/block/mirror.c
> @@ -1282,7 +1282,7 @@ void mirror_start(const char *job_id, BlockDriverState 
> *bs,
>       }
>       is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
>       base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
> -    mirror_start_job(job_id, bs, BLOCK_JOB_DEFAULT, target, replaces,
> +    mirror_start_job(job_id, bs, JOB_DEFAULT, target, replaces,
>                        speed, granularity, buf_size, backing_mode,
>                        on_source_error, on_target_error, unmap, NULL, NULL,
>                        &mirror_job_driver, is_none_mode, base, false,

Just an observation that this is a lot of parameters; would using boxed 
QAPI types make these calls any more obvious?  But that's a separate 
cleanup.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org