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

login
register
mail settings
Submitter Lei Li
Date Oct. 26, 2012, 11:21 a.m.
Message ID <1351250512-6386-4-git-send-email-lilei@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/194444/
State New
Headers show

Comments

Lei Li - Oct. 26, 2012, 11:21 a.m.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 hmp-commands.hx  |   19 ++++++++++++++++++
 hmp.c            |   19 ++++++++++++++++++
 hmp.h            |    1 +
 qapi-schema.json |   27 ++++++++++++++++++++++++++
 qemu-char.c      |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   40 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 161 insertions(+), 0 deletions(-)
Luiz Capitulino - Oct. 26, 2012, 5:39 p.m.
On Fri, 26 Oct 2012 19:21:51 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  hmp-commands.hx  |   19 ++++++++++++++++++
>  hmp.c            |   19 ++++++++++++++++++
>  hmp.h            |    1 +
>  qapi-schema.json |   27 ++++++++++++++++++++++++++
>  qemu-char.c      |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   40 +++++++++++++++++++++++++++++++++++++++
>  6 files changed, 161 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index a37b8e9..df294eb 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -842,6 +842,25 @@ is full/empty, for now just assume a drop behaver in these two commands.
>  ETEXI
>  
>      {
> +        .name       = "memchar_read",
> +        .args_type  = "chardev:s,size:i",
> +        .params     = "chardev size",
> +        .mhandler.cmd = hmp_memchar_read,
> +    },
> +
> +STEXI
> +@item memchar_read @var{chardev}
> +@findex memchar_read
> +Provide read interface for CirMemCharDriver. Read from cirmemchr
> +char device and return @var{size} of the data.
> +
> +@var{size} is the size of data want to read from. Refer to unencoded
> +size of the raw data, would adjust to the init size of the memchar
> +if the requested size is larger than it.
> +
> +ETEXI
> +
> +    {
>          .name       = "migrate",
>          .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
>          .params     = "[-d] [-b] [-i] uri",
> diff --git a/hmp.c b/hmp.c
> index 082985b..ef85736 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -698,6 +698,25 @@ 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;
> +    enum DataFormat format;

You don't need this variable.

> +    Error *errp = NULL;
> +
> +    format = DATA_FORMAT_UTF8;
> +    data = qmp_memchar_read(chardev, size, true, format, &errp);
> +    if (errp) {
> +        monitor_printf(mon, "%s\n", error_get_pretty(errp));
> +        error_free(errp);
> +        return;
> +    }
> +
> +    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 406ebb1..a5a0cfe 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 43ef6bc..a8c9430 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -372,6 +372,33 @@
>             '*format': 'DataFormat'} }
>  
>  ##
> +# @memchar-read:
> +#
> +# Provide read interface for memchardev. Read from memchar
> +# char device and return the data.
> +#
> +# @chardev: the name of the memchar char device.
> +#
> +# @size: the size to read in bytes.
> +#
> +# @format: #optional the format of the data want to read from
> +#          memchardev, by default is 'utf8'.
> +#
> +# Returns: The data read from memchar as string
> +#          If @chardev is not a valid memchr device, DeviceNotFound
> +#
> +# Notes: The option 'block' is not supported now due to the miss
> +#        feature in qmp. Will add it later when we gain the necessary
> +#        infrastructure enhancement. For now just assume 'drop' behaver
> +#        for this command.

Please, replace this note with an explanation of the current behavior. No
need to talk about the future.

> +#
> +# Since: 1.3
> +##
> +{ 'command': 'memchar-read',
> +  'data': {'chardev': 'str', 'size': 'int', '*format': 'DataFormat'},
> +  'returns': 'str' }
> +
> +##
>  # @CommandInfo:
>  #
>  # Information about a QMP command
> diff --git a/qemu-char.c b/qemu-char.c
> index 6114e29..cf88f71 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2761,6 +2761,61 @@ void qmp_memchar_write(const char *chardev, int64_t size,
>      }
>  }
>  
> +char *qmp_memchar_read(const char *chardev, int64_t size,
> +                       bool has_format, enum DataFormat format,
> +                       Error **errp)
> +{
> +    CharDriverState *chr;
> +    guchar *read_data;
> +    char *data = NULL;
> +    int ret;
> +
> +    read_data = g_malloc0(size);

This is unsafe, as qmp clients could pass any value here. You should check
the number of available bytes and allocate only that.

> +
> +    chr = qemu_chr_find(chardev);
> +    if (!chr) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
> +        goto fail;
> +    }
> +
> +    if (strcmp(chr->filename, "memchr") != 0) {
> +        error_setg(errp,"The %s is not memory char device\n", chardev);
> +        goto fail;
> +    }

The same comment I made for the write operation applies here.

