On Fri, 20 Jan 2012 09:30:45 +0100
Kevin Wolf <kw...@redhat.com> wrote:
> Am 20.01.2012 01:02, schrieb Eric Blake:
> > On 01/06/2012 07:01 AM, Stefan Hajnoczi wrote:
> >> Add block_job_cancel, which stops an active block streaming operation.
> >> When the operation has been cancelled the new BLOCK_JOB_CANCELLED event
> >> is emitted.
> >>
> >> Signed-off-by: Stefan Hajnoczi <stefa...@linux.vnet.ibm.com>
> >
> >> +++ b/hmp-commands.hx
> >> @@ -98,6 +98,20 @@ Set maximum speed for a background block operation.
> >> ETEXI
> >>
> >> {
> >> + .name = "block_job_cancel",
> >> + .args_type = "device:B",
> >> + .params = "device",
> >> + .help = "stop an active block streaming operation",
> >> + .mhandler.cmd = hmp_block_job_cancel,
> >> + },
> >> +
> >
> > Looking at this from libvirt's perspective, would it be possible to give
> > this a different name? Then libvirt would know that if
> > block_job_cancel_async exists, we have the official semantics; and if it
> > doesn't exist, then we can try block_job_cancel as a fallback to see if
> > we have the old blocking semantics.
> >
> > But by using the same name as the old unofficial blocking command, it's
> > difficult to tell if we should expect an event, or whether completion of
> > the command means completion of the cancel.
> >
> > On the other hand, I guess we can rely on completion of the command,
> > followed by reading block job status to see if the job is still in
> > flight, will tell us whether we need to worry about waiting for an event
> > - if the job is complete (whether or not this command was the blocking
> > variant), we are done; if the job is ongoing, we have the new semantics
> > and can expect an event; and that only leaves the race of calling the
> > command, then the job completes, then we query and see it done, then the
> > event comes, where we just have to be ready to ignore an unexpected event.
>
> You're quoting the HMP part, is that intentional? You shouldn't be using
> this at all.
>
> Anyway, are there even any qemu versions out there that implement an
> older interface?
>
> >> +##
> >> +# @block_job_cancel:
> >> +#
> >> +# Stop an active block streaming operation.
> >> +#
> >> +# This command returns immediately after marking the active block
> >> streaming
> >> +# operation for cancellation. It is an error to call this command if no
> >> +# operation is in progress.
> >> +#
> >> +# The operation will cancel as soon as possible and then emit the
> >> +# BLOCK_JOB_CANCELLED event. Before that happens the job is still
> >> visible when
> >> +# enumerated using query-block-jobs.
> >
> > Is there any policy on _ vs - in command names? It seems awkward to
> > have block_job_cancel but query-block-jobs.
>
> block_job_cancel is HMP, whereas query-block-jobs is a QMP command. QMP
> uses - consistently. Not sure if HMP is consistent, but it tends to use _.
This very series broke QMP's consistency because it was designed when we
were following HMP's inconsistencies...
