[Qemu-devel] [PATCH 0/4 V6] char: Add CirMemCharDriver and provide QMP interface

[Qemu-devel] [PATCH 0/4 V6] char: Add CirMemCharDriver and provide QMP interface

[Lei Li]

This patch series attempts to add new char backend CirMemCharDriver with
a circular buffer and expose it to users by introducing QMP interface
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',
           'format': 'str' } }

{ 'command': 'memchar-read',
  'data': {'chardev': 'str', 'size': 'int', 'format': 'str' },
  'returns': 'str' }

Expose CirMemCharDriver via the command line like:

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

Introduce HMP command 'console' like:

(qemu) console foo
foo: Input data

Note:
Now all of the feature were implemented, and the pervious comments
are fixed up too. Please comment and let me know if there is anything
else need to be improved. Your suggestion would be very appreciated!

Changes since v5:
  - Avoid writing the IAC information to the queue.
  - Grammar of the doc for command line options improved from Eric.

Changes since v4:
  - Get rid of all CongestionControl bits, and assume a dropping behavior
    based on Luiz's suggestion for now. Will add it when we add async
    support to QMP.
  - Squashed the patches about CirMemCharDriver in one.
  - Other fixups from Luiz.

Changes since v3:
  - Improve the algorithm of circular buffer based on Anthony's
    suggestion.
  - Some changes suggested by Luiz and Blue.
  - And other fixups.

Changes since v2:
  - Add congestion mechanism. For the 'block' option as sync command,
    will support it later when we gain the necessary infrastructure
    enhancement.
  - Add HMP 'console' command so that can interact with multiple
    chardevs via a single monitor socket.
  - Make the circular buffer backend and the current MemCharDriver
    live in parallel, expose a new char backend with circular buffer
    CirMemCharDriver suggested by Luiz.
  - Other fixs from Eric and Markus.

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.


Lei Li (4):
  qemu-char: Add new char backend CirMemCharDriver
  QAPI: Introduce memchar-write QMP command
  QAPI: Introduce memchar-read QMP command
  HMP: Introduce console command

 hmp-commands.hx  |   72 +++++++++++++++++++
 hmp.c            |   99 +++++++++++++++++++++++
 hmp.h            |    3 +
 monitor.c        |   15 ++++
 monitor.h        |    3 +
 qapi-schema.json |   96 +++++++++++++++++++++++++
 qemu-char.c      |  217 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-config.c    |    3 +
 qemu-options.hx  |   10 +++
 qmp-commands.hx  |   89 +++++++++++++++++++++
 10 files changed, 607 insertions(+), 0 deletions(-)

[Qemu-devel] [PATCH 1/4] qemu-char: Add new char backend CirMemCharDriver

[Lei Li]

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

diff --git a/qemu-char.c b/qemu-char.c
index b082bae..c3ec43d 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 */
@@ -2588,6 +2589,134 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr)
     return d->outbuf_size;
 }
 
+/*********************************************************/
+/*CircularMemory chardev*/
+
+typedef struct {
+    size_t size;
+    size_t producer;
+    size_t consumer;
+    uint8_t *cbuf;
+} CirMemCharDriver;
+
+static bool cirmem_chr_is_empty(const CharDriverState *chr)
+{
+    const CirMemCharDriver *d = chr->opaque;
+
+    return d->producer == d->consumer;
+}
+
+static bool cirmem_chr_is_full(const CharDriverState *chr)
+{
+    const CirMemCharDriver *d = chr->opaque;
+
+    return (d->producer - d->consumer) >= d->size;
+}
+
+static bool cirmem_chr_unlimit(const CharDriverState *chr)
+{
+    const CirMemCharDriver *d = chr->opaque;
+
+    return d->producer >= d->consumer;
+}
+
+static int cirmem_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
+{
+    CirMemCharDriver *d = chr->opaque;
+    int i;
+
+    if (!buf || (len < 0)) {
+        return -1;
+    }
+
+    for (i = 0; i < len; i++ ) {
+        if (cirmem_chr_unlimit(chr)) {
+            /* Avoid writing the IAC information to the queue. */
+            if ((unsigned char)buf[i] == IAC) {
+                continue;
+            }
+            d->cbuf[d->producer % d->size] = buf[i];
+            d->producer++;
+        } else {
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+static int cirmem_chr_read(CharDriverState *chr, uint8_t *buf, int len)
+{
+    CirMemCharDriver *d = chr->opaque;
+    int i;
+
+    if (cirmem_chr_is_empty(chr) || len < 0) {
+        return -1;
+    }
+
+    if (len > d->size) {
+        len = d->size;
+    }
+
+    for (i = 0; i < len; i++) {
+        if (cirmem_chr_unlimit(chr)) {
+            buf[i] = d->cbuf[d->consumer % d->size];
+            d->consumer++;
+
+            if (cirmem_chr_is_empty(chr)) {
+                break;
+            }
+        } else {
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+static void cirmem_chr_close(struct CharDriverState *chr)
+{
+    CirMemCharDriver *d = chr->opaque;
+
+    g_free(d->cbuf);
+    g_free(d);
+    chr->opaque = NULL;
+}
+
+static CharDriverState *qemu_chr_open_cirmemchr(QemuOpts *opts)
+{
+    CharDriverState *chr;
+    CirMemCharDriver *d;
+
+    chr = g_malloc0(sizeof(CharDriverState));
+    d = g_malloc(sizeof(*d));
+
+    d->size = qemu_opt_get_number(opts, "maxcapacity", 0);
+    if (d->size == 0) {
+        d->size = CBUFF_SIZE;
+    }
+
+    /* The size must be power of 2 */
+    if (d->size & (d->size - 1)) {
+        goto fail;
+    }
+
+    d->producer = 0;
+    d->consumer = 0;
+    d->cbuf = g_malloc0(d->size);
+
+    chr->opaque = d;
+    chr->chr_write = cirmem_chr_write;
+    chr->chr_close = cirmem_chr_close;
+
+    return chr;
+
+fail:
+    g_free(d);
+    g_free(chr);
+    return NULL;
+}
+
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
 {
     char host[65], port[33], width[8], height[8];
@@ -2652,6 +2781,16 @@ 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, "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) {
@@ -2725,6 +2864,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_cirmemchr },
 #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 cd1ec21..5553bb6 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -213,6 +213,9 @@ static QemuOptsList qemu_chardev_opts = {
         },{
             .name = "debug",
             .type = QEMU_OPT_NUMBER,
+        },{
+            .name = "maxcapacity",
+            .type = QEMU_OPT_NUMBER,
         },
         { /* end of list */ }
     },
diff --git a/qemu-options.hx b/qemu-options.hx
index 46f0539..9b2d792 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1680,6 +1680,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,maxcapacity=maxcapacity\n"
     "-chardev file,id=id,path=path[,mux=on|off]\n"
     "-chardev pipe,id=id,path=path[,mux=on|off]\n"
 #ifdef _WIN32
@@ -1718,6 +1719,7 @@ Backend is one of:
 @option{udp},
 @option{msmouse},
 @option{vc},
+@option{memchr},
 @option{file},
 @option{pipe},
 @option{console},
@@ -1824,6 +1826,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} ,maxcapacity=@var{maxcapacity}
+
+Create a circular buffer with fixed size indicated by optionally @option{maxcapacity}
+which will be default 64K if it is not given.
+
+@option{maxcapacity} specifies the max capacity of the size of circular buffer
+to create. Should be power of 2.
+
 @item -chardev file ,id=@var{id} ,path=@var{path}
 
 Log all traffic received from the guest to a file.
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/4] QAPI: Introduce memchar-write QMP command

[Lei Li]

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

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..a37b8e9 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -825,6 +825,23 @@ Inject an NMI on the given CPU (x86 only).
 ETEXI
 
     {
+        .name       = "memchar_write",
+        .args_type  = "chardev:s,data:s",
+        .params     = "chardev data",
+        .mhandler.cmd = hmp_memchar_write,
+    },
+
+STEXI
+@item memchar_write @var{chardev} @var{data}
+@findex memchar_write
+Provide writing interface for CirMemCharDriver. Write @var{data}
+to cirmemchr char device. Note that 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.
+
+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 2b97982..082985b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -683,6 +683,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;
+    const char *chardev = qdict_get_str(qdict, "chardev");
+    const char *data = qdict_get_str(qdict, "data");
+    enum DataFormat format;
+    Error *errp = NULL;
+
+    size = strlen(data);
+    format = DATA_FORMAT_UTF8;
+    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 71ea384..406ebb1 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 c615ee2..43ef6bc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -325,6 +325,53 @@
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
 
 ##
+# @DataFormat:
+#
+# An enumeration of data format. The default value would
+# be utf8.
+#
+# @utf8: The data format is 'utf8'.
+#
+# @base64: The data format is 'base64'.
+#
+# Note: The data format start with 'utf8' and 'base64',
+#       will support other data format as well.
+#
+# Since: 1.3
+##
+{ '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. Should be power of 2.
+#
+# @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
+#
+# 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-write',
+  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
+           '*format': 'DataFormat'} }
+
+##
 # @CommandInfo:
 #
 # Information about a QMP command
diff --git a/qemu-char.c b/qemu-char.c
index c3ec43d..6114e29 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2717,6 +2717,50 @@ fail:
     return NULL;
 }
 
+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;
+    }
+
+    if (strcmp(chr->filename, "memchr") != 0) {
+        error_setg(errp,"%s is not memory char device\n", chardev);
+        return;
+    }
+
+    /* XXX: For the sync command as 'block', waiting for the qmp
+     * to support necessary feature. Now just act as 'drop' */
+    if (cirmem_chr_is_full(chr)) {
+        error_setg(errp, "Failed to write to memchr %s", chardev);
+        return;
+    }
+
+    write_count = (gsize)size;
+
+    if (has_format && (format == DATA_FORMAT_BASE64)) {
+        write_data = g_base64_decode(data, &write_count);
+    } else {
+        write_data = (uint8_t *)data;
+    }
+
+    ret = cirmem_chr_write(chr, write_data, write_count);
+
+    if (ret < 0) {
+        error_setg(errp, "Failed to write to memchr %s", chardev);
+        return;
+    }
+}
+
 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 5ba8c48..7548b9b 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -466,6 +466,40 @@ 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?,control:s?",
+        .help       = "write size of data to memchar chardev",
+        .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, should be power of 2 (json-int)
+- "data": the source data writed to memchar (json-string)
+- "format": the data format write to memchardev, default is
+            utf8. (json-string, optional)
+          - Possible values: "utf8", "base64"
+
+Example:
+
+-> { "execute": "memchar-write",
+                "arguments": { "chardev": foo,
+                               "size": 8,
+                               "data": "abcdefgh",
+                               "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 3/4] QAPI: Introduce memchar-read QMP command

[Lei Li]

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;
+    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,
-- 
1.7.7.6

[Qemu-devel] [PATCH 4/4] HMP: Introduce console command

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 hmp-commands.hx |   25 +++++++++++++++++++++++++
 hmp.c           |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 hmp.h           |    1 +
 monitor.c       |   15 +++++++++++++++
 monitor.h       |    3 +++
 5 files changed, 96 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index df294eb..7cba42c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -861,6 +861,31 @@ if the requested size is larger than it.
 ETEXI
 
     {
+        .name       = "console",
+        .args_type  = "chardev:s",
+        .params     = "chardev",
+        .help       = "Connect to the serial console from within the"
+                      "monitor, allow to write data to memchardev"
+                      "'chardev'. Exit from the console and return back"
+                      "to monitor by typing 'ctrl-]'",
+        .mhandler.cmd = hmp_console,
+    },
+
+STEXI
+@item console @var{device}
+@findex console
+
+Connect to the serial console from within the monitor, allow to write data
+to memchardev @var{chardev}. Exit from the console and return back to
+monitor by typing 'ctrl-]'.
+@example
+(qemu) console foo
+foo: data string...
+@end example
+
+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 ef85736..d716410 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1255,3 +1255,55 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
     qmp_screendump(filename, &err);
     hmp_handle_error(mon, &err);
 }
+
+enum escape_char
+{
+    ESCAPE_CHAR_CTRL_GS = 0x1d  /* ctrl-] used for escape */
+};
+
+static void hmp_read_console(Monitor *mon, const char *data,
+                             void *opaque)
+{
+    CharDriverState *chr = opaque;
+    uint32_t size = strlen(data);
+    enum DataFormat format = DATA_FORMAT_UTF8;
+    enum escape_char console_escape = ESCAPE_CHAR_CTRL_GS;
+
+    Error *err = NULL;
+
+    if (*data == console_escape) {
+        monitor_resume(mon);
+        return;
+    }
+
+    qmp_memchar_write(chr->label, size, data, 0, format, &err);
+
+    if (err) {
+        monitor_printf(mon, "%s\n", error_get_pretty(err));
+        error_free(err);
+        return;
+    }
+
+    monitor_read_command(mon, 1);
+}
+
+void hmp_console(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "chardev");
+    CharDriverState *chr;
+    Error *err = NULL;
+
+    chr = qemu_chr_find(device);
+
+    if (!chr) {
+        error_set(&err, QERR_DEVICE_NOT_FOUND, device);
+        goto out;
+    }
+
+    if (monitor_read_console(mon, device, hmp_read_console, chr) < 0) {
+        monitor_printf(mon, "Connect to console %s failed\n", device);
+    }
+
+out:
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index a5a0cfe..5b54a79 100644
--- a/hmp.h
+++ b/hmp.h
@@ -77,5 +77,6 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_console(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/monitor.c b/monitor.c
index d17ae2d..7e90115 100644
--- a/monitor.c
+++ b/monitor.c
@@ -256,6 +256,21 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
     }
 }
 