> +
> +    /* XXX: For the sync command as 'block', waiting for the qmp
> +     * to support necessary feature. Now just act as 'drop'. */

Here too.

> +    if (cirmem_chr_is_empty(chr)) {
> +        error_setg(errp, "Failed to read from memchr %s", chardev);
> +        goto fail;
> +    }
> +
> +    if (size == 0) {
> +        size = CBUFF_SIZE;
> +    }

IMO, you should refuse size=0.

> +
> +    ret = cirmem_chr_read(chr, read_data, size);
> +
> +    if (ret < 0) {
> +        error_setg(errp, "Failed to read from memchr %s", chardev);
> +        goto fail;
> +    }
> +
> +    if (has_format && (format == DATA_FORMAT_BASE64)) {
> +       if (read_data) {
> +           data = g_base64_encode(read_data, (size_t)size);
> +       }
> +    } else {
> +        data = (char *)read_data;
> +    }
> +
> +    return data;
> +
> +fail:
> +    g_free(read_data);
> +    return NULL;
> +}
> +
>  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 7548b9b..7729fb0 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -500,6 +500,46 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "memchar-read",
> +        .args_type  = "chardev:s,size:i,format:s?",
> +        .help       = "return the size of data from memchar chardev",
> +        .mhandler.cmd_new = qmp_marshal_input_memchar_read,
> +    },
> +
> +SQMP
> +memchar-read
> +-------------
> +
> +Provide read interface for memchardev. Read from memchar
> +char device and return the data.
> +
> +Arguments:
> +
> +- "chardev": the name of the char device, must be unique (json-string)
> +- "size": the memory size wanted to read in bytes(refer to unencoded
> +          size of the raw data), would adjust to the init size of the
> +          memchar if the requested size is larger than it. (json-int)
> +- "format": the data format write to memchardev, default is
> +            utf8. (json-string, optional)
> +          - Possible values: "utf8", "base64"
> +
> +Example:
> +
> +-> { "execute": "memchar-read",
> +                "arguments": { "chardev": foo,
> +                               "size": 1000,
> +                               "format": "utf8" } }
> +<- { "return": "data string..." }
> +
> +Notes:
> +
> +We will add 'control' options for read and write command that specifies
> +behavior when the queue is full/empty, for now just assume a drop
> +behaver in these two commands.

Please drop this.

> +
> +EQMP
> +
> +    {
>          .name       = "xen-save-devices-state",
>          .args_type  = "filename:F",
>      .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,
Lei Li - Oct. 29, 2012, 4:09 a.m.
On 10/27/2012 01:39 AM, Luiz Capitulino wrote:
> On Fri, 26 Oct 2012 19:21:51 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>> ---
>>   hmp-commands.hx  |   19 ++++++++++++++++++
>>   hmp.c            |   19 ++++++++++++++++++
>>   hmp.h            |    1 +
>>   qapi-schema.json |   27 ++++++++++++++++++++++++++
>>   qemu-char.c      |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qmp-commands.hx  |   40 +++++++++++++++++++++++++++++++++++++++
>>   6 files changed, 161 insertions(+), 0 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index a37b8e9..df294eb 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -842,6 +842,25 @@ is full/empty, for now just assume a drop behaver in these two commands.
>>   ETEXI
>>   
>>       {
>> +        .name       = "memchar_read",
>> +        .args_type  = "chardev:s,size:i",
>> +        .params     = "chardev size",
>> +        .mhandler.cmd = hmp_memchar_read,
>> +    },
>> +
>> +STEXI
>> +@item memchar_read @var{chardev}
>> +@findex memchar_read
>> +Provide read interface for CirMemCharDriver. Read from cirmemchr
>> +char device and return @var{size} of the data.
>> +
>> +@var{size} is the size of data want to read from. Refer to unencoded
>> +size of the raw data, would adjust to the init size of the memchar
>> +if the requested size is larger than it.
>> +
>> +ETEXI
>> +
>> +    {
>>           .name       = "migrate",
>>           .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
>>           .params     = "[-d] [-b] [-i] uri",
>> diff --git a/hmp.c b/hmp.c
>> index 082985b..ef85736 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -698,6 +698,25 @@ 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;
>> +    enum DataFormat format;
> You don't need this variable.

ok.

>
>> +    Error *errp = NULL;
>> +
>> +    format = DATA_FORMAT_UTF8;
>> +    data = qmp_memchar_read(chardev, size, true, format, &errp);
>> +    if (errp) {
>> +        monitor_printf(mon, "%s\n", error_get_pretty(errp));
>> +        error_free(errp);
>> +        return;
>> +    }
>> +
>> +    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 406ebb1..a5a0cfe 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 43ef6bc..a8c9430 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -372,6 +372,33 @@
>>              '*format': 'DataFormat'} }
>>   
>>   ##
>> +# @memchar-read:
>> +#
>> +# Provide read interface for memchardev. Read from memchar
>> +# char device and return the data.
>> +#
>> +# @chardev: the name of the memchar char device.
>> +#
>> +# @size: the size to read in bytes.
>> +#
>> +# @format: #optional the format of the data want to read from
>> +#          memchardev, by default is 'utf8'.
>> +#
>> +# Returns: The data read from memchar as string
>> +#          If @chardev is not a valid memchr device, DeviceNotFound
>> +#
>> +# Notes: The option 'block' is not supported now due to the miss
>> +#        feature in qmp. Will add it later when we gain the necessary
>> +#        infrastructure enhancement. For now just assume 'drop' behaver
>> +#        for this command.
> Please, replace this note with an explanation of the current behavior. No
> need to talk about the future.

