[Qemu-devel] [RFC v2 ATCH 0/4] char: expose MemoryCharDriver to users and provide QMP interface

[Qemu-devel] [RFC v2 ATCH 0/4] char: expose MemoryCharDriver to users and provide QMP interface

[Lei Li]

This RFC series attempts to convert the MemCharDriver to use a circular
buffer for input and output, expose it to users by introducing QMP commands
memchar_write and memchar_read and via the command line like the other
CharDriverStates.

Serial ports in qemu always use CharDriverStates as there backends,
Right now, all of our backends always try to write the data from the
guest to a socket or file. The concern from OpenStack is that this could
lead to unbounded disk space usage since they log the serial output.
For more detail of the background info:
https://bugs.launchpad.net/nova/+bug/832507

So we want to use a circular buffer in QEMU instead, and then OpenStack
can periodically read the buffer in QEMU and log it.

The QMP commands introduced like:

{ 'command': 'memchar_write',
  'data': {'chardev': 'str', 'size': 'int', 'data': 'str'} }

{ 'command': 'memchar_read',
  'data': {'chardev': 'str', 'size': 'int'},
  'returns': 'str' }

Expose MemCharDriver via the command line like:

qemu -chardev memchr,max-capacity=640k,id=foo -serial chardev:foo

Note:
This series is just a incomplete sketch and not completely tested which
I am still struggling with, but I want to get this out ealier to have
your suggestion. Please comment and let me know if this seems like the
direction we should be headed, thanks!

TODO:
1) Add congestion mechanism.
2) Add HMP "console" command so that can interact with multiple
   chardevs via a single monitor socket

Changes since v1:
  - Exposing the MemCharDriver via command line.
  - Support base64 data format suggested by Anthony and Eric.
  - Follow the new rule for the name of qmp command from Eric.

  For the comments of MemCharDriver improvment, which I am working on and
  will send out within v3 with the rest feature implemented in few days.


Lei Li (6):
  qemu-char: Convert MemCharDriver to circular buffer
  monitor: Adjust qmp_human_monitor_command to new MemCharDriver
  QAPI: Introduce memchar_write QMP command
  QAPI: Introduce memchar_read QMP command
  Fix enumeration typo error
  Expose MemCharDriver via command line

 hmp-commands.hx        |   32 ++++++++
 hmp.c                  |   30 ++++++++
 hmp.h                  |    2 +
 monitor.c              |    8 ++-
 qapi-schema-guest.json |    2 +-
 qapi-schema.json       |   69 +++++++++++++++++-
 qemu-char.c            |  190 +++++++++++++++++++++++++++++++++++++++++++-----
 qemu-char.h            |    2 +-
 qemu-config.c          |    3 +
 qemu-options.hx        |   10 +++
 qmp-commands.hx        |   64 ++++++++++++++++
 11 files changed, 388 insertions(+), 24 deletions(-)

[Qemu-devel] [PATCH 1/6] qemu-char: Convert MemCharDriver to circular buffer

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 qemu-char.c |   96 +++++++++++++++++++++++++++++++++++++++++++++++-----------
 qemu-char.h |    2 +-
 2 files changed, 78 insertions(+), 20 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 398baf1..b21b93a 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2528,38 +2528,96 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
 /***********************************************************/
 /* Memory chardev */
 typedef struct {
-    size_t outbuf_size;
-    size_t outbuf_capacity;
-    uint8_t *outbuf;
+    size_t cbuf_capacity;
+    size_t cbuf_in;
+    size_t cbuf_out;
+    size_t cbuf_count;
+    uint8_t *cbuf;
 } MemoryDriver;
 