+int monitor_read_console(Monitor *mon, const char *device,
+                         ReadLineFunc *readline_func, void *opaque)
+{
+    char prompt[60];
+
+    if (!mon->rs) {
+        return -1;
+    }
+
+    snprintf(prompt, sizeof(prompt), "%s: ", device);
+    readline_start(mon->rs, prompt, 0, readline_func, opaque);
+
+    return 0;
+}
+
 void monitor_flush(Monitor *mon)
 {
     if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
diff --git a/monitor.h b/monitor.h
index b6e7d95..735bd1b 100644
--- a/monitor.h
+++ b/monitor.h
@@ -86,6 +86,9 @@ ReadLineState *monitor_get_rs(Monitor *mon);
 int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
                           void *opaque);
 
+int monitor_read_console(Monitor *mon, const char *device,
+                         ReadLineFunc *readline_func, void *opaque);
+
 int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
 
 int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 1/4] qemu-char: Add new char backend CirMemCharDriver

[Luiz Capitulino]

On Fri, 26 Oct 2012 19:21:49 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  qemu-char.c     |  140 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-config.c   |    3 +
>  qemu-options.hx |   10 ++++
>  3 files changed, 153 insertions(+), 0 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index b082bae..c3ec43d 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 */
> @@ -2588,6 +2589,134 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr)
>      return d->outbuf_size;
>  }
>  
> +/*********************************************************/
> +/*CircularMemory chardev*/
> +
> +typedef struct {
> +    size_t size;
> +    size_t producer;
> +    size_t consumer;
> +    uint8_t *cbuf;
> +} CirMemCharDriver;
> +
> +static bool cirmem_chr_is_empty(const CharDriverState *chr)
> +{
> +    const CirMemCharDriver *d = chr->opaque;
> +
> +    return d->producer == d->consumer;
> +}
> +
> +static bool cirmem_chr_is_full(const CharDriverState *chr)
> +{
> +    const CirMemCharDriver *d = chr->opaque;
> +
> +    return (d->producer - d->consumer) >= d->size;
> +}
> +
> +static bool cirmem_chr_unlimit(const CharDriverState *chr)
> +{
> +    const CirMemCharDriver *d = chr->opaque;
> +
> +    return d->producer >= d->consumer;
> +}

I'm not sure I get why this is needed. Are you trying to fix the problem
I mentioned in my last review that producer and consumer might overflow
for long running VMs? If yes, then the problem still exists.

The only fix I can think of is to re-initialize them. Of course that you
could do it in a way that they keep pointing to the right position in cbuf[].

> +
> +static int cirmem_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
> +{
> +    CirMemCharDriver *d = chr->opaque;
> +    int i;
> +
> +    if (!buf || (len < 0)) {
> +        return -1;
> +    }
> +
> +    for (i = 0; i < len; i++ ) {
> +        if (cirmem_chr_unlimit(chr)) {
> +            /* Avoid writing the IAC information to the queue. */
> +            if ((unsigned char)buf[i] == IAC) {
> +                continue;
> +            }
> +            d->cbuf[d->producer % d->size] = buf[i];
> +            d->producer++;
> +        } else {
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static int cirmem_chr_read(CharDriverState *chr, uint8_t *buf, int len)
> +{
> +    CirMemCharDriver *d = chr->opaque;
> +    int i;
> +
> +    if (cirmem_chr_is_empty(chr) || len < 0) {
> +        return -1;
> +    }
> +
> +    if (len > d->size) {
> +        len = d->size;
> +    }
> +
> +    for (i = 0; i < len; i++) {
> +        if (cirmem_chr_unlimit(chr)) {
> +            buf[i] = d->cbuf[d->consumer % d->size];
> +            d->consumer++;
> +
> +            if (cirmem_chr_is_empty(chr)) {
> +                break;
> +            }
> +        } else {
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static void cirmem_chr_close(struct CharDriverState *chr)
> +{
> +    CirMemCharDriver *d = chr->opaque;
> +
> +    g_free(d->cbuf);
> +    g_free(d);
> +    chr->opaque = NULL;
> +}
> +
> +static CharDriverState *qemu_chr_open_cirmemchr(QemuOpts *opts)
> +{
> +    CharDriverState *chr;
> +    CirMemCharDriver *d;
> +
> +    chr = g_malloc0(sizeof(CharDriverState));
> +    d = g_malloc(sizeof(*d));
> +
> +    d->size = qemu_opt_get_number(opts, "maxcapacity", 0);
> +    if (d->size == 0) {
> +        d->size = CBUFF_SIZE;
> +    }
> +
> +    /* The size must be power of 2 */

If this is enforced in vl.c (which I can't remember right now), then
you should turn this into an assert().

> +    if (d->size & (d->size - 1)) {
> +        goto fail;
> +    }
> +
> +    d->producer = 0;
> +    d->consumer = 0;
> +    d->cbuf = g_malloc0(d->size);
> +
> +    chr->opaque = d;
> +    chr->chr_write = cirmem_chr_write;
> +    chr->chr_close = cirmem_chr_close;
> +
> +    return chr;
> +
> +fail:
> +    g_free(d);
> +    g_free(chr);
> +    return NULL;
> +}
> +
>  QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
>  {
>      char host[65], port[33], width[8], height[8];
> @@ -2652,6 +2781,16 @@ 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, "memchr", &p)) {
> +        qemu_opt_set(opts, "backend", "memchr");
> +        qemu_opt_set(opts, "maxcapacity", p);
> +        return opts;

Duplication?

> +    }
>      if (strstart(filename, "tcp:", &p) ||
>          strstart(filename, "telnet:", &p)) {
>          if (sscanf(p, "%64[^:]:%32[^,]%n", host, port, &pos) < 2) {
> @@ -2725,6 +2864,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_cirmemchr },

What about my suggestion of calling this "memory"? All backends here are
chardevs, but we don't have vcchr, msmousechr, stdiochr etc.

>  #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 cd1ec21..5553bb6 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -213,6 +213,9 @@ static QemuOptsList qemu_chardev_opts = {
>          },{
>              .name = "debug",
>              .type = QEMU_OPT_NUMBER,
> +        },{
> +            .name = "maxcapacity",
> +            .type = QEMU_OPT_NUMBER,
>          },
>          { /* end of list */ }
>      },
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 46f0539..9b2d792 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1680,6 +1680,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,maxcapacity=maxcapacity\n"
>      "-chardev file,id=id,path=path[,mux=on|off]\n"
>      "-chardev pipe,id=id,path=path[,mux=on|off]\n"
>  #ifdef _WIN32
> @@ -1718,6 +1719,7 @@ Backend is one of:
>  @option{udp},
>  @option{msmouse},
>  @option{vc},
> +@option{memchr},
>  @option{file},
>  @option{pipe},
>  @option{console},
> @@ -1824,6 +1826,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} ,maxcapacity=@var{maxcapacity}
> +
> +Create a circular buffer with fixed size indicated by optionally @option{maxcapacity}
> +which will be default 64K if it is not given.
> +
> +@option{maxcapacity} specifies the max capacity of the size of circular buffer
> +to create. Should be power of 2.
> +
>  @item -chardev file ,id=@var{id} ,path=@var{path}
>  
>  Log all traffic received from the guest to a file.

Re: [Qemu-devel] [PATCH 2/4] QAPI: Introduce memchar-write QMP command

[Luiz Capitulino]

On Fri, 26 Oct 2012 19:21:50 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  hmp-commands.hx  |   17 +++++++++++++++++
>  hmp.c            |   15 +++++++++++++++
>  hmp.h            |    1 +
>  qapi-schema.json |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>  qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   34 ++++++++++++++++++++++++++++++++++
>  6 files changed, 158 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e0b537d..a37b8e9 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -825,6 +825,23 @@ Inject an NMI on the given CPU (x86 only).
>  ETEXI
>  
>      {
> +        .name       = "memchar_write",
> +        .args_type  = "chardev:s,data:s",
> +        .params     = "chardev data",
> +        .mhandler.cmd = hmp_memchar_write,
> +    },
> +
> +STEXI
> +@item memchar_write @var{chardev} @var{data}
> +@findex memchar_write
> +Provide writing interface for CirMemCharDriver. Write @var{data}
> +to cirmemchr char device. Note that 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.

You can drop everything after "Note".

> +
> +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 2b97982..082985b 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -683,6 +683,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;
> +    const char *chardev = qdict_get_str(qdict, "chardev");
> +    const char *data = qdict_get_str(qdict, "data");
> +    enum DataFormat format;

Why do you need this variable?

> +    Error *errp = NULL;
> +
> +    size = strlen(data);
> +    format = DATA_FORMAT_UTF8;
> +    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 71ea384..406ebb1 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 c615ee2..43ef6bc 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -325,6 +325,53 @@
>  { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
>  
>  ##
> +# @DataFormat:
> +#
> +# An enumeration of data format. The default value would
> +# be utf8.

Please, remove the "default value" part. This is decided by the command
using this type.

> +#
> +# @utf8: The data format is 'utf8'.
> +#
> +# @base64: The data format is 'base64'.
> +#
> +# Note: The data format start with 'utf8' and 'base64',
> +#       will support other data format as well.

Please, drop this note. It's not needed.

> +#
> +# Since: 1.3
> +##
> +{ '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. Should be power of 2.
> +#
> +# @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
> +#
> +# 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-write',
> +  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
> +           '*format': 'DataFormat'} }
> +
> +##
>  # @CommandInfo:
>  #
>  # Information about a QMP command
> diff --git a/qemu-char.c b/qemu-char.c
> index c3ec43d..6114e29 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2717,6 +2717,50 @@ fail:
>      return NULL;
>  }
>  
> +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;
> +    }
> +
> +    if (strcmp(chr->filename, "memchr") != 0) {
> +        error_setg(errp,"%s is not memory char device\n", chardev);
> +        return;
> +    }

This is wrong, as it's unreliable. A hackish (but hopefully correct) way
of doing this would be to check chr->init against the circmem init function.
But please, put this behind a function because it's ugly.

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

Please, replace this note with an explanation of the current behavior.

> +    if (cirmem_chr_is_full(chr)) {
> +        error_setg(errp, "Failed to write to memchr %s", chardev);
> +        return;
> +    }
> +
> +    write_count = (gsize)size;
> +
> +    if (has_format && (format == DATA_FORMAT_BASE64)) {
> +        write_data = g_base64_decode(data, &write_count);
> +    } else {
> +        write_data = (uint8_t *)data;
> +    }
> +
> +    ret = cirmem_chr_write(chr, write_data, write_count);

IIRC circmem_chr_write() doesn't check if write_count if a power of two,
what happens if it isn't?

> +
> +    if (ret < 0) {
> +        error_setg(errp, "Failed to write to memchr %s", chardev);
> +        return;
> +    }
> +}
> +
>  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 5ba8c48..7548b9b 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -466,6 +466,40 @@ 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?,control:s?",

You've dropped 'control', haven't you?

> +        .help       = "write size of data to memchar chardev",
> +        .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, should be power of 2 (json-int)
> +- "data": the source data writed to memchar (json-string)
> +- "format": the data format write to memchardev, default is
> +            utf8. (json-string, optional)
> +          - Possible values: "utf8", "base64"
> +
> +Example:
> +
> +-> { "execute": "memchar-write",
> +                "arguments": { "chardev": foo,
> +                               "size": 8,
> +                               "data": "abcdefgh",
> +                               "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/4] QAPI: Introduce memchar-read QMP command

[Luiz Capitulino]

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,

Re: [Qemu-devel] [PATCH 4/4] HMP: Introduce console command

[Luiz Capitulino]

On Fri, 26 Oct 2012 19:21:52 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

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

I still don't understand how this command, in its current form, is
different from memchar-write.

One more comment below.

> ---
>  hmp-commands.hx |   25 +++++++++++++++++++++++++
>  hmp.c           |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hmp.h           |    1 +
>  monitor.c       |   15 +++++++++++++++
>  monitor.h       |    3 +++
>  5 files changed, 96 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index df294eb..7cba42c 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -861,6 +861,31 @@ if the requested size is larger than it.
>  ETEXI
>  
>      {
> +        .name       = "console",
> +        .args_type  = "chardev:s",
> +        .params     = "chardev",
> +        .help       = "Connect to the serial console from within the"
> +                      "monitor, allow to write data to memchardev"
> +                      "'chardev'. Exit from the console and return back"
> +                      "to monitor by typing 'ctrl-]'",
> +        .mhandler.cmd = hmp_console,
> +    },
> +
> +STEXI
> +@item console @var{device}
> +@findex console
> +
> +Connect to the serial console from within the monitor, allow to write data
> +to memchardev @var{chardev}. Exit from the console and return back to
> +monitor by typing 'ctrl-]'.
> +@example
> +(qemu) console foo
> +foo: data string...
> +@end example
> +
> +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 ef85736..d716410 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1255,3 +1255,55 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
>      qmp_screendump(filename, &err);
>      hmp_handle_error(mon, &err);
>  }
> +
> +enum escape_char
> +{
> +    ESCAPE_CHAR_CTRL_GS = 0x1d  /* ctrl-] used for escape */
> +};
> +
> +static void hmp_read_console(Monitor *mon, const char *data,
> +                             void *opaque)
> +{
> +    CharDriverState *chr = opaque;
> +    uint32_t size = strlen(data);
> +    enum DataFormat format = DATA_FORMAT_UTF8;
> +    enum escape_char console_escape = ESCAPE_CHAR_CTRL_GS;
> +
> +    Error *err = NULL;
> +
> +    if (*data == console_escape) {
> +        monitor_resume(mon);
> +        return;
> +    }
> +
> +    qmp_memchar_write(chr->label, size, data, 0, format, &err);
> +
> +    if (err) {
> +        monitor_printf(mon, "%s\n", error_get_pretty(err));
> +        error_free(err);
> +        return;
> +    }
> +
> +    monitor_read_command(mon, 1);
> +}
> +
> +void hmp_console(Monitor *mon, const QDict *qdict)
> +{
> +    const char *device = qdict_get_str(qdict, "chardev");
> +    CharDriverState *chr;
> +    Error *err = NULL;
> +
> +    chr = qemu_chr_find(device);
> +
> +    if (!chr) {
> +        error_set(&err, QERR_DEVICE_NOT_FOUND, device);
> +        goto out;
> +    }

As I said before, I don't why chr is needed. It seems to me that passing
'device' down is enough.

> +
> +    if (monitor_read_console(mon, device, hmp_read_console, chr) < 0) {
> +        monitor_printf(mon, "Connect to console %s failed\n", device);
> +    }
> +
> +out:
> +    hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index a5a0cfe..5b54a79 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -77,5 +77,6 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
>  void hmp_closefd(Monitor *mon, const QDict *qdict);
>  void hmp_send_key(Monitor *mon, const QDict *qdict);
>  void hmp_screen_dump(Monitor *mon, const QDict *qdict);
> +void hmp_console(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/monitor.c b/monitor.c
> index d17ae2d..7e90115 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -256,6 +256,21 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>      }
>  }
>  
> +int monitor_read_console(Monitor *mon, const char *device,
> +                         ReadLineFunc *readline_func, void *opaque)
> +{
> +    char prompt[60];
> +
> +    if (!mon->rs) {
> +        return -1;
> +    }
> +
> +    snprintf(prompt, sizeof(prompt), "%s: ", device);
> +    readline_start(mon->rs, prompt, 0, readline_func, opaque);
> +
> +    return 0;
> +}
> +
>  void monitor_flush(Monitor *mon)
>  {
>      if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
> diff --git a/monitor.h b/monitor.h
> index b6e7d95..735bd1b 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -86,6 +86,9 @@ ReadLineState *monitor_get_rs(Monitor *mon);
>  int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>                            void *opaque);
>  
> +int monitor_read_console(Monitor *mon, const char *device,
> +                         ReadLineFunc *readline_func, void *opaque);
> +
>  int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
>  
>  int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);

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

[Lei Li]

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,
>


-- 
Lei

Re: [Qemu-devel] [PATCH 2/4] QAPI: Introduce memchar-write QMP command

[Lei Li]

On 10/27/2012 01:17 AM, Luiz Capitulino wrote:
> On Fri, 26 Oct 2012 19:21:50 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>> ---
>>   hmp-commands.hx  |   17 +++++++++++++++++
>>   hmp.c            |   15 +++++++++++++++
>>   hmp.h            |    1 +
>>   qapi-schema.json |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++++++++
>>   qmp-commands.hx  |   34 ++++++++++++++++++++++++++++++++++
>>   6 files changed, 158 insertions(+), 0 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index e0b537d..a37b8e9 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -825,6 +825,23 @@ Inject an NMI on the given CPU (x86 only).
>>   ETEXI
>>   
>>       {
>> +        .name       = "memchar_write",
>> +        .args_type  = "chardev:s,data:s",
>> +        .params     = "chardev data",
>> +        .mhandler.cmd = hmp_memchar_write,
>> +    },
>> +
>> +STEXI
>> +@item memchar_write @var{chardev} @var{data}
>> +@findex memchar_write
>> +Provide writing interface for CirMemCharDriver. Write @var{data}
>> +to cirmemchr char device. Note that 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.
> You can drop everything after "Note".
>
ok

>> +
>> +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 2b97982..082985b 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -683,6 +683,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;
>> +    const char *chardev = qdict_get_str(qdict, "chardev");
>> +    const char *data = qdict_get_str(qdict, "data");
>> +    enum DataFormat format;
> Why do you need this variable?

Sure, I will drop this.

>> +    Error *errp = NULL;
>> +
>> +    size = strlen(data);
>> +    format = DATA_FORMAT_UTF8;
>> +    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 71ea384..406ebb1 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 c615ee2..43ef6bc 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -325,6 +325,53 @@
>>   { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
>>   
>>   ##
>> +# @DataFormat:
>> +#
>> +# An enumeration of data format. The default value would
>> +# be utf8.
> Please, remove the "default value" part. This is decided by the command
> using this type.

Now the option format is optional, if it's not set then default by 'utf8'.
I think it's a reasonable behaver. :)

>> +#
>> +# @utf8: The data format is 'utf8'.
>> +#
>> +# @base64: The data format is 'base64'.
>> +#
>> +# Note: The data format start with 'utf8' and 'base64',
>> +#       will support other data format as well.
> Please, drop this note. It's not needed.
>
ok

>> +#
>> +# Since: 1.3
>> +##
>> +{ '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. Should be power of 2.
>> +#
>> +# @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
>> +#
>> +# 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.

Sure.

>> +#
>> +# Since: 1.3
>> +##
>> +{ 'command': 'memchar-write',
>> +  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
>> +           '*format': 'DataFormat'} }
>> +
>> +##
>>   # @CommandInfo:
>>   #
>>   # Information about a QMP command
>> diff --git a/qemu-char.c b/qemu-char.c
>> index c3ec43d..6114e29 100644
>> --- a/qemu-char.c
>> +++ b/qemu-char.c
>> @@ -2717,6 +2717,50 @@ fail:
>>       return NULL;
>>   }
>>   
>> +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;
>> +    }
>> +
>> +    if (strcmp(chr->filename, "memchr") != 0) {
>> +        error_setg(errp,"%s is not memory char device\n", chardev);
>> +        return;
>> +    }
> This is wrong, as it's unreliable. A hackish (but hopefully correct) way
> of doing this would be to check chr->init against the circmem init function.
> But please, put this behind a function because it's ugly.