ok.

>
>> +#
>> +# Since: 1.3
>> +##
>> +{ 'command': 'memchar-read',
>> +  'data': {'chardev': 'str', 'size': 'int', '*format': 'DataFormat'},
>> +  'returns': 'str' }
>> +
>> +##
>>   # @CommandInfo:
>>   #
>>   # Information about a QMP command
>> diff --git a/qemu-char.c b/qemu-char.c
>> index 6114e29..cf88f71 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -2761,6 +2761,61 @@ void qmp_memchar_write(const char *chardev, int64_t size,
>>       }
>>   }
>>   
>> +char *qmp_memchar_read(const char *chardev, int64_t size,
>> +                       bool has_format, enum DataFormat format,
>> +                       Error **errp)
>> +{
>> +    CharDriverState *chr;
>> +    guchar *read_data;
>> +    char *data = NULL;
>> +    int ret;
>> +
>> +    read_data = g_malloc0(size);
> This is unsafe, as qmp clients could pass any value here. You should check
> the number of available bytes and allocate only that.

This size is not the init size of ring buffer, it's the size user want to read
from the cirmem backend. And it'll be checked within cirmem_chr_write.

>
>> +
>> +    chr = qemu_chr_find(chardev);
>> +    if (!chr) {
>> +        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
>> +        goto fail;
>> +    }
>> +
>> +    if (strcmp(chr->filename, "memchr") != 0) {
>> +        error_setg(errp,"The %s is not memory char device\n", chardev);
>> +        goto fail;
>> +    }
> The same comment I made for the write operation applies here.
>
>> +
>> +    /* XXX: For the sync command as 'block', waiting for the qmp
>> +     * to support necessary feature. Now just act as 'drop'. */
> Here too.
>
>> +    if (cirmem_chr_is_empty(chr)) {
>> +        error_setg(errp, "Failed to read from memchr %s", chardev);
>> +        goto fail;
>> +    }
>> +
>> +    if (size == 0) {
>> +        size = CBUFF_SIZE;
>> +    }
> IMO, you should refuse size=0.

Do you think it's better to refuse it than giving a default size?

>
>> +
>> +    ret = cirmem_chr_read(chr, read_data, size);
>> +
>> +    if (ret < 0) {
>> +        error_setg(errp, "Failed to read from memchr %s", chardev);
>> +        goto fail;
>> +    }
>> +
>> +    if (has_format && (format == DATA_FORMAT_BASE64)) {
>> +       if (read_data) {
>> +           data = g_base64_encode(read_data, (size_t)size);
>> +       }
>> +    } else {
>> +        data = (char *)read_data;
>> +    }
>> +
>> +    return data;
>> +
>> +fail:
>> +    g_free(read_data);
>> +    return NULL;
>> +}
>> +
>>   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 7548b9b..7729fb0 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -500,6 +500,46 @@ Example:
>>   EQMP
>>   
>>       {
>> +        .name       = "memchar-read",
>> +        .args_type  = "chardev:s,size:i,format:s?",
>> +        .help       = "return the size of data from memchar chardev",
>> +        .mhandler.cmd_new = qmp_marshal_input_memchar_read,
>> +    },
>> +
>> +SQMP
>> +memchar-read
>> +-------------
>> +
>> +Provide read interface for memchardev. Read from memchar
>> +char device and return the data.
>> +
>> +Arguments:
>> +
>> +- "chardev": the name of the char device, must be unique (json-string)
>> +- "size": the memory size wanted to read in bytes(refer to unencoded
>> +          size of the raw data), would adjust to the init size of the
>> +          memchar if the requested size is larger than it. (json-int)
>> +- "format": the data format write to memchardev, default is
>> +            utf8. (json-string, optional)
>> +          - Possible values: "utf8", "base64"
>> +
>> +Example:
>> +
>> +-> { "execute": "memchar-read",
>> +                "arguments": { "chardev": foo,
>> +                               "size": 1000,
>> +                               "format": "utf8" } }
>> +<- { "return": "data string..." }
>> +
>> +Notes:
>> +
>> +We will add 'control' options for read and write command that specifies
>> +behavior when the queue is full/empty, for now just assume a drop
>> +behaver in these two commands.
> Please drop this.

