Re: [Qemu-devel] [PATCH v4 08/15] qmp: add block_job_cancel command

[Luiz Capitulino <lcapitulino@redhat.com>]

On Fri, 20 Jan 2012 09:30:45 +0100
Kevin Wolf <kwolf@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 <stefanha@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...