Re: [Qemu-devel] [PATCH v0 2/2] qmp: add block-set-copy-on-read command

[Eric Blake]

On 06/13/2018 10:47 AM, Denis Plotnikov wrote:
> The command enables/disables copy-on-read mode for VM's disk while
> VM is running.
>
> This is needed when using external disk readers to shape access pattern
> to the disk backend.
>
> Signed-off-by: Denis Plotnikov <address@hidden>
> ---

Deferring thoughts on the actual design for later; this is just a 
cursory implementation review for now.


> +++ b/qapi/block-core.json
> @@ -4701,6 +4701,26 @@
>   { 'command': 'block-set-write-threshold',
>     'data': { 'node-name': 'str', 'write-threshold': 'uint64' } }
>   
> +##
> +# @block-set-copy-on-read:
> +#
> +# Enables and disables the copy-on-read property of a block device.
> +#
> +# @device: device or graph node name on which copy-on-read must be set.
> +#
> +# Since: 2.12

We've missed the 2.12 release.  This should be since 3.0.

> +#
> +# Example:
> +#
> +# -> { "execute": "block-set-copy-on-read",
> +#      "arguments": { "device": "scsi0-0-0-0",
> +#                     "enable": true } }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'block-set-copy-on-read',
> +  'data': { 'device': 'str', 'enable': 'bool' } }

Missing documentation of @enable.

And just checking: is the current copy-on-read setting visible through 
an existing query-* command, or have you just introduced a write-only 
toggle?  If the state can be changed at runtime, then it needs to be 
easy to learn what the current state is.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org