Sure.

>
>> +
>> +EQMP
>> +
>> +    {
>>           .name       = "xen-save-devices-state",
>>           .args_type  = "filename:F",
>>       .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,
>
Luiz Capitulino - Oct. 29, 2012, 1:17 p.m.
On Mon, 29 Oct 2012 12:09:38 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> On 10/27/2012 01:39 AM, Luiz Capitulino wrote:
> > On Fri, 26 Oct 2012 19:21:51 +0800
> > Lei Li <lilei@linux.vnet.ibm.com> wrote:
> >
> >> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> >> ---
> >>   hmp-commands.hx  |   19 ++++++++++++++++++
> >>   hmp.c            |   19 ++++++++++++++++++
> >>   hmp.h            |    1 +
> >>   qapi-schema.json |   27 ++++++++++++++++++++++++++
> >>   qemu-char.c      |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>   qmp-commands.hx  |   40 +++++++++++++++++++++++++++++++++++++++
> >>   6 files changed, 161 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index a37b8e9..df294eb 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -842,6 +842,25 @@ is full/empty, for now just assume a drop behaver in these two commands.
> >>   ETEXI
> >>   
> >>       {
> >> +        .name       = "memchar_read",
> >> +        .args_type  = "chardev:s,size:i",
> >> +        .params     = "chardev size",
> >> +        .mhandler.cmd = hmp_memchar_read,
> >> +    },
> >> +
> >> +STEXI
> >> +@item memchar_read @var{chardev}
> >> +@findex memchar_read
> >> +Provide read interface for CirMemCharDriver. Read from cirmemchr
> >> +char device and return @var{size} of the data.
> >> +
> >> +@var{size} is the size of data want to read from. Refer to unencoded
> >> +size of the raw data, would adjust to the init size of the memchar
> >> +if the requested size is larger than it.
> >> +
> >> +ETEXI
> >> +
> >> +    {
> >>           .name       = "migrate",
> >>           .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
> >>           .params     = "[-d] [-b] [-i] uri",
> >> diff --git a/hmp.c b/hmp.c
> >> index 082985b..ef85736 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -698,6 +698,25 @@ 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;
> >> +    enum DataFormat format;
> > You don't need this variable.
> 
> ok.
> 
> >
> >> +    Error *errp = NULL;
> >> +
> >> +    format = DATA_FORMAT_UTF8;
> >> +    data = qmp_memchar_read(chardev, size, true, format, &errp);
> >> +    if (errp) {
> >> +        monitor_printf(mon, "%s\n", error_get_pretty(errp));
> >> +        error_free(errp);
> >> +        return;
> >> +    }
> >> +
> >> +    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 406ebb1..a5a0cfe 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 43ef6bc..a8c9430 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -372,6 +372,33 @@
> >>              '*format': 'DataFormat'} }
> >>   
> >>   ##
> >> +# @memchar-read:
> >> +#
> >> +# Provide read interface for memchardev. Read from memchar
> >> +# char device and return the data.
> >> +#
> >> +# @chardev: the name of the memchar char device.
> >> +#
> >> +# @size: the size to read in bytes.
> >> +#
> >> +# @format: #optional the format of the data want to read from
> >> +#          memchardev, by default is 'utf8'.
> >> +#
> >> +# Returns: The data read from memchar as string
> >> +#          If @chardev is not a valid memchr device, DeviceNotFound
> >> +#
> >> +# Notes: The option 'block' is not supported now due to the miss
> >> +#        feature in qmp. Will add it later when we gain the necessary
> >> +#        infrastructure enhancement. For now just assume 'drop' behaver
> >> +#        for this command.
> > Please, replace this note with an explanation of the current behavior. No
> > need to talk about the future.
> 
> ok.
> 
> >
> >> +#
> >> +# Since: 1.3
> >> +##
> >> +{ 'command': 'memchar-read',
> >> +  'data': {'chardev': 'str', 'size': 'int', '*format': 'DataFormat'},
> >> +  'returns': 'str' }
> >> +
> >> +##
> >>   # @CommandInfo:
> >>   #
> >>   # Information about a QMP command
> >> diff --git a/qemu-char.c b/qemu-char.c
> >> index 6114e29..cf88f71 100644
> >> --- a/qemu-char.c
> >> +++ b/qemu-char.c
> >> @@ -2761,6 +2761,61 @@ void qmp_memchar_write(const char *chardev, int64_t size,
> >>       }
> >>   }
> >>   
> >> +char *qmp_memchar_read(const char *chardev, int64_t size,
> >> +                       bool has_format, enum DataFormat format,
> >> +                       Error **errp)
> >> +{
> >> +    CharDriverState *chr;
> >> +    guchar *read_data;
> >> +    char *data = NULL;
> >> +    int ret;
> >> +
> >> +    read_data = g_malloc0(size);
> > This is unsafe, as qmp clients could pass any value here. You should check
> > the number of available bytes and allocate only that.
> 
> This size is not the init size of ring buffer, it's the size user want to read
> from the cirmem backend.

Yes, it's a temporary buffer and you allocate the exact value passed
by the user. What if s/he passes 2G?

My suggestion is to limit it to the amount of bytes available in the
circular buffer.

> And it'll be checked within cirmem_chr_write.

Not sure I get what you mean.

> 
> >
> >> +
> >> +    chr = qemu_chr_find(chardev);
> >> +    if (!chr) {
> >> +        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
> >> +        goto fail;
> >> +    }
> >> +
> >> +    if (strcmp(chr->filename, "memchr") != 0) {
> >> +        error_setg(errp,"The %s is not memory char device\n", chardev);
> >> +        goto fail;
> >> +    }
> > The same comment I made for the write operation applies here.
> >
> >> +
> >> +    /* XXX: For the sync command as 'block', waiting for the qmp
> >> +     * to support necessary feature. Now just act as 'drop'. */
> > Here too.
> >
> >> +    if (cirmem_chr_is_empty(chr)) {
> >> +        error_setg(errp, "Failed to read from memchr %s", chardev);
> >> +        goto fail;
> >> +    }
> >> +
> >> +    if (size == 0) {
> >> +        size = CBUFF_SIZE;
> >> +    }
> > IMO, you should refuse size=0.
> 
> Do you think it's better to refuse it than giving a default size?

Down QMP yes.

> 
> >
> >> +
> >> +    ret = cirmem_chr_read(chr, read_data, size);
> >> +
> >> +    if (ret < 0) {
> >> +        error_setg(errp, "Failed to read from memchr %s", chardev);
> >> +        goto fail;
> >> +    }
> >> +
> >> +    if (has_format && (format == DATA_FORMAT_BASE64)) {
> >> +       if (read_data) {
> >> +           data = g_base64_encode(read_data, (size_t)size);
> >> +       }
> >> +    } else {
> >> +        data = (char *)read_data;
> >> +    }
> >> +
> >> +    return data;
> >> +
> >> +fail:
> >> +    g_free(read_data);
> >> +    return NULL;
> >> +}
> >> +
> >>   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 7548b9b..7729fb0 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -500,6 +500,46 @@ Example:
> >>   EQMP
> >>   
> >>       {
> >> +        .name       = "memchar-read",
> >> +        .args_type  = "chardev:s,size:i,format:s?",
> >> +        .help       = "return the size of data from memchar chardev",
> >> +        .mhandler.cmd_new = qmp_marshal_input_memchar_read,
> >> +    },
> >> +
> >> +SQMP
> >> +memchar-read
> >> +-------------
> >> +
> >> +Provide read interface for memchardev. Read from memchar
> >> +char device and return the data.
> >> +
> >> +Arguments:
> >> +
> >> +- "chardev": the name of the char device, must be unique (json-string)
> >> +- "size": the memory size wanted to read in bytes(refer to unencoded
> >> +          size of the raw data), would adjust to the init size of the
> >> +          memchar if the requested size is larger than it. (json-int)
> >> +- "format": the data format write to memchardev, default is
> >> +            utf8. (json-string, optional)
> >> +          - Possible values: "utf8", "base64"
> >> +
> >> +Example:
> >> +
> >> +-> { "execute": "memchar-read",
> >> +                "arguments": { "chardev": foo,
> >> +                               "size": 1000,
> >> +                               "format": "utf8" } }
> >> +<- { "return": "data string..." }
> >> +
> >> +Notes:
> >> +
> >> +We will add 'control' options for read and write command that specifies
> >> +behavior when the queue is full/empty, for now just assume a drop
> >> +behaver in these two commands.
> > Please drop this.
> 
> Sure.
> 
> >
> >> +
> >> +EQMP
> >> +
> >> +    {
> >>           .name       = "xen-save-devices-state",
> >>           .args_type  = "filename:F",
> >>       .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,
> >
> 
>
Lei Li - Oct. 30, 2012, 10:22 a.m.
On 10/29/2012 09:17 PM, Luiz Capitulino wrote:
> On Mon, 29 Oct 2012 12:09:38 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> On 10/27/2012 01:39 AM, Luiz Capitulino wrote:
>>> On Fri, 26 Oct 2012 19:21:51 +0800
>>> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>>>
>>>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>>>> ---
>>>>    hmp-commands.hx  |   19 ++++++++++++++++++
>>>>    hmp.c            |   19 ++++++++++++++++++
>>>>    hmp.h            |    1 +
>>>>    qapi-schema.json |   27 ++++++++++++++++++++++++++
>>>>    qemu-char.c      |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    qmp-commands.hx  |   40 +++++++++++++++++++++++++++++++++++++++
>>>>    6 files changed, 161 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>>> index a37b8e9..df294eb 100644
>>>> --- a/hmp-commands.hx
>>>> +++ b/hmp-commands.hx
>>>> @@ -842,6 +842,25 @@ is full/empty, for now just assume a drop behaver in these two commands.
>>>>    ETEXI
>>>>    
>>>>        {
>>>> +        .name       = "memchar_read",
>>>> +        .args_type  = "chardev:s,size:i",
>>>> +        .params     = "chardev size",
>>>> +        .mhandler.cmd = hmp_memchar_read,
>>>> +    },
>>>> +
>>>> +STEXI
>>>> +@item memchar_read @var{chardev}
>>>> +@findex memchar_read
>>>> +Provide read interface for CirMemCharDriver. Read from cirmemchr
>>>> +char device and return @var{size} of the data.
>>>> +
>>>> +@var{size} is the size of data want to read from. Refer to unencoded
>>>> +size of the raw data, would adjust to the init size of the memchar
>>>> +if the requested size is larger than it.
>>>> +
>>>> +ETEXI
>>>> +
>>>> +    {
>>>>            .name       = "migrate",
>>>>            .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
>>>>            .params     = "[-d] [-b] [-i] uri",
>>>> diff --git a/hmp.c b/hmp.c
>>>> index 082985b..ef85736 100644
>>>> --- a/hmp.c
>>>> +++ b/hmp.c
>>>> @@ -698,6 +698,25 @@ 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;
>>>> +    enum DataFormat format;
>>> You don't need this variable.
>> ok.
>>
>>>> +    Error *errp = NULL;
>>>> +
>>>> +    format = DATA_FORMAT_UTF8;
>>>> +    data = qmp_memchar_read(chardev, size, true, format, &errp);
>>>> +    if (errp) {
>>>> +        monitor_printf(mon, "%s\n", error_get_pretty(errp));
>>>> +        error_free(errp);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    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 406ebb1..a5a0cfe 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 43ef6bc..a8c9430 100644
>>>> --- a/qapi-schema.json
>>>> +++ b/qapi-schema.json
>>>> @@ -372,6 +372,33 @@
>>>>               '*format': 'DataFormat'} }
>>>>    
>>>>    ##
>>>> +# @memchar-read:
>>>> +#
>>>> +# Provide read interface for memchardev. Read from memchar
>>>> +# char device and return the data.
>>>> +#
>>>> +# @chardev: the name of the memchar char device.
>>>> +#
>>>> +# @size: the size to read in bytes.
>>>> +#
>>>> +# @format: #optional the format of the data want to read from
>>>> +#          memchardev, by default is 'utf8'.
>>>> +#
>>>> +# Returns: The data read from memchar as string
>>>> +#          If @chardev is not a valid memchr device, DeviceNotFound
>>>> +#
>>>> +# Notes: The option 'block' is not supported now due to the miss
>>>> +#        feature in qmp. Will add it later when we gain the necessary
>>>> +#        infrastructure enhancement. For now just assume 'drop' behaver
>>>> +#        for this command.
>>> Please, replace this note with an explanation of the current behavior. No
>>> need to talk about the future.
>> ok.
>>
>>>> +#
>>>> +# Since: 1.3
>>>> +##
>>>> +{ 'command': 'memchar-read',
>>>> +  'data': {'chardev': 'str', 'size': 'int', '*format': 'DataFormat'},
>>>> +  'returns': 'str' }
>>>> +
>>>> +##
>>>>    # @CommandInfo:
>>>>    #
>>>>    # Information about a QMP command
>>>> diff --git a/qemu-char.c b/qemu-char.c
>>>> index 6114e29..cf88f71 100644
>>>> --- a/qemu-char.c
>>>> +++ b/qemu-char.c
>>>> @@ -2761,6 +2761,61 @@ void qmp_memchar_write(const char *chardev, int64_t size,
>>>>        }
>>>>    }
>>>>    
>>>> +char *qmp_memchar_read(const char *chardev, int64_t size,
>>>> +                       bool has_format, enum DataFormat format,
>>>> +                       Error **errp)
>>>> +{
>>>> +    CharDriverState *chr;
>>>> +    guchar *read_data;
>>>> +    char *data = NULL;
>>>> +    int ret;
>>>> +
>>>> +    read_data = g_malloc0(size);
>>> This is unsafe, as qmp clients could pass any value here. You should check
>>> the number of available bytes and allocate only that.
>> This size is not the init size of ring buffer, it's the size user want to read
>> from the cirmem backend.
> Yes, it's a temporary buffer and you allocate the exact value passed
> by the user. What if s/he passes 2G?
>
> My suggestion is to limit it to the amount of bytes available in the
> circular buffer.

Done.

>> And it'll be checked within cirmem_chr_write.
> Not sure I get what you mean.
>
>>>> +
>>>> +    chr = qemu_chr_find(chardev);
>>>> +    if (!chr) {
>>>> +        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
>>>> +        goto fail;
>>>> +    }
>>>> +
>>>> +    if (strcmp(chr->filename, "memchr") != 0) {
>>>> +        error_setg(errp,"The %s is not memory char device\n", chardev);
>>>> +        goto fail;
>>>> +    }
>>> The same comment I made for the write operation applies here.
>>>
>>>> +
>>>> +    /* XXX: For the sync command as 'block', waiting for the qmp
>>>> +     * to support necessary feature. Now just act as 'drop'. */
>>> Here too.
>>>
>>>> +    if (cirmem_chr_is_empty(chr)) {
>>>> +        error_setg(errp, "Failed to read from memchr %s", chardev);
>>>> +        goto fail;
>>>> +    }
>>>> +
>>>> +    if (size == 0) {
>>>> +        size = CBUFF_SIZE;
>>>> +    }
>>> IMO, you should refuse size=0.
>> Do you think it's better to refuse it than giving a default size?
> Down QMP yes.

OK, done.

>>>> +
>>>> +    ret = cirmem_chr_read(chr, read_data, size);
>>>> +
>>>> +    if (ret < 0) {
>>>> +        error_setg(errp, "Failed to read from memchr %s", chardev);
>>>> +        goto fail;
>>>> +    }
>>>> +
>>>> +    if (has_format && (format == DATA_FORMAT_BASE64)) {
>>>> +       if (read_data) {
>>>> +           data = g_base64_encode(read_data, (size_t)size);
>>>> +       }
>>>> +    } else {
>>>> +        data = (char *)read_data;
>>>> +    }
>>>> +
>>>> +    return data;
>>>> +
>>>> +fail:
>>>> +    g_free(read_data);
>>>> +    return NULL;
>>>> +}
>>>> +
>>>>    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 7548b9b..7729fb0 100644
>>>> --- a/qmp-commands.hx
>>>> +++ b/qmp-commands.hx
>>>> @@ -500,6 +500,46 @@ Example:
>>>>    EQMP
>>>>    
>>>>        {
>>>> +        .name       = "memchar-read",
>>>> +        .args_type  = "chardev:s,size:i,format:s?",
>>>> +        .help       = "return the size of data from memchar chardev",
>>>> +        .mhandler.cmd_new = qmp_marshal_input_memchar_read,
>>>> +    },
>>>> +
>>>> +SQMP
>>>> +memchar-read
>>>> +-------------
>>>> +
>>>> +Provide read interface for memchardev. Read from memchar
>>>> +char device and return the data.
>>>> +
>>>> +Arguments:
>>>> +
>>>> +- "chardev": the name of the char device, must be unique (json-string)
>>>> +- "size": the memory size wanted to read in bytes(refer to unencoded
>>>> +          size of the raw data), would adjust to the init size of the
>>>> +          memchar if the requested size is larger than it. (json-int)
>>>> +- "format": the data format write to memchardev, default is
>>>> +            utf8. (json-string, optional)
>>>> +          - Possible values: "utf8", "base64"
>>>> +
>>>> +Example:
>>>> +
>>>> +-> { "execute": "memchar-read",
>>>> +                "arguments": { "chardev": foo,
>>>> +                               "size": 1000,
>>>> +                               "format": "utf8" } }
>>>> +<- { "return": "data string..." }
>>>> +
>>>> +Notes:
>>>> +
>>>> +We will add 'control' options for read and write command that specifies
>>>> +behavior when the queue is full/empty, for now just assume a drop
>>>> +behaver in these two commands.
>>> Please drop this.
>> Sure.
>>
>>>> +
>>>> +EQMP
>>>> +
>>>> +    {
>>>>            .name       = "xen-save-devices-state",
>>>>            .args_type  = "filename:F",
>>>>        .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,
>>

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index a37b8e9..df294eb 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -842,6 +842,25 @@  is full/empty, for now just assume a drop behaver in these two commands.
 ETEXI
 
     {
+        .name       = "memchar_read",
+        .args_type  = "chardev:s,size:i",
+        .params     = "chardev size",
+        .mhandler.cmd = hmp_memchar_read,
+    },
+
+STEXI
+@item memchar_read @var{chardev}
+@findex memchar_read
+Provide read interface for CirMemCharDriver. Read from cirmemchr
+char device and return @var{size} of the data.
+
+@var{size} is the size of data want to read from. Refer to unencoded
+size of the raw data, would adjust to the init size of the memchar
+if the requested size is larger than it.
+
+ETEXI
+
+    {
         .name       = "migrate",
         .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
         .params     = "[-d] [-b] [-i] uri",
diff --git a/hmp.c b/hmp.c
index 082985b..ef85736 100644
--- a/hmp.c
+++ b/hmp.c
@@ -698,6 +698,25 @@  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;
+    enum DataFormat format;
+    Error *errp = NULL;
+
+    format = DATA_FORMAT_UTF8;
+    data = qmp_memchar_read(chardev, size, true, format, &errp);
+    if (errp) {
+        monitor_printf(mon, "%s\n", error_get_pretty(errp));
+        error_free(errp);
+        return;
+    }
+
+    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 406ebb1..a5a0cfe 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 43ef6bc..a8c9430 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -372,6 +372,33 @@ 
            '*format': 'DataFormat'} }
 
 ##