Hmm, I guess it's because the chr->filename could be NULL. but we can still
get it since CharDriverState keeps the inited type of backend. I am not sure
that check chr->init against the cirmem init funciton could work, I'll have
a try.

>
>> +
>> +    /* XXX: For the sync command as 'block', waiting for the qmp
>> +     * to support necessary feature. Now just act as 'drop' */
> Please, replace this note with an explanation of the current behavior.

ok.

>> +    if (cirmem_chr_is_full(chr)) {
>> +        error_setg(errp, "Failed to write to memchr %s", chardev);
>> +        return;
>> +    }
>> +
>> +    write_count = (gsize)size;
>> +
>> +    if (has_format && (format == DATA_FORMAT_BASE64)) {
>> +        write_data = g_base64_decode(data, &write_count);
>> +    } else {
>> +        write_data = (uint8_t *)data;
>> +    }
>> +
>> +    ret = cirmem_chr_write(chr, write_data, write_count);
> IIRC circmem_chr_write() doesn't check if write_count if a power of two,
> what happens if it isn't?

I have considered your suggestion on this in v4. The reason why I did not
add the assert is that I think we have already check with this when cirmem
init, so there is no need to check it again. Correct me if I am wrong. :)

>> +
>> +    if (ret < 0) {
>> +        error_setg(errp, "Failed to write to memchr %s", chardev);
>> +        return;
>> +    }
>> +}
>> +
>>   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 5ba8c48..7548b9b 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -466,6 +466,40 @@ 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?,control:s?",
> You've dropped 'control', haven't you?

Yes, sorry I forgot to drop here.. :(

>
>> +        .help       = "write size of data to memchar chardev",
>> +        .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, should be power of 2 (json-int)
>> +- "data": the source data writed to memchar (json-string)
>> +- "format": the data format write to memchardev, default is
>> +            utf8. (json-string, optional)
>> +          - Possible values: "utf8", "base64"
>> +
>> +Example:
>> +
>> +-> { "execute": "memchar-write",
>> +                "arguments": { "chardev": foo,
>> +                               "size": 8,
>> +                               "data": "abcdefgh",
>> +                               "format": "utf8" } }
>> +<- { "return": {} }
>> +
>> +EQMP
>> +
>> +    {
>>           .name       = "xen-save-devices-state",
>>           .args_type  = "filename:F",
>>       .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,
>


-- 
Lei

Re: [Qemu-devel] [PATCH 4/4] HMP: Introduce console command

[Lei Li]

On 10/27/2012 01:43 AM, Luiz Capitulino wrote:
> On Fri, 26 Oct 2012 19:21:52 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> I still don't understand how this command, in its current form, is
> different from memchar-write.
>
> One more comment below.

Hi Luiz,

Yes, I have replied to it in patch series v4. You can look at it
as link below:

https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg04568.html

>
>> ---
>>   hmp-commands.hx |   25 +++++++++++++++++++++++++
>>   hmp.c           |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   hmp.h           |    1 +
>>   monitor.c       |   15 +++++++++++++++
>>   monitor.h       |    3 +++
>>   5 files changed, 96 insertions(+), 0 deletions(-)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index df294eb..7cba42c 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -861,6 +861,31 @@ if the requested size is larger than it.
>>   ETEXI
>>   
>>       {
>> +        .name       = "console",
>> +        .args_type  = "chardev:s",
>> +        .params     = "chardev",
>> +        .help       = "Connect to the serial console from within the"
>> +                      "monitor, allow to write data to memchardev"
>> +                      "'chardev'. Exit from the console and return back"
>> +                      "to monitor by typing 'ctrl-]'",
>> +        .mhandler.cmd = hmp_console,
>> +    },
>> +
>> +STEXI
>> +@item console @var{device}
>> +@findex console
>> +
>> +Connect to the serial console from within the monitor, allow to write data
>> +to memchardev @var{chardev}. Exit from the console and return back to
>> +monitor by typing 'ctrl-]'.
>> +@example
>> +(qemu) console foo
>> +foo: data string...
>> +@end example
>> +
>> +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 ef85736..d716410 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -1255,3 +1255,55 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
>>       qmp_screendump(filename, &err);
>>       hmp_handle_error(mon, &err);
>>   }
>> +
>> +enum escape_char
>> +{
>> +    ESCAPE_CHAR_CTRL_GS = 0x1d  /* ctrl-] used for escape */
>> +};
>> +
>> +static void hmp_read_console(Monitor *mon, const char *data,
>> +                             void *opaque)
>> +{
>> +    CharDriverState *chr = opaque;
>> +    uint32_t size = strlen(data);
>> +    enum DataFormat format = DATA_FORMAT_UTF8;
>> +    enum escape_char console_escape = ESCAPE_CHAR_CTRL_GS;
>> +
>> +    Error *err = NULL;
>> +
>> +    if (*data == console_escape) {
>> +        monitor_resume(mon);
>> +        return;
>> +    }
>> +
>> +    qmp_memchar_write(chr->label, size, data, 0, format, &err);
>> +
>> +    if (err) {
>> +        monitor_printf(mon, "%s\n", error_get_pretty(err));
>> +        error_free(err);
>> +        return;
>> +    }
>> +
>> +    monitor_read_command(mon, 1);
>> +}
>> +
>> +void hmp_console(Monitor *mon, const QDict *qdict)
>> +{
>> +    const char *device = qdict_get_str(qdict, "chardev");
>> +    CharDriverState *chr;
>> +    Error *err = NULL;
>> +
>> +    chr = qemu_chr_find(device);
>> +
>> +    if (!chr) {
>> +        error_set(&err, QERR_DEVICE_NOT_FOUND, device);
>> +        goto out;
>> +    }
> As I said before, I don't why chr is needed. It seems to me that passing
> 'device' down is enough.
>
>> +
>> +    if (monitor_read_console(mon, device, hmp_read_console, chr) < 0) {
>> +        monitor_printf(mon, "Connect to console %s failed\n", device);
>> +    }
>> +
>> +out:
>> +    hmp_handle_error(mon, &err);
>> +}
>> diff --git a/hmp.h b/hmp.h
>> index a5a0cfe..5b54a79 100644
>> --- a/hmp.h
>> +++ b/hmp.h
>> @@ -77,5 +77,6 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
>>   void hmp_closefd(Monitor *mon, const QDict *qdict);
>>   void hmp_send_key(Monitor *mon, const QDict *qdict);
>>   void hmp_screen_dump(Monitor *mon, const QDict *qdict);
>> +void hmp_console(Monitor *mon, const QDict *qdict);
>>   
>>   #endif
>> diff --git a/monitor.c b/monitor.c
>> index d17ae2d..7e90115 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -256,6 +256,21 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>>       }
>>   }
>>   
>> +int monitor_read_console(Monitor *mon, const char *device,
>> +                         ReadLineFunc *readline_func, void *opaque)
>> +{
>> +    char prompt[60];
>> +
>> +    if (!mon->rs) {
>> +        return -1;
>> +    }
>> +
>> +    snprintf(prompt, sizeof(prompt), "%s: ", device);
>> +    readline_start(mon->rs, prompt, 0, readline_func, opaque);
>> +
>> +    return 0;
>> +}
>> +
>>   void monitor_flush(Monitor *mon)
>>   {
>>       if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
>> diff --git a/monitor.h b/monitor.h
>> index b6e7d95..735bd1b 100644
>> --- a/monitor.h
>> +++ b/monitor.h
>> @@ -86,6 +86,9 @@ ReadLineState *monitor_get_rs(Monitor *mon);
>>   int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>>                             void *opaque);
>>   
>> +int monitor_read_console(Monitor *mon, const char *device,
>> +                         ReadLineFunc *readline_func, void *opaque);
>> +
>>   int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
>>   
>>   int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
>