+static int mem_chr_is_empty(CharDriverState *chr)
+{
+    MemoryDriver *d = chr->opaque;
+
+    return d->cbuf_count == 0;
+}
+
+static int mem_chr_is_full(CharDriverState *chr)
+{
+    MemoryDriver *d = chr->opaque;
+
+    return d->cbuf_count == d->cbuf_capacity;
+}
+
 static int mem_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
 {
     MemoryDriver *d = chr->opaque;
+    int left;
 
-    /* TODO: the QString implementation has the same code, we should
-     * introduce a generic way to do this in cutils.c */
-    if (d->outbuf_capacity < d->outbuf_size + len) {
-        /* grow outbuf */
-        d->outbuf_capacity += len;
-        d->outbuf_capacity *= 2;
-        d->outbuf = g_realloc(d->outbuf, d->outbuf_capacity);
+    if (d->cbuf_capacity < len) {
+        return -1;
     }
 
-    memcpy(d->outbuf + d->outbuf_size, buf, len);
-    d->outbuf_size += len;
+    left = d->cbuf_capacity - d->cbuf_count % d->cbuf_capacity;
+
+    /* Some of cbuf need to be overwrited */
+    if (left < len) {
+        memcpy(d->cbuf + d->cbuf_in, buf, left);
+        memcpy(d->cbuf + d->cbuf_out, buf + left, len - left);
+        d->cbuf_out = (d->cbuf_out + len - left) % d->cbuf_capacity;
+        d->cbuf_count = d->cbuf_count + left;
+    } else {
+        /* Completely overwrite */
+        if (mem_chr_is_full(chr)) {
+            d->cbuf_out = (d->cbuf_out + len) % d->cbuf_capacity;
+        } else {
+        /* Enough cbuf to write */
+        d->cbuf_count += len;
+        }
+        memcpy(d->cbuf + d->cbuf_in, buf, len);
+    }
+
+    d->cbuf_in = (d->cbuf_in + len) % d->cbuf_capacity;
 
     return len;
 }
 
-void qemu_chr_init_mem(CharDriverState *chr)
+static void mem_chr_read(CharDriverState *chr, uint8_t *buf, int len)
+{
+    MemoryDriver *d = chr->opaque;
+    int left;
+
+    if (mem_chr_is_empty(chr)) {
+        return;
+    }
+
+    left = d->cbuf_capacity - d->cbuf_count % d->cbuf_capacity;
+
+    if (d->cbuf_capacity < len) {
+        len = d->cbuf_capacity;
+    }
+
+    if (left < len) {
+        memcpy(buf, d->cbuf + d->cbuf_out, left);
+        memcpy(buf + left, d->cbuf + d->cbuf_out + left, len - left);
+    } else {
+        memcpy(buf, d->cbuf + d->cbuf_out, len);
+    }
+
+    d->cbuf_out = (d->cbuf_out + len) % d->cbuf_capacity;
+    d->cbuf_count -= len;
+}
+
+void qemu_chr_init_mem(CharDriverState *chr, size_t size)
 {
     MemoryDriver *d;
 
     d = g_malloc(sizeof(*d));
-    d->outbuf_size = 0;
-    d->outbuf_capacity = 4096;
-    d->outbuf = g_malloc0(d->outbuf_capacity);
+    d->cbuf_capacity = size;
+    d->cbuf_in = 0;
+    d->cbuf_out = 0;
+    d->cbuf_count = 0;
+    d->cbuf = g_malloc0(d->cbuf_capacity);
 
     memset(chr, 0, sizeof(*chr));
     chr->opaque = d;
@@ -2569,7 +2627,7 @@ void qemu_chr_init_mem(CharDriverState *chr)
 QString *qemu_chr_mem_to_qs(CharDriverState *chr)
 {
     MemoryDriver *d = chr->opaque;
-    return qstring_from_substr((char *) d->outbuf, 0, d->outbuf_size - 1);
+    return qstring_from_substr((char *) d->cbuf, 0, d->cbuf_count - 1);
 }
 
 /* NOTE: this driver can not be closed with qemu_chr_delete()! */
@@ -2577,7 +2635,7 @@ void qemu_chr_close_mem(CharDriverState *chr)
 {
     MemoryDriver *d = chr->opaque;
 
-    g_free(d->outbuf);
+    g_free(d->cbuf);
     g_free(chr->opaque);
     chr->opaque = NULL;
     chr->chr_write = NULL;
@@ -2586,7 +2644,7 @@ void qemu_chr_close_mem(CharDriverState *chr)
 size_t qemu_chr_mem_osize(const CharDriverState *chr)
 {
     const MemoryDriver *d = chr->opaque;
-    return d->outbuf_size;
+    return d->cbuf_count;
 }
 
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
diff --git a/qemu-char.h b/qemu-char.h
index 486644b..d8d90cc 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -243,7 +243,7 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd);
 extern int term_escape_char;
 
 /* memory chardev */
-void qemu_chr_init_mem(CharDriverState *chr);
+void qemu_chr_init_mem(CharDriverState *chr, size_t size);
 void qemu_chr_close_mem(CharDriverState *chr);
 QString *qemu_chr_mem_to_qs(CharDriverState *chr);
 size_t qemu_chr_mem_osize(const CharDriverState *chr);
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/6] monitor: Adjust qmp_human_monitor_command to new MemCharDriver

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 monitor.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 480f583..ab4650b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -642,7 +642,13 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
     CharDriverState mchar;
 
     memset(&hmp, 0, sizeof(hmp));
-    qemu_chr_init_mem(&mchar);
+
+    /* Since the backend of MemCharDriver convert to a circular
+     * buffer with fixed size, so should indicate the init memory
+     * size.
+     *
+     * XXX:  is 4096 as init memory enough for this? */
+    qemu_chr_init_mem(&mchar, 4096);
     hmp.chr = &mchar;
 
     old_mon = cur_mon;
-- 
1.7.7.6

[Qemu-devel] [PATCH 3/6] QAPI: Introduce memchar_write QMP command

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 hmp-commands.hx  |   16 ++++++++++++++++
 hmp.c            |   15 +++++++++++++++
 hmp.h            |    1 +
 qapi-schema.json |   28 ++++++++++++++++++++++++++++
 qemu-char.c      |   36 ++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   33 +++++++++++++++++++++++++++++++++
 6 files changed, 129 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f6104b0..829aea1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -797,6 +797,22 @@ Inject an NMI on the given CPU (x86 only).
 ETEXI
 
     {
+        .name       = "memchar-write",
+        .args_type  = "chardev:s,size:i,data:s,format:s?",
+        .params     = "chardev size data",
+        .help       = "Provide writing interface for memchardev. Write"
+                      "'data' to memchr char device with size 'size'",
+        .mhandler.cmd = hmp_memchar_write,
+    },
+
+STEXI
+@item memchar-write @var{chardev} @var{size} @var{data} @var{format}
+@findex memchar-write
+Provide writing interface for memchardev. Write @var{data}
+to memchr char device with size @var{size}.
+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 81c8acb..c1164eb 100644
--- a/hmp.c
+++ b/hmp.c
@@ -668,6 +668,21 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &errp);
 }
 
+void hmp_memchar_write(Monitor *mon, const QDict *qdict)
+{
+    uint32_t size = qdict_get_int(qdict, "size");
+    const char *chardev = qdict_get_str(qdict, "chardev");
+    const char *data = qdict_get_str(qdict, "data");
+    int con = qdict_get_try_bool(qdict, "utf8", 0);
+    enum DataFormat format;
+    Error *errp = NULL;
+
+    format = con ? DATA_FORMAT_UTF8 : DATA_FORMAT_BASE64;
+    qmp_memchar_write(chardev, size, data, true, format, &errp);
+
+    hmp_handle_error(mon, &errp);
+}
+
 static void hmp_cont_cb(void *opaque, int err)
 {
     if (!err) {
diff --git a/hmp.h b/hmp.h
index 7dd93bf..15e1311 100644
--- a/hmp.h
+++ b/hmp.h
@@ -43,6 +43,7 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
 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_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 bd8ad74..2fc1a27 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -235,6 +235,34 @@
 ##
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
 
+{ 'enum': 'DataFormat'
+  'data': [ 'utf8', 'base64' ] }
+
+##
+# @memchar-write:
+#
+# Provide writing interface for memchardev. Write data to memchar
+# char device.
+#
+# @chardev: the name of the memchar char device.
+#
+# @size: the size to write in bytes.
+#
+# @data: the source data write to memchar.
+#
+# @format: #optional the format of the data write to memchardev, by
+#          default is 'utf8'.
+#
+# Returns: Nothing on success
+#          If @chardev is not a valid memchr device, DeviceNotFound
+#          If an I/O error occurs while writing, IOError
+#
+# Since: 1.3
+##
+{ 'command': 'memchar-write',
+  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
+           '*format': 'DataFormat'} }
+
 ##
 # @CommandInfo:
 #
diff --git a/qemu-char.c b/qemu-char.c
index b21b93a..d8f3238 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2647,6 +2647,42 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr)
     return d->cbuf_count;
 }
 