+# @memchar-read:
+#
+# Provide read interface for memchardev. Read from memchar
+# char device and return the data.
+#
+# @chardev: the name of the memchar char device.
+#
+# @size: the size to read in bytes.
+#
+# @format: #optional the format of the data want to read from
+#          memchardev, by default is 'utf8'.
+#
+# Returns: The data read from memchar as string
+#          If @chardev is not a valid memchr device, DeviceNotFound
+#
+# Notes: The option 'block' is not supported now due to the miss
+#        feature in qmp. Will add it later when we gain the necessary
+#        infrastructure enhancement. For now just assume 'drop' behaver
+#        for this command.
+#
+# Since: 1.3
+##
+{ 'command': 'memchar-read',
+  'data': {'chardev': 'str', 'size': 'int', '*format': 'DataFormat'},
+  'returns': 'str' }
+
+##
 # @CommandInfo:
 #
 # Information about a QMP command
diff --git a/qemu-char.c b/qemu-char.c
index 6114e29..cf88f71 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2761,6 +2761,61 @@  void qmp_memchar_write(const char *chardev, int64_t size,
     }
 }
 
+char *qmp_memchar_read(const char *chardev, int64_t size,
+                       bool has_format, enum DataFormat format,
+                       Error **errp)
+{
+    CharDriverState *chr;
+    guchar *read_data;
+    char *data = NULL;
+    int ret;
+
+    read_data = g_malloc0(size);
+
+    chr = qemu_chr_find(chardev);
+    if (!chr) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
+        goto fail;
+    }
+
+    if (strcmp(chr->filename, "memchr") != 0) {
+        error_setg(errp,"The %s is not memory char device\n", chardev);
+        goto fail;
+    }
+
+    /* XXX: For the sync command as 'block', waiting for the qmp
+     * to support necessary feature. Now just act as 'drop'. */
+    if (cirmem_chr_is_empty(chr)) {
+        error_setg(errp, "Failed to read from memchr %s", chardev);
+        goto fail;
+    }
+
+    if (size == 0) {
+        size = CBUFF_SIZE;
+    }
+
+    ret = cirmem_chr_read(chr, read_data, size);
+
+    if (ret < 0) {
+        error_setg(errp, "Failed to read from memchr %s", chardev);
+        goto fail;
+    }
+
+    if (has_format && (format == DATA_FORMAT_BASE64)) {
+       if (read_data) {
+           data = g_base64_encode(read_data, (size_t)size);
+       }
+    } else {
+        data = (char *)read_data;
+    }
+
+    return data;
+
+fail:
+    g_free(read_data);
+    return NULL;
+}
+
 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 7548b9b..7729fb0 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -500,6 +500,46 @@  Example:
 EQMP
 
     {
+        .name       = "memchar-read",
+        .args_type  = "chardev:s,size:i,format:s?",
+        .help       = "return the size of data from memchar chardev",
+        .mhandler.cmd_new = qmp_marshal_input_memchar_read,
+    },
+
+SQMP
+memchar-read
+-------------
+
+Provide read interface for memchardev. Read from memchar
+char device and return the data.
+
+Arguments:
+
+- "chardev": the name of the char device, must be unique (json-string)
+- "size": the memory size wanted to read in bytes(refer to unencoded
+          size of the raw data), would adjust to the init size of the
+          memchar if the requested size is larger than it. (json-int)
+- "format": the data format write to memchardev, default is
+            utf8. (json-string, optional)
+          - Possible values: "utf8", "base64"
+
+Example:
+
+-> { "execute": "memchar-read",
+                "arguments": { "chardev": foo,
+                               "size": 1000,
+                               "format": "utf8" } }
+<- { "return": "data string..." }
+
+Notes:
+
+We will add 'control' options for read and write command that specifies
+behavior when the queue is full/empty, for now just assume a drop
+behaver in these two commands.
+
+EQMP
+
+    {
         .name       = "xen-save-devices-state",
         .args_type  = "filename:F",
     .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,