Am 15.05.2018 um 00:31 hat Max Reitz geschrieben:
> On 2018-05-09 18:26, Kevin Wolf wrote:
> > This adds QMP commands that control the transition between states of the
> > job lifecycle.
> >
> > Signed-off-by: Kevin Wolf <kw...@redhat.com>
> > ---
> > qapi/job.json | 112 ++++++++++++++++++++++++++++++++++++++++++++++
> > job-qmp.c | 140
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > MAINTAINERS | 1 +
> > Makefile.objs | 2 +-
> > trace-events | 9 ++++
> > 5 files changed, 263 insertions(+), 1 deletion(-)
> > create mode 100644 job-qmp.c
> >
> > diff --git a/qapi/job.json b/qapi/job.json
> > index bd88a358d0..7b84158292 100644
> > --- a/qapi/job.json
> > +++ b/qapi/job.json
> > @@ -63,3 +63,115 @@
> > { 'event': 'JOB_STATUS_CHANGE',
> > 'data': { 'id': 'str',
> > 'status': 'JobStatus' } }
> > +
> > +##
> > +# @job-pause:
> > +#
> > +# Pause an active job.
> > +#
> > +# This command returns immediately after marking the active job for
> > pausing.
> > +# Pausing an already paused job has no cumulative effect; a single
> > job-resume
> > +# command will resume the job.
>
> Pausing an already paused job is, in fact, an error.
>
> (Which should be noted here instead of making it appear like it'd be
> idempotent.)
Aye. Interestingly, I managed to fix it below in job-resume, but missed
it here. I should also stick in a patch somewhere early in the series
that fixes the documentation of block-job-pause/resume.
> > +##
> > +# @job-cancel:
> > +#
> > +# Instruct an active background job to cancel at the next opportunity.
> > +# This command returns immediately after marking the active job for
> > +# cancellation.
> > +#
> > +# The job will cancel as soon as possible and then emit a JOB_STATUS_CHANGE
> > +# event. Usually, the status will change to ABORTING, but it is possible
> > that
> > +# a job successfully completes (e.g. because it was almost done and there
> > was
> > +# no opportunity to cancel earlier than completing the job) and
> > transitions to
> > +# PENDING instead.
> > +#
> > +# Note that if you issue 'job-cancel' after a mirror block job has
> > indicated
> > +# (via the event BLOCK_JOB_READY, and by transitioning into the READY
> > state)
> > +# that the source and destination are synchronized, then the job always
> > +# completes successfully and transitions to PENDING as well as triggering
> > the
> > +# event BLOCK_JOB_COMPLETED, to indicate that the mirroring has ended and
> > the
> > +# destination now has a point-in-time copy tied to the time of the
> > +# cancellation.
> > +#
> > +# @id: The job identifier.
> > +#
> > +# @force: If true, and the job is already in the READY state, abandon the
> > job
> > +# immediately (even if it is paused) instead of waiting for the
> > +# destination to complete its final synchronization
>
> The note on "final synchronization" is extremely mirror-specific. I see
> three choices here:
>
> (1) If mirror stays the only job with this special cancel semantics,
> then we are free to note that this is a mirror-specific flag here.
>
> (2) Even if some other job might come along at some point where use of
> @force may make sense, that doesn't stop us from now noting that only
> mirror supports this, which helps readers understand what "destination"
> and "final synchronization" mean.
>
> (Yes, so (1) and (2) are basically the same.)
>
> (3) We try to find some general description and drop the last part.
> Like "If a job would normally decide to complete instead of actually
> aborting, this flag can be used to convince it otherwise." But that's
> so handwavy, I'd rather just mark it as a special mirror flag for now.
Or how about this one?
(4) Mirror is really abusing cancel for a second completion mode and we
don't want to have this kind of ugliness in job-cancel. Remove
@force from the schema and internally always use force=true. For
now, block-job-cancel does the job (no pun intended) for mirror, and
maybe we can later introduce a way to select completion mode with
job-complete.
This would also get us rid of that whole long paragraph above that
explains how mirror jobs have an exceptional behaviour.
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 3df8d58e49..253e0356f3 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -66,7 +66,7 @@ chardev-obj-y = chardev/
> > # block-obj-y is code used by both qemu system emulation and qemu-img
> >
> > block-obj-y += nbd/
> > -block-obj-y += block.o blockjob.o job.o
> > +block-obj-y += block.o blockjob.o job.o job-qmp.o
>
> Shouldn't this be in common-obj-y like blockdev?
Seems to build with that change, so it can't be wrong...
Kevin
signature.asc
Description: PGP signature
