[4/5] QAPI: Introduce memchar-read QMP command

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(-)

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.

On Wed, Sep 12, 2012 at 11:57 AM, Lei Li <lilei@linux.vnet.ibm.com> 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(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index fe11926..1d2fccc 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -816,6 +816,31 @@ Support 'utf8' and 'base64', by default is 'utf8'.
>  @var{control} is option('block', 'drop') for read and write command
>  that specifies behavior when the queue is full/empty. By default is
>  'drop'. Note that the 'block' option is not supported now.
> +
> +ETEXI
> +
> +    {
> +        .name       = "memchar_read",
> +        .args_type  = "chardev:s,size:i,format:s?,control:s?",
> +        .params     = "chardev size [format] [control]",
> +        .help       = "Provide read interface for CirMemCharDriver. Read from"
> +                      "it and return 'size' of the data",
> +        .mhandler.cmd = hmp_memchar_read,
> +    },
> +
> +STEXI
> +@item memchar_read @var{chardev} @var{size}
> +@findex memchar_read
> +Provide read interface for CirMemCharDriver. Read from cirmemchr
> +char device and return @var{size} of the data.
> +
> +@var{format} is the format of the data read from CirMemCharDriver.
> +Support 'utf8' and 'base64', by default is 'utf8'.
> +
> +@var{control} is option['block', 'drop'] for read and write command
> +that specifies behavior when the queue is full/empty. By default is
> +'drop'. Note that the 'block' option is not supported now.
> +
>  ETEXI
>
>      {
> diff --git a/hmp.c b/hmp.c
> index 97f5058..4397981 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -690,6 +690,24 @@ void hmp_memchar_write(Monitor *mon, const QDict *qdict)
>      hmp_handle_error(mon, &errp);
>  }
>
> +void hmp_memchar_read(Monitor *mon, const QDict *qdict)
> +{
> +    uint32_t size = qdict_get_int(qdict, "size");
> +    const char *chardev = qdict_get_str(qdict, "chardev");
> +    char *data;
> +    int val = qdict_get_try_bool(qdict, "base64", 0);
> +    enum DataFormat format;
> +    int con = qdict_get_try_bool(qdict, "block", 0);
> +    enum CongestionControl control;
> +    Error *errp = NULL;
> +
> +    format = val ? DATA_FORMAT_BASE64 : DATA_FORMAT_UTF8;
> +    control = con ? CONGESTION_CONTROL_BLOCK : CONGESTION_CONTROL_DROP;
> +    data = qmp_memchar_read(chardev, size, true, format,
> +                            true, control, &errp);
> +    monitor_printf(mon, "%s\n", data);
> +}
> +
>  static void hmp_cont_cb(void *opaque, int err)
>  {
>      if (!err) {
> diff --git a/hmp.h b/hmp.h
> index 44b6463..ed2cda4 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -44,6 +44,7 @@ void hmp_cpu(Monitor *mon, const QDict *qdict);
>  void hmp_memsave(Monitor *mon, const QDict *qdict);
>  void hmp_pmemsave(Monitor *mon, const QDict *qdict);
>  void hmp_memchar_write(Monitor *mon, const QDict *qdict);
> +void hmp_memchar_read(Monitor *mon, const QDict *qdict);
>  void hmp_cont(Monitor *mon, const QDict *qdict);
>  void hmp_system_wakeup(Monitor *mon, const QDict *qdict);
>  void hmp_inject_nmi(Monitor *mon, const QDict *qdict);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 371239a..5274b86 100644
> --- a/qapi-schema.json
> +++ 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' }
> +
> +##
>  # @CommandInfo:
>  #
>  # Information about a QMP command
> diff --git a/qemu-char.c b/qemu-char.c
> index be1d79a..bb3ddb9 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2748,6 +2748,54 @@ void qmp_memchar_write(const char *chardev, int64_t size,
>      g_free(write_data);
>  }
>
> +char *qmp_memchar_read(const char *chardev, int64_t size,
> +                       bool has_format, enum DataFormat format,
> +                       bool has_control, enum CongestionControl control,
> +                       Error **errp)
> +{
> +    CharDriverState *chr;
> +    guchar *read_data;
> +    char *data = NULL;
> +
> +    read_data = g_malloc0(size);
> +
> +    chr = qemu_chr_find(chardev);
> +    if(!chr) {

Space between 'if' and '('

> +        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
> +        return NULL;
> +    }
> +
> +    /* XXX: For the sync command as 'block', waiting for the qmp
> + *  *      * to support necessary feature. Now just act as 'drop'. */

Wrong indentation, * * looks spurious.

> +    if (cirmem_chr_is_empty(chr)) {
> +        if (has_control && (control == CONGESTION_CONTROL_BLOCK)) {
> +            g_free(data);
> +            return NULL;
> +        } else {
> +            error_setg(errp, "Failed to read from memchr %s", chardev);
> +            return NULL;
> +        }
> +    }
> +
> +    if (size == 0) {
> +        size = CBUFF_SIZE;
> +    }
> +
> +    cirmem_chr_read(chr, read_data, size);
> +
> +    if (has_format && (format == DATA_FORMAT_BASE64)) {
> +       if (read_data) {
> +           data = g_base64_encode(read_data, (size_t)size);
> +       }
> +    } else {
> +        data = (char *)read_data;
> +    }
> +
> +    g_free(read_data);
> +
> +    return data;
> +}
> +
>  QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
>  {
>      char host[65], port[33], width[8], height[8];
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 5a1b86b..fab3e4e 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -505,6 +505,43 @@ Example:
>  EQMP
>
>      {
> +        .name       = "memchar-read",
> +        .args_type  = "chardev:s,size:i",
> +        .mhandler.cmd_new = qmp_marshal_input_memchar_read,
> +    },
> +
> +SQMP
> +memchar-read
> +-------------
> +
> +Provide read interface for CirMemCharDriver. Read from cirmemchar
> +char device and return the data.
> +
> +Arguments:
> +
> +- "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"
> +
> +- "control": options for read and write command that specifies
> +             behavior when the queue is full/empty, default is
> +             drop. (json-string, optional)
> +          - Possible values: "drop", "block"
> +
> +Example:
> +
> +-> { "execute": "memchar-read",
> +                "arguments": { "chardev": foo,
> +                               "size": 1000,
> +                               "format": "utf8" }
> +<- { "return": "data string..." }
> +
> +EQMP
> +
> +    {
>          .name       = "xen-save-devices-state",
>          .args_type  = "filename:F",
>      .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,
> --
> 1.7.7.6
>
>