-- 
Lei

Re: [Qemu-devel] [PATCH 1/4] qemu-char: Add new char backend CirMemCharDriver

[Lei Li]

On 10/27/2012 12:47 AM, Luiz Capitulino wrote:
> On Fri, 26 Oct 2012 19:21:49 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>> ---
>>   qemu-char.c     |  140 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-config.c   |    3 +
>>   qemu-options.hx |   10 ++++
>>   3 files changed, 153 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu-char.c b/qemu-char.c
>> index b082bae..c3ec43d 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 */
>> @@ -2588,6 +2589,134 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr)
>>       return d->outbuf_size;
>>   }
>>   
>> +/*********************************************************/
>> +/*CircularMemory chardev*/
>> +
>> +typedef struct {
>> +    size_t size;
>> +    size_t producer;
>> +    size_t consumer;
>> +    uint8_t *cbuf;
>> +} CirMemCharDriver;
>> +
>> +static bool cirmem_chr_is_empty(const CharDriverState *chr)
>> +{
>> +    const CirMemCharDriver *d = chr->opaque;
>> +
>> +    return d->producer == d->consumer;
>> +}
>> +
>> +static bool cirmem_chr_is_full(const CharDriverState *chr)
>> +{
>> +    const CirMemCharDriver *d = chr->opaque;
>> +
>> +    return (d->producer - d->consumer) >= d->size;
>> +}
>> +
>> +static bool cirmem_chr_unlimit(const CharDriverState *chr)
>> +{
>> +    const CirMemCharDriver *d = chr->opaque;
>> +
>> +    return d->producer >= d->consumer;
>> +}
> I'm not sure I get why this is needed. Are you trying to fix the problem
> I mentioned in my last review that producer and consumer might overflow
> for long running VMs? If yes, then the problem still exists.
>
> The only fix I can think of is to re-initialize them. Of course that you
> could do it in a way that they keep pointing to the right position in cbuf[].
>
>> +
>> +static int cirmem_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>> +{
>> +    CirMemCharDriver *d = chr->opaque;
>> +    int i;
>> +
>> +    if (!buf || (len < 0)) {
>> +        return -1;
>> +    }
>> +
>> +    for (i = 0; i < len; i++ ) {
>> +        if (cirmem_chr_unlimit(chr)) {
>> +            /* Avoid writing the IAC information to the queue. */
>> +            if ((unsigned char)buf[i] == IAC) {
>> +                continue;
>> +            }
>> +            d->cbuf[d->producer % d->size] = buf[i];
>> +            d->producer++;
>> +        } else {
>> +            return -1;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int cirmem_chr_read(CharDriverState *chr, uint8_t *buf, int len)
>> +{
>> +    CirMemCharDriver *d = chr->opaque;
>> +    int i;
>> +
>> +    if (cirmem_chr_is_empty(chr) || len < 0) {
>> +        return -1;
>> +    }
>> +
>> +    if (len > d->size) {
>> +        len = d->size;
>> +    }
>> +
>> +    for (i = 0; i < len; i++) {
>> +        if (cirmem_chr_unlimit(chr)) {
>> +            buf[i] = d->cbuf[d->consumer % d->size];
>> +            d->consumer++;
>> +
>> +            if (cirmem_chr_is_empty(chr)) {
>> +                break;
>> +            }
>> +        } else {
>> +            return -1;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void cirmem_chr_close(struct CharDriverState *chr)
>> +{
>> +    CirMemCharDriver *d = chr->opaque;
>> +
>> +    g_free(d->cbuf);
>> +    g_free(d);
>> +    chr->opaque = NULL;
>> +}
>> +
>> +static CharDriverState *qemu_chr_open_cirmemchr(QemuOpts *opts)
>> +{
>> +    CharDriverState *chr;
>> +    CirMemCharDriver *d;
>> +
>> +    chr = g_malloc0(sizeof(CharDriverState));
>> +    d = g_malloc(sizeof(*d));
>> +
>> +    d->size = qemu_opt_get_number(opts, "maxcapacity", 0);
>> +    if (d->size == 0) {
>> +        d->size = CBUFF_SIZE;
>> +    }
>> +
>> +    /* The size must be power of 2 */
> If this is enforced in vl.c (which I can't remember right now), then
> you should turn this into an assert().

It is just enforced in here, not in vl.c.

>> +    if (d->size & (d->size - 1)) {
>> +        goto fail;
>> +    }
>> +
>> +    d->producer = 0;
>> +    d->consumer = 0;
>> +    d->cbuf = g_malloc0(d->size);
>> +
>> +    chr->opaque = d;
>> +    chr->chr_write = cirmem_chr_write;
>> +    chr->chr_close = cirmem_chr_close;
>> +
>> +    return chr;
>> +
>> +fail:
>> +    g_free(d);
>> +    g_free(chr);
>> +    return NULL;
>> +}
>> +
>>   QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
>>   {
>>       char host[65], port[33], width[8], height[8];
>> @@ -2652,6 +2781,16 @@ 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, "memchr", &p)) {
>> +        qemu_opt_set(opts, "backend", "memchr");
>> +        qemu_opt_set(opts, "maxcapacity", p);
>> +        return opts;
> Duplication?

Yeah...

>> +    }
>>       if (strstart(filename, "tcp:", &p) ||
>>           strstart(filename, "telnet:", &p)) {
>>           if (sscanf(p, "%64[^:]:%32[^,]%n", host, port, &pos) < 2) {
>> @@ -2725,6 +2864,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_cirmemchr },
> What about my suggestion of calling this "memory"? All backends here are
> chardevs, but we don't have vcchr, msmousechr, stdiochr etc.

Yes, it make sense.

>>   #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 cd1ec21..5553bb6 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -213,6 +213,9 @@ static QemuOptsList qemu_chardev_opts = {
>>           },{
>>               .name = "debug",
>>               .type = QEMU_OPT_NUMBER,
>> +        },{
>> +            .name = "maxcapacity",
>> +            .type = QEMU_OPT_NUMBER,
>>           },
>>           { /* end of list */ }
>>       },
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 46f0539..9b2d792 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -1680,6 +1680,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,maxcapacity=maxcapacity\n"
>>       "-chardev file,id=id,path=path[,mux=on|off]\n"
>>       "-chardev pipe,id=id,path=path[,mux=on|off]\n"
>>   #ifdef _WIN32
>> @@ -1718,6 +1719,7 @@ Backend is one of:
>>   @option{udp},
>>   @option{msmouse},
>>   @option{vc},
>> +@option{memchr},
>>   @option{file},
>>   @option{pipe},
>>   @option{console},
>> @@ -1824,6 +1826,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} ,maxcapacity=@var{maxcapacity}
>> +
>> +Create a circular buffer with fixed size indicated by optionally @option{maxcapacity}
>> +which will be default 64K if it is not given.
>> +
>> +@option{maxcapacity} specifies the max capacity of the size of circular buffer
>> +to create. Should be power of 2.
>> +
>>   @item -chardev file ,id=@var{id} ,path=@var{path}
>>   
>>   Log all traffic received from the guest to a file.
>


-- 
Lei

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

[Luiz Capitulino]

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,
> >
> 
> 

Re: [Qemu-devel] [PATCH 2/4] QAPI: Introduce memchar-write QMP command

[Luiz Capitulino]

On Mon, 29 Oct 2012 12:10:24 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> On 10/27/2012 01:17 AM, Luiz Capitulino wrote:
> > On Fri, 26 Oct 2012 19:21:50 +0800
> > Lei Li <lilei@linux.vnet.ibm.com> wrote:
> >
> >> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> >> ---
> >>   hmp-commands.hx  |   17 +++++++++++++++++
> >>   hmp.c            |   15 +++++++++++++++
> >>   hmp.h            |    1 +
> >>   qapi-schema.json |   47 +++++++++++++++++++++++++++++++++++++++++++++++
> >>   qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++++++++
> >>   qmp-commands.hx  |   34 ++++++++++++++++++++++++++++++++++
> >>   6 files changed, 158 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index e0b537d..a37b8e9 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -825,6 +825,23 @@ Inject an NMI on the given CPU (x86 only).
> >>   ETEXI
> >>   
> >>       {
> >> +        .name       = "memchar_write",
> >> +        .args_type  = "chardev:s,data:s",
> >> +        .params     = "chardev data",
> >> +        .mhandler.cmd = hmp_memchar_write,
> >> +    },
> >> +
> >> +STEXI
> >> +@item memchar_write @var{chardev} @var{data}
> >> +@findex memchar_write
> >> +Provide writing interface for CirMemCharDriver. Write @var{data}
> >> +to cirmemchr char device. Note that 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.
> > You can drop everything after "Note".
> >
> ok
> 
> >> +
> >> +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 2b97982..082985b 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -683,6 +683,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;
> >> +    const char *chardev = qdict_get_str(qdict, "chardev");
> >> +    const char *data = qdict_get_str(qdict, "data");
> >> +    enum DataFormat format;
> > Why do you need this variable?
> 
> Sure, I will drop this.
> 
> >> +    Error *errp = NULL;
> >> +
> >> +    size = strlen(data);
> >> +    format = DATA_FORMAT_UTF8;
> >> +    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 71ea384..406ebb1 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 c615ee2..43ef6bc 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -325,6 +325,53 @@
> >>   { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
> >>   
> >>   ##
> >> +# @DataFormat:
> >> +#
> >> +# An enumeration of data format. The default value would
> >> +# be utf8.
> > Please, remove the "default value" part. This is decided by the command
> > using this type.
> 
> Now the option format is optional, if it's not set then default by 'utf8'.
> I think it's a reasonable behaver. :)

What I meant is that the right place to say what the value is is in
the command documentation, not in @DataFormat doc. Here, only having
"An enumeration of data encodings" is fine.

> >> +#
> >> +# @utf8: The data format is 'utf8'.
> >> +#
> >> +# @base64: The data format is 'base64'.
> >> +#
> >> +# Note: The data format start with 'utf8' and 'base64',
> >> +#       will support other data format as well.
> > Please, drop this note. It's not needed.
> >
> ok
> 
> >> +#
> >> +# Since: 1.3
> >> +##
> >> +{ '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. Should be power of 2.
> >> +#
> >> +# @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
> >> +#
> >> +# 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.
> 
> Sure.
> 
> >> +#
> >> +# Since: 1.3
> >> +##
> >> +{ 'command': 'memchar-write',
> >> +  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
> >> +           '*format': 'DataFormat'} }
> >> +
> >> +##
> >>   # @CommandInfo:
> >>   #
> >>   # Information about a QMP command
> >> diff --git a/qemu-char.c b/qemu-char.c
> >> index c3ec43d..6114e29 100644
> >> --- a/qemu-char.c
> >> +++ b/qemu-char.c
> >> @@ -2717,6 +2717,50 @@ fail:
> >>       return NULL;
> >>   }
> >>   
> >> +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;
> >> +    }
> >> +
> >> +    if (strcmp(chr->filename, "memchr") != 0) {
> >> +        error_setg(errp,"%s is not memory char device\n", chardev);
> >> +        return;
> >> +    }
> > This is wrong, as it's unreliable. A hackish (but hopefully correct) way
> > of doing this would be to check chr->init against the circmem init function.
> > But please, put this behind a function because it's ugly.
> 
> Hmm, I guess it's because the chr->filename could be NULL. but we can still
> get it since CharDriverState keeps the inited type of backend. I am not sure
> that check chr->init against the cirmem init funciton could work, I'll have
> a try.
> 
> >
> >> +
> >> +    /* XXX: For the sync command as 'block', waiting for the qmp
> >> +     * to support necessary feature. Now just act as 'drop' */
> > Please, replace this note with an explanation of the current behavior.
> 
> ok.
> 
> >> +    if (cirmem_chr_is_full(chr)) {
> >> +        error_setg(errp, "Failed to write to memchr %s", chardev);
> >> +        return;
> >> +    }
> >> +
> >> +    write_count = (gsize)size;
> >> +
> >> +    if (has_format && (format == DATA_FORMAT_BASE64)) {
> >> +        write_data = g_base64_decode(data, &write_count);
> >> +    } else {
> >> +        write_data = (uint8_t *)data;
> >> +    }
> >> +
> >> +    ret = cirmem_chr_write(chr, write_data, write_count);
> > IIRC circmem_chr_write() doesn't check if write_count if a power of two,
> > what happens if it isn't?
> 
> I have considered your suggestion on this in v4. The reason why I did not
> add the assert is that I think we have already check with this when cirmem
> init, so there is no need to check it again. Correct me if I am wrong. :)

I was actually asking an honest question: does cirmem_chr_write() assume
that len is a power of two? If yes, what if the user doesn't pass a
power of two value to qmp_memchar_write()?

> 
> >> +
> >> +    if (ret < 0) {
> >> +        error_setg(errp, "Failed to write to memchr %s", chardev);
> >> +        return;
> >> +    }
> >> +}
> >> +
> >>   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 5ba8c48..7548b9b 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -466,6 +466,40 @@ 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?,control:s?",
> > You've dropped 'control', haven't you?
> 
> Yes, sorry I forgot to drop here.. :(
> 
> >
> >> +        .help       = "write size of data to memchar chardev",
> >> +        .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, should be power of 2 (json-int)
> >> +- "data": the source data writed to memchar (json-string)
> >> +- "format": the data format write to memchardev, default is
> >> +            utf8. (json-string, optional)
> >> +          - Possible values: "utf8", "base64"
> >> +
> >> +Example:
> >> +
> >> +-> { "execute": "memchar-write",
> >> +                "arguments": { "chardev": foo,
> >> +                               "size": 8,
> >> +                               "data": "abcdefgh",
> >> +                               "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 4/4] HMP: Introduce console command

[Luiz Capitulino]

On Mon, 29 Oct 2012 12:18:03 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> On 10/27/2012 01:43 AM, Luiz Capitulino wrote:
> > On Fri, 26 Oct 2012 19:21:52 +0800
> > Lei Li <lilei@linux.vnet.ibm.com> wrote:
> >
> >> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> > I still don't understand how this command, in its current form, is
> > different from memchar-write.
> >
> > One more comment below.
> 
> Hi Luiz,
> 
> Yes, I have replied to it in patch series v4. You can look at it
> as link below:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg04568.html

Unfortunately your answer doesn't answer my (honest) question. Can you actually
show me how the console command is better than the memchar-write one? Maybe
I'm missing something obvious...

> 
> >
> >> ---
> >>   hmp-commands.hx |   25 +++++++++++++++++++++++++
> >>   hmp.c           |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>   hmp.h           |    1 +
> >>   monitor.c       |   15 +++++++++++++++
> >>   monitor.h       |    3 +++
> >>   5 files changed, 96 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index df294eb..7cba42c 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -861,6 +861,31 @@ if the requested size is larger than it.
> >>   ETEXI
> >>   
> >>       {
> >> +        .name       = "console",
> >> +        .args_type  = "chardev:s",
> >> +        .params     = "chardev",
> >> +        .help       = "Connect to the serial console from within the"
> >> +                      "monitor, allow to write data to memchardev"
> >> +                      "'chardev'. Exit from the console and return back"
> >> +                      "to monitor by typing 'ctrl-]'",
> >> +        .mhandler.cmd = hmp_console,
> >> +    },
> >> +
> >> +STEXI
> >> +@item console @var{device}
> >> +@findex console
> >> +
> >> +Connect to the serial console from within the monitor, allow to write data
> >> +to memchardev @var{chardev}. Exit from the console and return back to
> >> +monitor by typing 'ctrl-]'.
> >> +@example
> >> +(qemu) console foo
> >> +foo: data string...
> >> +@end example
> >> +
> >> +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 ef85736..d716410 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -1255,3 +1255,55 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
> >>       qmp_screendump(filename, &err);
> >>       hmp_handle_error(mon, &err);
> >>   }
> >> +
> >> +enum escape_char
> >> +{
> >> +    ESCAPE_CHAR_CTRL_GS = 0x1d  /* ctrl-] used for escape */
> >> +};
> >> +
> >> +static void hmp_read_console(Monitor *mon, const char *data,
> >> +                             void *opaque)
> >> +{
> >> +    CharDriverState *chr = opaque;
> >> +    uint32_t size = strlen(data);
> >> +    enum DataFormat format = DATA_FORMAT_UTF8;
> >> +    enum escape_char console_escape = ESCAPE_CHAR_CTRL_GS;
> >> +
> >> +    Error *err = NULL;
> >> +
> >> +    if (*data == console_escape) {
> >> +        monitor_resume(mon);
> >> +        return;
> >> +    }
> >> +
> >> +    qmp_memchar_write(chr->label, size, data, 0, format, &err);
> >> +
> >> +    if (err) {
> >> +        monitor_printf(mon, "%s\n", error_get_pretty(err));
> >> +        error_free(err);
> >> +        return;
> >> +    }
> >> +
> >> +    monitor_read_command(mon, 1);
> >> +}
> >> +
> >> +void hmp_console(Monitor *mon, const QDict *qdict)
> >> +{
> >> +    const char *device = qdict_get_str(qdict, "chardev");
> >> +    CharDriverState *chr;
> >> +    Error *err = NULL;
> >> +
> >> +    chr = qemu_chr_find(device);
> >> +
> >> +    if (!chr) {
> >> +        error_set(&err, QERR_DEVICE_NOT_FOUND, device);
> >> +        goto out;
> >> +    }
> > As I said before, I don't why chr is needed. It seems to me that passing
> > 'device' down is enough.
> >
> >> +
> >> +    if (monitor_read_console(mon, device, hmp_read_console, chr) < 0) {
> >> +        monitor_printf(mon, "Connect to console %s failed\n", device);
> >> +    }
> >> +
> >> +out:
> >> +    hmp_handle_error(mon, &err);
> >> +}
> >> diff --git a/hmp.h b/hmp.h
> >> index a5a0cfe..5b54a79 100644
> >> --- a/hmp.h
> >> +++ b/hmp.h
> >> @@ -77,5 +77,6 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
> >>   void hmp_closefd(Monitor *mon, const QDict *qdict);
> >>   void hmp_send_key(Monitor *mon, const QDict *qdict);
> >>   void hmp_screen_dump(Monitor *mon, const QDict *qdict);
> >> +void hmp_console(Monitor *mon, const QDict *qdict);
> >>   
> >>   #endif
> >> diff --git a/monitor.c b/monitor.c
> >> index d17ae2d..7e90115 100644
> >> --- a/monitor.c
> >> +++ b/monitor.c
> >> @@ -256,6 +256,21 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> >>       }
> >>   }
> >>   
> >> +int monitor_read_console(Monitor *mon, const char *device,
> >> +                         ReadLineFunc *readline_func, void *opaque)
> >> +{
> >> +    char prompt[60];
> >> +
> >> +    if (!mon->rs) {
> >> +        return -1;
> >> +    }
> >> +
> >> +    snprintf(prompt, sizeof(prompt), "%s: ", device);
> >> +    readline_start(mon->rs, prompt, 0, readline_func, opaque);
> >> +
> >> +    return 0;
> >> +}
> >> +
> >>   void monitor_flush(Monitor *mon)
> >>   {
> >>       if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
> >> diff --git a/monitor.h b/monitor.h
> >> index b6e7d95..735bd1b 100644
> >> --- a/monitor.h
> >> +++ b/monitor.h
> >> @@ -86,6 +86,9 @@ ReadLineState *monitor_get_rs(Monitor *mon);
> >>   int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> >>                             void *opaque);
> >>   
> >> +int monitor_read_console(Monitor *mon, const char *device,
> >> +                         ReadLineFunc *readline_func, void *opaque);
> >> +
> >>   int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
> >>   
> >>   int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
> >
> 
> 

Re: [Qemu-devel] [PATCH 2/4] QAPI: Introduce memchar-write QMP command

[Lei Li]

On 10/29/2012 09:23 PM, Luiz Capitulino wrote:
> On Mon, 29 Oct 2012 12:10:24 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> On 10/27/2012 01:17 AM, Luiz Capitulino wrote:
>>> On Fri, 26 Oct 2012 19:21:50 +0800
>>> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>>>
>>>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>>>> ---
>>>>    hmp-commands.hx  |   17 +++++++++++++++++
>>>>    hmp.c            |   15 +++++++++++++++
>>>>    hmp.h            |    1 +
>>>>    qapi-schema.json |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>    qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++++++++
>>>>    qmp-commands.hx  |   34 ++++++++++++++++++++++++++++++++++
>>>>    6 files changed, 158 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>>> index e0b537d..a37b8e9 100644
>>>> --- a/hmp-commands.hx
>>>> +++ b/hmp-commands.hx
>>>> @@ -825,6 +825,23 @@ Inject an NMI on the given CPU (x86 only).
>>>>    ETEXI
>>>>    
>>>>        {
>>>> +        .name       = "memchar_write",
>>>> +        .args_type  = "chardev:s,data:s",
>>>> +        .params     = "chardev data",
>>>> +        .mhandler.cmd = hmp_memchar_write,
>>>> +    },
>>>> +
>>>> +STEXI
>>>> +@item memchar_write @var{chardev} @var{data}
>>>> +@findex memchar_write
>>>> +Provide writing interface for CirMemCharDriver. Write @var{data}
>>>> +to cirmemchr char device. Note that 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.
>>> You can drop everything after "Note".
>>>
>> ok
>>
>>>> +
>>>> +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 2b97982..082985b 100644
>>>> --- a/hmp.c
>>>> +++ b/hmp.c
>>>> @@ -683,6 +683,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;
>>>> +    const char *chardev = qdict_get_str(qdict, "chardev");
>>>> +    const char *data = qdict_get_str(qdict, "data");
>>>> +    enum DataFormat format;
>>> Why do you need this variable?
>> Sure, I will drop this.
>>
>>>> +    Error *errp = NULL;
>>>> +
>>>> +    size = strlen(data);
>>>> +    format = DATA_FORMAT_UTF8;
>>>> +    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 71ea384..406ebb1 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 c615ee2..43ef6bc 100644
>>>> --- a/qapi-schema.json
>>>> +++ b/qapi-schema.json
>>>> @@ -325,6 +325,53 @@
>>>>    { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
>>>>    
>>>>    ##
>>>> +# @DataFormat:
>>>> +#
>>>> +# An enumeration of data format. The default value would
>>>> +# be utf8.
>>> Please, remove the "default value" part. This is decided by the command
>>> using this type.
>> Now the option format is optional, if it's not set then default by 'utf8'.
>> I think it's a reasonable behaver. :)
> What I meant is that the right place to say what the value is is in
> the command documentation, not in @DataFormat doc. Here, only having
> "An enumeration of data encodings" is fine.

OK, done.

>>>> +#
>>>> +# @utf8: The data format is 'utf8'.
>>>> +#
>>>> +# @base64: The data format is 'base64'.
>>>> +#
>>>> +# Note: The data format start with 'utf8' and 'base64',
>>>> +#       will support other data format as well.
>>> Please, drop this note. It's not needed.
>>>
>> ok
>>
>>>> +#
>>>> +# Since: 1.3
>>>> +##
>>>> +{ '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. Should be power of 2.
>>>> +#
>>>> +# @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
>>>> +#
>>>> +# 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.
>> Sure.
>>
>>>> +#
>>>> +# Since: 1.3
>>>> +##
>>>> +{ 'command': 'memchar-write',
>>>> +  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
>>>> +           '*format': 'DataFormat'} }
>>>> +
>>>> +##
>>>>    # @CommandInfo:
>>>>    #
>>>>    # Information about a QMP command
>>>> diff --git a/qemu-char.c b/qemu-char.c
>>>> index c3ec43d..6114e29 100644
>>>> --- a/qemu-char.c
>>>> +++ b/qemu-char.c
>>>> @@ -2717,6 +2717,50 @@ fail:
>>>>        return NULL;
>>>>    }
>>>>    
>>>> +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;
>>>> +    }
>>>> +
>>>> +    if (strcmp(chr->filename, "memchr") != 0) {
>>>> +        error_setg(errp,"%s is not memory char device\n", chardev);
>>>> +        return;
>>>> +    }
>>> This is wrong, as it's unreliable. A hackish (but hopefully correct) way
>>> of doing this would be to check chr->init against the circmem init function.
>>> But please, put this behind a function because it's ugly.
>> Hmm, I guess it's because the chr->filename could be NULL. but we can still
>> get it since CharDriverState keeps the inited type of backend. I am not sure
>> that check chr->init against the cirmem init funciton could work, I'll have
>> a try.
>>
>>>> +
>>>> +    /* XXX: For the sync command as 'block', waiting for the qmp
>>>> +     * to support necessary feature. Now just act as 'drop' */
>>> Please, replace this note with an explanation of the current behavior.
>> ok.
>>
>>>> +    if (cirmem_chr_is_full(chr)) {
>>>> +        error_setg(errp, "Failed to write to memchr %s", chardev);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    write_count = (gsize)size;
>>>> +
>>>> +    if (has_format && (format == DATA_FORMAT_BASE64)) {
>>>> +        write_data = g_base64_decode(data, &write_count);
>>>> +    } else {
>>>> +        write_data = (uint8_t *)data;
>>>> +    }
>>>> +
>>>> +    ret = cirmem_chr_write(chr, write_data, write_count);
>>> IIRC circmem_chr_write() doesn't check if write_count if a power of two,
>>> what happens if it isn't?
>> I have considered your suggestion on this in v4. The reason why I did not
>> add the assert is that I think we have already check with this when cirmem
>> init, so there is no need to check it again. Correct me if I am wrong. :)
> I was actually asking an honest question: does cirmem_chr_write() assume
> that len is a power of two? If yes, what if the user doesn't pass a
> power of two value to qmp_memchar_write()?

