Re: [Qemu-devel] [RFC V1 11/12] qmp: Add block-pause-dedup.

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]

On 01/16/2013 09:25 AM, Benoît Canet wrote:
> ---
>  blockdev.c       |   18 ++++++++++++++++++
>  qapi-schema.json |   18 ++++++++++++++++++
>  qmp-commands.hx  |   23 +++++++++++++++++++++++
>  3 files changed, 59 insertions(+)
> 
>  
>  ##
> +# @block-pause-dedup:
> +#
> +# This command pause the deduplication on a device that support it.

s/support/supports/

> +#
> +# @device:   the name of the device to pause the deduplication on
> +#
> +# Returns: nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#          If @device is not deduplicated, DeviceNotDeduplicated

I don't think you need this second error.  A generic error is good
enough unless we can prove that having a dedicated error class makes
algorithmic sense for a given client, and I can't come up with such a
scenario off the top of my head for libvirt.

> +SQMP
> +block-pause-dedup
> +------------
> +
> +Pause the deduplication on a device that support it.

s/support/supports/

I notice that between this and patch 12, you are adding two very similar
commands (block-pause-dedup, block-resume-dedup); would it be any
simpler to add a single command instead:

{ 'command': 'block-dedup-control',
  'data': { 'device': 'str', 'enable': 'bool' } }

where the user calls:

{ "execute": "block-dedup-control",
  "arguments": { "device": "ide0-hd0", "enable": false } }

to pause, and "enable":true to resume?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]