Re: [Qemu-devel] [PATCH 4/5] QAPI: Introduce memchar-read QMP command

[Eric Blake <eblake@redhat.com>]

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

On 09/12/2012 05:57 AM, Lei Li wrote:
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  hmp-commands.hx  |   25 +++++++++++++++++++++++++
>  hmp.c            |   18 ++++++++++++++++++
>  hmp.h            |    1 +
>  qapi-schema.json |   27 +++++++++++++++++++++++++++
>  qemu-char.c      |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   37 +++++++++++++++++++++++++++++++++++++
>  6 files changed, 156 insertions(+), 0 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -305,6 +305,33 @@
>             '*control': 'CongestionControl'} }
>  
>  ##
> +# @memchar-read:
> +#
> +# Provide read interface for CirMemCharDriver. Read from cirmemchar
> +# char device and return the data.
> +#
> +# @chardev: the name of the cirmemchar char device.
> +#
> +# @size: the size to read in bytes.
> +#
> +# @format: #optional the format of the data want to read from
> +#          CirMemCharDriver, by default is 'utf8'.
> +#
> +# @control: #optional options for read and write command that specifies
> +#           behavior when the queue is full/empty.
> +#
> +# Returns: The data read from cirmemchar as string.
> +#          If @chardev is not a valid memchr device, DeviceNotFound
> +#          If an I/O error occurs while reading, IOError
> +#
> +# Since: 1.3
> +##
> +{ 'command': 'memchar-read',
> +  'data': {'chardev': 'str', 'size': 'int', '*format': 'DataFormat',
> +           '*control': 'CongestionControl'},
> +           'returns': 'str' }

What happens if the data to be read contains embedded NUL, but the
requested 'format' can't express that?  What happens if there is less
data available than the maximum requested size?  I'm wondering if the
return should be a JSON struct, { 'data':'str', 'size':'int' }, in order
to allow for the case of short read returns.

> +- "chardev": the name of the char device, must be unique (json-string)
> +- "size": the memory size in bytes, init size of the cirmemchar
> +          by default (json-int)
> +- "format": the data format write to CirMemCharDriver, default is
> +            utf8. (json-string, optional)
> +          - Possible values: "utf8", "base64"

Also, you probably want to make it crystal-clear whether size is
referring to the unencoded size of the raw data, or the encoded size
after conversion to utf8 or base64.

-- 
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: 617 bytes --]