[Qemu-devel] [PULL 0/4] QMP queue

[Qemu-devel] [PULL 0/4] QMP queue

[Luiz Capitulino]

The changes (since d05ef160453e98546a4197496dc8a3cb2defac53) are available
in the following repository:

    git://repo.or.cz/qemu/qmp-unstable.git queue/qmp

Luiz Capitulino (4):
  qstring: add qstring_get_length()
  Monitor: Make output buffer dynamic
  hmp: human-monitor-command: stop using the Memory chardev driver
  chardev: drop the Memory chardev driver

 include/qapi/qmp/qstring.h |  1 +
 monitor.c                  | 60 +++++++++++++++++++++++++------------------
 qemu-char.c                | 64 ----------------------------------------------
 qobject/qstring.c          |  8 ++++++
 4 files changed, 44 insertions(+), 89 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PULL 1/4] qstring: add qstring_get_length()

[Luiz Capitulino]

Long overdue.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/qapi/qmp/qstring.h | 1 +
 qobject/qstring.c          | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 0e690f4..1bc3666 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -26,6 +26,7 @@ typedef struct QString {
 QString *qstring_new(void);
 QString *qstring_from_str(const char *str);
 QString *qstring_from_substr(const char *str, int start, int end);
+size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
 void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
diff --git a/qobject/qstring.c b/qobject/qstring.c
index 5f7376c..607b7a1 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -32,6 +32,14 @@ QString *qstring_new(void)
 }
 
 /**
+ * qstring_get_length(): Get the length of a QString
+ */
+size_t qstring_get_length(const QString *qstring)
+{
+    return qstring->length;
+}
+
+/**
  * qstring_from_substr(): Create a new QString from a C string substring
  *
  * Return string reference
-- 
1.8.1.4

[Qemu-devel] [PULL 2/4] Monitor: Make output buffer dynamic

[Luiz Capitulino]

Commit f628926bb423fa8a7e0b114511400ea9df38b76a changed monitor_flush()
to retry on qemu_chr_fe_write() errors. However, the Monitor's output
buffer can keep growing while the retry is not issued and this can
cause the buffer to overflow.

To reproduce this issue, just start qemu and type on the Monitor:

(qemu) ?

This will cause an assertion to trig.

To fix this problem this commit makes the Monitor buffer dynamic,
which means that it can grow as much as needed.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 monitor.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/monitor.c b/monitor.c
index 4ec1db9..8712c53 100644
--- a/monitor.c
+++ b/monitor.c
@@ -188,8 +188,7 @@ struct Monitor {
     int reset_seen;
     int flags;
     int suspend_cnt;
-    uint8_t outbuf[1024];
-    int outbuf_index;
+    QString *outbuf;
     ReadLineState *rs;
     MonitorControl *mc;
     CPUArchState *mon_cpu;
@@ -271,45 +270,52 @@ static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
 void monitor_flush(Monitor *mon)
 {
     int rc;
+    size_t len;
+    const char *buf;
+
+    buf = qstring_get_str(mon->outbuf);
+    len = qstring_get_length(mon->outbuf);
 
-    if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
-        rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
-        if (rc == mon->outbuf_index) {
+    if (mon && len && !mon->mux_out) {
+        rc = qemu_chr_fe_write(mon->chr, (const uint8_t *) buf, len);
+        if (rc == len) {
             /* all flushed */
-            mon->outbuf_index = 0;
+            QDECREF(mon->outbuf);
+            mon->outbuf = qstring_new();
             return;
         }
         if (rc > 0) {
             /* partinal write */
-            memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc);
-            mon->outbuf_index -= rc;
+            QString *tmp = qstring_from_str(buf + rc);
+            QDECREF(mon->outbuf);
+            mon->outbuf = tmp;
         }
         qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon);
     }
 }
 
-/* flush at every end of line or if the buffer is full */
+/* flush at every end of line */
 static void monitor_puts(Monitor *mon, const char *str)
 {
     char c;
 
     for(;;) {
-        assert(mon->outbuf_index < sizeof(mon->outbuf) - 1);
         c = *str++;
         if (c == '\0')
             break;
-        if (c == '\n')
-            mon->outbuf[mon->outbuf_index++] = '\r';
-        mon->outbuf[mon->outbuf_index++] = c;
-        if (mon->outbuf_index >= (sizeof(mon->outbuf) - 1)
-            || c == '\n')
+        if (c == '\n') {
+            qstring_append_chr(mon->outbuf, '\r');
+        }
+        qstring_append_chr(mon->outbuf, c);
+        if (c == '\n') {
             monitor_flush(mon);
+        }
     }
 }
 
 void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 {
-    char buf[4096];
+    char *buf;
 
     if (!mon)
         return;
@@ -318,8 +324,9 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
         return;
     }
 
-    vsnprintf(buf, sizeof(buf), fmt, ap);
+    buf = g_strdup_vprintf(fmt, ap);
     monitor_puts(mon, buf);
+    g_free(buf);
 }
 
 void monitor_printf(Monitor *mon, const char *fmt, ...)
@@ -671,6 +678,8 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
     CharDriverState mchar;
 
     memset(&hmp, 0, sizeof(hmp));
+    hmp.outbuf = qstring_new();
+
     qemu_chr_init_mem(&mchar);
     hmp.chr = &mchar;
 
@@ -699,6 +708,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
     }
 
 out:
+    QDECREF(hmp.outbuf);
     qemu_chr_close_mem(hmp.chr);
     return output;
 }
@@ -4749,6 +4759,7 @@ void monitor_init(CharDriverState *chr, int flags)
     }
 
     mon = g_malloc0(sizeof(*mon));
+    mon->outbuf = qstring_new();
 
     mon->chr = chr;
     mon->flags = flags;
-- 
1.8.1.4

[Qemu-devel] [PULL 3/4] hmp: human-monitor-command: stop using the Memory chardev driver

[Luiz Capitulino]

The Memory chardev driver was added because, as the Monitor's output
buffer was static, we needed a way to accumulate the output of an
HMP commmand when ran by human-monitor-command.

However, the Monitor's output buffer is now dynamic, so it's possible
for the human-monitor-command to use it instead of the Memory chardev
driver.

This commit does that change, but there are two important
observations about it:

 1. We need a way to signal to the Monitor that it shouldn't call
    chardev functions when flushing its output. This is done
    by adding a new flag to the Monitor object called skip_flush
	(which is set to true by qmp_human_monitor_command())

 2. The current code has buffered semantics: QMP clients will
    only see a command's output if it flushes its output with
	a new-line character. This commit changes this to unbuffered,
	which means that QMP clients will see a command's output
	whenever the command prints anything.

	I don't think this will matter in practice though, as I believe
	all HMP commands print the new-line character anyway.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 monitor.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/monitor.c b/monitor.c
index 8712c53..b4bda77 100644
--- a/monitor.c
+++ b/monitor.c
@@ -188,6 +188,7 @@ struct Monitor {
     int reset_seen;
     int flags;
     int suspend_cnt;
+    bool skip_flush;
     QString *outbuf;
     ReadLineState *rs;
     MonitorControl *mc;
@@ -273,6 +274,10 @@ void monitor_flush(Monitor *mon)
     size_t len;
     const char *buf;
 
+    if (mon->skip_flush) {
+        return;
+    }
+
     buf = qstring_get_str(mon->outbuf);
     len = qstring_get_length(mon->outbuf);
 
@@ -675,13 +680,10 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
 {
     char *output = NULL;
     Monitor *old_mon, hmp;
-    CharDriverState mchar;
 
     memset(&hmp, 0, sizeof(hmp));
     hmp.outbuf = qstring_new();
-
-    qemu_chr_init_mem(&mchar);
-    hmp.chr = &mchar;
+    hmp.skip_flush = true;
 
     old_mon = cur_mon;
     cur_mon = &hmp;
@@ -699,17 +701,14 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
     handle_user_command(&hmp, command_line);
     cur_mon = old_mon;
 
-    if (qemu_chr_mem_osize(hmp.chr) > 0) {
-        QString *str = qemu_chr_mem_to_qs(hmp.chr);
-        output = g_strdup(qstring_get_str(str));
-        QDECREF(str);
+    if (qstring_get_length(hmp.outbuf) > 0) {
+        output = g_strdup(qstring_get_str(hmp.outbuf));
     } else {
         output = g_strdup("");
     }
 
 out:
     QDECREF(hmp.outbuf);
-    qemu_chr_close_mem(hmp.chr);
     return output;
 }
 
-- 
1.8.1.4

[Qemu-devel] [PULL 4/4] chardev: drop the Memory chardev driver

[Luiz Capitulino]

It's not used anymore since the last commit.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-char.c | 64 -------------------------------------------------------------
 1 file changed, 64 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index e5eb8dd..0f7f32d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2796,70 +2796,6 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     return NULL;
 }
 
-/***********************************************************/
-/* Memory chardev */
-typedef struct {
-    size_t outbuf_size;
-    size_t outbuf_capacity;
-    uint8_t *outbuf;
-} MemoryDriver;
-
-static int mem_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
-{
-    MemoryDriver *d = chr->opaque;
-
-    /* TODO: the QString implementation has the same code, we should
-     * introduce a generic way to do this in cutils.c */
-    if (d->outbuf_capacity < d->outbuf_size + len) {
-        /* grow outbuf */
-        d->outbuf_capacity += len;
-        d->outbuf_capacity *= 2;
-        d->outbuf = g_realloc(d->outbuf, d->outbuf_capacity);
-    }
-
-    memcpy(d->outbuf + d->outbuf_size, buf, len);
-    d->outbuf_size += len;
-
-    return len;
-}
-
-void qemu_chr_init_mem(CharDriverState *chr)
-{
-    MemoryDriver *d;
-
-    d = g_malloc(sizeof(*d));
-    d->outbuf_size = 0;
-    d->outbuf_capacity = 4096;
-    d->outbuf = g_malloc0(d->outbuf_capacity);
-
-    memset(chr, 0, sizeof(*chr));
-    chr->opaque = d;
-    chr->chr_write = mem_chr_write;
-}
-
-QString *qemu_chr_mem_to_qs(CharDriverState *chr)
-{
-    MemoryDriver *d = chr->opaque;
-    return qstring_from_substr((char *) d->outbuf, 0, d->outbuf_size - 1);
-}
-
-/* NOTE: this driver can not be closed with qemu_chr_delete()! */
-void qemu_chr_close_mem(CharDriverState *chr)
-{
-    MemoryDriver *d = chr->opaque;
-
-    g_free(d->outbuf);
-    g_free(chr->opaque);
-    chr->opaque = NULL;
-    chr->chr_write = NULL;
-}
-
-size_t qemu_chr_mem_osize(const CharDriverState *chr)
-{
-    const MemoryDriver *d = chr->opaque;
-    return d->outbuf_size;
-}
-
 /*********************************************************/
 /* Ring buffer chardev */
 
-- 
1.8.1.4