[Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Jan Kiszka]

More than one year ago I posted some patches to add a monitor command
callend device_show. The purpose of that command is to dump the state of
some qdev device based on its vmstate.

To improve the usability of that interface, the previous series also
tried to create a canonical qdev tree path name format and even added
monitor command expansion. That format created quite a few discussions,
and we never came to some conclusion.

As device_show is still a useful tool for debugging but qdev structuring
and addressing may significantly change in the near future, I decided to
reanimate those patches while avoiding almost any change of the
addressing scheme. The only one I propose is automatic ID assignment for
anonymous devices so that any qdev device is in principle addressable
(e.g. APIC and IO-APIC on x86).

As back then, device_show remains HMP-only to avoid any stable ABI
issues around QMP.

CC: Luiz Capitulino <lcapitulino@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>

Jan Kiszka (6):
  monitor: return length of printed string via monitor_[v]printf
  Add base64 encoder/decoder
  QMP: Reserve namespace for complex object classes
  QMP: Add QBuffer
  monitor: Add basic device state visualization
  qdev: Generate IDs for anonymous devices

 Makefile             |    2 +-
 Makefile.objs        |    4 +-
 QMP/qmp-spec.txt     |   24 ++++-
 base64.c             |  202 +++++++++++++++++++++++++++++++++++++
 base64.h             |   19 ++++
 check-qbuffer.c      |  172 ++++++++++++++++++++++++++++++++
 configure            |    2 +-
 hmp-commands.hx      |   16 +++
 hw/hw.h              |    2 +
 hw/qdev-properties.c |    2 +-
 hw/qdev.c            |  271 ++++++++++++++++++++++++++++++++++++++++++++++++--
 hw/qdev.h            |    3 +
 monitor.c            |   23 +++--
 monitor.h            |    4 +-
 qbuffer.c            |  117 ++++++++++++++++++++++
 qbuffer.h            |   33 ++++++
 qemu-tool.c          |    6 +-
 qerror.c             |    4 +
 qerror.h             |    3 +
 qjson.c              |   15 +++
 qobject.h            |    1 +
 21 files changed, 894 insertions(+), 31 deletions(-)
 create mode 100644 base64.c
 create mode 100644 base64.h
 create mode 100644 check-qbuffer.c
 create mode 100644 qbuffer.c
 create mode 100644 qbuffer.h

-- 
1.7.3.4

[Qemu-devel] [PATCH 1/6] monitor: return length of printed string via monitor_[v]printf

[Jan Kiszka]

This simply forwards the result of the internal vsnprintf to the callers
of monitor_printf and monitor_vprintf. When invoked over a QMP session
or in absence of an active monitor, -1 is returned.

CC: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 monitor.c   |   23 +++++++++++++++--------
 monitor.h   |    4 ++--
 qemu-tool.c |    6 ++++--
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/monitor.c b/monitor.c
index ada51d0..a1bba27 100644
--- a/monitor.c
+++ b/monitor.c
@@ -270,29 +270,36 @@ static void monitor_puts(Monitor *mon, const char *str)
     }
 }
 
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 {
     char buf[4096];
+    int ret;
 
-    if (!mon)
-        return;
-
+    if (!mon) {
+        return -1;
+    }
     mon_print_count_inc(mon);
 
     if (monitor_ctrl_mode(mon)) {
-        return;
+        return -1;
     }
 
-    vsnprintf(buf, sizeof(buf), fmt, ap);
+    ret = vsnprintf(buf, sizeof(buf), fmt, ap);
     monitor_puts(mon, buf);
+
+    return ret;
 }
 
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+int monitor_printf(Monitor *mon, const char *fmt, ...)
 {
     va_list ap;
+    int ret;
+
     va_start(ap, fmt);
-    monitor_vprintf(mon, fmt, ap);
+    ret = monitor_vprintf(mon, fmt, ap);
     va_end(ap);
+
+    return ret;
 }
 
 void monitor_print_filename(Monitor *mon, const char *filename)
diff --git a/monitor.h b/monitor.h
index 4f2d328..873ba22 100644
--- a/monitor.h
+++ b/monitor.h
@@ -52,9 +52,9 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
 
 int monitor_get_fd(Monitor *mon, const char *fdname);
 
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
     GCC_FMT_ATTR(2, 0);
-void monitor_printf(Monitor *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+int monitor_printf(Monitor *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void monitor_print_filename(Monitor *mon, const char *filename);
 void monitor_flush(Monitor *mon);
 
diff --git a/qemu-tool.c b/qemu-tool.c
index eb89fe0..b0a032d7b 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -44,12 +44,14 @@ void monitor_set_error(Monitor *mon, QError *qerror)
 {
 }
 
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 {
+    return -1;
 }
 
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+int monitor_printf(Monitor *mon, const char *fmt, ...)
 {
+    return -1;
 }
 
 void monitor_print_filename(Monitor *mon, const char *filename)
-- 
1.7.3.4

[Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder

[Jan Kiszka]

Will be used by QBuffer.

CC: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 Makefile.objs |    2 +-
 base64.c      |  202 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 base64.h      |   19 ++++++
 3 files changed, 222 insertions(+), 1 deletions(-)
 create mode 100644 base64.c
 create mode 100644 base64.h

diff --git a/Makefile.objs b/Makefile.objs
index d1f3e5d..41febf6 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -2,7 +2,7 @@
 # QObject
 qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
 qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
-qobject-obj-y += qerror.o error.o
+qobject-obj-y += qerror.o error.o base64.o
 
 #######################################################################
 # oslib-obj-y is code depending on the OS (win32 vs posix)
diff --git a/base64.c b/base64.c
new file mode 100644
index 0000000..f24d039
--- /dev/null
+++ b/base64.c
@@ -0,0 +1,202 @@
+/*
+ * Base64 encoder/decoder conforming to RFC 4648
+ * (based on Mozilla's nsprpub/lib/libc/src/base64.c)
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <inttypes.h>
+#include "base64.h"
+
+static const char base[] =
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
+
+static void encode3to4(const uint8_t *src, char *dest)
+{
+    uint32_t b32 = 0;
+    int i, j = 18;
+
+    for (i = 0; i < 3; i++) {
+        b32 <<= 8;
+        b32 |= src[i];
+    }
+    for (i = 0; i < 4; i++) {
+        dest[i] = base[(b32 >> j) & 0x3F];
+        j -= 6;
+    }
+}
+
+static void encode2to4(const uint8_t *src, char *dest)
+{
+    dest[0] = base[(src[0] >> 2) & 0x3F];
+    dest[1] = base[((src[0] & 0x03) << 4) | ((src[1] >> 4) & 0x0F)];
+    dest[2] = base[(src[1] & 0x0F) << 2];
+    dest[3] = '=';
+}
+
+static void encode1to4(const uint8_t *src, char *dest)
+{
+    dest[0] = base[(src[0] >> 2) & 0x3F];
+    dest[1] = base[(src[0] & 0x03) << 4];
+    dest[2] = '=';
+    dest[3] = '=';
+}
+
+/*
+ * Encode data in 'src' of length 'srclen' to a base64 string, saving the
+ * null-terminated result in 'dest'. The size of the destition buffer must be
+ * at least ((srclen + 2) / 3) * 4 + 1.
+ */
+void base64_encode(const uint8_t *src, size_t srclen, char *dest)
+{
+    while (srclen >= 3) {
+        encode3to4(src, dest);
+        src += 3;
+        dest += 4;
+        srclen -= 3;
+    }
+    switch (srclen) {
+    case 2:
+        encode2to4(src, dest);
+        dest += 4;
+        break;
+    case 1:
+        encode1to4(src, dest);
+        dest += 4;
+        break;
+    case 0:
+        break;
+    }
+    dest[0] = 0;
+}
+
+static int32_t codetovalue(char c)
+{
+    if (c >= 'A' && c <= 'Z') {
+        return c - 'A';
+    } else if (c >= 'a' && c <= 'z') {
+        return c - 'a' + 26;
+    } else if (c >= '0' && c <= '9') {
+        return c - '0' + 52;
+    } else if (c == '+') {
+        return 62;
+    } else if (c == '/') {
+        return 63;
+    } else {
+        return -1;
+    }
+}
+
+static int decode4to3(const char *src, uint8_t *dest)
+{
+    uint32_t b32 = 0;
+    int32_t bits;
+    int i;
+
+    for (i = 0; i < 4; i++) {
+        bits = codetovalue(src[i]);
+        if (bits < 0) {
+            return bits;
+        }
+        b32 <<= 6;
+        b32 |= bits;
+    }
+    dest[0] = (b32 >> 16) & 0xFF;
+    dest[1] = (b32 >> 8) & 0xFF;
+    dest[2] = b32 & 0xFF;
+
+    return 0;
+}
+
+static int decode3to2(const char *src, uint8_t *dest)
+{
+    uint32_t b32 = 0;
+    int32_t bits;
+
+    bits = codetovalue(src[0]);
+    if (bits < 0) {
+        return bits;
+    }
+    b32 = (uint32_t)bits;
+    b32 <<= 6;
+
+    bits = codetovalue(src[1]);
+    if (bits < 0) {
+        return bits;
+    }
+    b32 |= (uint32_t)bits;
+    b32 <<= 4;
+
+    bits = codetovalue(src[2]);
+    if (bits < 0) {
+        return bits;
+    }
+    b32 |= ((uint32_t)bits) >> 2;
+
+    dest[0] = (b32 >> 8) & 0xFF;
+    dest[1] = b32 & 0xFF;
+
+    return 0;
+}
+
+static int decode2to1(const char *src, uint8_t *dest)
+{
+    uint32_t b32;
+    int32_t bits;
+
+    bits = codetovalue(src[0]);
+    if (bits < 0) {
+        return bits;
+    }
+    b32 = (uint32_t)bits << 2;
+
+    bits = codetovalue(src[1]);
+    if (bits < 0) {
+        return bits;
+    }
+    b32 |= ((uint32_t)bits) >> 4;
+
+    dest[0] = b32;
+
+    return 0;
+}
+
+/*
+ * Convert string 'src' of length 'srclen' from base64 to binary form,
+ * saving the result in 'dest'. The size of the destination buffer must be at
+ * least srclen * 3 / 4.
+ *
+ * Returns 0 on success, -1 on conversion error.
+ */
+int base64_decode(const char *src, size_t srclen, uint8_t *dest)
+{
+    int ret;
+
+    while (srclen >= 4) {
+        ret = decode4to3(src, dest);
+        if (ret < 0) {
+            return ret;
+        }
+        src += 4;
+        dest += 3;
+        srclen -= 4;
+    }
+
+    switch (srclen) {
+    case 3:
+        return decode3to2(src, dest);
+    case 2:
+        return decode2to1(src, dest);
+    case 1:
+        return -1;
+    default: /* 0 */
+        return 0;
+    }
+}
diff --git a/base64.h b/base64.h
new file mode 100644
index 0000000..07e72a8
--- /dev/null
+++ b/base64.h
@@ -0,0 +1,19 @@
+/*
+ * Base64 encoder/decoder conforming to RFC 4648
+ * (based on Mozilla's nsprpub/lib/libc/src/base64.c)
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <inttypes.h>
+#include <stddef.h>
+
+void base64_encode(const uint8_t *src, size_t srclen, char *dest);
+int base64_decode(const char *src, size_t srclen, uint8_t *dest);
-- 
1.7.3.4

[Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes

[Jan Kiszka]

This reserves JSON objects that contain the key '__class__' for QMP-specific
complex objects. First user will be the buffer class.

CC: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 QMP/qmp-spec.txt |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 9d30a8c..fa1dd62 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -146,6 +146,15 @@ The format is:
 For a listing of supported asynchronous events, please, refer to the
 qmp-events.txt file.
 
+2.6 Complex object classes
+--------------------------
+
+JSON objects that contain the key-value pair '"__class__": json-string' are
+reserved for QMP-specific complex object classes that. QMP specifies which
+further keys each of these objects include and how they are encoded.
+
+So far, no complex object class is specified.
+
 3. QMP Examples
 ===============
 
@@ -229,9 +238,10 @@ avoid modifying QMP.  Both upstream and downstream need to take care to
 preserve long-term compatibility and interoperability.
 
 To help with that, QMP reserves JSON object member names beginning with
-'__' (double underscore) for downstream use ("downstream names").  This
-means upstream will never use any downstream names for its commands,
-arguments, errors, asynchronous events, and so forth.
+'__' (double underscore) for downstream use ("downstream names").  Downstream
+names MUST NOT end with '__' as this pattern is reserved for QMP-defined JSON
+object classes.  Upstream will never use any downstream names for its
+commands, arguments, errors, asynchronous events, and so forth.
 
 Any new names downstream wishes to add must begin with '__'.  To
 ensure compatibility with other downstreams, it is strongly
-- 
1.7.3.4

[Qemu-devel] [PATCH 4/6] QMP: Add QBuffer

[Jan Kiszka]

This introduces a buffer object for use with QMP. As a buffer is not
natively encodable in JSON, we encode it as a base64 string and
encapsulate the result in the new QMP object class "buffer".

The first use case for this is pushing the content of buffers that are
part of a device state into a qdict.

CC: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 Makefile         |    2 +-
 Makefile.objs    |    2 +-
 QMP/qmp-spec.txt |   10 +++-
 check-qbuffer.c  |  172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure        |    2 +-
 qbuffer.c        |  117 ++++++++++++++++++++++++++++++++++++
 qbuffer.h        |   33 ++++++++++
 qjson.c          |   15 +++++
 qobject.h        |    1 +
 9 files changed, 350 insertions(+), 4 deletions(-)
 create mode 100644 check-qbuffer.c
 create mode 100644 qbuffer.c
 create mode 100644 qbuffer.h

diff --git a/Makefile b/Makefile
index 8606849..231c0ac 100644
--- a/Makefile
+++ b/Makefile
@@ -151,7 +151,7 @@ qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trac
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $@")
 
-check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o test-coroutine.o: $(GENERATED_HEADERS)
+check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o test-coroutine.o check-qbuffer: $(GENERATED_HEADERS)
 
 CHECK_PROG_DEPS = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o
 
diff --git a/Makefile.objs b/Makefile.objs
index 41febf6..dc79057 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,6 +1,6 @@
 #######################################################################
 # QObject
-qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
+qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qbuffer.o
 qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
 qobject-obj-y += qerror.o error.o base64.o
 
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index fa1dd62..820e39d 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -153,7 +153,15 @@ JSON objects that contain the key-value pair '"__class__": json-string' are
 reserved for QMP-specific complex object classes that. QMP specifies which
 further keys each of these objects include and how they are encoded.
 
-So far, no complex object class is specified.
+2.6.1 Buffer class
+------------------
+
+This QMP object class allows to transport binary data. A buffer object
+consists of the following keys:
+
+{ "__class__": "buffer", "data": json-string }
+
+The data string is base64 encoded according to RFC 4648.
 
 3. QMP Examples
 ===============
diff --git a/check-qbuffer.c b/check-qbuffer.c
new file mode 100644
index 0000000..b490230
--- /dev/null
+++ b/check-qbuffer.c
@@ -0,0 +1,172 @@
+/*
+ * QBuffer unit-tests.
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include <check.h>
+
+#include "qbuffer.h"
+#include "qemu-common.h"
+
+const char data[] = "some data";
+
+START_TEST(qbuffer_from_data_test)
+{
+    QBuffer *qbuffer;
+
+    qbuffer = qbuffer_from_data(data, sizeof(data));
+    fail_unless(qbuffer != NULL);
+    fail_unless(qbuffer->base.refcnt == 1);
+    fail_unless(memcmp(data, qbuffer->data, sizeof(data)) == 0);
+    fail_unless(qbuffer->size == sizeof(data));
+    fail_unless(qobject_type(QOBJECT(qbuffer)) == QTYPE_QBUFFER);
+
+    /* destroy doesn't exit yet */
+    qemu_free(qbuffer->data);
+    qemu_free(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_destroy_test)
+{
+    QBuffer *qbuffer = qbuffer_from_data(data, sizeof(data));
+
+    QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_get_data_test)
+{
+    QBuffer *qbuffer;
+    const void *ret_data;
+
+    qbuffer = qbuffer_from_data(data, sizeof(data));
+    ret_data = qbuffer_get_data(qbuffer);
+    fail_unless(memcmp(ret_data, data, sizeof(data)) == 0);
+
+    QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_get_size_test)
+{
+    QBuffer *qbuffer;
+
+    qbuffer = qbuffer_from_data(data, sizeof(data));
+    fail_unless(qbuffer_get_size(qbuffer) == sizeof(data));
+
+    QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_from_qstring_test)
+{
+    const struct {
+        const char *encoded;
+        const char *decoded;
+    } pattern[3] = {
+        {
+            .encoded = "SGVsbG8sIFFCdWZmZXIhCg==",
+            .decoded = "Hello, QBuffer!",
+        },
+        {
+             .encoded = "SGVsbG8gUUJ1ZmZlcgo=",
+             .decoded = "Hello QBuffer",
+        },
+        {
+             .encoded = "SGVsbG8gUUJ1ZmZlciEK===",
+             .decoded = "Hello QBuffer!",
+        },
+    };
+    QBuffer *qbuffer;
+    QString *qstring;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(pattern); i++) {
+        qstring = qstring_from_str(pattern[i].encoded);
+        qbuffer = qbuffer_from_qstring(qstring);
+        QDECREF(qstring);
+
+        fail_unless(qbuffer != NULL);
+        fail_unless(memcmp(qbuffer_get_data(qbuffer), pattern[i].decoded,
+                    sizeof(pattern[i].decoded)) == 0);
+
+        QDECREF(qbuffer);
+    }
+}
+END_TEST
+
+START_TEST(qbuffer_from_invalid_qstring_test)
+{
+    const char *pattern[] = {
+        "SGVsbG8sIFFCdWZmZXIhC",
+        "SGVsbG8gU=UJ1ZmZlcgo",
+        "SGVsbG8gUUJ1*ZmZlciEK",
+    };
+    QBuffer *qbuffer;
+    QString *qstring;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(pattern); i++) {
+        qstring = qstring_from_str(pattern[i]);
+        qbuffer = qbuffer_from_qstring(qstring);
+        QDECREF(qstring);
+
+        fail_unless(qbuffer == NULL);
+    }
+}
+END_TEST
+
+START_TEST(qobject_to_qbuffer_test)
+{
+    QBuffer *qbuffer;
+
+    qbuffer = qbuffer_from_data(data, sizeof(data));
+    fail_unless(qobject_to_qbuffer(QOBJECT(qbuffer)) == qbuffer);
+
+    QDECREF(qbuffer);
+}
+END_TEST
+
+static Suite *qbuffer_suite(void)
+{
+    Suite *s;
+    TCase *qbuffer_public_tcase;
+
+    s = suite_create("QBuffer test-suite");
+
+    qbuffer_public_tcase = tcase_create("Public Interface");
+    suite_add_tcase(s, qbuffer_public_tcase);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_from_data_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_destroy_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_get_data_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_get_size_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_from_qstring_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_from_invalid_qstring_test);
+    tcase_add_test(qbuffer_public_tcase, qobject_to_qbuffer_test);
+
+    return s;
+}
+
+int main(void)
+{
+    int nf;
+    Suite *s;
+    SRunner *sr;
+
+    s = qbuffer_suite();
+    sr = srunner_create(s);
+
+    srunner_run_all(sr, CK_NORMAL);
+    nf = srunner_ntests_failed(sr);
+    srunner_free(sr);
+
+    return (nf == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
diff --git a/configure b/configure
index 1340c33..3521898 100755
--- a/configure
+++ b/configure
@@ -2630,7 +2630,7 @@ if test "$softmmu" = yes ; then
       tools="qemu-ga\$(EXESUF) $tools"
     fi
     if [ "$check_utests" = "yes" ]; then
-      tools="check-qint check-qstring check-qdict check-qlist $tools"
+      tools="check-qint check-qstring check-qdict check-qlist check-qbuffer $tools"
       tools="check-qfloat check-qjson $tools"
     fi
   fi
diff --git a/qbuffer.c b/qbuffer.c
new file mode 100644
index 0000000..4252a1e
--- /dev/null
+++ b/qbuffer.c
@@ -0,0 +1,117 @@
+/*
+ * QBuffer Module
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qbuffer.h"
+#include "qobject.h"
+#include "qemu-common.h"
+#include "base64.h"
+
+static void qbuffer_destroy_obj(QObject *obj);
+
+static const QType qbuffer_type = {
+    .code = QTYPE_QBUFFER,
+    .destroy = qbuffer_destroy_obj,
+};
+
+/**
+ * qbuffer_from_data(): Create a new QBuffer from an existing data blob
+ *
+ * Returns strong reference.
+ */
+QBuffer *qbuffer_from_data(const void *data, size_t size)
+{
+    QBuffer *qb;
+
+    qb = g_malloc(sizeof(*qb));
+    qb->data = g_malloc(size);
+    memcpy(qb->data, data, size);
+    qb->size = size;
+    QOBJECT_INIT(qb, &qbuffer_type);
+
+    return qb;
+}
+
+/**
+ * qbuffer_from_qstring(): Create a new QBuffer from a QString object that
+ * contains the data as a stream of hex-encoded bytes
+ *
+ * Returns strong reference.
+ */
+QBuffer *qbuffer_from_qstring(const QString *string)
+{
+    const char *str = qstring_get_str(string);
+    size_t str_len;
+    QBuffer *qb;
+
+    qb = g_malloc(sizeof(*qb));
+
+    str_len = strlen(str);
+    while (str_len > 0 && str[str_len - 1] == '=') {
+        str_len--;
+    }
+    qb->size = (str_len / 4) * 3 + ((str_len % 4) * 3) / 4;
+    qb->data = g_malloc(qb->size);
+
+    QOBJECT_INIT(qb, &qbuffer_type);
+
+    if (base64_decode(str, str_len, qb->data) < 0) {
+        qbuffer_destroy_obj(QOBJECT(qb));
+        return NULL;
+    }
+
+    return qb;
+}
+
+/**
+ * qbuffer_get_data(): Return pointer to stored data
+ *
+ * NOTE: Should be used with caution, if the object is deallocated
+ * this pointer becomes invalid.
+ */
+const void *qbuffer_get_data(const QBuffer *qb)
+{
+    return qb->data;
+}
+
+/**
+ * qbuffer_get_size(): Return size of stored data
+ */
+size_t qbuffer_get_size(const QBuffer *qb)
+{
+    return qb->size;
+}
+
+/**
+ * qobject_to_qbool(): Convert a QObject into a QBuffer
+ */
+QBuffer *qobject_to_qbuffer(const QObject *obj)
+{
+    if (qobject_type(obj) != QTYPE_QBUFFER) {
+        return NULL;
+    }
+
+    return container_of(obj, QBuffer, base);
+}
+
+/**
+ * qbuffer_destroy_obj(): Free all memory allocated by a QBuffer object
+ */
+static void qbuffer_destroy_obj(QObject *obj)
+{
+    QBuffer *qb;
+
+    assert(obj != NULL);
+    qb = qobject_to_qbuffer(obj);
+    g_free(qb->data);
+    g_free(qb);
+}
diff --git a/qbuffer.h b/qbuffer.h
new file mode 100644
index 0000000..2e01078
--- /dev/null
+++ b/qbuffer.h
@@ -0,0 +1,33 @@
+/*
+ * QBuffer Module
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QBUFFER_H
+#define QBUFFER_H
+
+#include <stdint.h>
+#include "qobject.h"
+#include "qstring.h"
+
+typedef struct QBuffer {
+    QObject_HEAD;
+    void *data;
+    size_t size;
+} QBuffer;
+
+QBuffer *qbuffer_from_data(const void *data, size_t size);
+QBuffer *qbuffer_from_qstring(const QString *string);
+const void *qbuffer_get_data(const QBuffer *qb);
+size_t qbuffer_get_size(const QBuffer *qb);
+QBuffer *qobject_to_qbuffer(const QObject *obj);
+
+#endif /* QBUFFER_H */
diff --git a/qjson.c b/qjson.c
index f9c8e77..d415f83 100644
--- a/qjson.c
+++ b/qjson.c
@@ -19,7 +19,9 @@
 #include "qlist.h"
 #include "qbool.h"
 #include "qfloat.h"
+#include "qbuffer.h"
 #include "qdict.h"
+#include "base64.h"
 
 typedef struct JSONParsingState
 {
@@ -268,6 +270,19 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         }
         break;
     }
+    case QTYPE_QBUFFER: {
+        QBuffer *val = qobject_to_qbuffer(obj);
+        size_t data_size = qbuffer_get_size(val);
+        size_t str_len = ((data_size + 2) / 3) * 4;
+        char *buffer = g_malloc(str_len + 1);
+
+        base64_encode(qbuffer_get_data(val), data_size, buffer);
+        qstring_append(str, "{\"__class__\": \"buffer\", \"data\": \"");
+        qstring_append(str, buffer);
+        qstring_append(str, "\"}");
+        g_free(buffer);
+        break;
+    }
     case QTYPE_QERROR:
         /* XXX: should QError be emitted? */
     case QTYPE_NONE:
diff --git a/qobject.h b/qobject.h
index d42386d..4ec932b 100644
--- a/qobject.h
+++ b/qobject.h
@@ -44,6 +44,7 @@ typedef enum {
     QTYPE_QFLOAT,
     QTYPE_QBOOL,
     QTYPE_QERROR,
+    QTYPE_QBUFFER,
 } qtype_code;
 
 struct QObject;
-- 
1.7.3.4

[Qemu-devel] [PATCH 5/6] monitor: Add basic device state visualization

[Jan Kiszka]

This introduces device_show, a monitor command that saves the vmstate of
a qdev device and visualizes it. Buffers are cut after 16 byte by
default, but the full content can be requested via '-f'. To pretty-print
sub-arrays, vmstate is extended to store the start index name. A new
qerror is introduced to signal a missing vmstate. QMP is not supported
as we cannot provide a stable interface, at least at this point.

CC: Luiz Capitulino <lcapitulino@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hmp-commands.hx |   16 ++++
 hw/hw.h         |    2 +
 hw/qdev.c       |  248 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h       |    2 +
 qerror.c        |    4 +
 qerror.h        |    3 +
 6 files changed, 275 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 0ccfb28..2fc5b3b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -586,6 +586,22 @@ Remove device @var{id}.
 ETEXI
 
     {
+        .name       = "device_show",
+        .args_type  = "full:-f,id:s",
+        .params     = "[-f] device",
+        .help       = "show device state (specify -f for full buffer dumping)",
+        .user_print = device_user_print,
+        .mhandler.cmd_new = do_device_show,
+    },
+
+STEXI
+@item device_show [@code{-f}] @var{id}
+
+Show state of device @var{id} in a human-readable form. Buffers are cut
+after 16 bytes unless a full dump is requested via @code{-f}.
+ETEXI
+
+    {
         .name       = "cpu",
         .args_type  = "index:i",
         .params     = "index",
diff --git a/hw/hw.h b/hw/hw.h
index a124da9..5bc55ac 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -304,6 +304,7 @@ enum VMStateFlags {
 
 typedef struct {
     const char *name;
+    const char *start_index;
     size_t offset;
     size_t size;
     size_t start;
@@ -437,6 +438,7 @@ extern const VMStateInfo vmstate_info_unused_buffer;
     .size       = sizeof(_type),                                     \
     .flags      = VMS_ARRAY,                                         \
     .offset     = vmstate_offset_sub_array(_state, _field, _type, _start), \
+    .start_index = (stringify(_start)),                              \
 }
 
 #define VMSTATE_ARRAY_INT32_UNSAFE(_field, _state, _field_num, _info, _type) {\
diff --git a/hw/qdev.c b/hw/qdev.c
index c463c52..3b0e1af 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,6 +29,9 @@
 #include "qdev.h"
 #include "sysemu.h"
 #include "monitor.h"
+#include "qjson.h"
+#include "qint.h"
+#include "qbuffer.h"
 
 static int qdev_hotplug = 0;
 static bool qdev_hot_added = false;
@@ -956,3 +959,248 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
 
     return strdup(path);
 }
+
+#define NAME_COLUMN_WIDTH 23
+
+static void print_field(Monitor *mon, const QDict *qfield, int indent);
+
+static void print_elem(Monitor *mon, const QObject *qelem, size_t size,
+                       int column_pos, int indent)
+{
+    int64_t data_size;
+    const void *data;
+    int n;
+
+    if (qobject_type(qelem) == QTYPE_QDICT) {
+        if (column_pos >= 0) {
+            monitor_printf(mon, ".\n");
+        }
+    } else {
+        monitor_printf(mon, ":");
+        column_pos++;
+        if (column_pos < NAME_COLUMN_WIDTH) {
+            monitor_printf(mon, "%*c", NAME_COLUMN_WIDTH - column_pos, ' ');
+        }
+    }
+
+    switch (qobject_type(qelem)) {
+    case QTYPE_QDICT:
+        print_field(mon, qobject_to_qdict(qelem), indent + 2);
+        break;
+    case QTYPE_QBUFFER:
+        data = qbuffer_get_data(qobject_to_qbuffer(qelem));
+        data_size = qbuffer_get_size(qobject_to_qbuffer(qelem));
+        for (n = 0; n < data_size; ) {
+            monitor_printf(mon, " %02x", *((uint8_t *)data+n));
+            if (++n < size) {
+                if (n % 16 == 0) {
+                    monitor_printf(mon, "\n%*c", NAME_COLUMN_WIDTH, ' ');
+                } else if (n % 8 == 0) {
+                    monitor_printf(mon, " -");
+                }
+            }
+        }
+        if (data_size < size) {
+            monitor_printf(mon, " ...");
+        }
+        monitor_printf(mon, "\n");
+        break;
+    case QTYPE_QINT:
+        monitor_printf(mon, " %0*" PRIx64 "\n", (int)size * 2,
+                       qint_get_int(qobject_to_qint(qelem)));
+        break;
+    default:
+        assert(0);
+    }
+}
+
+static void print_field(Monitor *mon, const QDict *qfield, int indent)
+{
+    const char *name = qdict_get_str(qfield, "name");
+    const char *start = qdict_get_try_str(qfield, "start");
+    int64_t size = qdict_get_int(qfield, "size");
+    QList *qlist = qdict_get_qlist(qfield, "elems");
+    QListEntry *entry, *sub_entry;
+    QList *sub_list;
+    int elem_no = 0;
+
+    QLIST_FOREACH_ENTRY(qlist, entry) {
+        QObject *qelem = qlist_entry_obj(entry);
+        int pos = indent + strlen(name);
+
+        if (qobject_type(qelem) == QTYPE_QLIST) {
+            monitor_printf(mon, "%*c%s", indent, ' ', name);
+            if (start) {
+                pos += monitor_printf(mon, "[%s+%02x]", start, elem_no);
+            } else {
+                pos += monitor_printf(mon, "[%02x]", elem_no);
+            }
+            sub_list = qobject_to_qlist(qelem);
+            QLIST_FOREACH_ENTRY(sub_list, sub_entry) {
+                print_elem(mon, qlist_entry_obj(sub_entry), size, pos,
+                           indent + 2);
+                pos = -1;
+            }
+        } else {
+            if (elem_no == 0) {
+                monitor_printf(mon, "%*c%s", indent, ' ', name);
+            } else {
+                pos = -1;
+            }
+            print_elem(mon, qelem, size, pos, indent);
+        }
+        elem_no++;
+    }
+}
+
+void device_user_print(Monitor *mon, const QObject *data)
+{
+    QDict *qdict = qobject_to_qdict(data);
+    QList *qlist = qdict_get_qlist(qdict, "fields");
+    QListEntry *entry;
+
+    monitor_printf(mon, "dev: %s, id \"%s\", version %" PRId64 "\n",
+                   qdict_get_str(qdict, "device"),
+                   qdict_get_str(qdict, "id"),
+                   qdict_get_int(qdict, "version"));
+
+    QLIST_FOREACH_ENTRY(qlist, entry) {
+        print_field(mon, qobject_to_qdict(qlist_entry_obj(entry)), 2);
+    }
+}
+
+static size_t parse_vmstate(const VMStateDescription *vmsd, void *opaque,
+                            QList *qlist, bool full_buffers)
+{
+    VMStateField *field = vmsd->fields;
+    size_t overall_size = 0;
+
+    if (vmsd->pre_save) {
+        vmsd->pre_save(opaque);
+    }
+    while (field->name) {
+        if (!field->field_exists ||
+            field->field_exists(opaque, vmsd->version_id)) {
+            void *base_addr = opaque + field->offset;
+            int i, n_elems = 1;
+            int is_array = 1;
+            size_t size = field->size;
+            size_t real_size = 0;
+            size_t dump_size;
+            QDict *qfield = qdict_new();
+            QList *qelems = qlist_new();
+
+            qlist_append_obj(qlist, QOBJECT(qfield));
+
+            qdict_put_obj(qfield, "name",
+                          QOBJECT(qstring_from_str(field->name)));
+            qdict_put_obj(qfield, "elems", QOBJECT(qelems));
+
+            if (field->flags & VMS_VBUFFER) {
+                size = *(int32_t *)(opaque + field->size_offset);
+                if (field->flags & VMS_MULTIPLY) {
+                    size *= field->size;
+                }
+            }
+            if (field->start_index) {
+                qdict_put_obj(qfield, "start",
+                              QOBJECT(qstring_from_str(field->start_index)));
+            }
+
+            if (field->flags & VMS_ARRAY) {
+                n_elems = field->num;
+            } else if (field->flags & VMS_VARRAY_INT32) {
+                n_elems = *(int32_t *)(opaque + field->num_offset);
+            } else if (field->flags & VMS_VARRAY_UINT16) {
+                n_elems = *(uint16_t *)(opaque + field->num_offset);
+            } else {
+                is_array = 0;
+            }
+            if (field->flags & VMS_POINTER) {
+                base_addr = *(void **)base_addr + field->start;
+            }
+            for (i = 0; i < n_elems; i++) {
+                void *addr = base_addr + size * i;
+                QList *sub_elems = qelems;
+                int val;
+
+                if (is_array) {
+                    sub_elems = qlist_new();
+                    qlist_append_obj(qelems, QOBJECT(sub_elems));
+                }
+                if (field->flags & VMS_ARRAY_OF_POINTER) {
+                    addr = *(void **)addr;
+                }
+                if (field->flags & VMS_STRUCT) {
+                    real_size = parse_vmstate(field->vmsd, addr,
+                                              sub_elems, full_buffers);
+                } else {
+                    real_size = size;
+                    if (field->flags & (VMS_BUFFER | VMS_VBUFFER)) {
+                        dump_size = (full_buffers || size <= 16) ? size : 16;
+                        qlist_append_obj(sub_elems,
+                                QOBJECT(qbuffer_from_data(addr, dump_size)));
+                    } else {
+                        switch (size) {
+                        case 1:
+                            val = *(uint8_t *)addr;
+                            break;
+                        case 2:
+                            val = *(uint16_t *)addr;
+                            break;
+                        case 4:
+                            val = *(uint32_t *)addr;
+                            break;
+                        case 8:
+                            val = *(uint64_t *)addr;
+                            break;
+                        default:
+                            assert(0);
+                        }
+                        qlist_append_obj(sub_elems,
+                                         QOBJECT(qint_from_int(val)));
+                    }
+                }
+                overall_size += real_size;
+            }
+            qdict_put_obj(qfield, "size", QOBJECT(qint_from_int(real_size)));
+        }
+        field++;
+    }
+    return overall_size;
+}
+
+int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    const VMStateDescription *vmsd;
+    DeviceState *dev;
+    QList *qlist;
+    int name_len;
+    char *name;
+
+    dev = qdev_find_recursive(main_system_bus, id);
+    if (!dev) {
+        return -1;
+    }
+
+    vmsd = dev->info->vmsd;
+    if (!vmsd || vmsd->unmigratable) {
+        qerror_report(QERR_DEVICE_NO_STATE, dev->info->name);
+        error_printf_unless_qmp("Note: device may simply lack complete qdev "
+                                "conversion\n");
+        return -1;
+    }
+
+    name_len = strlen(dev->info->name) + 16;
+    name = g_malloc(name_len);
+    snprintf(name, name_len, "%s", dev->info->name);
+    *ret_data = qobject_from_jsonf("{ 'device': %s, 'id': %s, 'version': %d }",
+                                   name, dev->id ? : "", vmsd->version_id);
+    g_free(name);
+    qlist = qlist_new();
+    parse_vmstate(vmsd, dev, qlist, qdict_get_try_bool(qdict, "full", false));
+    qdict_put_obj(qobject_to_qdict(*ret_data), "fields", QOBJECT(qlist));
+
+    return 0;
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index 8a13ec9..912fc45 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -215,6 +215,8 @@ void do_info_qtree(Monitor *mon);
 void do_info_qdm(Monitor *mon);
 int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+void device_user_print(Monitor *mon, const QObject *data);
+int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 /*** qdev-properties.c ***/
 
diff --git a/qerror.c b/qerror.c
index 3d64b80..8aed20c 100644
--- a/qerror.c
+++ b/qerror.c
@@ -105,6 +105,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' does not support hotplugging",
     },
     {
+        .error_fmt = QERR_DEVICE_NO_STATE,
+        .desc      = "No state available for device '%(device)'",
+    },
+    {
         .error_fmt = QERR_DUPLICATE_ID,
         .desc      = "Duplicate ID '%(id)' for %(object)",
     },
diff --git a/qerror.h b/qerror.h
index 8058456..17b40ab 100644
--- a/qerror.h
+++ b/qerror.h
@@ -94,6 +94,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_DEVICE_NO_HOTPLUG \
     "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
 
+#define QERR_DEVICE_NO_STATE \
+    "{ 'class': 'DeviceNoState', 'data': { 'device': %s } }"
+
 #define QERR_DUPLICATE_ID \
     "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
 
-- 
1.7.3.4

[Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices

[Jan Kiszka]

In order to address devices for that the user forgot or is even unable
(no_user) to provide an ID, assign an automatically generated one. Such
IDs have the format #<number>, thus are outside the name space availing
to users. Don't use them for bus naming to avoid any other user-visible
change.

CC: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/qdev-properties.c |    2 +-
 hw/qdev.c            |   25 +++++++++++++------------
 hw/qdev.h            |    1 +
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0c0c292..4e39cef 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -682,7 +682,7 @@ int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *va
     if (res < 0) {
         error_report("Can't attach drive %s to %s.%s: %s",
                      bdrv_get_device_name(value),
-                     dev->id ? dev->id : dev->info->name,
+                     dev->id != dev->auto_id ? dev->id : dev->info->name,
                      name, strerror(-res));
         return -1;
     }
diff --git a/hw/qdev.c b/hw/qdev.c
index 3b0e1af..807902b 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -85,6 +85,7 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name)
 
 static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
 {
+    static unsigned int auto_id;
     DeviceState *dev;
 
     assert(bus->info == info->bus_info);
@@ -94,6 +95,8 @@ static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
     qdev_prop_set_defaults(dev, dev->info->props);
     qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
     qdev_prop_set_globals(dev);
+    snprintf(dev->auto_id, sizeof(dev->auto_id), "#%u", ++auto_id);
+    dev->id = dev->auto_id;
     QLIST_INSERT_HEAD(&bus->children, dev, sibling);
     if (qdev_hotplug) {
         assert(bus->allow_hotplug);
@@ -574,8 +577,9 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id)
     BusState *child;
 
     QLIST_FOREACH(dev, &bus->children, sibling) {
-        if (dev->id && strcmp(dev->id, id) == 0)
+        if (strcmp(dev->id, id) == 0) {
             return dev;
+        }
         QLIST_FOREACH(child, &dev->child_bus, sibling) {
             ret = qdev_find_recursive(child, id);
             if (ret) {
@@ -591,8 +595,8 @@ static void qbus_list_bus(DeviceState *dev)
     BusState *child;
     const char *sep = " ";
 
-    error_printf("child busses at \"%s\":",
-                 dev->id ? dev->id : dev->info->name);
+    error_printf("child busses at \"%s\"/\"%s\":",
+                 dev->info->name, dev->id);
     QLIST_FOREACH(child, &dev->child_bus, sibling) {
         error_printf("%s\"%s\"", sep, child->name);
         sep = ", ";
@@ -607,9 +611,7 @@ static void qbus_list_dev(BusState *bus)
 
     error_printf("devices at \"%s\":", bus->name);
     QLIST_FOREACH(dev, &bus->children, sibling) {
-        error_printf("%s\"%s\"", sep, dev->info->name);
-        if (dev->id)
-            error_printf("/\"%s\"", dev->id);
+        error_printf("%s\"%s\"/\"%s\"", sep, dev->info->name, dev->id);
         sep = ", ";
     }
     error_printf("\n");
@@ -638,7 +640,7 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)
      *   (3) driver alias, if present
      */
     QLIST_FOREACH(dev, &bus->children, sibling) {
-        if (dev->id  &&  strcmp(dev->id, elem) == 0) {
+        if (strcmp(dev->id, elem) == 0) {
             return dev;
         }
     }
@@ -754,8 +756,8 @@ void qbus_create_inplace(BusState *bus, BusInfo *info,
     if (name) {
         /* use supplied name */
         bus->name = g_strdup(name);
-    } else if (parent && parent->id) {
-        /* parent device has id -> use it for bus name */
+    } else if (parent && parent->id != parent->auto_id) {
+        /* parent device has user-assigned id -> use it for bus name */
         len = strlen(parent->id) + 16;
         buf = g_malloc(len);
         snprintf(buf, len, "%s.%d", parent->id, parent->num_child_bus);
@@ -850,8 +852,7 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
 static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     BusState *child;
-    qdev_printf("dev: %s, id \"%s\"\n", dev->info->name,
-                dev->id ? dev->id : "");
+    qdev_printf("dev: %s, id \"%s\"\n", dev->info->name, dev->id);
     indent += 2;
     if (dev->num_gpio_in) {
         qdev_printf("gpio-in %d\n", dev->num_gpio_in);
@@ -1196,7 +1197,7 @@ int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data)
     name = g_malloc(name_len);
     snprintf(name, name_len, "%s", dev->info->name);
     *ret_data = qobject_from_jsonf("{ 'device': %s, 'id': %s, 'version': %d }",
-                                   name, dev->id ? : "", vmsd->version_id);
+                                   name, dev->id, vmsd->version_id);
     g_free(name);
     qlist = qlist_new();
     parse_vmstate(vmsd, dev, qlist, qdict_get_try_bool(qdict, "full", false));
diff --git a/hw/qdev.h b/hw/qdev.h
index 912fc45..d028409 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -31,6 +31,7 @@ enum {
    so that it can be embedded in individual device state structures.  */
 struct DeviceState {
     const char *id;
+    char auto_id[16];
     enum DevState state;
     QemuOpts *opts;
     int hotplugged;
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder

[Peter Maydell]

On 26 August 2011 15:48, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Will be used by QBuffer.

Is it possible to use the glib base64 encode/decode routines instead
of rolling our own here?

-- PMM

Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder

[Jan Kiszka]

On 2011-08-26 17:21, Peter Maydell wrote:
> On 26 August 2011 15:48, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Will be used by QBuffer.
> 
> Is it possible to use the glib base64 encode/decode routines instead
> of rolling our own here?

Yeah, times are changing. Need to check what's there and how to use it.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder

[Jan Kiszka]

On 2011-08-26 17:23, Jan Kiszka wrote:
> On 2011-08-26 17:21, Peter Maydell wrote:
>> On 26 August 2011 15:48, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> Will be used by QBuffer.
>>
>> Is it possible to use the glib base64 encode/decode routines instead
>> of rolling our own here?
> 
> Yeah, times are changing. Need to check what's there and how to use it.

Requires glib >= 2.12, we are currently at >= 2.0, right? Would it be OK
to raise the entry barrier?

I'm also still looking for error handling of g_base64_decode. I guess
one is supposed to compare returned length against some expected value.
Well, it's glib...

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder

[Jan Kiszka]

On 2011-08-26 17:47, Jan Kiszka wrote:
> On 2011-08-26 17:23, Jan Kiszka wrote:
>> On 2011-08-26 17:21, Peter Maydell wrote:
>>> On 26 August 2011 15:48, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> Will be used by QBuffer.
>>>
>>> Is it possible to use the glib base64 encode/decode routines instead
>>> of rolling our own here?
>>
>> Yeah, times are changing. Need to check what's there and how to use it.
> 
> Requires glib >= 2.12, we are currently at >= 2.0, right? Would it be OK
> to raise the entry barrier?
> 
> I'm also still looking for error handling of g_base64_decode. I guess
> one is supposed to compare returned length against some expected value.
> Well, it's glib...

The length check is not sufficient, and glib's decoder fails on my
invalid input string tests.

I think proper error detection in base64 inputs is a worthwhile feature
that glib lacks. So I'll stick with roll-our-own (actually, it's
Mozilla's version).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] [PATCH v2 4/6] QMP: Add QBuffer

[Jan Kiszka]

This introduces a buffer object for use with QMP. As a buffer is not
natively encodable in JSON, we encode it as a base64 string and
encapsulate the result in the new QMP object class "buffer".

The first use case for this is pushing the content of buffers that are
part of a device state into a qdict.

CC: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - fixed unit tests

 Makefile         |    5 +-
 Makefile.objs    |    2 +-
 QMP/qmp-spec.txt |   10 +++-
 check-qbuffer.c  |  172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure        |    2 +-
 qbuffer.c        |  117 ++++++++++++++++++++++++++++++++++++
 qbuffer.h        |   33 ++++++++++
 qjson.c          |   15 +++++
 qobject.h        |    1 +
 9 files changed, 352 insertions(+), 5 deletions(-)
 create mode 100644 check-qbuffer.c
 create mode 100644 qbuffer.c
 create mode 100644 qbuffer.h

diff --git a/Makefile b/Makefile
index 8606849..9dc2cf4 100644
--- a/Makefile
+++ b/Makefile
@@ -151,7 +151,7 @@ qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(oslib-obj-y) $(trac
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $@")
 
-check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o test-coroutine.o: $(GENERATED_HEADERS)
+check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o test-coroutine.o check-qbuffer.o: $(GENERATED_HEADERS)
 
 CHECK_PROG_DEPS = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o
 
@@ -160,7 +160,8 @@ check-qstring: check-qstring.o qstring.o $(CHECK_PROG_DEPS)
 check-qdict: check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o qlist.o $(CHECK_PROG_DEPS)
 check-qlist: check-qlist.o qlist.o qint.o $(CHECK_PROG_DEPS)
 check-qfloat: check-qfloat.o qfloat.o $(CHECK_PROG_DEPS)
-check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o error.o qerror.o qemu-error.o $(CHECK_PROG_DEPS)
+check-qbuffer: check-qbuffer.o qbuffer.o qstring.o base64.o $(CHECK_PROG_DEPS)
+check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o qbuffer.o base64.o json-streamer.o json-lexer.o json-parser.o error.o qerror.o qemu-error.o $(CHECK_PROG_DEPS)
 test-coroutine: test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y) $(CHECK_PROG_DEPS)
 
 $(qapi-obj-y): $(GENERATED_HEADERS)
diff --git a/Makefile.objs b/Makefile.objs
index 41febf6..dc79057 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,6 +1,6 @@
 #######################################################################
 # QObject
-qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
+qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qbuffer.o
 qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
 qobject-obj-y += qerror.o error.o base64.o
 
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index fa1dd62..820e39d 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -153,7 +153,15 @@ JSON objects that contain the key-value pair '"__class__": json-string' are
 reserved for QMP-specific complex object classes that. QMP specifies which
 further keys each of these objects include and how they are encoded.
 
-So far, no complex object class is specified.
+2.6.1 Buffer class
+------------------
+
+This QMP object class allows to transport binary data. A buffer object
+consists of the following keys:
+
+{ "__class__": "buffer", "data": json-string }
+
+The data string is base64 encoded according to RFC 4648.
 
 3. QMP Examples
 ===============
diff --git a/check-qbuffer.c b/check-qbuffer.c
new file mode 100644
index 0000000..0876e9f
--- /dev/null
+++ b/check-qbuffer.c
@@ -0,0 +1,172 @@
+/*
+ * QBuffer unit-tests.
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include <check.h>
+
+#include "qbuffer.h"
+#include "qemu-common.h"
+
+const char data[] = "some data";
+
+START_TEST(qbuffer_from_data_test)
+{
+    QBuffer *qbuffer;
+
+    qbuffer = qbuffer_from_data(data, sizeof(data));
+    fail_unless(qbuffer != NULL);
+    fail_unless(qbuffer->base.refcnt == 1);
+    fail_unless(memcmp(data, qbuffer->data, sizeof(data)) == 0);
+    fail_unless(qbuffer->size == sizeof(data));
+    fail_unless(qobject_type(QOBJECT(qbuffer)) == QTYPE_QBUFFER);
+
+    /* destroy doesn't exit yet */
+    g_free(qbuffer->data);
+    g_free(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_destroy_test)
+{
+    QBuffer *qbuffer = qbuffer_from_data(data, sizeof(data));
+
+    QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_get_data_test)
+{
+    QBuffer *qbuffer;
+    const void *ret_data;
+
+    qbuffer = qbuffer_from_data(data, sizeof(data));
+    ret_data = qbuffer_get_data(qbuffer);
+    fail_unless(memcmp(ret_data, data, sizeof(data)) == 0);
+
+    QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_get_size_test)
+{
+    QBuffer *qbuffer;
+
+    qbuffer = qbuffer_from_data(data, sizeof(data));
+    fail_unless(qbuffer_get_size(qbuffer) == sizeof(data));
+
+    QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_from_qstring_test)
+{
+    const struct {
+        const char *encoded;
+        const char *decoded;
+    } pattern[3] = {
+        {
+            .encoded = "SGVsbG8sIFFCdWZmZXIhCg==",
+            .decoded = "Hello, QBuffer!",
+        },
+        {
+             .encoded = "SGVsbG8gUUJ1ZmZlcgo=",
+             .decoded = "Hello QBuffer",
+        },
+        {
+             .encoded = "SGVsbG8gUUJ1ZmZlciEK===",
+             .decoded = "Hello QBuffer!",
+        },
+    };
+    QBuffer *qbuffer;
+    QString *qstring;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(pattern); i++) {
+        qstring = qstring_from_str(pattern[i].encoded);
+        qbuffer = qbuffer_from_qstring(qstring);
+        QDECREF(qstring);
+
+        fail_unless(qbuffer != NULL);
+        fail_unless(memcmp(qbuffer_get_data(qbuffer), pattern[i].decoded,
+                    sizeof(pattern[i].decoded)) == 0);
+
+        QDECREF(qbuffer);
+    }
+}
+END_TEST
+
+START_TEST(qbuffer_from_invalid_qstring_test)
+{
+    const char *pattern[] = {
+        "SGVsbG8sIFFCdWZmZXIhC",
+        "SGVsbG8gU=UJ1ZmZlcgo",
+        "SGVsbG8gUUJ1*ZmZlciEK",
+    };
+    QBuffer *qbuffer;
+    QString *qstring;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(pattern); i++) {
+        qstring = qstring_from_str(pattern[i]);
+        qbuffer = qbuffer_from_qstring(qstring);
+        QDECREF(qstring);
+
+        fail_unless(qbuffer == NULL);
+    }
+}
+END_TEST
+
+START_TEST(qobject_to_qbuffer_test)
+{
+    QBuffer *qbuffer;
+
+    qbuffer = qbuffer_from_data(data, sizeof(data));
+    fail_unless(qobject_to_qbuffer(QOBJECT(qbuffer)) == qbuffer);
+
+    QDECREF(qbuffer);
+}
+END_TEST
+
+static Suite *qbuffer_suite(void)
+{
+    Suite *s;
+    TCase *qbuffer_public_tcase;
+
+    s = suite_create("QBuffer test-suite");
+
+    qbuffer_public_tcase = tcase_create("Public Interface");
+    suite_add_tcase(s, qbuffer_public_tcase);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_from_data_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_destroy_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_get_data_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_get_size_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_from_qstring_test);
+    tcase_add_test(qbuffer_public_tcase, qbuffer_from_invalid_qstring_test);
+    tcase_add_test(qbuffer_public_tcase, qobject_to_qbuffer_test);
+
+    return s;
+}
+
+int main(void)
+{
+    int nf;
+    Suite *s;
+    SRunner *sr;
+
+    s = qbuffer_suite();
+    sr = srunner_create(s);
+
+    srunner_run_all(sr, CK_NORMAL);
+    nf = srunner_ntests_failed(sr);
+    srunner_free(sr);
+
+    return (nf == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
diff --git a/configure b/configure
index 1340c33..3521898 100755
--- a/configure
+++ b/configure
@@ -2630,7 +2630,7 @@ if test "$softmmu" = yes ; then
       tools="qemu-ga\$(EXESUF) $tools"
     fi
     if [ "$check_utests" = "yes" ]; then
-      tools="check-qint check-qstring check-qdict check-qlist $tools"
+      tools="check-qint check-qstring check-qdict check-qlist check-qbuffer $tools"
       tools="check-qfloat check-qjson $tools"
     fi
   fi
diff --git a/qbuffer.c b/qbuffer.c
new file mode 100644
index 0000000..4252a1e
--- /dev/null
+++ b/qbuffer.c
@@ -0,0 +1,117 @@
+/*
+ * QBuffer Module
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qbuffer.h"
+#include "qobject.h"
+#include "qemu-common.h"
+#include "base64.h"
+
+static void qbuffer_destroy_obj(QObject *obj);
+
+static const QType qbuffer_type = {
+    .code = QTYPE_QBUFFER,
+    .destroy = qbuffer_destroy_obj,
+};
+
+/**
+ * qbuffer_from_data(): Create a new QBuffer from an existing data blob
+ *
+ * Returns strong reference.
+ */
+QBuffer *qbuffer_from_data(const void *data, size_t size)
+{
+    QBuffer *qb;
+
+    qb = g_malloc(sizeof(*qb));
+    qb->data = g_malloc(size);
+    memcpy(qb->data, data, size);
+    qb->size = size;
+    QOBJECT_INIT(qb, &qbuffer_type);
+
+    return qb;
+}
+
+/**
+ * qbuffer_from_qstring(): Create a new QBuffer from a QString object that
+ * contains the data as a stream of hex-encoded bytes
+ *
+ * Returns strong reference.
+ */
+QBuffer *qbuffer_from_qstring(const QString *string)
+{
+    const char *str = qstring_get_str(string);
+    size_t str_len;
+    QBuffer *qb;
+
+    qb = g_malloc(sizeof(*qb));
+
+    str_len = strlen(str);
+    while (str_len > 0 && str[str_len - 1] == '=') {
+        str_len--;
+    }
+    qb->size = (str_len / 4) * 3 + ((str_len % 4) * 3) / 4;
+    qb->data = g_malloc(qb->size);
+
+    QOBJECT_INIT(qb, &qbuffer_type);
+
+    if (base64_decode(str, str_len, qb->data) < 0) {
+        qbuffer_destroy_obj(QOBJECT(qb));
+        return NULL;
+    }
+
+    return qb;
+}
+
+/**
+ * qbuffer_get_data(): Return pointer to stored data
+ *
+ * NOTE: Should be used with caution, if the object is deallocated
+ * this pointer becomes invalid.
+ */
+const void *qbuffer_get_data(const QBuffer *qb)
+{
+    return qb->data;
+}
+
+/**
+ * qbuffer_get_size(): Return size of stored data
+ */
+size_t qbuffer_get_size(const QBuffer *qb)
+{
+    return qb->size;
+}
+
+/**
+ * qobject_to_qbool(): Convert a QObject into a QBuffer
+ */
+QBuffer *qobject_to_qbuffer(const QObject *obj)
+{
+    if (qobject_type(obj) != QTYPE_QBUFFER) {
+        return NULL;
+    }
+
+    return container_of(obj, QBuffer, base);
+}
+
+/**
+ * qbuffer_destroy_obj(): Free all memory allocated by a QBuffer object
+ */
+static void qbuffer_destroy_obj(QObject *obj)
+{
+    QBuffer *qb;
+
+    assert(obj != NULL);
+    qb = qobject_to_qbuffer(obj);
+    g_free(qb->data);
+    g_free(qb);
+}
diff --git a/qbuffer.h b/qbuffer.h
new file mode 100644
index 0000000..2e01078
--- /dev/null
+++ b/qbuffer.h
@@ -0,0 +1,33 @@
+/*
+ * QBuffer Module
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QBUFFER_H
+#define QBUFFER_H
+
+#include <stdint.h>
+#include "qobject.h"
+#include "qstring.h"
+
+typedef struct QBuffer {
+    QObject_HEAD;
+    void *data;
+    size_t size;
+} QBuffer;
+
+QBuffer *qbuffer_from_data(const void *data, size_t size);
+QBuffer *qbuffer_from_qstring(const QString *string);
+const void *qbuffer_get_data(const QBuffer *qb);
+size_t qbuffer_get_size(const QBuffer *qb);
+QBuffer *qobject_to_qbuffer(const QObject *obj);
+
+#endif /* QBUFFER_H */
diff --git a/qjson.c b/qjson.c
index f9c8e77..d415f83 100644
--- a/qjson.c
+++ b/qjson.c
@@ -19,7 +19,9 @@
 #include "qlist.h"
 #include "qbool.h"
 #include "qfloat.h"
+#include "qbuffer.h"
 #include "qdict.h"
+#include "base64.h"
 
 typedef struct JSONParsingState
 {
@@ -268,6 +270,19 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent)
         }
         break;
     }
+    case QTYPE_QBUFFER: {
+        QBuffer *val = qobject_to_qbuffer(obj);
+        size_t data_size = qbuffer_get_size(val);
+        size_t str_len = ((data_size + 2) / 3) * 4;
+        char *buffer = g_malloc(str_len + 1);
+
+        base64_encode(qbuffer_get_data(val), data_size, buffer);
+        qstring_append(str, "{\"__class__\": \"buffer\", \"data\": \"");
+        qstring_append(str, buffer);
+        qstring_append(str, "\"}");
+        g_free(buffer);
+        break;
+    }
     case QTYPE_QERROR:
         /* XXX: should QError be emitted? */
     case QTYPE_NONE:
diff --git a/qobject.h b/qobject.h
index d42386d..4ec932b 100644
--- a/qobject.h
+++ b/qobject.h
@@ -44,6 +44,7 @@ typedef enum {
     QTYPE_QFLOAT,
     QTYPE_QBOOL,
     QTYPE_QERROR,
+    QTYPE_QBUFFER,
 } qtype_code;
 
 struct QObject;

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Anthony Liguori]

On 08/26/2011 09:48 AM, Jan Kiszka wrote:
> More than one year ago I posted some patches to add a monitor command
> callend device_show. The purpose of that command is to dump the state of
> some qdev device based on its vmstate.

I have a series that does the same thing as this in a totally different 
way.  Instead of a VMState interpreter, it converts all marshalling to 
go through a Visitor interface.  This makes it extensible to devices 
that don't currently support VMState.

Regards,

Anthony Liguori

>
> To improve the usability of that interface, the previous series also
> tried to create a canonical qdev tree path name format and even added
> monitor command expansion. That format created quite a few discussions,
> and we never came to some conclusion.
>
> As device_show is still a useful tool for debugging but qdev structuring
> and addressing may significantly change in the near future, I decided to
> reanimate those patches while avoiding almost any change of the
> addressing scheme. The only one I propose is automatic ID assignment for
> anonymous devices so that any qdev device is in principle addressable
> (e.g. APIC and IO-APIC on x86).
>
> As back then, device_show remains HMP-only to avoid any stable ABI
> issues around QMP.
>
> CC: Luiz Capitulino<lcapitulino@redhat.com>
> CC: Markus Armbruster<armbru@redhat.com>
>
> Jan Kiszka (6):
>    monitor: return length of printed string via monitor_[v]printf
>    Add base64 encoder/decoder
>    QMP: Reserve namespace for complex object classes
>    QMP: Add QBuffer
>    monitor: Add basic device state visualization
>    qdev: Generate IDs for anonymous devices
>
>   Makefile             |    2 +-
>   Makefile.objs        |    4 +-
>   QMP/qmp-spec.txt     |   24 ++++-
>   base64.c             |  202 +++++++++++++++++++++++++++++++++++++
>   base64.h             |   19 ++++
>   check-qbuffer.c      |  172 ++++++++++++++++++++++++++++++++
>   configure            |    2 +-
>   hmp-commands.hx      |   16 +++
>   hw/hw.h              |    2 +
>   hw/qdev-properties.c |    2 +-
>   hw/qdev.c            |  271 ++++++++++++++++++++++++++++++++++++++++++++++++--
>   hw/qdev.h            |    3 +
>   monitor.c            |   23 +++--
>   monitor.h            |    4 +-
>   qbuffer.c            |  117 ++++++++++++++++++++++
>   qbuffer.h            |   33 ++++++
>   qemu-tool.c          |    6 +-
>   qerror.c             |    4 +
>   qerror.h             |    3 +
>   qjson.c              |   15 +++
>   qobject.h            |    1 +
>   21 files changed, 894 insertions(+), 31 deletions(-)
>   create mode 100644 base64.c
>   create mode 100644 base64.h
>   create mode 100644 check-qbuffer.c
>   create mode 100644 qbuffer.c
>   create mode 100644 qbuffer.h
>

Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices

[Anthony Liguori]

On 08/26/2011 09:48 AM, Jan Kiszka wrote:
> In order to address devices for that the user forgot or is even unable
> (no_user) to provide an ID, assign an automatically generated one. Such
> IDs have the format #<number>, thus are outside the name space availing
> to users. Don't use them for bus naming to avoid any other user-visible
> change.

I don't think this is a very nice approach.  Why not eliminate anonymous 
devices entirely and use a parent derived name for devices that are not 
created by the user?

Regards,

Anthony Liguori

>
> CC: Markus Armbruster<armbru@redhat.com>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> ---
>   hw/qdev-properties.c |    2 +-
>   hw/qdev.c            |   25 +++++++++++++------------
>   hw/qdev.h            |    1 +
>   3 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 0c0c292..4e39cef 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -682,7 +682,7 @@ int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *va
>       if (res<  0) {
>           error_report("Can't attach drive %s to %s.%s: %s",
>                        bdrv_get_device_name(value),
> -                     dev->id ? dev->id : dev->info->name,
> +                     dev->id != dev->auto_id ? dev->id : dev->info->name,
>                        name, strerror(-res));
>           return -1;
>       }
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 3b0e1af..807902b 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -85,6 +85,7 @@ static DeviceInfo *qdev_find_info(BusInfo *bus_info, const char *name)
>
>   static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
>   {
> +    static unsigned int auto_id;
>       DeviceState *dev;
>
>       assert(bus->info == info->bus_info);
> @@ -94,6 +95,8 @@ static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
>       qdev_prop_set_defaults(dev, dev->info->props);
>       qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
>       qdev_prop_set_globals(dev);
> +    snprintf(dev->auto_id, sizeof(dev->auto_id), "#%u", ++auto_id);
> +    dev->id = dev->auto_id;
>       QLIST_INSERT_HEAD(&bus->children, dev, sibling);
>       if (qdev_hotplug) {
>           assert(bus->allow_hotplug);
> @@ -574,8 +577,9 @@ DeviceState *qdev_find_recursive(BusState *bus, const char *id)
>       BusState *child;
>
>       QLIST_FOREACH(dev,&bus->children, sibling) {
> -        if (dev->id&&  strcmp(dev->id, id) == 0)
> +        if (strcmp(dev->id, id) == 0) {
>               return dev;
> +        }
>           QLIST_FOREACH(child,&dev->child_bus, sibling) {
>               ret = qdev_find_recursive(child, id);
>               if (ret) {
> @@ -591,8 +595,8 @@ static void qbus_list_bus(DeviceState *dev)
>       BusState *child;
>       const char *sep = " ";
>
> -    error_printf("child busses at \"%s\":",
> -                 dev->id ? dev->id : dev->info->name);
> +    error_printf("child busses at \"%s\"/\"%s\":",
> +                 dev->info->name, dev->id);
>       QLIST_FOREACH(child,&dev->child_bus, sibling) {
>           error_printf("%s\"%s\"", sep, child->name);
>           sep = ", ";
> @@ -607,9 +611,7 @@ static void qbus_list_dev(BusState *bus)
>
>       error_printf("devices at \"%s\":", bus->name);
>       QLIST_FOREACH(dev,&bus->children, sibling) {
> -        error_printf("%s\"%s\"", sep, dev->info->name);
> -        if (dev->id)
> -            error_printf("/\"%s\"", dev->id);
> +        error_printf("%s\"%s\"/\"%s\"", sep, dev->info->name, dev->id);
>           sep = ", ";
>       }
>       error_printf("\n");
> @@ -638,7 +640,7 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)
>        *   (3) driver alias, if present
>        */
>       QLIST_FOREACH(dev,&bus->children, sibling) {
> -        if (dev->id&&   strcmp(dev->id, elem) == 0) {
> +        if (strcmp(dev->id, elem) == 0) {
>               return dev;
>           }
>       }
> @@ -754,8 +756,8 @@ void qbus_create_inplace(BusState *bus, BusInfo *info,
>       if (name) {
>           /* use supplied name */
>           bus->name = g_strdup(name);
> -    } else if (parent&&  parent->id) {
> -        /* parent device has id ->  use it for bus name */
> +    } else if (parent&&  parent->id != parent->auto_id) {
> +        /* parent device has user-assigned id ->  use it for bus name */
>           len = strlen(parent->id) + 16;
>           buf = g_malloc(len);
>           snprintf(buf, len, "%s.%d", parent->id, parent->num_child_bus);
> @@ -850,8 +852,7 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
>   static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
>   {
>       BusState *child;
> -    qdev_printf("dev: %s, id \"%s\"\n", dev->info->name,
> -                dev->id ? dev->id : "");
> +    qdev_printf("dev: %s, id \"%s\"\n", dev->info->name, dev->id);
>       indent += 2;
>       if (dev->num_gpio_in) {
>           qdev_printf("gpio-in %d\n", dev->num_gpio_in);
> @@ -1196,7 +1197,7 @@ int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data)
>       name = g_malloc(name_len);
>       snprintf(name, name_len, "%s", dev->info->name);
>       *ret_data = qobject_from_jsonf("{ 'device': %s, 'id': %s, 'version': %d }",
> -                                   name, dev->id ? : "", vmsd->version_id);
> +                                   name, dev->id, vmsd->version_id);
>       g_free(name);
>       qlist = qlist_new();
>       parse_vmstate(vmsd, dev, qlist, qdict_get_try_bool(qdict, "full", false));
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 912fc45..d028409 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -31,6 +31,7 @@ enum {
>      so that it can be embedded in individual device state structures.  */
>   struct DeviceState {
>       const char *id;
> +    char auto_id[16];
>       enum DevState state;
>       QemuOpts *opts;
>       int hotplugged;

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Jan Kiszka]

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

On 2011-08-29 21:22, Anthony Liguori wrote:
> On 08/26/2011 09:48 AM, Jan Kiszka wrote:
>> More than one year ago I posted some patches to add a monitor command
>> callend device_show. The purpose of that command is to dump the state of
>> some qdev device based on its vmstate.
> 
> I have a series that does the same thing as this in a totally different
> way.  Instead of a VMState interpreter, it converts all marshalling to
> go through a Visitor interface.  This makes it extensible to devices
> that don't currently support VMState.

For obtaining a raw picture of some device state, there is still gdb.
What this feature is targeting at is a refined view, something VMState
already represents as it (in the absence of bugs) contains the complete
persistent state, and only that.

Once we have QOM in place and also sorted out how VMState 2.0 may look
like and how it can be extracted from or expressed within the device
model, we will probably change the internals of this feature again.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices

[Jan Kiszka]

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

On 2011-08-29 21:23, Anthony Liguori wrote:
> On 08/26/2011 09:48 AM, Jan Kiszka wrote:
>> In order to address devices for that the user forgot or is even unable
>> (no_user) to provide an ID, assign an automatically generated one. Such
>> IDs have the format #<number>, thus are outside the name space availing
>> to users. Don't use them for bus naming to avoid any other user-visible
>> change.
> 
> I don't think this is a very nice approach.  Why not eliminate anonymous
> devices entirely and use a parent derived name for devices that are not
> created by the user?

This eliminates anonymous devices completely. So I guess you are asking
for a different naming scheme, something like <parent-id>.child#<no>
e.g.? Well, we would end up with fairly long names when a complete
hierarchy is anonymous. What would be the benefit?

I'm really just looking for some simple, temporary workaround without
touching the existing fragile naming scheme. What we really need is full
path addressing, but that without preserving all the legacy.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices

[Anthony Liguori]

On 08/29/2011 03:56 PM, Jan Kiszka wrote:
> On 2011-08-29 21:23, Anthony Liguori wrote:
>> On 08/26/2011 09:48 AM, Jan Kiszka wrote:
>>> In order to address devices for that the user forgot or is even unable
>>> (no_user) to provide an ID, assign an automatically generated one. Such
>>> IDs have the format #<number>, thus are outside the name space availing
>>> to users. Don't use them for bus naming to avoid any other user-visible
>>> change.
>>
>> I don't think this is a very nice approach.  Why not eliminate anonymous
>> devices entirely and use a parent derived name for devices that are not
>> created by the user?
>
> This eliminates anonymous devices completely. So I guess you are asking
> for a different naming scheme, something like<parent-id>.child#<no>
> e.g.? Well, we would end up with fairly long names when a complete
> hierarchy is anonymous. What would be the benefit?

No, I'm saying that whenever a device is created, it should be given a 
non-random name.  IOW, the names of these devices should be stable.

> I'm really just looking for some simple, temporary workaround without
> touching the existing fragile naming scheme. What we really need is full
> path addressing, but that without preserving all the legacy.

Yeah, I understand, and I hesitated making any grander suggestions here, 
but I'm not sure how much work it would be to just remove any caller 
that passes NULL for ID and replace it with something more meaningful. 
I think that's a helpful clean up long term no matter what.

Regards,

Anthony Liguori

> Jan
>
>

Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices

[Jan Kiszka]

On 2011-08-29 23:19, Anthony Liguori wrote:
> On 08/29/2011 03:56 PM, Jan Kiszka wrote:
>> On 2011-08-29 21:23, Anthony Liguori wrote:
>>> On 08/26/2011 09:48 AM, Jan Kiszka wrote:
>>>> In order to address devices for that the user forgot or is even unable
>>>> (no_user) to provide an ID, assign an automatically generated one. Such
>>>> IDs have the format #<number>, thus are outside the name space availing
>>>> to users. Don't use them for bus naming to avoid any other user-visible
>>>> change.
>>>
>>> I don't think this is a very nice approach.  Why not eliminate anonymous
>>> devices entirely and use a parent derived name for devices that are not
>>> created by the user?
>>
>> This eliminates anonymous devices completely. So I guess you are asking
>> for a different naming scheme, something like<parent-id>.child#<no>
>> e.g.? Well, we would end up with fairly long names when a complete
>> hierarchy is anonymous. What would be the benefit?
> 
> No, I'm saying that whenever a device is created, it should be given a
> non-random name.  IOW, the names of these devices should be stable.
> 
>> I'm really just looking for some simple, temporary workaround without
>> touching the existing fragile naming scheme. What we really need is full
>> path addressing, but that without preserving all the legacy.
> 
> Yeah, I understand, and I hesitated making any grander suggestions here,
> but I'm not sure how much work it would be to just remove any caller
> that passes NULL for ID and replace it with something more meaningful. I
> think that's a helpful clean up long term no matter what.

That won't solve the problem of finding a unique device name. If we want
to derive it from stable device properties (bus addresses etc.), we
first of all have to define them for all types of devices. And that's
basically were the discussion exploded last year IIRC.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder

[Luiz Capitulino]

On Fri, 26 Aug 2011 20:02:37 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> On 2011-08-26 17:47, Jan Kiszka wrote:
> > On 2011-08-26 17:23, Jan Kiszka wrote:
> >> On 2011-08-26 17:21, Peter Maydell wrote:
> >>> On 26 August 2011 15:48, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>>> Will be used by QBuffer.
> >>>
> >>> Is it possible to use the glib base64 encode/decode routines instead
> >>> of rolling our own here?
> >>
> >> Yeah, times are changing. Need to check what's there and how to use it.
> > 
> > Requires glib >= 2.12, we are currently at >= 2.0, right? Would it be OK
> > to raise the entry barrier?
> > 
> > I'm also still looking for error handling of g_base64_decode. I guess
> > one is supposed to compare returned length against some expected value.
> > Well, it's glib...
> 
> The length check is not sufficient, and glib's decoder fails on my
> invalid input string tests.
> 
> I think proper error detection in base64 inputs is a worthwhile feature
> that glib lacks. So I'll stick with roll-our-own (actually, it's
> Mozilla's version).

The guest-agent is using glib's to implement file read & write, I think
it should switch to Jan's implementation?

Re: [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes

[Luiz Capitulino]

On Fri, 26 Aug 2011 16:48:13 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> This reserves JSON objects that contain the key '__class__' for QMP-specific
> complex objects. First user will be the buffer class.
> 
> CC: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  QMP/qmp-spec.txt |   16 +++++++++++++---
>  1 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
> index 9d30a8c..fa1dd62 100644
> --- a/QMP/qmp-spec.txt
> +++ b/QMP/qmp-spec.txt
> @@ -146,6 +146,15 @@ The format is:
>  For a listing of supported asynchronous events, please, refer to the
>  qmp-events.txt file.
>  
> +2.6 Complex object classes
> +--------------------------
> +
> +JSON objects that contain the key-value pair '"__class__": json-string' are
> +reserved for QMP-specific complex object classes that. QMP specifies which

Can I just drop the period or is it misplaced?

> +further keys each of these objects include and how they are encoded.
> +
> +So far, no complex object class is specified.
> +
>  3. QMP Examples
>  ===============
>  
> @@ -229,9 +238,10 @@ avoid modifying QMP.  Both upstream and downstream need to take care to
>  preserve long-term compatibility and interoperability.
>  
>  To help with that, QMP reserves JSON object member names beginning with
> -'__' (double underscore) for downstream use ("downstream names").  This
> -means upstream will never use any downstream names for its commands,
> -arguments, errors, asynchronous events, and so forth.
> +'__' (double underscore) for downstream use ("downstream names").  Downstream
> +names MUST NOT end with '__' as this pattern is reserved for QMP-defined JSON
> +object classes.  Upstream will never use any downstream names for its
> +commands, arguments, errors, asynchronous events, and so forth.
>  
>  Any new names downstream wishes to add must begin with '__'.  To
>  ensure compatibility with other downstreams, it is strongly

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Luiz Capitulino]

On Fri, 26 Aug 2011 16:48:10 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> More than one year ago I posted some patches to add a monitor command
> callend device_show. The purpose of that command is to dump the state of
> some qdev device based on its vmstate.
> 
> To improve the usability of that interface, the previous series also
> tried to create a canonical qdev tree path name format and even added
> monitor command expansion. That format created quite a few discussions,
> and we never came to some conclusion.
> 
> As device_show is still a useful tool for debugging but qdev structuring
> and addressing may significantly change in the near future, I decided to
> reanimate those patches while avoiding almost any change of the
> addressing scheme. The only one I propose is automatic ID assignment for
> anonymous devices so that any qdev device is in principle addressable
> (e.g. APIC and IO-APIC on x86).
> 
> As back then, device_show remains HMP-only to avoid any stable ABI
> issues around QMP.
> 
> CC: Luiz Capitulino <lcapitulino@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>

This looks good to me and I'm willing to apply it. Only issue I'd like to see
resolved is the base64 encoder/decoder. This series introduces a new one,
while the guest agent uses glib's.

I think we should choose one of them and stick to it.

Any other objections apart from that? Markus?

Re: [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes

[Jan Kiszka]

On 2011-09-02 19:23, Luiz Capitulino wrote:
> On Fri, 26 Aug 2011 16:48:13 +0200
> Jan Kiszka <jan.kiszka@siemens.com> wrote:
> 
>> This reserves JSON objects that contain the key '__class__' for QMP-specific
>> complex objects. First user will be the buffer class.
>>
>> CC: Luiz Capitulino <lcapitulino@redhat.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  QMP/qmp-spec.txt |   16 +++++++++++++---
>>  1 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
>> index 9d30a8c..fa1dd62 100644
>> --- a/QMP/qmp-spec.txt
>> +++ b/QMP/qmp-spec.txt
>> @@ -146,6 +146,15 @@ The format is:
>>  For a listing of supported asynchronous events, please, refer to the
>>  qmp-events.txt file.
>>  
>> +2.6 Complex object classes
>> +--------------------------
>> +
>> +JSON objects that contain the key-value pair '"__class__": json-string' are
>> +reserved for QMP-specific complex object classes that. QMP specifies which
> 
> Can I just drop the period or is it misplaced?

Actually, 'that' is misplaced.

> 
>> +further keys each of these objects include and how they are encoded.
>> +
>> +So far, no complex object class is specified.
>> +

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes

[Anthony Liguori]

On 09/02/2011 12:23 PM, Luiz Capitulino wrote:
> On Fri, 26 Aug 2011 16:48:13 +0200
> Jan Kiszka<jan.kiszka@siemens.com>  wrote:
>
>> This reserves JSON objects that contain the key '__class__' for QMP-specific
>> complex objects. First user will be the buffer class.

BTW, we need to teach QAPI how handle these types.

QAPI already has the information about the class that it's working with. 
  I think that means you can probably do something like this (completely 
untested):

diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 4419a31..9895792 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -105,6 +105,8 @@ static void qmp_output_start_struct(Visitor *v, void 
**obj,

      qmp_output_add(qov, name, dict);
      qmp_output_push(qov, dict);
+
+    visit_type_str(v, "__class__", kind, errp);
  }

  static void qmp_output_end_struct(Visitor *v, Error **errp)

That will add class information for every type that gets written by 
QAPI.  Likewise:

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index fcf8bf9..77e7ab4 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -86,6 +86,7 @@ static void qmp_input_start_struct(Visitor *v, void 
**obj, con
  {
      QmpInputVisitor *qiv = to_qiv(v);
      const QObject *qobj = qmp_input_get_object(qiv, name);
+    char *type;

      if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
          error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -101,6 +102,15 @@ static void qmp_input_start_struct(Visitor *v, void 
**obj,
      if (obj) {
          *obj = g_malloc0(size);
      }
+
+    visit_type_str(v, &type, "__class__", errp);
+    if (error_is_set(errp)) {
+        return;
+    }

Will do type checking on incoming data.

Maybe we should relax that and only do the type checking if __class__ is 
specified..

Regards,

Anthony Liguori

>>
>> CC: Luiz Capitulino<lcapitulino@redhat.com>
>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
>> ---
>>   QMP/qmp-spec.txt |   16 +++++++++++++---
>>   1 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
>> index 9d30a8c..fa1dd62 100644
>> --- a/QMP/qmp-spec.txt
>> +++ b/QMP/qmp-spec.txt
>> @@ -146,6 +146,15 @@ The format is:
>>   For a listing of supported asynchronous events, please, refer to the
>>   qmp-events.txt file.
>>
>> +2.6 Complex object classes
>> +--------------------------
>> +
>> +JSON objects that contain the key-value pair '"__class__": json-string' are
>> +reserved for QMP-specific complex object classes that. QMP specifies which
>
> Can I just drop the period or is it misplaced?
>
>> +further keys each of these objects include and how they are encoded.
>> +
>> +So far, no complex object class is specified.
>> +
>>   3. QMP Examples
>>   ===============
>>
>> @@ -229,9 +238,10 @@ avoid modifying QMP.  Both upstream and downstream need to take care to
>>   preserve long-term compatibility and interoperability.
>>
>>   To help with that, QMP reserves JSON object member names beginning with
>> -'__' (double underscore) for downstream use ("downstream names").  This
>> -means upstream will never use any downstream names for its commands,
>> -arguments, errors, asynchronous events, and so forth.
>> +'__' (double underscore) for downstream use ("downstream names").  Downstream
>> +names MUST NOT end with '__' as this pattern is reserved for QMP-defined JSON
>> +object classes.  Upstream will never use any downstream names for its
>> +commands, arguments, errors, asynchronous events, and so forth.
>>
>>   Any new names downstream wishes to add must begin with '__'.  To
>>   ensure compatibility with other downstreams, it is strongly
>

[Qemu-devel] required glib version? Re: [PATCH 2/6] Add base64 encoder/decoder

[Gerd Hoffmann]

On 08/26/11 17:47, Jan Kiszka wrote:
> On 2011-08-26 17:23, Jan Kiszka wrote:

>> [ using glib base64 decoder ]
>
> Requires glib>= 2.12, we are currently at>= 2.0, right? Would it be OK
> to raise the entry barrier?

In master it currently is >= 2.20 due to v9fs_init_worker_threads using 
g_thread_get_initialized which was added in 2.20 according to the docs. 
  Which makes the build fail on rhel-5 (shipping with glib 2.12).

Guess we'll need to clearly define which minimum glib version we want 
require.  And whenever we want apply this to the whole code base or 
allow different minimum requirements for different components.

Requiring glib 2.20 for all components isn't going to fly as we 
certainly want be able to run the qemu guest agent almost everythere. 
Requiring something newer than 2.0 for the qemu emulator might be 
reasonable of there are good reasons (aka useful glib features) though.

Comments?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Michael S. Tsirkin]

On Fri, Aug 26, 2011 at 04:48:10PM +0200, Jan Kiszka wrote:
> More than one year ago I posted some patches to add a monitor command
> callend device_show. The purpose of that command is to dump the state of
> some qdev device based on its vmstate.
> 
> To improve the usability of that interface, the previous series also
> tried to create a canonical qdev tree path name format and even added
> monitor command expansion. That format created quite a few discussions,
> and we never came to some conclusion.
> 
> As device_show is still a useful tool for debugging but qdev structuring
> and addressing may significantly change in the near future, I decided to
> reanimate those patches while avoiding almost any change of the
> addressing scheme. The only one I propose is automatic ID assignment for
> anonymous devices so that any qdev device is in principle addressable
> (e.g. APIC and IO-APIC on x86).
> 
> As back then, device_show remains HMP-only to avoid any stable ABI
> issues around QMP.

I'm afraid that won't be enough to stop people
scripting this command - libvirt accessed
HMP for years.

On the other hand, no QMP command means e.g.
libvirt users don't get any benefit from this.

What I think will solve these problems, for both HMP and QMP,
is an explicit 'debug_unstable' or 'debug_unsupported' command that will
expose all kind of debugging functionality making it
very explicit that it's an unsupported debugging utility.

Proposed syntax:

debug_unstable <subcommand> <options>

Example:

debug_unstable device_show -all


-- 
MSt

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Jan Kiszka]

On 2011-09-06 16:48, Michael S. Tsirkin wrote:
> On Fri, Aug 26, 2011 at 04:48:10PM +0200, Jan Kiszka wrote:
>> More than one year ago I posted some patches to add a monitor command
>> callend device_show. The purpose of that command is to dump the state of
>> some qdev device based on its vmstate.
>>
>> To improve the usability of that interface, the previous series also
>> tried to create a canonical qdev tree path name format and even added
>> monitor command expansion. That format created quite a few discussions,
>> and we never came to some conclusion.
>>
>> As device_show is still a useful tool for debugging but qdev structuring
>> and addressing may significantly change in the near future, I decided to
>> reanimate those patches while avoiding almost any change of the
>> addressing scheme. The only one I propose is automatic ID assignment for
>> anonymous devices so that any qdev device is in principle addressable
>> (e.g. APIC and IO-APIC on x86).
>>
>> As back then, device_show remains HMP-only to avoid any stable ABI
>> issues around QMP.
> 
> I'm afraid that won't be enough to stop people
> scripting this command - libvirt accessed
> HMP for years.
> 
> On the other hand, no QMP command means e.g.
> libvirt users don't get any benefit from this.
> 
> What I think will solve these problems, for both HMP and QMP,
> is an explicit 'debug_unstable' or 'debug_unsupported' command that will
> expose all kind of debugging functionality making it
> very explicit that it's an unsupported debugging utility.
> 
> Proposed syntax:
> 
> debug_unstable <subcommand> <options>
> 
> Example:
> 
> debug_unstable device_show -all

For HMP, this would needlessly complicate the user interface, nothing I
would support. People scripting things on top of HMP are generally doing
this on their own risk and cannot expect output stability.

device_show is like info qtree: the output will naturally change as the
emulated hardware evolves, information is added/removed, or we simply
improve the layout. Recent changes on info network are an example for
the latter.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Anthony Liguori]

On 09/06/2011 10:45 AM, Jan Kiszka wrote:
> On 2011-09-06 16:48, Michael S. Tsirkin wrote:
>> I'm afraid that won't be enough to stop people
>> scripting this command - libvirt accessed
>> HMP for years.
>>
>> On the other hand, no QMP command means e.g.
>> libvirt users don't get any benefit from this.
>>
>> What I think will solve these problems, for both HMP and QMP,
>> is an explicit 'debug_unstable' or 'debug_unsupported' command that will
>> expose all kind of debugging functionality making it
>> very explicit that it's an unsupported debugging utility.
>>
>> Proposed syntax:
>>
>> debug_unstable<subcommand>  <options>
>>
>> Example:
>>
>> debug_unstable device_show -all
>
> For HMP, this would needlessly complicate the user interface, nothing I
> would support. People scripting things on top of HMP are generally doing
> this on their own risk and cannot expect output stability.
>
> device_show is like info qtree: the output will naturally change as the
> emulated hardware evolves, information is added/removed, or we simply
> improve the layout. Recent changes on info network are an example for
> the latter.

Yeah, I'm not worried about stability.  HMP commands that aren't exposed 
as QMP commands are inherently unstable and should not be scripted to.

I'm still contemplating how we go about doing this.  This series 
introduces a couple new concepts like QMP class hinting anonymous IDs. 
I'm concerned that we'll further complicate the need to support 
backwards compatibility.

Would the command be useful if you couldn't address devices?  If it just 
dumped the full machine state all at once?  That would at least obviate 
the need to add anonymous IDs.

Regards,

Anthony Liguori

>
> Jan
>

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Jan Kiszka]

On 2011-09-06 17:51, Anthony Liguori wrote:
> I'm still contemplating how we go about doing this.  This series 
> introduces a couple new concepts like QMP class hinting anonymous IDs. 
> I'm concerned that we'll further complicate the need to support 
> backwards compatibility.

Anonymous IDs must not be considered stable. If you prefer, we could try
to filter them out for QMP use. Need to check though if there is a good
location to catch them, but the rule is trivial.

I will also happily drop anonymous IDs again once we have converted all
devices to stable QOM full path addressing. But that will simply take
too much time to wait for it I'm afraid.

> 
> Would the command be useful if you couldn't address devices?  If it just 
> dumped the full machine state all at once?  That would at least obviate 
> the need to add anonymous IDs.

Theoretically usable, but extremely unhandy. I would not like so see
such an interface exposed to users.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Anthony Liguori]

On 09/06/2011 11:05 AM, Jan Kiszka wrote:
> On 2011-09-06 17:51, Anthony Liguori wrote:
>> I'm still contemplating how we go about doing this.  This series
>> introduces a couple new concepts like QMP class hinting anonymous IDs.
>> I'm concerned that we'll further complicate the need to support
>> backwards compatibility.
>
> Anonymous IDs must not be considered stable. If you prefer, we could try
> to filter them out for QMP use. Need to check though if there is a good
> location to catch them, but the rule is trivial.
>
> I will also happily drop anonymous IDs again once we have converted all
> devices to stable QOM full path addressing. But that will simply take
> too much time to wait for it I'm afraid.
>
>>
>> Would the command be useful if you couldn't address devices?  If it just
>> dumped the full machine state all at once?  That would at least obviate
>> the need to add anonymous IDs.
>
> Theoretically usable, but extremely unhandy. I would not like so see
> such an interface exposed to users.

What about just taking the device type (iow the savevm section name)?

In most scenarios, it's probably easier for a user and will still only 
have a single match.

Regards,

Anthony Liguori

>
> Jan
>

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Michael S. Tsirkin]

On Tue, Sep 06, 2011 at 10:51:26AM -0500, Anthony Liguori wrote:
> On 09/06/2011 10:45 AM, Jan Kiszka wrote:
> >On 2011-09-06 16:48, Michael S. Tsirkin wrote:
> >>I'm afraid that won't be enough to stop people
> >>scripting this command - libvirt accessed
> >>HMP for years.
> >>
> >>On the other hand, no QMP command means e.g.
> >>libvirt users don't get any benefit from this.
> >>
> >>What I think will solve these problems, for both HMP and QMP,
> >>is an explicit 'debug_unstable' or 'debug_unsupported' command that will
> >>expose all kind of debugging functionality making it
> >>very explicit that it's an unsupported debugging utility.
> >>
> >>Proposed syntax:
> >>
> >>debug_unstable<subcommand>  <options>
> >>
> >>Example:
> >>
> >>debug_unstable device_show -all
> >
> >For HMP, this would needlessly complicate the user interface, nothing I
> >would support. People scripting things on top of HMP are generally doing
> >this on their own risk and cannot expect output stability.
> >
> >device_show is like info qtree: the output will naturally change as the
> >emulated hardware evolves, information is added/removed, or we simply
> >improve the layout. Recent changes on info network are an example for
> >the latter.
> 
> Yeah, I'm not worried about stability.  HMP commands that aren't
> exposed as QMP commands are inherently unstable and should not be
> scripted to.

They are also not accessible when using libvirt, right?
Which means almost all cases I care about: debugging on my laptop
I can easily attach with gdb and inspect state.

> I'm still contemplating how we go about doing this.  This series
> introduces a couple new concepts like QMP class hinting anonymous
> IDs. I'm concerned that we'll further complicate the need to support
> backwards compatibility.

Dazed and confused. Above you stated these commands are
inherently unstable and no need to support?

> Would the command be useful if you couldn't address devices?  If it
> just dumped the full machine state all at once?  That would at least
> obviate the need to add anonymous IDs.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >Jan
> >

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Anthony Liguori]

On 09/06/2011 11:09 AM, Michael S. Tsirkin wrote:
> On Tue, Sep 06, 2011 at 10:51:26AM -0500, Anthony Liguori wrote:
>> On 09/06/2011 10:45 AM, Jan Kiszka wrote:
>>> On 2011-09-06 16:48, Michael S. Tsirkin wrote:
>>>> I'm afraid that won't be enough to stop people
>>>> scripting this command - libvirt accessed
>>>> HMP for years.
>>>>
>>>> On the other hand, no QMP command means e.g.
>>>> libvirt users don't get any benefit from this.
>>>>
>>>> What I think will solve these problems, for both HMP and QMP,
>>>> is an explicit 'debug_unstable' or 'debug_unsupported' command that will
>>>> expose all kind of debugging functionality making it
>>>> very explicit that it's an unsupported debugging utility.
>>>>
>>>> Proposed syntax:
>>>>
>>>> debug_unstable<subcommand>   <options>
>>>>
>>>> Example:
>>>>
>>>> debug_unstable device_show -all
>>>
>>> For HMP, this would needlessly complicate the user interface, nothing I
>>> would support. People scripting things on top of HMP are generally doing
>>> this on their own risk and cannot expect output stability.
>>>
>>> device_show is like info qtree: the output will naturally change as the
>>> emulated hardware evolves, information is added/removed, or we simply
>>> improve the layout. Recent changes on info network are an example for
>>> the latter.
>>
>> Yeah, I'm not worried about stability.  HMP commands that aren't
>> exposed as QMP commands are inherently unstable and should not be
>> scripted to.
>
> They are also not accessible when using libvirt, right?

$ virsh human-monitor-passthrough GuestName device_show foo

Should work.

> Which means almost all cases I care about: debugging on my laptop
> I can easily attach with gdb and inspect state.
>
>> I'm still contemplating how we go about doing this.  This series
>> introduces a couple new concepts like QMP class hinting anonymous
>> IDs. I'm concerned that we'll further complicate the need to support
>> backwards compatibility.
>
> Dazed and confused. Above you stated these commands are
> inherently unstable and no need to support?

Right, my concern is that the unstable command results in changes that 
are visible in stable interfaces.  As Jan suggested, we could try to 
hide this from those commands but that seems a bit ugly to me.

Regards,

Anthony Liguori

>> Would the command be useful if you couldn't address devices?  If it
>> just dumped the full machine state all at once?  That would at least
>> obviate the need to add anonymous IDs.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>> Jan
>>>
>

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Jan Kiszka]

On 2011-09-06 18:09, Michael S. Tsirkin wrote:
> On Tue, Sep 06, 2011 at 10:51:26AM -0500, Anthony Liguori wrote:
>> On 09/06/2011 10:45 AM, Jan Kiszka wrote:
>>> On 2011-09-06 16:48, Michael S. Tsirkin wrote:
>>>> I'm afraid that won't be enough to stop people
>>>> scripting this command - libvirt accessed
>>>> HMP for years.
>>>>
>>>> On the other hand, no QMP command means e.g.
>>>> libvirt users don't get any benefit from this.
>>>>
>>>> What I think will solve these problems, for both HMP and QMP,
>>>> is an explicit 'debug_unstable' or 'debug_unsupported' command that will
>>>> expose all kind of debugging functionality making it
>>>> very explicit that it's an unsupported debugging utility.
>>>>
>>>> Proposed syntax:
>>>>
>>>> debug_unstable<subcommand>  <options>
>>>>
>>>> Example:
>>>>
>>>> debug_unstable device_show -all
>>>
>>> For HMP, this would needlessly complicate the user interface, nothing I
>>> would support. People scripting things on top of HMP are generally doing
>>> this on their own risk and cannot expect output stability.
>>>
>>> device_show is like info qtree: the output will naturally change as the
>>> emulated hardware evolves, information is added/removed, or we simply
>>> improve the layout. Recent changes on info network are an example for
>>> the latter.
>>
>> Yeah, I'm not worried about stability.  HMP commands that aren't
>> exposed as QMP commands are inherently unstable and should not be
>> scripted to.
> 
> They are also not accessible when using libvirt, right?
> Which means almost all cases I care about: debugging on my laptop
> I can easily attach with gdb and inspect state.

HMP passthrough or - I bet that's what you rather want - monitor
passthrough from gdb.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Jan Kiszka]

On 2011-09-06 18:08, Anthony Liguori wrote:
> On 09/06/2011 11:05 AM, Jan Kiszka wrote:
>> On 2011-09-06 17:51, Anthony Liguori wrote:
>>> I'm still contemplating how we go about doing this.  This series
>>> introduces a couple new concepts like QMP class hinting anonymous IDs.
>>> I'm concerned that we'll further complicate the need to support
>>> backwards compatibility.
>>
>> Anonymous IDs must not be considered stable. If you prefer, we could try
>> to filter them out for QMP use. Need to check though if there is a good
>> location to catch them, but the rule is trivial.
>>
>> I will also happily drop anonymous IDs again once we have converted all
>> devices to stable QOM full path addressing. But that will simply take
>> too much time to wait for it I'm afraid.
>>
>>>
>>> Would the command be useful if you couldn't address devices?  If it just
>>> dumped the full machine state all at once?  That would at least obviate
>>> the need to add anonymous IDs.
>>
>> Theoretically usable, but extremely unhandy. I would not like so see
>> such an interface exposed to users.
> 
> What about just taking the device type (iow the savevm section name)?
> 
> In most scenarios, it's probably easier for a user and will still only 
> have a single match.

This naming is not yet exposed via any user interface. We would have to
extend info qtree, not sure if we want this...

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Michael S. Tsirkin]

On Tue, Sep 06, 2011 at 11:28:09AM -0500, Anthony Liguori wrote:
> On 09/06/2011 11:09 AM, Michael S. Tsirkin wrote:
> >On Tue, Sep 06, 2011 at 10:51:26AM -0500, Anthony Liguori wrote:
> >>On 09/06/2011 10:45 AM, Jan Kiszka wrote:
> >>>On 2011-09-06 16:48, Michael S. Tsirkin wrote:
> >>>>I'm afraid that won't be enough to stop people
> >>>>scripting this command - libvirt accessed
> >>>>HMP for years.
> >>>>
> >>>>On the other hand, no QMP command means e.g.
> >>>>libvirt users don't get any benefit from this.
> >>>>
> >>>>What I think will solve these problems, for both HMP and QMP,
> >>>>is an explicit 'debug_unstable' or 'debug_unsupported' command that will
> >>>>expose all kind of debugging functionality making it
> >>>>very explicit that it's an unsupported debugging utility.
> >>>>
> >>>>Proposed syntax:
> >>>>
> >>>>debug_unstable<subcommand>   <options>
> >>>>
> >>>>Example:
> >>>>
> >>>>debug_unstable device_show -all
> >>>
> >>>For HMP, this would needlessly complicate the user interface, nothing I
> >>>would support. People scripting things on top of HMP are generally doing
> >>>this on their own risk and cannot expect output stability.
> >>>
> >>>device_show is like info qtree: the output will naturally change as the
> >>>emulated hardware evolves, information is added/removed, or we simply
> >>>improve the layout. Recent changes on info network are an example for
> >>>the latter.
> >>
> >>Yeah, I'm not worried about stability.  HMP commands that aren't
> >>exposed as QMP commands are inherently unstable and should not be
> >>scripted to.
> >
> >They are also not accessible when using libvirt, right?
> 
> $ virsh human-monitor-passthrough GuestName device_show foo
> 
> Should work.

So how, in the end, will user know it's unsupported?
I don't agree with 'all HMP is unstable' as people
will use it and will come to depend on it.

What I'm asking is something like 'enable_unsupported_commands',
which will expose all kind of stuff.
Could also be extra stuff in existing commands too -
e.g. info pci is a natural way to see config space
for devices.

-- 
MST

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Kevin Wolf]

Am 06.09.2011 19:05, schrieb Michael S. Tsirkin:
> On Tue, Sep 06, 2011 at 11:28:09AM -0500, Anthony Liguori wrote:
>> On 09/06/2011 11:09 AM, Michael S. Tsirkin wrote:
>>> On Tue, Sep 06, 2011 at 10:51:26AM -0500, Anthony Liguori wrote:
>>>> On 09/06/2011 10:45 AM, Jan Kiszka wrote:
>>>>> On 2011-09-06 16:48, Michael S. Tsirkin wrote:
>>>>>> I'm afraid that won't be enough to stop people
>>>>>> scripting this command - libvirt accessed
>>>>>> HMP for years.
>>>>>>
>>>>>> On the other hand, no QMP command means e.g.
>>>>>> libvirt users don't get any benefit from this.
>>>>>>
>>>>>> What I think will solve these problems, for both HMP and QMP,
>>>>>> is an explicit 'debug_unstable' or 'debug_unsupported' command that will
>>>>>> expose all kind of debugging functionality making it
>>>>>> very explicit that it's an unsupported debugging utility.
>>>>>>
>>>>>> Proposed syntax:
>>>>>>
>>>>>> debug_unstable<subcommand>   <options>
>>>>>>
>>>>>> Example:
>>>>>>
>>>>>> debug_unstable device_show -all
>>>>>
>>>>> For HMP, this would needlessly complicate the user interface, nothing I
>>>>> would support. People scripting things on top of HMP are generally doing
>>>>> this on their own risk and cannot expect output stability.
>>>>>
>>>>> device_show is like info qtree: the output will naturally change as the
>>>>> emulated hardware evolves, information is added/removed, or we simply
>>>>> improve the layout. Recent changes on info network are an example for
>>>>> the latter.
>>>>
>>>> Yeah, I'm not worried about stability.  HMP commands that aren't
>>>> exposed as QMP commands are inherently unstable and should not be
>>>> scripted to.
>>>
>>> They are also not accessible when using libvirt, right?
>>
>> $ virsh human-monitor-passthrough GuestName device_show foo
>>
>> Should work.
> 
> So how, in the end, will user know it's unsupported?
> I don't agree with 'all HMP is unstable' as people
> will use it and will come to depend on it.

Why unsupported? It's just not a stable API to script against. It's a
user interface, not a programming interface. So as long as you use it
manually, that's perfectly fine.

Would you expect that virt-manager never changes its GUI because there
might be some scripts that try to achieve things by screen scraping? The
interface is just not meant for such uses, and if you use it in that way
and it breaks with the next version it's your own fault.

Kevin

Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices

[Gleb Natapov]

On Wed, Aug 31, 2011 at 08:31:26PM +0200, Jan Kiszka wrote:
> On 2011-08-29 23:19, Anthony Liguori wrote:
> > On 08/29/2011 03:56 PM, Jan Kiszka wrote:
> >> On 2011-08-29 21:23, Anthony Liguori wrote:
> >>> On 08/26/2011 09:48 AM, Jan Kiszka wrote:
> >>>> In order to address devices for that the user forgot or is even unable
> >>>> (no_user) to provide an ID, assign an automatically generated one. Such
> >>>> IDs have the format #<number>, thus are outside the name space availing
> >>>> to users. Don't use them for bus naming to avoid any other user-visible
> >>>> change.
> >>>
> >>> I don't think this is a very nice approach.  Why not eliminate anonymous
> >>> devices entirely and use a parent derived name for devices that are not
> >>> created by the user?
> >>
> >> This eliminates anonymous devices completely. So I guess you are asking
> >> for a different naming scheme, something like<parent-id>.child#<no>
> >> e.g.? Well, we would end up with fairly long names when a complete
> >> hierarchy is anonymous. What would be the benefit?
> > 
> > No, I'm saying that whenever a device is created, it should be given a
> > non-random name.  IOW, the names of these devices should be stable.
> > 
> >> I'm really just looking for some simple, temporary workaround without
> >> touching the existing fragile naming scheme. What we really need is full
> >> path addressing, but that without preserving all the legacy.
> > 
> > Yeah, I understand, and I hesitated making any grander suggestions here,
> > but I'm not sure how much work it would be to just remove any caller
> > that passes NULL for ID and replace it with something more meaningful. I
> > think that's a helpful clean up long term no matter what.
> 
> That won't solve the problem of finding a unique device name. If we want
> to derive it from stable device properties (bus addresses etc.), we
> first of all have to define them for all types of devices. And that's
> basically were the discussion exploded last year IIRC.
> 
Why not use the OpenFirmware naming that we already have for some
devices instead of inventing something new?

--
			Gleb.

Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices

[Jan Kiszka]

On 2011-09-07 11:50, Gleb Natapov wrote:
> On Wed, Aug 31, 2011 at 08:31:26PM +0200, Jan Kiszka wrote:
>> On 2011-08-29 23:19, Anthony Liguori wrote:
>>> On 08/29/2011 03:56 PM, Jan Kiszka wrote:
>>>> On 2011-08-29 21:23, Anthony Liguori wrote:
>>>>> On 08/26/2011 09:48 AM, Jan Kiszka wrote:
>>>>>> In order to address devices for that the user forgot or is even unable
>>>>>> (no_user) to provide an ID, assign an automatically generated one. Such
>>>>>> IDs have the format #<number>, thus are outside the name space availing
>>>>>> to users. Don't use them for bus naming to avoid any other user-visible
>>>>>> change.
>>>>>
>>>>> I don't think this is a very nice approach.  Why not eliminate anonymous
>>>>> devices entirely and use a parent derived name for devices that are not
>>>>> created by the user?
>>>>
>>>> This eliminates anonymous devices completely. So I guess you are asking
>>>> for a different naming scheme, something like<parent-id>.child#<no>
>>>> e.g.? Well, we would end up with fairly long names when a complete
>>>> hierarchy is anonymous. What would be the benefit?
>>>
>>> No, I'm saying that whenever a device is created, it should be given a
>>> non-random name.  IOW, the names of these devices should be stable.
>>>
>>>> I'm really just looking for some simple, temporary workaround without
>>>> touching the existing fragile naming scheme. What we really need is full
>>>> path addressing, but that without preserving all the legacy.
>>>
>>> Yeah, I understand, and I hesitated making any grander suggestions here,
>>> but I'm not sure how much work it would be to just remove any caller
>>> that passes NULL for ID and replace it with something more meaningful. I
>>> think that's a helpful clean up long term no matter what.
>>
>> That won't solve the problem of finding a unique device name. If we want
>> to derive it from stable device properties (bus addresses etc.), we
>> first of all have to define them for all types of devices. And that's
>> basically were the discussion exploded last year IIRC.
>>
> Why not use the OpenFirmware naming that we already have for some
> devices instead of inventing something new?

Because I do not want to establish any path names before QOM conversion
(including potential device reorganization) has been started.
Specifically as I do not need naming for "some" devices, but for all.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices

[Gleb Natapov]

On Wed, Sep 07, 2011 at 12:27:23PM +0200, Jan Kiszka wrote:
> On 2011-09-07 11:50, Gleb Natapov wrote:
> > On Wed, Aug 31, 2011 at 08:31:26PM +0200, Jan Kiszka wrote:
> >> On 2011-08-29 23:19, Anthony Liguori wrote:
> >>> On 08/29/2011 03:56 PM, Jan Kiszka wrote:
> >>>> On 2011-08-29 21:23, Anthony Liguori wrote:
> >>>>> On 08/26/2011 09:48 AM, Jan Kiszka wrote:
> >>>>>> In order to address devices for that the user forgot or is even unable
> >>>>>> (no_user) to provide an ID, assign an automatically generated one. Such
> >>>>>> IDs have the format #<number>, thus are outside the name space availing
> >>>>>> to users. Don't use them for bus naming to avoid any other user-visible
> >>>>>> change.
> >>>>>
> >>>>> I don't think this is a very nice approach.  Why not eliminate anonymous
> >>>>> devices entirely and use a parent derived name for devices that are not
> >>>>> created by the user?
> >>>>
> >>>> This eliminates anonymous devices completely. So I guess you are asking
> >>>> for a different naming scheme, something like<parent-id>.child#<no>
> >>>> e.g.? Well, we would end up with fairly long names when a complete
> >>>> hierarchy is anonymous. What would be the benefit?
> >>>
> >>> No, I'm saying that whenever a device is created, it should be given a
> >>> non-random name.  IOW, the names of these devices should be stable.
> >>>
> >>>> I'm really just looking for some simple, temporary workaround without
> >>>> touching the existing fragile naming scheme. What we really need is full
> >>>> path addressing, but that without preserving all the legacy.
> >>>
> >>> Yeah, I understand, and I hesitated making any grander suggestions here,
> >>> but I'm not sure how much work it would be to just remove any caller
> >>> that passes NULL for ID and replace it with something more meaningful. I
> >>> think that's a helpful clean up long term no matter what.
> >>
> >> That won't solve the problem of finding a unique device name. If we want
> >> to derive it from stable device properties (bus addresses etc.), we
> >> first of all have to define them for all types of devices. And that's
> >> basically were the discussion exploded last year IIRC.
> >>
> > Why not use the OpenFirmware naming that we already have for some
> > devices instead of inventing something new?
> 
> Because I do not want to establish any path names before QOM conversion
> (including potential device reorganization) has been started.
In theory device paths are dictated by HW topology, not today's flavor of
QEMU object model.

> Specifically as I do not need naming for "some" devices, but for all.
> 
It can be extended. We already have three types of device naming. One is
used in qdev, another is used for migration and yet another one for
passing device names to firmware. We should converge to a single one :)

--
			Gleb.

Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices

[Jan Kiszka]

On 2011-09-07 12:34, Gleb Natapov wrote:
> On Wed, Sep 07, 2011 at 12:27:23PM +0200, Jan Kiszka wrote:
>> On 2011-09-07 11:50, Gleb Natapov wrote:
>>> On Wed, Aug 31, 2011 at 08:31:26PM +0200, Jan Kiszka wrote:
>>>> On 2011-08-29 23:19, Anthony Liguori wrote:
>>>>> On 08/29/2011 03:56 PM, Jan Kiszka wrote:
>>>>>> On 2011-08-29 21:23, Anthony Liguori wrote:
>>>>>>> On 08/26/2011 09:48 AM, Jan Kiszka wrote:
>>>>>>>> In order to address devices for that the user forgot or is even unable
>>>>>>>> (no_user) to provide an ID, assign an automatically generated one. Such
>>>>>>>> IDs have the format #<number>, thus are outside the name space availing
>>>>>>>> to users. Don't use them for bus naming to avoid any other user-visible
>>>>>>>> change.
>>>>>>>
>>>>>>> I don't think this is a very nice approach.  Why not eliminate anonymous
>>>>>>> devices entirely and use a parent derived name for devices that are not
>>>>>>> created by the user?
>>>>>>
>>>>>> This eliminates anonymous devices completely. So I guess you are asking
>>>>>> for a different naming scheme, something like<parent-id>.child#<no>
>>>>>> e.g.? Well, we would end up with fairly long names when a complete
>>>>>> hierarchy is anonymous. What would be the benefit?
>>>>>
>>>>> No, I'm saying that whenever a device is created, it should be given a
>>>>> non-random name.  IOW, the names of these devices should be stable.
>>>>>
>>>>>> I'm really just looking for some simple, temporary workaround without
>>>>>> touching the existing fragile naming scheme. What we really need is full
>>>>>> path addressing, but that without preserving all the legacy.
>>>>>
>>>>> Yeah, I understand, and I hesitated making any grander suggestions here,
>>>>> but I'm not sure how much work it would be to just remove any caller
>>>>> that passes NULL for ID and replace it with something more meaningful. I
>>>>> think that's a helpful clean up long term no matter what.
>>>>
>>>> That won't solve the problem of finding a unique device name. If we want
>>>> to derive it from stable device properties (bus addresses etc.), we
>>>> first of all have to define them for all types of devices. And that's
>>>> basically were the discussion exploded last year IIRC.
>>>>
>>> Why not use the OpenFirmware naming that we already have for some
>>> devices instead of inventing something new?
>>
>> Because I do not want to establish any path names before QOM conversion
>> (including potential device reorganization) has been started.
> In theory device paths are dictated by HW topology, not today's flavor of
> QEMU object model.

There will be changes in the object composition, but predicting them
today and modeling this on top of current qdev is nothing I want to try.

> 
>> Specifically as I do not need naming for "some" devices, but for all.
>>
> It can be extended. We already have three types of device naming. One is
> used in qdev, another is used for migration and yet another one for
> passing device names to firmware. We should converge to a single one :)

Yes, but that's beyond what this patch set can achieve or what will
happen in foreseeable time.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Michael S. Tsirkin]

On Wed, Sep 07, 2011 at 11:37:20AM +0200, Kevin Wolf wrote:
> Am 06.09.2011 19:05, schrieb Michael S. Tsirkin:
> > On Tue, Sep 06, 2011 at 11:28:09AM -0500, Anthony Liguori wrote:
> >> On 09/06/2011 11:09 AM, Michael S. Tsirkin wrote:
> >>> On Tue, Sep 06, 2011 at 10:51:26AM -0500, Anthony Liguori wrote:
> >>>> On 09/06/2011 10:45 AM, Jan Kiszka wrote:
> >>>>> On 2011-09-06 16:48, Michael S. Tsirkin wrote:
> >>>>>> I'm afraid that won't be enough to stop people
> >>>>>> scripting this command - libvirt accessed
> >>>>>> HMP for years.
> >>>>>>
> >>>>>> On the other hand, no QMP command means e.g.
> >>>>>> libvirt users don't get any benefit from this.
> >>>>>>
> >>>>>> What I think will solve these problems, for both HMP and QMP,
> >>>>>> is an explicit 'debug_unstable' or 'debug_unsupported' command that will
> >>>>>> expose all kind of debugging functionality making it
> >>>>>> very explicit that it's an unsupported debugging utility.
> >>>>>>
> >>>>>> Proposed syntax:
> >>>>>>
> >>>>>> debug_unstable<subcommand>   <options>
> >>>>>>
> >>>>>> Example:
> >>>>>>
> >>>>>> debug_unstable device_show -all
> >>>>>
> >>>>> For HMP, this would needlessly complicate the user interface, nothing I
> >>>>> would support. People scripting things on top of HMP are generally doing
> >>>>> this on their own risk and cannot expect output stability.
> >>>>>
> >>>>> device_show is like info qtree: the output will naturally change as the
> >>>>> emulated hardware evolves, information is added/removed, or we simply
> >>>>> improve the layout. Recent changes on info network are an example for
> >>>>> the latter.
> >>>>
> >>>> Yeah, I'm not worried about stability.  HMP commands that aren't
> >>>> exposed as QMP commands are inherently unstable and should not be
> >>>> scripted to.
> >>>
> >>> They are also not accessible when using libvirt, right?
> >>
> >> $ virsh human-monitor-passthrough GuestName device_show foo
> >>
> >> Should work.
> > 
> > So how, in the end, will user know it's unsupported?
> > I don't agree with 'all HMP is unstable' as people
> > will use it and will come to depend on it.
> 
> Why unsupported? It's just not a stable API to script against. It's a
> user interface, not a programming interface. So as long as you use it
> manually, that's perfectly fine.
> 
> Would you expect that virt-manager never changes its GUI because there
> might be some scripts that try to achieve things by screen scraping? The
> interface is just not meant for such uses, and if you use it in that way
> and it breaks with the next version it's your own fault.
> 
> Kevin

I certainly think that radically changing a GUI is very bad usability.
Screen scarping is a silly example, but documentation does appear
e.g. on the web, and GUI changes invalidate it. I won't mention
recent examples of user unhappiness - it takes years to live down
such trauma.

If we expose qemu internals in a command, that's a very
bad idea to give such command to the user, because
1. users will come to depend on the command, whatever you tell them
2. the fact it's there means there is some unaddressed need,
   so we plaster over it with unsupported commands instead
   of addressing it properly

But if the command is not for users at all, if it's
for qemu debugging, then exposing internals is a very
logical thing.  Only problem is - we must make it very very clear
which commands are for qemu debugging.

-- 
MST

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Jan Kiszka]

On 2011-09-07 15:06, Michael S. Tsirkin wrote:
> But if the command is not for users at all, if it's
> for qemu debugging, then exposing internals is a very
> logical thing.  Only problem is - we must make it very very clear
> which commands are for qemu debugging.

This command it also for users, to debug guests.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Michael S. Tsirkin]

On Wed, Sep 07, 2011 at 03:13:00PM +0200, Jan Kiszka wrote:
> On 2011-09-07 15:06, Michael S. Tsirkin wrote:
> > But if the command is not for users at all, if it's
> > for qemu debugging, then exposing internals is a very
> > logical thing.  Only problem is - we must make it very very clear
> > which commands are for qemu debugging.
> 
> This command it also for users, to debug guests.
> 
> Jan

Hmm, guest visible state must be stable by definition.
So why can't we make the interfaces stable then?

> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Anthony Liguori]

On 09/07/2011 08:17 AM, Michael S. Tsirkin wrote:
> On Wed, Sep 07, 2011 at 03:13:00PM +0200, Jan Kiszka wrote:
>> On 2011-09-07 15:06, Michael S. Tsirkin wrote:
>>> But if the command is not for users at all, if it's
>>> for qemu debugging, then exposing internals is a very
>>> logical thing.  Only problem is - we must make it very very clear
>>> which commands are for qemu debugging.
>>
>> This command it also for users, to debug guests.
>>
>> Jan
>
> Hmm, guest visible state must be stable by definition.
> So why can't we make the interfaces stable then?

Because right now how you reference devices cannot be stable.

Going back to a previous thread, what if the command took a qdev class 
type as a filter?

That would be a stable name, not require anonymous IDs, and would have 
the property of usually being a single device.

Regards,

Anthony Liguori

>
>> --
>> Siemens AG, Corporate Technology, CT T DE IT 1
>> Corporate Competence Center Embedded Linux
>

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Jan Kiszka]

On 2011-09-07 15:23, Anthony Liguori wrote:
> On 09/07/2011 08:17 AM, Michael S. Tsirkin wrote:
>> On Wed, Sep 07, 2011 at 03:13:00PM +0200, Jan Kiszka wrote:
>>> On 2011-09-07 15:06, Michael S. Tsirkin wrote:
>>>> But if the command is not for users at all, if it's
>>>> for qemu debugging, then exposing internals is a very
>>>> logical thing.  Only problem is - we must make it very very clear
>>>> which commands are for qemu debugging.
>>>
>>> This command it also for users, to debug guests.
>>>
>>> Jan
>>
>> Hmm, guest visible state must be stable by definition.
>> So why can't we make the interfaces stable then?
> 
> Because right now how you reference devices cannot be stable.
> 
> Going back to a previous thread, what if the command took a qdev class 
> type as a filter?
> 
> That would be a stable name, not require anonymous IDs, and would have 
> the property of usually being a single device.

It's surely not usual that there is only a single instance of a device
type. That's why my old series introduced (unstable) instance numbering.
I refrained for warming those patches up again.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded

[Michael S. Tsirkin]

On Wed, Sep 07, 2011 at 08:23:10AM -0500, Anthony Liguori wrote:
> On 09/07/2011 08:17 AM, Michael S. Tsirkin wrote:
> >On Wed, Sep 07, 2011 at 03:13:00PM +0200, Jan Kiszka wrote:
> >>On 2011-09-07 15:06, Michael S. Tsirkin wrote:
> >>>But if the command is not for users at all, if it's
> >>>for qemu debugging, then exposing internals is a very
> >>>logical thing.  Only problem is - we must make it very very clear
> >>>which commands are for qemu debugging.
> >>
> >>This command it also for users, to debug guests.
> >>
> >>Jan
> >
> >Hmm, guest visible state must be stable by definition.
> >So why can't we make the interfaces stable then?
> 
> Because right now how you reference devices cannot be stable.

For devices that do have IDs, we can support this
on QMP as well then?

-- 
MST