Ah, there must be some misunderstanding here... :) We let the size of ring
buffer inited as power of 2, not the write_count. And it has already been
checked when we init the CirMemCharDriver. That's why I did not
add the assert in the function cirmem_chr_write.

>
>>>> +
>>>> +    if (ret < 0) {
>>>> +        error_setg(errp, "Failed to write to memchr %s", chardev);
>>>> +        return;
>>>> +    }
>>>> +}
>>>> +
>>>>    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 5ba8c48..7548b9b 100644
>>>> --- a/qmp-commands.hx
>>>> +++ b/qmp-commands.hx
>>>> @@ -466,6 +466,40 @@ 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?,control:s?",
>>> You've dropped 'control', haven't you?
>> Yes, sorry I forgot to drop here.. :(
>>
>>>> +        .help       = "write size of data to memchar chardev",
>>>> +        .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, should be power of 2 (json-int)
>>>> +- "data": the source data writed to memchar (json-string)
>>>> +- "format": the data format write to memchardev, default is
>>>> +            utf8. (json-string, optional)
>>>> +          - Possible values: "utf8", "base64"
>>>> +
>>>> +Example:
>>>> +
>>>> +-> { "execute": "memchar-write",
>>>> +                "arguments": { "chardev": foo,
>>>> +                               "size": 8,
>>>> +                               "data": "abcdefgh",
>>>> +                               "format": "utf8" } }
>>>> +<- { "return": {} }
>>>> +
>>>> +EQMP
>>>> +
>>>> +    {
>>>>            .name       = "xen-save-devices-state",
>>>>            .args_type  = "filename:F",
>>>>        .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,
>>


-- 
Lei

Re: [Qemu-devel] [PATCH 4/4] HMP: Introduce console command

[Lei Li]

On 10/29/2012 09:26 PM, Luiz Capitulino wrote:
> On Mon, 29 Oct 2012 12:18:03 +0800
> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>
>> On 10/27/2012 01:43 AM, Luiz Capitulino wrote:
>>> On Fri, 26 Oct 2012 19:21:52 +0800
>>> Lei Li <lilei@linux.vnet.ibm.com> wrote:
>>>
>>>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
>>> I still don't understand how this command, in its current form, is
>>> different from memchar-write.
>>>
>>> One more comment below.
>> Hi Luiz,
>>
>> Yes, I have replied to it in patch series v4. You can look at it
>> as link below:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg04568.html
> Unfortunately your answer doesn't answer my (honest) question. Can you actually
> show me how the console command is better than the memchar-write one? Maybe
> I'm missing something obvious...

As the example I post in the link, I think the main benefit for console command
is that it's more friendly.

A simple example, when we try to input "hello world" to this char device,
the memchar_write command would like this:

(qemu) memchar_write foo "hello world"

We need to add quotation marks for the string due to the monitor behaver.(Would
treat as another argument for the string which has space in it.)

The console command can avoid this like:

(qemu) console foo
foo: hello world
\r / ^]
(qemu)

It drops you into an interactive mode where it's like your at the serial console
and \r or ctrl-] escape sequences takes you back to the monitor suggested by Anthony.

I hope this can answer your question. :)

>>>> ---
>>>>    hmp-commands.hx |   25 +++++++++++++++++++++++++
>>>>    hmp.c           |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    hmp.h           |    1 +
>>>>    monitor.c       |   15 +++++++++++++++
>>>>    monitor.h       |    3 +++
>>>>    5 files changed, 96 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>>> index df294eb..7cba42c 100644
>>>> --- a/hmp-commands.hx
>>>> +++ b/hmp-commands.hx
>>>> @@ -861,6 +861,31 @@ if the requested size is larger than it.
>>>>    ETEXI
>>>>    
>>>>        {
>>>> +        .name       = "console",
>>>> +        .args_type  = "chardev:s",
>>>> +        .params     = "chardev",
>>>> +        .help       = "Connect to the serial console from within the"
>>>> +                      "monitor, allow to write data to memchardev"
>>>> +                      "'chardev'. Exit from the console and return back"
>>>> +                      "to monitor by typing 'ctrl-]'",
>>>> +        .mhandler.cmd = hmp_console,
>>>> +    },
>>>> +
>>>> +STEXI
>>>> +@item console @var{device}
>>>> +@findex console
>>>> +
>>>> +Connect to the serial console from within the monitor, allow to write data
>>>> +to memchardev @var{chardev}. Exit from the console and return back to
>>>> +monitor by typing 'ctrl-]'.
>>>> +@example
>>>> +(qemu) console foo
>>>> +foo: data string...
>>>> +@end example
>>>> +
>>>> +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 ef85736..d716410 100644
>>>> --- a/hmp.c
>>>> +++ b/hmp.c
>>>> @@ -1255,3 +1255,55 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
>>>>        qmp_screendump(filename, &err);
>>>>        hmp_handle_error(mon, &err);
>>>>    }
>>>> +
>>>> +enum escape_char
>>>> +{
>>>> +    ESCAPE_CHAR_CTRL_GS = 0x1d  /* ctrl-] used for escape */
>>>> +};
>>>> +
>>>> +static void hmp_read_console(Monitor *mon, const char *data,
>>>> +                             void *opaque)
>>>> +{
>>>> +    CharDriverState *chr = opaque;
>>>> +    uint32_t size = strlen(data);
>>>> +    enum DataFormat format = DATA_FORMAT_UTF8;
>>>> +    enum escape_char console_escape = ESCAPE_CHAR_CTRL_GS;
>>>> +
>>>> +    Error *err = NULL;
>>>> +
>>>> +    if (*data == console_escape) {
>>>> +        monitor_resume(mon);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    qmp_memchar_write(chr->label, size, data, 0, format, &err);
>>>> +
>>>> +    if (err) {
>>>> +        monitor_printf(mon, "%s\n", error_get_pretty(err));
>>>> +        error_free(err);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    monitor_read_command(mon, 1);
>>>> +}
>>>> +
>>>> +void hmp_console(Monitor *mon, const QDict *qdict)
>>>> +{
>>>> +    const char *device = qdict_get_str(qdict, "chardev");
>>>> +    CharDriverState *chr;
>>>> +    Error *err = NULL;
>>>> +
>>>> +    chr = qemu_chr_find(device);
>>>> +
>>>> +    if (!chr) {
>>>> +        error_set(&err, QERR_DEVICE_NOT_FOUND, device);
>>>> +        goto out;
>>>> +    }
>>> As I said before, I don't why chr is needed. It seems to me that passing
>>> 'device' down is enough.
>>>
>>>> +
>>>> +    if (monitor_read_console(mon, device, hmp_read_console, chr) < 0) {
>>>> +        monitor_printf(mon, "Connect to console %s failed\n", device);
>>>> +    }
>>>> +
>>>> +out:
>>>> +    hmp_handle_error(mon, &err);
>>>> +}
>>>> diff --git a/hmp.h b/hmp.h
>>>> index a5a0cfe..5b54a79 100644
>>>> --- a/hmp.h
>>>> +++ b/hmp.h
>>>> @@ -77,5 +77,6 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
>>>>    void hmp_closefd(Monitor *mon, const QDict *qdict);
>>>>    void hmp_send_key(Monitor *mon, const QDict *qdict);
>>>>    void hmp_screen_dump(Monitor *mon, const QDict *qdict);
>>>> +void hmp_console(Monitor *mon, const QDict *qdict);
>>>>    
>>>>    #endif
>>>> diff --git a/monitor.c b/monitor.c
>>>> index d17ae2d..7e90115 100644
>>>> --- a/monitor.c
>>>> +++ b/monitor.c
>>>> @@ -256,6 +256,21 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>>>>        }
>>>>    }
>>>>    
>>>> +int monitor_read_console(Monitor *mon, const char *device,
>>>> +                         ReadLineFunc *readline_func, void *opaque)
>>>> +{
>>>> +    char prompt[60];
>>>> +
>>>> +    if (!mon->rs) {
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    snprintf(prompt, sizeof(prompt), "%s: ", device);
>>>> +    readline_start(mon->rs, prompt, 0, readline_func, opaque);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>>    void monitor_flush(Monitor *mon)
>>>>    {
>>>>        if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
>>>> diff --git a/monitor.h b/monitor.h
>>>> index b6e7d95..735bd1b 100644
>>>> --- a/monitor.h
>>>> +++ b/monitor.h
>>>> @@ -86,6 +86,9 @@ ReadLineState *monitor_get_rs(Monitor *mon);
>>>>    int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>>>>                              void *opaque);
>>>>    
>>>> +int monitor_read_console(Monitor *mon, const char *device,
>>>> +                         ReadLineFunc *readline_func, void *opaque);
>>>> +
>>>>    int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
>>>>    
>>>>    int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
>>


-- 
Lei

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

[Lei Li]

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,
>>


-- 
Lei

Re: [Qemu-devel] [PATCH 4/4] HMP: Introduce console command

[Luiz Capitulino]

On Tue, 30 Oct 2012 18:22:34 +0800
Lei Li <lilei@linux.vnet.ibm.com> wrote:

> On 10/29/2012 09:26 PM, Luiz Capitulino wrote:
> > On Mon, 29 Oct 2012 12:18:03 +0800
> > Lei Li <lilei@linux.vnet.ibm.com> wrote:
> >
> >> On 10/27/2012 01:43 AM, Luiz Capitulino wrote:
> >>> On Fri, 26 Oct 2012 19:21:52 +0800
> >>> Lei Li <lilei@linux.vnet.ibm.com> wrote:
> >>>
> >>>> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> >>> I still don't understand how this command, in its current form, is
> >>> different from memchar-write.
> >>>
> >>> One more comment below.
> >> Hi Luiz,
> >>
> >> Yes, I have replied to it in patch series v4. You can look at it
> >> as link below:
> >>
> >> https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg04568.html
> > Unfortunately your answer doesn't answer my (honest) question. Can you actually
> > show me how the console command is better than the memchar-write one? Maybe
> > I'm missing something obvious...
> 
> As the example I post in the link, I think the main benefit for console command
> is that it's more friendly.
> 
> A simple example, when we try to input "hello world" to this char device,
> the memchar_write command would like this:
> 
> (qemu) memchar_write foo "hello world"
> 
> We need to add quotation marks for the string due to the monitor behaver.(Would
> treat as another argument for the string which has space in it.)
> 
> The console command can avoid this like:
> 
> (qemu) console foo
> foo: hello world
> \r / ^]
> (qemu)
> 
> It drops you into an interactive mode where it's like your at the serial console
> and \r or ctrl-] escape sequences takes you back to the monitor suggested by Anthony.

I really think this is wrong. In its current form, this command is
memchar_write with a prompt.

My understanding of a console command is that you attach to the serial port,
see stuff being written by the guest and is able to also write stuff.

> I hope this can answer your question. :)
> 
> >>>> ---
> >>>>    hmp-commands.hx |   25 +++++++++++++++++++++++++
> >>>>    hmp.c           |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>    hmp.h           |    1 +
> >>>>    monitor.c       |   15 +++++++++++++++
> >>>>    monitor.h       |    3 +++
> >>>>    5 files changed, 96 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >>>> index df294eb..7cba42c 100644
> >>>> --- a/hmp-commands.hx
> >>>> +++ b/hmp-commands.hx
> >>>> @@ -861,6 +861,31 @@ if the requested size is larger than it.
> >>>>    ETEXI
> >>>>    
> >>>>        {
> >>>> +        .name       = "console",
> >>>> +        .args_type  = "chardev:s",
> >>>> +        .params     = "chardev",
> >>>> +        .help       = "Connect to the serial console from within the"
> >>>> +                      "monitor, allow to write data to memchardev"
> >>>> +                      "'chardev'. Exit from the console and return back"
> >>>> +                      "to monitor by typing 'ctrl-]'",
> >>>> +        .mhandler.cmd = hmp_console,
> >>>> +    },
> >>>> +
> >>>> +STEXI
> >>>> +@item console @var{device}
> >>>> +@findex console
> >>>> +
> >>>> +Connect to the serial console from within the monitor, allow to write data
> >>>> +to memchardev @var{chardev}. Exit from the console and return back to
> >>>> +monitor by typing 'ctrl-]'.
> >>>> +@example
> >>>> +(qemu) console foo
> >>>> +foo: data string...
> >>>> +@end example
> >>>> +
> >>>> +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 ef85736..d716410 100644
> >>>> --- a/hmp.c
> >>>> +++ b/hmp.c
> >>>> @@ -1255,3 +1255,55 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
> >>>>        qmp_screendump(filename, &err);
> >>>>        hmp_handle_error(mon, &err);
> >>>>    }
> >>>> +
> >>>> +enum escape_char
> >>>> +{
> >>>> +    ESCAPE_CHAR_CTRL_GS = 0x1d  /* ctrl-] used for escape */
> >>>> +};
> >>>> +
> >>>> +static void hmp_read_console(Monitor *mon, const char *data,
> >>>> +                             void *opaque)
> >>>> +{
> >>>> +    CharDriverState *chr = opaque;
> >>>> +    uint32_t size = strlen(data);
> >>>> +    enum DataFormat format = DATA_FORMAT_UTF8;
> >>>> +    enum escape_char console_escape = ESCAPE_CHAR_CTRL_GS;
> >>>> +
> >>>> +    Error *err = NULL;
> >>>> +
> >>>> +    if (*data == console_escape) {
> >>>> +        monitor_resume(mon);
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    qmp_memchar_write(chr->label, size, data, 0, format, &err);
> >>>> +
> >>>> +    if (err) {
> >>>> +        monitor_printf(mon, "%s\n", error_get_pretty(err));
> >>>> +        error_free(err);
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    monitor_read_command(mon, 1);
> >>>> +}
> >>>> +
> >>>> +void hmp_console(Monitor *mon, const QDict *qdict)
> >>>> +{
> >>>> +    const char *device = qdict_get_str(qdict, "chardev");
> >>>> +    CharDriverState *chr;
> >>>> +    Error *err = NULL;
> >>>> +
> >>>> +    chr = qemu_chr_find(device);
> >>>> +
> >>>> +    if (!chr) {
> >>>> +        error_set(&err, QERR_DEVICE_NOT_FOUND, device);
> >>>> +        goto out;
> >>>> +    }
> >>> As I said before, I don't why chr is needed. It seems to me that passing
> >>> 'device' down is enough.
> >>>
> >>>> +
> >>>> +    if (monitor_read_console(mon, device, hmp_read_console, chr) < 0) {
> >>>> +        monitor_printf(mon, "Connect to console %s failed\n", device);
> >>>> +    }
> >>>> +
> >>>> +out:
> >>>> +    hmp_handle_error(mon, &err);
> >>>> +}
> >>>> diff --git a/hmp.h b/hmp.h
> >>>> index a5a0cfe..5b54a79 100644
> >>>> --- a/hmp.h
> >>>> +++ b/hmp.h
> >>>> @@ -77,5 +77,6 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
> >>>>    void hmp_closefd(Monitor *mon, const QDict *qdict);
> >>>>    void hmp_send_key(Monitor *mon, const QDict *qdict);
> >>>>    void hmp_screen_dump(Monitor *mon, const QDict *qdict);
> >>>> +void hmp_console(Monitor *mon, const QDict *qdict);
> >>>>    
> >>>>    #endif
> >>>> diff --git a/monitor.c b/monitor.c
> >>>> index d17ae2d..7e90115 100644
> >>>> --- a/monitor.c
> >>>> +++ b/monitor.c
> >>>> @@ -256,6 +256,21 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> >>>>        }
> >>>>    }
> >>>>    
> >>>> +int monitor_read_console(Monitor *mon, const char *device,
> >>>> +                         ReadLineFunc *readline_func, void *opaque)
> >>>> +{
> >>>> +    char prompt[60];
> >>>> +
> >>>> +    if (!mon->rs) {
> >>>> +        return -1;
> >>>> +    }
> >>>> +
> >>>> +    snprintf(prompt, sizeof(prompt), "%s: ", device);
> >>>> +    readline_start(mon->rs, prompt, 0, readline_func, opaque);
> >>>> +
> >>>> +    return 0;
> >>>> +}
> >>>> +
> >>>>    void monitor_flush(Monitor *mon)
> >>>>    {
> >>>>        if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
> >>>> diff --git a/monitor.h b/monitor.h
> >>>> index b6e7d95..735bd1b 100644
> >>>> --- a/monitor.h
> >>>> +++ b/monitor.h
> >>>> @@ -86,6 +86,9 @@ ReadLineState *monitor_get_rs(Monitor *mon);
> >>>>    int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> >>>>                              void *opaque);
> >>>>    
> >>>> +int monitor_read_console(Monitor *mon, const char *device,
> >>>> +                         ReadLineFunc *readline_func, void *opaque);
> >>>> +
> >>>>    int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
> >>>>    
> >>>>    int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
> >>
> 
> 

[Qemu-devel] [PATCH 2/4] QAPI: Introduce memchar-write QMP command

[Lei Li]

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

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 0934b9b..e546c76 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -837,6 +837,22 @@ STEXI
 @item nmi @var{cpu}
 @findex nmi
 Inject an NMI on the given CPU (x86 only).
+
+ETEXI
+
+    {
+        .name       = "memchar_write",
+        .args_type  = "chardev:s,data:s",
+        .params     = "chardev data",
+        .mhandler.cmd = hmp_memchar_write,
+    },
+
+STEXI
+@item memchar_write @var{chardev} @var{data}
+@findex memchar_write
+Provide writing interface for CirMemCharDriver. Write @var{data}
+to char device 'memory'.
+
 ETEXI
 
     {
diff --git a/hmp.c b/hmp.c
index c7b6ba0..546d687 100644
--- a/hmp.c
+++ b/hmp.c
@@ -684,6 +684,19 @@ 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;
+    const char *chardev = qdict_get_str(qdict, "chardev");
+    const char *data = qdict_get_str(qdict, "data");
+    Error *errp = NULL;
+
+    size = strlen(data);
+    qmp_memchar_write(chardev, size, data, false, 0, &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 44be683..06d6ea2 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 6d7252b..c34e9ac 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -325,6 +325,47 @@
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
 
 ##
+# @DataFormat:
+#
+# An enumeration of data format.
+#
+# @utf8: The data format is 'utf8'.
+#
+# @base64: The data format is 'base64'.
+#
+# Since: 1.4
+##
+{ 'enum': 'DataFormat'
+  'data': [ 'utf8', 'base64' ] }
+
+##
+# @memchar-write:
+#
+# Provide writing interface for memchardev. Write data to char
+# device 'memory'.
+#
+# @chardev: the name of the memory 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 chardev 'memory',
+#          by default is 'utf8'.
+#
+# Returns: Nothing on success
+#          If @chardev is not a valid char device, DeviceNotFound
+#
+# Notes: For now assume 'drop' behaver, which would result in writes
+#        dropping queued data.
+#
+# Since: 1.4
+##
+{ 'command': 'memchar-write',
+  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
+           '*format': 'DataFormat'} }
+
+##
 # @CommandInfo:
 #
 # Information about a QMP command
diff --git a/qemu-char.c b/qemu-char.c
index 950c543..a3a07e0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2768,6 +2768,54 @@ fail:
     return NULL;
 }
 
+static bool qemu_is_chr(const CharDriverState *chr, const char *filename)
+{
+    return strcmp(chr->filename, filename);
+}
+
+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;
+    }
+
+    if (qemu_is_chr(chr, "memory")) {
+        error_setg(errp,"%s is not memory char device\n", chardev);
+        return;
+    }
+
+    /* XXX: Drop the coming data when the buffer is full. */
+    if (cirmem_chr_is_full(chr)) {
+        error_setg(errp, "Memory device %s is full", chardev);
+        return;
+    }
+
+    write_count = (gsize)size;
+
+    if (has_format && (format == DATA_FORMAT_BASE64)) {
+        write_data = g_base64_decode(data, &write_count);
+    } else {
+        write_data = (uint8_t *)data;
+    }
+
+    ret = cirmem_chr_write(chr, write_data, write_count);
+
+    if (ret < 0) {
+        error_setg(errp, "Failed to write to device %s", chardev);
+        return;
+    }
+}
+
 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 cbf1280..8ad06d0 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -466,6 +466,40 @@ 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?",
+        .help       = "write size of data to memory chardev",
+        .mhandler.cmd_new = qmp_marshal_input_memchar_write,
+    },
+
+SQMP
+memchar-write
+-------------
+
+Provide writing interface for CirMemCharDriver. Write data to memory
+char device.
+
+Arguments:
+
+- "chardev": the name of the char device, must be unique (json-string)
+- "size": the memory size, in bytes, should be power of 2 (json-int)
+- "data": the source data write to memory (json-string)
+- "format": the data format write to memory, default is
+            utf8. (json-string, optional)
+          - Possible values: "utf8", "base64"
+
+Example:
+
+-> { "execute": "memchar-write",
+                "arguments": { "chardev": foo,
+                               "size": 8,
+                               "data": "abcdefgh",
+                               "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 2/4] QAPI: Introduce memchar-write QMP command

[Lei Li]

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

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..a60ba69 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -840,6 +840,21 @@ Inject an NMI on the given CPU (x86 only).
 ETEXI
 
     {
+        .name       = "memchar_write",
+        .args_type  = "chardev:s,data:s",
+        .params     = "chardev data",
+        .mhandler.cmd = hmp_memchar_write,
+    },
+
+STEXI
+@item memchar_write @var{chardev} @var{data}
+@findex memchar_write
+Provide writing interface for CirMemCharDriver. Write @var{data}
+to char device 'memory'.
+
+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 180ba2b..05b8c21 100644
--- a/hmp.c
+++ b/hmp.c
@@ -683,6 +683,19 @@ 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;
+    const char *chardev = qdict_get_str(qdict, "chardev");
+    const char *data = qdict_get_str(qdict, "data");
+    Error *errp = NULL;
+
+    size = strlen(data);
+    qmp_memchar_write(chardev, size, data, false, 0, &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 0ab03be..3ea9896 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 5dfa052..d9fd635 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -325,6 +325,47 @@
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
 
 ##
+# @DataFormat:
+#
+# An enumeration of data format.
+#
+# @utf8: The data format is 'utf8'.
+#
+# @base64: The data format is 'base64'.
+#
+# Since: 1.4
+##
+{ 'enum': 'DataFormat'
+  'data': [ 'utf8', 'base64' ] }
+
+##
+# @memchar-write:
+#
+# Provide writing interface for memchardev. Write data to char
+# device 'memory'.
+#
+# @chardev: the name of the memory 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 chardev 'memory',
+#          by default is 'utf8'.
+#
+# Returns: Nothing on success
+#          If @chardev is not a valid char device, DeviceNotFound
+#
+# Notes: For now assume 'drop' behaver, which would result in writes
+#        dropping queued data.
+#
+# Since: 1.4
+##
+{ 'command': 'memchar-write',
+  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
+           '*format': 'DataFormat'} }
+
+##
 # @CommandInfo:
 #
 # Information about a QMP command
diff --git a/qemu-char.c b/qemu-char.c
index 3e45ce6..a407087 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2724,6 +2724,54 @@ fail:
     return NULL;
 }
 
+static bool qemu_is_chr(const CharDriverState *chr, const char *filename)
+{
+    return strcmp(chr->filename, filename);
+}
+
+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;
+    }
+
+    if (qemu_is_chr(chr, "memory")) {
+        error_setg(errp,"%s is not memory char device\n", chardev);
+        return;
+    }
+
+    /* XXX: Drop the coming data when the buffer is full. */
+    if (cirmem_chr_is_full(chr)) {
+        error_setg(errp, "Memory device %s is full", chardev);
+        return;
+    }
+
+    write_count = (gsize)size;
+
+    if (has_format && (format == DATA_FORMAT_BASE64)) {
+        write_data = g_base64_decode(data, &write_count);
+    } else {
+        write_data = (uint8_t *)data;
+    }
+
+    ret = cirmem_chr_write(chr, write_data, write_count);
+
+    if (ret < 0) {
+        error_setg(errp, "Failed to write to device %s", chardev);
+        return;
+    }
+}
+
 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 5c692d0..ae3ab5a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -466,6 +466,40 @@ 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?",
