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

[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 <address@hidden>

> +++ 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.

> +##
> +# @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.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature