[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 22/42] job: Move BlockJobCreateFlags to Job
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH 22/42] job: Move BlockJobCreateFlags to Job |
Date: |
Wed, 16 May 2018 22:53:44 +0200 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 16.05.2018 um 21:13 hat Eric Blake geschrieben:
> 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.
This series doesn't expose it anyway. We can later decide whether we
want to add it or not. A sophisticated management tool like libvirt that
needs to manage individual nodes and cope with daemon restarts will
never make use of them, but they might still be useful in hand-crafted
scripts for defined special cases.
> > +++ 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.
I'm not sure if we have a QAPI type that matches this. But maybe it
could become a QAPI struct and very few extra parameters.
Kevin
- Re: [Qemu-block] [PATCH 23/42] blockjob: Split block_job_event_pending(), (continued)
- [Qemu-block] [PATCH 19/42] job: Add job_sleep_ns(), Kevin Wolf, 2018/05/09
- [Qemu-block] [PATCH 24/42] job: Add job_event_*(), Kevin Wolf, 2018/05/09
- [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
- [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