+void qmp_memchar_write(const char *chardev, int64_t size,
+                       const char *data, bool has_format,
+                       enum DataFormat format,
+
+                       Error **errp)
+{
+    CharDriverState *chr;
+    guchar *write_data;
+    int ret;
+    gsize write_count;
+
+    chr = qemu_chr_find(chardev);
+
+    if(!chr) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
+        return;
+    }
+
+    write_count = (gsize)size;
+
+    if (has_format) {
+        if (format == DATA_FORMAT_BASE64) {
+            write_data = g_base64_decode(data, &write_count);
+        }
+    } else {
+        write_data = (uint8_t *)data;
+    }
+
+    ret = mem_chr_write(chr, write_data, size);
+    if (ret <= 0) {
+        error_set(errp, QERR_IO_ERROR);
+    }
+    g_free(write_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 3745a21..3c747cd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -441,6 +441,39 @@ Note: inject-nmi fails when the guest doesn't support injecting.
 EQMP
 
     {
+        .name       = "memchar-write",
+        .args_type  = "chardev:s,size:i,data:s,format:s?",
+        .mhandler.cmd_new = qmp_marshal_input_memchar_write,
+    },
+
+SQMP
+memchar-write
+-------------
+
+Provide writing interface for memchardev. Write data to memchar
+char device.
+
+Arguments:
+
+- "chardev": the name of the char device, must be unique (json-string)
+- "size": the memory size, in bytes (json-int)
+- "data": the source data writed to memchar (json-string)
+- "format": the data format write to memchardev
+  (DataFormat, optional, support "utf8" and "base64" now, default "utf8")
+
+
+Example:
+
+-> { "execute": "memchar-write",
+                "arguments": { "chardev": foo,
+                               "size": 1000,
+                               "data": "data string...",
+                               "format": "utf8" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "xen-save-devices-state",
         .args_type  = "filename:F",
     .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,
-- 
1.7.7.6

[Qemu-devel] [PATCH 4/6] QAPI: Introduce memchar_read QMP command

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 hmp-commands.hx  |   16 ++++++++++++++++
 hmp.c            |   15 +++++++++++++++
 hmp.h            |    1 +
 qapi-schema.json |   23 +++++++++++++++++++++++
 qemu-char.c      |   32 ++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   31 +++++++++++++++++++++++++++++++
 6 files changed, 118 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 829aea1..9f61633 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -813,6 +813,22 @@ to memchr char device with size @var{size}.
 ETEXI
 
     {
+        .name       = "memchar-read",
+        .args_type  = "chardev:s,size:i,format:s?",
+        .params     = "chardev size",
+        .help       = "Provide read interface for memchardev. Read from"
+                      "memchr char device and return 'size' of the data",
+        .mhandler.cmd = hmp_memchar_read,
+    },
+
+STEXI
+@item memchar-read @var{chardev} @var{size} @var{format}
+@findex memchar-read
+Provide read interface for memchardev. Read from memchr
+char device and return @var{size} of the data.
+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 c1164eb..0c0182a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -683,6 +683,21 @@ 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 con = qdict_get_try_bool(qdict, "utf8", 0);
+    enum DataFormat format;
+    Error *errp = NULL;
+
+    format = con ? DATA_FORMAT_UTF8 : DATA_FORMAT_BASE64;
+
+    data = qmp_memchar_read(chardev, size, true, format, &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 15e1311..b734e31 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 2fc1a27..1346bcc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -264,6 +264,29 @@
            '*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 write 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
+#          If an I/O error occurs while reading, IOError
+#
+# Since: 1.2
+##
+{ '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 d8f3238..ff6651b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2683,6 +2683,38 @@ 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;
+
+    read_data = g_malloc0(size);
+
+    chr = qemu_chr_find(chardev);
+    if(!chr) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
+        return NULL;
+    }
+
+    mem_chr_read(chr, read_data, size);
+
+    if (has_format) {
+        if (format == DATA_FORMAT_BASE64) {
+            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 3c747cd..a9890db 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -474,6 +474,37 @@ 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 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 in bytes, init size of the memchar
+          by default (json-int)
+- "format": the data format want to read from mechardev.
+  (DataFormat, optional, support "utf8" and "base64" now, default "utf8")
+
+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

[Qemu-devel] [PATCH 5/6] Fix enumeration typo error

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 qapi-schema-guest.json |    2 +-
 qapi-schema.json       |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
index d955cf1..ed0eb69 100644
--- a/qapi-schema-guest.json
+++ b/qapi-schema-guest.json
@@ -293,7 +293,7 @@
 ##
 # @GuestFsFreezeStatus
 #
-# An enumation of filesystem freeze states
+# An enumeration of filesystem freeze states
 #
 # @thawed: filesystems thawed/unfrozen
 #
diff --git a/qapi-schema.json b/qapi-schema.json
index 1346bcc..8e13a05 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -118,7 +118,7 @@
 ##
 # @RunState
 #
-# An enumation of VM run states.
+# An enumeration of VM run states.
 #
 # @debug: QEMU is running on a debugger
 #
@@ -836,7 +836,7 @@
 ##
 # @SpiceQueryMouseMode
 #
-# An enumation of Spice mouse states.
+# An enumeration of Spice mouse states.
 #
 # @client: Mouse cursor position is determined by the client.
 #
-- 
1.7.7.6

[Qemu-devel] [PATCH 6/6] Expose MemCharDriver via command line

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 qemu-char.c     |   24 ++++++++++++++++++++++++
 qemu-config.c   |    3 +++
 qemu-options.hx |   10 ++++++++++
 3 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index ff6651b..36f4ecc 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -99,6 +99,7 @@
 #include "ui/qemu-spice.h"
 
 #define READ_BUF_LEN 4096
+#define CBUFF_SIZE 65536
 
 /***********************************************************/
 /* character device */
@@ -2647,6 +2648,23 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr)
     return d->cbuf_count;
 }
 
+static CharDriverState *qemu_chr_open_memchr(QemuOpts *opts)
+{
+    CharDriverState *chr;
+    size_t capacity;
+
+    chr = g_malloc0(sizeof(CharDriverState));
+
+    capacity = qemu_opt_get_number(opts, "maxcapacity", 0);
+    if (capacity == 0) {
+        capacity = CBUFF_SIZE;
+    }
+
+    qemu_chr_init_mem(chr, capacity);
+
+    return chr;
+}
+
 void qmp_memchar_write(const char *chardev, int64_t size,
                        const char *data, bool has_format,
                        enum DataFormat format,
@@ -2779,6 +2797,11 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
         qemu_opt_set(opts, "path", p);
         return opts;
     }
+    if (strstart(filename, "memchr", &p)) {
+        qemu_opt_set(opts, "backend", "memchr");
+        qemu_opt_set(opts, "maxcapacity", p);
+        return opts;
+    }
     if (strstart(filename, "tcp:", &p) ||
         strstart(filename, "telnet:", &p)) {
         if (sscanf(p, "%64[^:]:%32[^,]%n", host, port, &pos) < 2) {
@@ -2852,6 +2875,7 @@ static const struct {
     { .name = "udp",       .open = qemu_chr_open_udp },
     { .name = "msmouse",   .open = qemu_chr_open_msmouse },
     { .name = "vc",        .open = text_console_init },
+    { .name = "memchr",    .open = qemu_chr_open_memchr },
 #ifdef _WIN32
     { .name = "file",      .open = qemu_chr_open_win_file_out },
     { .name = "pipe",      .open = qemu_chr_open_win_pipe },
diff --git a/qemu-config.c b/qemu-config.c
index c05ffbc..7118d0e 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -114,6 +114,9 @@ static QemuOptsList qemu_drive_opts = {
             .name = "copy-on-read",
             .type = QEMU_OPT_BOOL,
             .help = "copy read data from backing file into image file",
+        },{
+            .name = "max-capacity",
+            .type = QEMU_OPT_NUMBER,
         },
         { /* end of list */ }
     },
diff --git a/qemu-options.hx b/qemu-options.hx
index 3c411c4..1ccf295 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1647,6 +1647,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
     "-chardev msmouse,id=id[,mux=on|off]\n"
     "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n"
     "         [,mux=on|off]\n"
+    "-chardev memchr,id=id,max-capacity=max-capacity\n"
     "-chardev file,id=id,path=path[,mux=on|off]\n"
     "-chardev pipe,id=id,path=path[,mux=on|off]\n"
 #ifdef _WIN32
@@ -1685,6 +1686,7 @@ Backend is one of:
 @option{udp},
 @option{msmouse},
 @option{vc},
+@option{memchr},
 @option{file},
 @option{pipe},
 @option{console},
@@ -1791,6 +1793,14 @@ the console, in pixels.
 @option{cols} and @option{rows} specify that the console be sized to fit a text
 console with the given dimensions.
 
+@item -chardev memchr ,id=@var{id} ,max-capacity=@var{max-capacity}
+
+Create a circular buffer with fixed size indicated by optionally @option{max-capacity}
+which will be default 64K if it is not given.
+
+@option{max-capacity} specify the max capacity of the size of circular buffer
+want to create.
+
 @item -chardev file ,id=@var{id} ,path=@var{path}
 
 Log all traffic received from the guest to a file.
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 3/6] QAPI: Introduce memchar_write QMP command

[Eric Blake]

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

On 08/22/2012 11:14 PM, Lei Li wrote:
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>

Subject line uses '_', but the QMP command uses '-' [1]

> ---
>  hmp-commands.hx  |   16 ++++++++++++++++
>  hmp.c            |   15 +++++++++++++++
>  hmp.h            |    1 +
>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>  qemu-char.c      |   36 ++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   33 +++++++++++++++++++++++++++++++++
>  6 files changed, 129 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f6104b0..829aea1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -797,6 +797,22 @@ Inject an NMI on the given CPU (x86 only).
>  ETEXI
>  
>      {
> +        .name       = "memchar-write",

HMP commands should use '_', not '-'.

> +++ b/qapi-schema.json
> @@ -235,6 +235,34 @@
>  ##
>  { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
>  
> +{ 'enum': 'DataFormat'
> +  'data': [ 'utf8', 'base64' ] }

Missing documentation for DataFormat (see for example how @ErrorClass is
documented).

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

Re: [Qemu-devel] [PATCH 4/6] QAPI: Introduce memchar_read QMP command

[Eric Blake]

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

On 08/22/2012 11:14 PM, Lei Li wrote:
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>

Again, subject line should use '-' not '_' for QMP.

> ---
>  hmp-commands.hx  |   16 ++++++++++++++++
>  hmp.c            |   15 +++++++++++++++
>  hmp.h            |    1 +
>  qapi-schema.json |   23 +++++++++++++++++++++++
>  qemu-char.c      |   32 ++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   31 +++++++++++++++++++++++++++++++
>  6 files changed, 118 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 829aea1..9f61633 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -813,6 +813,22 @@ to memchr char device with size @var{size}.
>  ETEXI
>  
>      {
> +        .name       = "memchar-read",

while the HMP command should use '_' not '-'.

> +++ b/qapi-schema.json
> @@ -264,6 +264,29 @@
>             '*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.

Why two spaces?

> +#
> +# @size: the size to write in bytes.

s/write/read/

> +#
> +# @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
> +#          If an I/O error occurs while reading, IOError
> +#
> +# Since: 1.2

1.3

> +##
> +{ 'command': 'memchar-read',
> +  'data': {'chardev': 'str', 'size': 'int', '*format': 'DataFormat'},
> +  'returns': 'str' }

While writing can default to UTF-8, I'm worried about reading - does
encoding in UTF-8 allow transmission of NUL bytes through JSON, or do
you have to use base64 to allow full binary data through?  What happens
if the data read includes a NUL byte that can't be encoded back into the
requested DataFormat?

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

Re: [Qemu-devel] [PATCH 5/6] Fix enumeration typo error

[Eric Blake]

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

On 08/22/2012 11:14 PM, Lei Li wrote:
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  qapi-schema-guest.json |    2 +-
>  qapi-schema.json       |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

This is independent.  Have you considered posting it to the trivial
patch queue, possibly for 1.2, since it improves documentation?

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

Re: [Qemu-devel] [PATCH 3/6] QAPI: Introduce memchar_write QMP command

[Lei Li]

On 08/23/2012 01:42 PM, Eric Blake wrote:
> On 08/22/2012 11:14 PM, Lei Li wrote:
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> Subject line uses '_', but the QMP command uses '-' [1]
>
>> ---
>>   hmp-commands.hx  |   16 ++++++++++++++++
>>   hmp.c            |   15 +++++++++++++++
>>   hmp.h            |    1 +
>>   qapi-schema.json |   28 ++++++++++++++++++++++++++++
>>   qemu-char.c      |   36 ++++++++++++++++++++++++++++++++++++
>>   qmp-commands.hx  |   33 +++++++++++++++++++++++++++++++++
>>   6 files changed, 129 insertions(+), 0 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index f6104b0..829aea1 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -797,6 +797,22 @@ Inject an NMI on the given CPU (x86 only).
>>   ETEXI
>>   
>>       {
>> +        .name       = "memchar-write",
> HMP commands should use '_', not '-'.
>
>> +++ b/qapi-schema.json
>> @@ -235,6 +235,34 @@
>>   ##
>>   { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
>>   
>> +{ 'enum': 'DataFormat'
>> +  'data': [ 'utf8', 'base64' ] }
> Missing documentation for DataFormat (see for example how @ErrorClass is
> documented).
>
Sure, I will look into it and add documentation in v3, thanks!



-- 
Lei

Re: [Qemu-devel] [PATCH 4/6] QAPI: Introduce memchar_read QMP command

[Lei Li]

On 08/23/2012 01:46 PM, Eric Blake wrote:
> On 08/22/2012 11:14 PM, Lei Li wrote:
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> Again, subject line should use '-' not '_' for QMP.
>
>> ---
>>   hmp-commands.hx  |   16 ++++++++++++++++
>>   hmp.c            |   15 +++++++++++++++
>>   hmp.h            |    1 +
>>   qapi-schema.json |   23 +++++++++++++++++++++++
>>   qemu-char.c      |   32 ++++++++++++++++++++++++++++++++
>>   qmp-commands.hx  |   31 +++++++++++++++++++++++++++++++
>>   6 files changed, 118 insertions(+), 0 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 829aea1..9f61633 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -813,6 +813,22 @@ to memchr char device with size @var{size}.
>>   ETEXI
>>   
>>       {
>> +        .name       = "memchar-read",
> while the HMP command should use '_' not '-'.
>
>> +++ b/qapi-schema.json
>> @@ -264,6 +264,29 @@
>>              '*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.
> Why two spaces?
>
>> +#
>> +# @size: the size to write in bytes.
> s/write/read/
>
>> +#
>> +# @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
>> +#          If an I/O error occurs while reading, IOError
>> +#
>> +# Since: 1.2
> 1.3
>
>> +##
>> +{ 'command': 'memchar-read',
>> +  'data': {'chardev': 'str', 'size': 'int', '*format': 'DataFormat'},
>> +  'returns': 'str' }
> While writing can default to UTF-8, I'm worried about reading - does
> encoding in UTF-8 allow transmission of NUL bytes through JSON, or do
> you have to use base64 to allow full binary data through?  What happens
> if the data read includes a NUL byte that can't be encoded back into the
> requested DataFormat?

Yes, you are right. g_base64_encode did not support NULL bytes encoding, so
encoding in UTF-8 should not allow transmission of NULL bytes through JSON.
Thanks for pointing this out!


-- 
Lei

Re: [Qemu-devel] [PATCH 1/6] qemu-char: Convert MemCharDriver to circular buffer

[Luiz Capitulino]

On Thu, 23 Aug 2012 13:14:21 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  qemu-char.c |   96 +++++++++++++++++++++++++++++++++++++++++++++++-----------
>  qemu-char.h |    2 +-
>  2 files changed, 78 insertions(+), 20 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 398baf1..b21b93a 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2528,38 +2528,96 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
>  /***********************************************************/
>  /* Memory chardev */
>  typedef struct {
> -    size_t outbuf_size;
> -    size_t outbuf_capacity;
> -    uint8_t *outbuf;
> +    size_t cbuf_capacity;
> +    size_t cbuf_in;
> +    size_t cbuf_out;
> +    size_t cbuf_count;
> +    uint8_t *cbuf;
>  } MemoryDriver;
>  
> +static int mem_chr_is_empty(CharDriverState *chr)
> +{
> +    MemoryDriver *d = chr->opaque;
> +
> +    return d->cbuf_count == 0;
> +}
> +
> +static int mem_chr_is_full(CharDriverState *chr)
> +{
> +    MemoryDriver *d = chr->opaque;
> +
> +    return d->cbuf_count == d->cbuf_capacity;
> +}

Please, make them return a bool and chr can be const.

> +
>  static int mem_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>  {
>      MemoryDriver *d = chr->opaque;
> +    int left;
>  
> -    /* TODO: the QString implementation has the same code, we should
> -     * introduce a generic way to do this in cutils.c */
> -    if (d->outbuf_capacity < d->outbuf_size + len) {
> -        /* grow outbuf */
> -        d->outbuf_capacity += len;
> -        d->outbuf_capacity *= 2;
> -        d->outbuf = g_realloc(d->outbuf, d->outbuf_capacity);
> +    if (d->cbuf_capacity < len) {
> +        return -1;
>      }

This is the first time I look at a circular buffer implementation, but
I'd expect this too work: ie. you just write the last bytes that fit on
the buffer.

>  
> -    memcpy(d->outbuf + d->outbuf_size, buf, len);
> -    d->outbuf_size += len;
> +    left = d->cbuf_capacity - d->cbuf_count % d->cbuf_capacity;
> +
> +    /* Some of cbuf need to be overwrited */
> +    if (left < len) {
> +        memcpy(d->cbuf + d->cbuf_in, buf, left);
> +        memcpy(d->cbuf + d->cbuf_out, buf + left, len - left);
> +        d->cbuf_out = (d->cbuf_out + len - left) % d->cbuf_capacity;
> +        d->cbuf_count = d->cbuf_count + left;
> +    } else {
> +        /* Completely overwrite */
> +        if (mem_chr_is_full(chr)) {
> +            d->cbuf_out = (d->cbuf_out + len) % d->cbuf_capacity;
> +        } else {
> +        /* Enough cbuf to write */
> +        d->cbuf_count += len;
> +        }
> +        memcpy(d->cbuf + d->cbuf_in, buf, len);
> +    }

Couldn't this be made simpler by having a pointer to d->cbuf that points
to where we are, then we just made that pointer points to the beginning
of the buffer every time we cross its end? Just an idea.

> +
> +    d->cbuf_in = (d->cbuf_in + len) % d->cbuf_capacity;
>  
>      return len;
>  }
>  
> -void qemu_chr_init_mem(CharDriverState *chr)
> +static void mem_chr_read(CharDriverState *chr, uint8_t *buf, int len)
> +{
> +    MemoryDriver *d = chr->opaque;
> +    int left;
> +
> +    if (mem_chr_is_empty(chr)) {
> +        return;
> +    }
> +
> +    left = d->cbuf_capacity - d->cbuf_count % d->cbuf_capacity;
> +
> +    if (d->cbuf_capacity < len) {
> +        len = d->cbuf_capacity;
> +    }
> +
> +    if (left < len) {
> +        memcpy(buf, d->cbuf + d->cbuf_out, left);
> +        memcpy(buf + left, d->cbuf + d->cbuf_out + left, len - left);
> +    } else {
> +        memcpy(buf, d->cbuf + d->cbuf_out, len);
> +    }
> +
> +    d->cbuf_out = (d->cbuf_out + len) % d->cbuf_capacity;
> +    d->cbuf_count -= len;
> +}
> +
> +void qemu_chr_init_mem(CharDriverState *chr, size_t size)

Won't this break bisect?

>  {
>      MemoryDriver *d;
>  
>      d = g_malloc(sizeof(*d));
> -    d->outbuf_size = 0;
> -    d->outbuf_capacity = 4096;
> -    d->outbuf = g_malloc0(d->outbuf_capacity);
> +    d->cbuf_capacity = size;
> +    d->cbuf_in = 0;
> +    d->cbuf_out = 0;
> +    d->cbuf_count = 0;
> +    d->cbuf = g_malloc0(d->cbuf_capacity);
>  
>      memset(chr, 0, sizeof(*chr));
>      chr->opaque = d;
> @@ -2569,7 +2627,7 @@ void qemu_chr_init_mem(CharDriverState *chr)
>  QString *qemu_chr_mem_to_qs(CharDriverState *chr)
>  {
>      MemoryDriver *d = chr->opaque;
> -    return qstring_from_substr((char *) d->outbuf, 0, d->outbuf_size - 1);
> +    return qstring_from_substr((char *) d->cbuf, 0, d->cbuf_count - 1);
>  }
>  
>  /* NOTE: this driver can not be closed with qemu_chr_delete()! */
> @@ -2577,7 +2635,7 @@ void qemu_chr_close_mem(CharDriverState *chr)
>  {
>      MemoryDriver *d = chr->opaque;
>  
> -    g_free(d->outbuf);
> +    g_free(d->cbuf);
>      g_free(chr->opaque);
>      chr->opaque = NULL;
>      chr->chr_write = NULL;
> @@ -2586,7 +2644,7 @@ void qemu_chr_close_mem(CharDriverState *chr)
>  size_t qemu_chr_mem_osize(const CharDriverState *chr)
>  {
>      const MemoryDriver *d = chr->opaque;
> -    return d->outbuf_size;
> +    return d->cbuf_count;
>  }
>  
>  QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
> diff --git a/qemu-char.h b/qemu-char.h
> index 486644b..d8d90cc 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -243,7 +243,7 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd);
>  extern int term_escape_char;
>  
>  /* memory chardev */
> -void qemu_chr_init_mem(CharDriverState *chr);
> +void qemu_chr_init_mem(CharDriverState *chr, size_t size);
>  void qemu_chr_close_mem(CharDriverState *chr);
>  QString *qemu_chr_mem_to_qs(CharDriverState *chr);
>  size_t qemu_chr_mem_osize(const CharDriverState *chr);

Re: [Qemu-devel] [PATCH 2/6] monitor: Adjust qmp_human_monitor_command to new MemCharDriver

[Luiz Capitulino]

On Thu, 23 Aug 2012 13:14:22 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  monitor.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 480f583..ab4650b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -642,7 +642,13 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
>      CharDriverState mchar;
>  
>      memset(&hmp, 0, sizeof(hmp));
> -    qemu_chr_init_mem(&mchar);
> +
> +    /* Since the backend of MemCharDriver convert to a circular
> +     * buffer with fixed size, so should indicate the init memory
> +     * size.
> +     *
> +     * XXX:  is 4096 as init memory enough for this? */
> +    qemu_chr_init_mem(&mchar, 4096);

I'm not sure I like this. The end result will be that hmp commands writing
more than 4096 bytes will simply fail or return garbage (if the circular buffer
is changed to allow writing more than it supports) today they would just work.

Although it's always possible to increase the buffer size, we would only realize
this is needed when the bug is triggered, which means it has a high chance
of happening in production. IOW, this would be a regression.

The only solution I can think of is to make the circular buffer and the
current MemoryDriver live in parallel. Actually, you really seem to be
adding something else.

>      hmp.chr = &mchar;
>  
>      old_mon = cur_mon;

Re: [Qemu-devel] [PATCH 3/6] QAPI: Introduce memchar_write QMP command

[Luiz Capitulino]

On Thu, 23 Aug 2012 13:14:23 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  hmp-commands.hx  |   16 ++++++++++++++++
>  hmp.c            |   15 +++++++++++++++
>  hmp.h            |    1 +
>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>  qemu-char.c      |   36 ++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   33 +++++++++++++++++++++++++++++++++
>  6 files changed, 129 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f6104b0..829aea1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -797,6 +797,22 @@ Inject an NMI on the given CPU (x86 only).
>  ETEXI
>  
>      {
> +        .name       = "memchar-write",
> +        .args_type  = "chardev:s,size:i,data:s,format:s?",
> +        .params     = "chardev size data",
> +        .help       = "Provide writing interface for memchardev. Write"
> +                      "'data' to memchr char device with size 'size'",
> +        .mhandler.cmd = hmp_memchar_write,
> +    },
> +
> +STEXI
> +@item memchar-write @var{chardev} @var{size} @var{data} @var{format}
> +@findex memchar-write
> +Provide writing interface for memchardev. Write @var{data}
> +to memchr char device with size @var{size}.
> +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 81c8acb..c1164eb 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -668,6 +668,21 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
>      hmp_handle_error(mon, &errp);
>  }
>  
> +void hmp_memchar_write(Monitor *mon, const QDict *qdict)
> +{
> +    uint32_t size = qdict_get_int(qdict, "size");
> +    const char *chardev = qdict_get_str(qdict, "chardev");
> +    const char *data = qdict_get_str(qdict, "data");
> +    int con = qdict_get_try_bool(qdict, "utf8", 0);
> +    enum DataFormat format;
> +    Error *errp = NULL;
> +
> +    format = con ? DATA_FORMAT_UTF8 : DATA_FORMAT_BASE64;
> +    qmp_memchar_write(chardev, size, data, true, format, &errp);
> +
> +    hmp_handle_error(mon, &errp);
> +}
> +
>  static void hmp_cont_cb(void *opaque, int err)
>  {
>      if (!err) {
> diff --git a/hmp.h b/hmp.h
> index 7dd93bf..15e1311 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -43,6 +43,7 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
>  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_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 bd8ad74..2fc1a27 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -235,6 +235,34 @@
>  ##
>  { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
>  
> +{ 'enum': 'DataFormat'
> +  'data': [ 'utf8', 'base64' ] }
> +
> +##
> +# @memchar-write:
> +#
> +# Provide writing interface for memchardev. Write data to memchar
> +# char device.
> +#
> +# @chardev: the name of the memchar char device.
> +#
> +# @size: the size to write in bytes.
> +#
> +# @data: the source data write to memchar.
> +#
> +# @format: #optional the format of the data write to memchardev, by
> +#          default is 'utf8'.
> +#
> +# Returns: Nothing on success
> +#          If @chardev is not a valid memchr device, DeviceNotFound
> +#          If an I/O error occurs while writing, IOError
> +#
> +# Since: 1.3
> +##
> +{ 'command': 'memchar-write',
> +  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
> +           '*format': 'DataFormat'} }
> +
>  ##
>  # @CommandInfo:
>  #
> diff --git a/qemu-char.c b/qemu-char.c
> index b21b93a..d8f3238 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2647,6 +2647,42 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr)
>      return d->cbuf_count;
>  }
>  
> +void qmp_memchar_write(const char *chardev, int64_t size,
> +                       const char *data, bool has_format,
> +                       enum DataFormat format,
> +
> +                       Error **errp)
> +{
> +    CharDriverState *chr;
> +    guchar *write_data;
> +    int ret;
> +    gsize write_count;
> +
> +    chr = qemu_chr_find(chardev);
> +
> +    if(!chr) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
> +        return;
> +    }
> +
> +    write_count = (gsize)size;
> +
> +    if (has_format) {
> +        if (format == DATA_FORMAT_BASE64) {
> +            write_data = g_base64_decode(data, &write_count);
> +        }
> +    } else {
> +        write_data = (uint8_t *)data;
> +    }
> +
> +    ret = mem_chr_write(chr, write_data, size);
> +    if (ret <= 0) {
> +        error_set(errp, QERR_IO_ERROR);

We're only using the QERR_ macros for the errors we have to maintain
compatibility (they're listed in ErrorClass in the schema) all other
errors can use the error_setg() function:

 http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg04980.html

> +    }
> +    g_free(write_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 3745a21..3c747cd 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -441,6 +441,39 @@ Note: inject-nmi fails when the guest doesn't support injecting.
>  EQMP
>  
>      {
> +        .name       = "memchar-write",
> +        .args_type  = "chardev:s,size:i,data:s,format:s?",
> +        .mhandler.cmd_new = qmp_marshal_input_memchar_write,
> +    },
> +
> +SQMP
> +memchar-write
> +-------------
> +
> +Provide writing interface for memchardev. Write data to memchar
> +char device.
> +
> +Arguments:
> +
> +- "chardev": the name of the char device, must be unique (json-string)
> +- "size": the memory size, in bytes (json-int)
> +- "data": the source data writed to memchar (json-string)
> +- "format": the data format write to memchardev
> +  (DataFormat, optional, support "utf8" and "base64" now, default "utf8")
> +
> +
> +Example:
> +
> +-> { "execute": "memchar-write",
> +                "arguments": { "chardev": foo,
> +                               "size": 1000,
> +                               "data": "data string...",
> +                               "format": "utf8" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>          .name       = "xen-save-devices-state",
>          .args_type  = "filename:F",
>      .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,

Re: [Qemu-devel] [PATCH 5/6] Fix enumeration typo error

[Luiz Capitulino]

On Thu, 23 Aug 2012 13:14:25 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

PS: CC'ing qemu-trivial as per Eric's suggestion.

> ---
>  qapi-schema-guest.json |    2 +-
>  qapi-schema.json       |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
> index d955cf1..ed0eb69 100644
> --- a/qapi-schema-guest.json
> +++ b/qapi-schema-guest.json
> @@ -293,7 +293,7 @@
>  ##
>  # @GuestFsFreezeStatus
>  #
> -# An enumation of filesystem freeze states
> +# An enumeration of filesystem freeze states
>  #
>  # @thawed: filesystems thawed/unfrozen
>  #
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 1346bcc..8e13a05 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -118,7 +118,7 @@
>  ##
>  # @RunState
>  #
> -# An enumation of VM run states.
> +# An enumeration of VM run states.
>  #
>  # @debug: QEMU is running on a debugger
>  #
> @@ -836,7 +836,7 @@
>  ##
>  # @SpiceQueryMouseMode
>  #
> -# An enumation of Spice mouse states.
> +# An enumeration of Spice mouse states.
>  #
>  # @client: Mouse cursor position is determined by the client.
>  #

Re: [Qemu-devel] [PATCH 5/6] Fix enumeration typo error

[Stefan Hajnoczi]

On Thu, Aug 23, 2012 at 01:14:25PM +0800, Lei Li wrote:
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  qapi-schema-guest.json |    2 +-
>  qapi-schema.json       |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

Re: [Qemu-devel] [PATCH 6/6] Expose MemCharDriver via command line

[Markus Armbruster]

Lei Li <lilei@linux.vnet.ibm.com> writes:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  qemu-char.c     |   24 ++++++++++++++++++++++++
>  qemu-config.c   |    3 +++
>  qemu-options.hx |   10 ++++++++++
>  3 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index ff6651b..36f4ecc 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -99,6 +99,7 @@
>  #include "ui/qemu-spice.h"
>  
>  #define READ_BUF_LEN 4096
> +#define CBUFF_SIZE 65536
>  
>  /***********************************************************/
>  /* character device */
> @@ -2647,6 +2648,23 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr)
>      return d->cbuf_count;
>  }
>  
> +static CharDriverState *qemu_chr_open_memchr(QemuOpts *opts)
> +{
> +    CharDriverState *chr;
> +    size_t capacity;
> +
> +    chr = g_malloc0(sizeof(CharDriverState));
> +
> +    capacity = qemu_opt_get_number(opts, "maxcapacity", 0);
> +    if (capacity == 0) {
> +        capacity = CBUFF_SIZE;
> +    }
> +
> +    qemu_chr_init_mem(chr, capacity);
> +
> +    return chr;
> +}
> +
>  void qmp_memchar_write(const char *chardev, int64_t size,
>                         const char *data, bool has_format,
>                         enum DataFormat format,
> @@ -2779,6 +2797,11 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
>          qemu_opt_set(opts, "path", p);
>          return opts;
>      }
> +    if (strstart(filename, "memchr", &p)) {
> +        qemu_opt_set(opts, "backend", "memchr");
> +        qemu_opt_set(opts, "maxcapacity", p);
> +        return opts;
> +    }
>      if (strstart(filename, "tcp:", &p) ||
>          strstart(filename, "telnet:", &p)) {
>          if (sscanf(p, "%64[^:]:%32[^,]%n", host, port, &pos) < 2) {
> @@ -2852,6 +2875,7 @@ static const struct {
>      { .name = "udp",       .open = qemu_chr_open_udp },
>      { .name = "msmouse",   .open = qemu_chr_open_msmouse },
>      { .name = "vc",        .open = text_console_init },
> +    { .name = "memchr",    .open = qemu_chr_open_memchr },
>  #ifdef _WIN32
>      { .name = "file",      .open = qemu_chr_open_win_file_out },
>      { .name = "pipe",      .open = qemu_chr_open_win_pipe },

Is extending the legacy syntax worthwhile?

[...]

Re: [Qemu-devel] [PATCH 3/6] QAPI: Introduce memchar_write QMP command

[Markus Armbruster]

Lei Li <lilei@linux.vnet.ibm.com> writes:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  hmp-commands.hx  |   16 ++++++++++++++++
>  hmp.c            |   15 +++++++++++++++
>  hmp.h            |    1 +
>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>  qemu-char.c      |   36 ++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   33 +++++++++++++++++++++++++++++++++
>  6 files changed, 129 insertions(+), 0 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f6104b0..829aea1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -797,6 +797,22 @@ Inject an NMI on the given CPU (x86 only).
>  ETEXI
>  
>      {
> +        .name       = "memchar-write",
> +        .args_type  = "chardev:s,size:i,data:s,format:s?",
> +        .params     = "chardev size data",

Make this "chardev size data [format]", and

> +        .help       = "Provide writing interface for memchardev. Write"
> +                      "'data' to memchr char device with size 'size'",

document format here.

> +        .mhandler.cmd = hmp_memchar_write,
> +    },
> +
> +STEXI
> +@item memchar-write @var{chardev} @var{size} @var{data} @var{format}

Argument format is optional, so make this [@var{format}].

What's the purpose of the size argument?  More below.

> +@findex memchar-write
> +Provide writing interface for memchardev. Write @var{data}
> +to memchr char device with size @var{size}.
> +ETEXI

Document format, please.

> +
> +    {
>          .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 81c8acb..c1164eb 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -668,6 +668,21 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
>      hmp_handle_error(mon, &errp);
>  }
>  
> +void hmp_memchar_write(Monitor *mon, const QDict *qdict)
> +{
> +    uint32_t size = qdict_get_int(qdict, "size");
> +    const char *chardev = qdict_get_str(qdict, "chardev");
> +    const char *data = qdict_get_str(qdict, "data");
> +    int con = qdict_get_try_bool(qdict, "utf8", 0);
> +    enum DataFormat format;
> +    Error *errp = NULL;
> +
> +    format = con ? DATA_FORMAT_UTF8 : DATA_FORMAT_BASE64;
> +    qmp_memchar_write(chardev, size, data, true, format, &errp);
> +
> +    hmp_handle_error(mon, &errp);
> +}
> +
>  static void hmp_cont_cb(void *opaque, int err)
>  {
>      if (!err) {
> diff --git a/hmp.h b/hmp.h
> index 7dd93bf..15e1311 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -43,6 +43,7 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
>  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_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 bd8ad74..2fc1a27 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -235,6 +235,34 @@
>  ##
>  { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
>  
> +{ 'enum': 'DataFormat'
> +  'data': [ 'utf8', 'base64' ] }
> +
> +##
> +# @memchar-write:
> +#
> +# Provide writing interface for memchardev. Write data to memchar
> +# char device.
> +#
> +# @chardev: the name of the memchar char device.
> +#
> +# @size: the size to write in bytes.
> +#
> +# @data: the source data write to memchar.
> +#
> +# @format: #optional the format of the data write to memchardev, by
> +#          default is 'utf8'.
> +#
> +# Returns: Nothing on success
> +#          If @chardev is not a valid memchr device, DeviceNotFound
> +#          If an I/O error occurs while writing, IOError
> +#
> +# Since: 1.3
> +##
> +{ 'command': 'memchar-write',
> +  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
> +           '*format': 'DataFormat'} }
> +
>  ##
>  # @CommandInfo:
>  #
> diff --git a/qemu-char.c b/qemu-char.c
> index b21b93a..d8f3238 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2647,6 +2647,42 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr)
>      return d->cbuf_count;
>  }
>  
> +void qmp_memchar_write(const char *chardev, int64_t size,
> +                       const char *data, bool has_format,
> +                       enum DataFormat format,
> +
> +                       Error **errp)
> +{
> +    CharDriverState *chr;
> +    guchar *write_data;
> +    int ret;
> +    gsize write_count;
> +
> +    chr = qemu_chr_find(chardev);
> +
> +    if(!chr) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, chardev);
> +        return;
> +    }
> +
> +    write_count = (gsize)size;
> +
> +    if (has_format) {
> +        if (format == DATA_FORMAT_BASE64) {
> +            write_data = g_base64_decode(data, &write_count);

Overwrites write_count.  Argument size is ignored.

> +        }
> +    } else {
> +        write_data = (uint8_t *)data;
> +    }
> +
> +    ret = mem_chr_write(chr, write_data, size);

I'm afraid this can read beyond the of data[] when !has_format.
Consider 'memchar_write foo "short and sweet" 1234567890'.

> +    if (ret <= 0) {
> +        error_set(errp, QERR_IO_ERROR);
> +    }
> +    g_free(write_data);

If has_format, this frees memory allocated by g_base64_decode().  Else,
it frees argument data.  Oops.

> +
> +}
> +
>  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 3745a21..3c747cd 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -441,6 +441,39 @@ Note: inject-nmi fails when the guest doesn't support injecting.
>  EQMP
>  
>      {
> +        .name       = "memchar-write",
> +        .args_type  = "chardev:s,size:i,data:s,format:s?",
> +        .mhandler.cmd_new = qmp_marshal_input_memchar_write,
> +    },
> +
> +SQMP
> +memchar-write
> +-------------
> +
> +Provide writing interface for memchardev. Write data to memchar
> +char device.
> +
> +Arguments:
> +
> +- "chardev": the name of the char device, must be unique (json-string)
> +- "size": the memory size, in bytes (json-int)
> +- "data": the source data writed to memchar (json-string)
> +- "format": the data format write to memchardev
> +  (DataFormat, optional, support "utf8" and "base64" now, default "utf8")
> +
> +
> +Example:
> +
> +-> { "execute": "memchar-write",
> +                "arguments": { "chardev": foo,
> +                               "size": 1000,
> +                               "data": "data string...",
> +                               "format": "utf8" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>          .name       = "xen-save-devices-state",
>          .args_type  = "filename:F",
>      .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,

Re: [Qemu-devel] [PATCH 3/6] QAPI: Introduce memchar_write QMP command

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 08/22/2012 11:14 PM, Lei Li wrote:
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>
> Subject line uses '_', but the QMP command uses '-' [1]
>
>> ---
>>  hmp-commands.hx  |   16 ++++++++++++++++
>>  hmp.c            |   15 +++++++++++++++
>>  hmp.h            |    1 +
>>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>>  qemu-char.c      |   36 ++++++++++++++++++++++++++++++++++++
>>  qmp-commands.hx  |   33 +++++++++++++++++++++++++++++++++
>>  6 files changed, 129 insertions(+), 0 deletions(-)
>> 
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index f6104b0..829aea1 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -797,6 +797,22 @@ Inject an NMI on the given CPU (x86 only).
>>  ETEXI
>>  
>>      {
>> +        .name       = "memchar-write",
>
> HMP commands should use '_', not '-'.

Both '_' and '-' occur in HMP command names.  There are more occurences
of '_', though.

Otherwise identical HMP and QMP commands differing only in '_' vs. '-'
is ugly.  Maybe we should simply map '_' in HMP command names to '-' and
be done with it.

[...]

Re: [Qemu-devel] [PATCH 2/6] monitor: Adjust qmp_human_monitor_command to new MemCharDriver

[Lei Li]

On 08/31/2012 02:51 AM, Luiz Capitulino wrote:
> On Thu, 23 Aug 2012 13:14:22 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>> ---
>>   monitor.c |    8 +++++++-
>>   1 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 480f583..ab4650b 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -642,7 +642,13 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
>>       CharDriverState mchar;
>>   
>>       memset(&hmp, 0, sizeof(hmp));
>> -    qemu_chr_init_mem(&mchar);
>> +
>> +    /* Since the backend of MemCharDriver convert to a circular
>> +     * buffer with fixed size, so should indicate the init memory
>> +     * size.
>> +     *
>> +     * XXX:  is 4096 as init memory enough for this? */
>> +    qemu_chr_init_mem(&mchar, 4096);
> I'm not sure I like this. The end result will be that hmp commands writing
> more than 4096 bytes will simply fail or return garbage (if the circular buffer
> is changed to allow writing more than it supports) today they would just work.
>
> Although it's always possible to increase the buffer size, we would only realize
> this is needed when the bug is triggered, which means it has a high chance
> of happening in production. IOW, this would be a regression.
>
> The only solution I can think of is to make the circular buffer and the
> current MemoryDriver live in parallel. Actually, you really seem to be
> adding something else.

Hi Luiz,

Thanks for your review. Yes, I agree with that it's not a good solution for the
old user human command. Make the circular buffer and the current MemoryDriver
live in parallel, do you mean don't change the current MemoryDriver, choose to
expose a new char device backend with circular buffer?

>>       hmp.chr = &mchar;
>>   
>>       old_mon = cur_mon;
>


-- 
Lei

Re: [Qemu-devel] [PATCH 2/6] monitor: Adjust qmp_human_monitor_command to new MemCharDriver

[Luiz Capitulino]

On Tue, 04 Sep 2012 00:14:03 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> On 08/31/2012 02:51 AM, Luiz Capitulino wrote:
> > On Thu, 23 Aug 2012 13:14:22 +0800
> > Lei Li <lilei@linux.vnet.ibm.com> wrote:
> >
> >> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> >> ---
> >>   monitor.c |    8 +++++++-
> >>   1 files changed, 7 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/monitor.c b/monitor.c
> >> index 480f583..ab4650b 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -642,7 +642,13 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
> >>       CharDriverState mchar;
> >>   
> >>       memset(&hmp, 0, sizeof(hmp));
> >> -    qemu_chr_init_mem(&mchar);
> >> +
> >> +    /* Since the backend of MemCharDriver convert to a circular
> >> +     * buffer with fixed size, so should indicate the init memory
> >> +     * size.
> >> +     *
> >> +     * XXX:  is 4096 as init memory enough for this? */
> >> +    qemu_chr_init_mem(&mchar, 4096);
> > I'm not sure I like this. The end result will be that hmp commands writing
> > more than 4096 bytes will simply fail or return garbage (if the circular buffer
> > is changed to allow writing more than it supports) today they would just work.
> >
> > Although it's always possible to increase the buffer size, we would only realize
> > this is needed when the bug is triggered, which means it has a high chance
> > of happening in production. IOW, this would be a regression.
> >
> > The only solution I can think of is to make the circular buffer and the
> > current MemoryDriver live in parallel. Actually, you really seem to be
> > adding something else.
> 
> Hi Luiz,
> 
> Thanks for your review. Yes, I agree with that it's not a good solution for the
> old user human command. Make the circular buffer and the current MemoryDriver
> live in parallel, do you mean don't change the current MemoryDriver, choose to
> expose a new char device backend with circular buffer?

Yes, you could add CircularMemoryDriver for example.

> 
> >>       hmp.chr = &mchar;
> >>   
> >>       old_mon = cur_mon;
> >
> 
>