+        .help       = "write size of data to memory chardev",
+        .mhandler.cmd_new = qmp_marshal_input_memchar_write,
+    },
+
+SQMP
+memchar-write
+-------------
+
+Provide writing interface for CirMemCharDriver. Write data to memory
+char device.
+
+Arguments:
+
+- "chardev": the name of the char device, must be unique (json-string)
+- "size": the memory size, in bytes, should be power of 2 (json-int)
+- "data": the source data write to memory (json-string)
+- "format": the data format write to memory, default is
+            utf8. (json-string, optional)
+          - Possible values: "utf8", "base64"
+
+Example:
+
+-> { "execute": "memchar-write",
+                "arguments": { "chardev": foo,
+                               "size": 8,
+                               "data": "abcdefgh",
+                               "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 2/4] QAPI: Introduce memchar-write QMP command

[Lei Li]

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

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f916385..305b0d7 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -840,6 +840,21 @@ Inject an NMI on the given CPU (x86 only).
 ETEXI
 
     {
+        .name       = "memchar_write",
+        .args_type  = "chardev:s,data:s",
+        .params     = "chardev data",
+        .mhandler.cmd = hmp_memchar_write,
+    },
+
+STEXI
+@item memchar_write @var{chardev} @var{data}
+@findex memchar_write
+Provide writing interface for CirMemCharDriver. Write @var{data}
+to char device 'memory'.
+
+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 895a343..1ffb3d6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -682,6 +682,19 @@ 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;
+    const char *chardev = qdict_get_str(qdict, "chardev");
+    const char *data = qdict_get_str(qdict, "data");
+    Error *errp = NULL;
+
+    size = strlen(data);
+    qmp_memchar_write(chardev, size, data, false, 0, &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 34eb2b3..6c3ec45 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 542e3ac..f0acaac 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -325,6 +325,47 @@
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
 
 ##
+# @DataFormat:
+#
+# An enumeration of data format.
+#
+# @utf8: The data format is 'utf8'.
+#
+# @base64: The data format is 'base64'.
+#
+# Since: 1.3
+##
+{ 'enum': 'DataFormat'
+  'data': [ 'utf8', 'base64' ] }
+
+##
+# @memchar-write:
+#
+# Provide writing interface for memchardev. Write data to char
+# device 'memory'.
+#
+# @chardev: the name of the memory char device.
+#
+# @size: the size to write in bytes. Should be power of 2.
+#
+# @data: the source data write to memchar.
+#
+# @format: #optional the format of the data write to chardev 'memory',
+#          by default is 'utf8'.
+#
+# Returns: Nothing on success
+#          If @chardev is not a valid char device, DeviceNotFound
+#
+# Notes: For now assume 'drop' behaver, which would result in writes
+#        dropping queued data.
+#
+# Since: 1.3
+##
+{ 'command': 'memchar-write',
+  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
+           '*format': 'DataFormat'} }
+
+##
 # @CommandInfo:
 #
 # Information about a QMP command
diff --git a/qemu-char.c b/qemu-char.c
index 11c320f..92b4cec 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2723,6 +2723,54 @@ fail:
     return NULL;
 }
 
+static bool qemu_is_chr(const CharDriverState *chr, const char *filename)
+{
+    return strcmp(chr->filename, filename);
+}
+
+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;
+    }
+
+    if (qemu_is_chr(chr, "memory")) {
+        error_setg(errp,"%s is not memory char device\n", chardev);
+        return;
+    }
+
+    /* XXX: Drop the coming data when the buffer is full. */
+    if (cirmem_chr_is_full(chr)) {
+        error_setg(errp, "Memory device %s is full", chardev);
+        return;
+    }
+
+    write_count = (gsize)size;
+
+    if (has_format && (format == DATA_FORMAT_BASE64)) {
+        write_data = g_base64_decode(data, &write_count);
+    } else {
+        write_data = (uint8_t *)data;
+    }
+
+    ret = cirmem_chr_write(chr, write_data, write_count);
+
+    if (ret < 0) {
+        error_setg(errp, "Failed to write to device %s", chardev);
+        return;
+    }
+}
+
 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 5c692d0..ae3ab5a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -466,6 +466,40 @@ 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?",
+        .help       = "write size of data to memory chardev",
+        .mhandler.cmd_new = qmp_marshal_input_memchar_write,
+    },
+
+SQMP
+memchar-write
+-------------
+
+Provide writing interface for CirMemCharDriver. Write data to memory
+char device.
+
+Arguments:
+
+- "chardev": the name of the char device, must be unique (json-string)
+- "size": the memory size, in bytes, should be power of 2 (json-int)
+- "data": the source data write to memory (json-string)
+- "format": the data format write to memory, default is
+            utf8. (json-string, optional)
+          - Possible values: "utf8", "base64"
+
+Example:
+
+-> { "execute": "memchar-write",
+                "arguments": { "chardev": foo,
+                               "size": 8,
+                               "data": "abcdefgh",
+                               "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 2/4] QAPI: Introduce memchar-write QMP command

[Lei Li]

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

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..a37b8e9 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -825,6 +825,23 @@ Inject an NMI on the given CPU (x86 only).
 ETEXI
 
     {
+        .name       = "memchar_write",
+        .args_type  = "chardev:s,data:s",
+        .params     = "chardev data",
+        .mhandler.cmd = hmp_memchar_write,
+    },
+
+STEXI
+@item memchar_write @var{chardev} @var{data}
+@findex memchar_write
+Provide writing interface for CirMemCharDriver. Write @var{data}
+to cirmemchr char device. Note that 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.
+
+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 2b97982..082985b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -683,6 +683,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;
+    const char *chardev = qdict_get_str(qdict, "chardev");
+    const char *data = qdict_get_str(qdict, "data");
+    enum DataFormat format;
+    Error *errp = NULL;
+
+    size = strlen(data);
+    format = DATA_FORMAT_UTF8;
+    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 71ea384..406ebb1 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 c615ee2..43ef6bc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -325,6 +325,53 @@
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
 
 ##
+# @DataFormat:
+#
+# An enumeration of data format. The default value would
+# be utf8.
+#
+# @utf8: The data format is 'utf8'.
+#
+# @base64: The data format is 'base64'.
+#
+# Note: The data format start with 'utf8' and 'base64',
+#       will support other data format as well.
+#
+# Since: 1.3
+##
+{ '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. Should be power of 2.
+#
+# @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
+#
+# 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-write',
+  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
+           '*format': 'DataFormat'} }
+
+##
 # @CommandInfo:
 #
 # Information about a QMP command
diff --git a/qemu-char.c b/qemu-char.c
index 45d2a86..78fc634 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2713,6 +2713,50 @@ fail:
     return NULL;
 }
 
+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;
+    }
+
+    if (strcmp(chr->filename, "memchr") != 0) {
+        error_setg(errp,"%s is not memory char device\n", chardev);
+        return;
+    }
+
+    /* XXX: For the sync command as 'block', waiting for the qmp
+     * to support necessary feature. Now just act as 'drop' */
+    if (cirmem_chr_is_full(chr)) {
+        error_setg(errp, "Failed to write to memchr %s", chardev);
+        return;
+    }
+
+    write_count = (gsize)size;
+
+    if (has_format && (format == DATA_FORMAT_BASE64)) {
+        write_data = g_base64_decode(data, &write_count);
+    } else {
+        write_data = (uint8_t *)data;
+    }
+
+    ret = cirmem_chr_write(chr, write_data, write_count);
+
+    if (ret < 0) {
+        error_setg(errp, "Failed to write to memchr %s", chardev);
+        return;
+    }
+}
+
 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 5ba8c48..7548b9b 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -466,6 +466,40 @@ 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?,control:s?",
+        .help       = "write size of data to memchar chardev",
+        .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, should be power of 2 (json-int)
+- "data": the source data writed to memchar (json-string)
+- "format": the data format write to memchardev, default is
+            utf8. (json-string, optional)
+          - Possible values: "utf8", "base64"
+
+Example:
+
+-> { "execute": "memchar-write",
+                "arguments": { "chardev": foo,
+                               "size": 8,
+                               "data": "abcdefgh",
+                               "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 2/4] QAPI: Introduce memchar-write QMP command

[Lei Li]

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

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..a37b8e9 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -825,6 +825,23 @@ Inject an NMI on the given CPU (x86 only).
 ETEXI
 
     {
+        .name       = "memchar_write",
+        .args_type  = "chardev:s,data:s",
+        .params     = "chardev data",
+        .mhandler.cmd = hmp_memchar_write,
+    },
+
+STEXI
+@item memchar_write @var{chardev} @var{data}
+@findex memchar_write
+Provide writing interface for CirMemCharDriver. Write @var{data}
+to cirmemchr char device. Note that 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.
+
+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 2b97982..082985b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -683,6 +683,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;
+    const char *chardev = qdict_get_str(qdict, "chardev");
+    const char *data = qdict_get_str(qdict, "data");
+    enum DataFormat format;
+    Error *errp = NULL;
+
+    size = strlen(data);
+    format = DATA_FORMAT_UTF8;
+    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 71ea384..406ebb1 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 c615ee2..43ef6bc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -325,6 +325,53 @@
 { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
 
 ##
+# @DataFormat:
+#
+# An enumeration of data format. The default value would
+# be utf8.
+#
+# @utf8: The data format is 'utf8'.
+#
+# @base64: The data format is 'base64'.
+#
+# Note: The data format start with 'utf8' and 'base64',
+#       will support other data format as well.
+#
+# Since: 1.3
+##
+{ '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. Should be power of 2.
+#
+# @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
+#
+# 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-write',
+  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
+           '*format': 'DataFormat'} }
+
+##
 # @CommandInfo:
 #
 # Information about a QMP command
diff --git a/qemu-char.c b/qemu-char.c
index 45d2a86..78fc634 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2713,6 +2713,50 @@ fail:
     return NULL;
 }
 
+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;
+    }
+
+    if (strcmp(chr->filename, "memchr") != 0) {
+        error_setg(errp,"%s is not memory char device\n", chardev);
+        return;
+    }
+
+    /* XXX: For the sync command as 'block', waiting for the qmp
+     * to support necessary feature. Now just act as 'drop' */
+    if (cirmem_chr_is_full(chr)) {
+        error_setg(errp, "Failed to write to memchr %s", chardev);
+        return;
+    }
+
+    write_count = (gsize)size;
+
+    if (has_format && (format == DATA_FORMAT_BASE64)) {
+        write_data = g_base64_decode(data, &write_count);
+    } else {
+        write_data = (uint8_t *)data;
+    }
+
+    ret = cirmem_chr_write(chr, write_data, write_count);
+
+    if (ret < 0) {
+        error_setg(errp, "Failed to write to memchr %s", chardev);
+        return;
+    }
+}
+
 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 5ba8c48..7548b9b 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -466,6 +466,40 @@ 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?,control:s?",
+        .help       = "write size of data to memchar chardev",
+        .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, should be power of 2 (json-int)
+- "data": the source data writed to memchar (json-string)
+- "format": the data format write to memchardev, default is
+            utf8. (json-string, optional)
+          - Possible values: "utf8", "base64"
+
+Example:
+
+-> { "execute": "memchar-write",
+                "arguments": { "chardev": foo,
+                               "size": 8,
+                               "data": "abcdefgh",
+                               "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