* [Qemu-devel] [PATCH 1/6] monitor: return length of printed string via monitor_[v]printf
2011-08-26 14:48 [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Jan Kiszka
@ 2011-08-26 14:48 ` Jan Kiszka
2011-08-26 14:48 ` [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder Jan Kiszka
` (7 subsequent siblings)
8 siblings, 0 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 14:48 UTC (permalink / raw)
To: Anthony Liguori, qemu-devel; +Cc: Luiz Capitulino
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder
2011-08-26 14:48 [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Jan Kiszka
2011-08-26 14:48 ` [Qemu-devel] [PATCH 1/6] monitor: return length of printed string via monitor_[v]printf Jan Kiszka
@ 2011-08-26 14:48 ` Jan Kiszka
2011-08-26 15:21 ` Peter Maydell
2011-08-26 14:48 ` [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes Jan Kiszka
` (6 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 14:48 UTC (permalink / raw)
To: Anthony Liguori, qemu-devel; +Cc: Luiz Capitulino
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder
2011-08-26 14:48 ` [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder Jan Kiszka
@ 2011-08-26 15:21 ` Peter Maydell
2011-08-26 15:23 ` Jan Kiszka
0 siblings, 1 reply; 45+ messages in thread
From: Peter Maydell @ 2011-08-26 15:21 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Anthony Liguori, qemu-devel, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder
2011-08-26 15:21 ` Peter Maydell
@ 2011-08-26 15:23 ` Jan Kiszka
2011-08-26 15:47 ` Jan Kiszka
0 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 15:23 UTC (permalink / raw)
To: Peter Maydell; +Cc: Anthony Liguori, qemu-devel, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder
2011-08-26 15:23 ` Jan Kiszka
@ 2011-08-26 15:47 ` Jan Kiszka
2011-08-26 18:02 ` Jan Kiszka
2011-09-05 13:55 ` [Qemu-devel] required glib version? " Gerd Hoffmann
0 siblings, 2 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 15:47 UTC (permalink / raw)
To: Peter Maydell, Anthony Liguori; +Cc: qemu-devel, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder
2011-08-26 15:47 ` Jan Kiszka
@ 2011-08-26 18:02 ` Jan Kiszka
2011-09-02 17:22 ` Luiz Capitulino
2011-09-05 13:55 ` [Qemu-devel] required glib version? " Gerd Hoffmann
1 sibling, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 18:02 UTC (permalink / raw)
To: Peter Maydell, Anthony Liguori; +Cc: qemu-devel, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder
2011-08-26 18:02 ` Jan Kiszka
@ 2011-09-02 17:22 ` Luiz Capitulino
0 siblings, 0 replies; 45+ messages in thread
From: Luiz Capitulino @ 2011-09-02 17:22 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Peter Maydell, Anthony Liguori, qemu-devel, mdroth
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?
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] required glib version? Re: [PATCH 2/6] Add base64 encoder/decoder
2011-08-26 15:47 ` Jan Kiszka
2011-08-26 18:02 ` Jan Kiszka
@ 2011-09-05 13:55 ` Gerd Hoffmann
1 sibling, 0 replies; 45+ messages in thread
From: Gerd Hoffmann @ 2011-09-05 13:55 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Peter Maydell, Anthony Liguori, qemu-devel, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes
2011-08-26 14:48 [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Jan Kiszka
2011-08-26 14:48 ` [Qemu-devel] [PATCH 1/6] monitor: return length of printed string via monitor_[v]printf Jan Kiszka
2011-08-26 14:48 ` [Qemu-devel] [PATCH 2/6] Add base64 encoder/decoder Jan Kiszka
@ 2011-08-26 14:48 ` Jan Kiszka
2011-09-02 17:23 ` Luiz Capitulino
2011-08-26 14:48 ` [Qemu-devel] [PATCH 4/6] QMP: Add QBuffer Jan Kiszka
` (5 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 14:48 UTC (permalink / raw)
To: Anthony Liguori, qemu-devel; +Cc: Luiz Capitulino
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes
2011-08-26 14:48 ` [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes Jan Kiszka
@ 2011-09-02 17:23 ` Luiz Capitulino
2011-09-02 17:47 ` Jan Kiszka
2011-09-02 18:02 ` Anthony Liguori
0 siblings, 2 replies; 45+ messages in thread
From: Luiz Capitulino @ 2011-09-02 17:23 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Anthony Liguori, qemu-devel
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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes
2011-09-02 17:23 ` Luiz Capitulino
@ 2011-09-02 17:47 ` Jan Kiszka
2011-09-02 18:02 ` Anthony Liguori
1 sibling, 0 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-09-02 17:47 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: Anthony Liguori, qemu-devel
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes
2011-09-02 17:23 ` Luiz Capitulino
2011-09-02 17:47 ` Jan Kiszka
@ 2011-09-02 18:02 ` Anthony Liguori
1 sibling, 0 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-09-02 18:02 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: Jan Kiszka, qemu-devel
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
>
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 4/6] QMP: Add QBuffer
2011-08-26 14:48 [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Jan Kiszka
` (2 preceding siblings ...)
2011-08-26 14:48 ` [Qemu-devel] [PATCH 3/6] QMP: Reserve namespace for complex object classes Jan Kiszka
@ 2011-08-26 14:48 ` Jan Kiszka
2011-08-26 18:23 ` [Qemu-devel] [PATCH v2 " Jan Kiszka
2011-08-26 14:48 ` [Qemu-devel] [PATCH 5/6] monitor: Add basic device state visualization Jan Kiszka
` (4 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 14:48 UTC (permalink / raw)
To: Anthony Liguori, qemu-devel; +Cc: Luiz Capitulino
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH v2 4/6] QMP: Add QBuffer
2011-08-26 14:48 ` [Qemu-devel] [PATCH 4/6] QMP: Add QBuffer Jan Kiszka
@ 2011-08-26 18:23 ` Jan Kiszka
0 siblings, 0 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 18:23 UTC (permalink / raw)
To: Anthony Liguori, qemu-devel; +Cc: Luiz Capitulino
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;
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 5/6] monitor: Add basic device state visualization
2011-08-26 14:48 [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Jan Kiszka
` (3 preceding siblings ...)
2011-08-26 14:48 ` [Qemu-devel] [PATCH 4/6] QMP: Add QBuffer Jan Kiszka
@ 2011-08-26 14:48 ` Jan Kiszka
2011-08-26 14:48 ` [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices Jan Kiszka
` (3 subsequent siblings)
8 siblings, 0 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 14:48 UTC (permalink / raw)
To: Anthony Liguori, qemu-devel; +Cc: Markus Armbruster, Luiz Capitulino
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices
2011-08-26 14:48 [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Jan Kiszka
` (4 preceding siblings ...)
2011-08-26 14:48 ` [Qemu-devel] [PATCH 5/6] monitor: Add basic device state visualization Jan Kiszka
@ 2011-08-26 14:48 ` Jan Kiszka
2011-08-29 19:23 ` Anthony Liguori
2011-08-29 19:22 ` [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Anthony Liguori
` (2 subsequent siblings)
8 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-08-26 14:48 UTC (permalink / raw)
To: Anthony Liguori, qemu-devel; +Cc: Markus Armbruster
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
^ permalink raw reply related [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices
2011-08-26 14:48 ` [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices Jan Kiszka
@ 2011-08-29 19:23 ` Anthony Liguori
2011-08-29 20:56 ` Jan Kiszka
0 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-08-29 19:23 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Markus Armbruster
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;
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices
2011-08-29 19:23 ` Anthony Liguori
@ 2011-08-29 20:56 ` Jan Kiszka
2011-08-29 21:19 ` Anthony Liguori
0 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-08-29 20:56 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Markus Armbruster
[-- 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 --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices
2011-08-29 20:56 ` Jan Kiszka
@ 2011-08-29 21:19 ` Anthony Liguori
2011-08-31 18:31 ` Jan Kiszka
0 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-08-29 21:19 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Markus Armbruster
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
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices
2011-08-29 21:19 ` Anthony Liguori
@ 2011-08-31 18:31 ` Jan Kiszka
2011-09-07 9:50 ` Gleb Natapov
0 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-08-31 18:31 UTC (permalink / raw)
To: Anthony Liguori; +Cc: qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices
2011-08-31 18:31 ` Jan Kiszka
@ 2011-09-07 9:50 ` Gleb Natapov
2011-09-07 10:27 ` Jan Kiszka
0 siblings, 1 reply; 45+ messages in thread
From: Gleb Natapov @ 2011-09-07 9:50 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Markus Armbruster
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.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices
2011-09-07 9:50 ` Gleb Natapov
@ 2011-09-07 10:27 ` Jan Kiszka
2011-09-07 10:34 ` Gleb Natapov
0 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-09-07 10:27 UTC (permalink / raw)
To: Gleb Natapov; +Cc: qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices
2011-09-07 10:27 ` Jan Kiszka
@ 2011-09-07 10:34 ` Gleb Natapov
2011-09-07 10:58 ` Jan Kiszka
0 siblings, 1 reply; 45+ messages in thread
From: Gleb Natapov @ 2011-09-07 10:34 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Markus Armbruster
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.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices
2011-09-07 10:34 ` Gleb Natapov
@ 2011-09-07 10:58 ` Jan Kiszka
0 siblings, 0 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-09-07 10:58 UTC (permalink / raw)
To: Gleb Natapov; +Cc: qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-08-26 14:48 [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Jan Kiszka
` (5 preceding siblings ...)
2011-08-26 14:48 ` [Qemu-devel] [PATCH 6/6] qdev: Generate IDs for anonymous devices Jan Kiszka
@ 2011-08-29 19:22 ` Anthony Liguori
2011-08-29 20:54 ` Jan Kiszka
2011-09-02 17:27 ` Luiz Capitulino
2011-09-06 14:48 ` Michael S. Tsirkin
8 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-08-29 19:22 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Luiz Capitulino, qemu-devel, Markus Armbruster
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
>
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-08-29 19:22 ` [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Anthony Liguori
@ 2011-08-29 20:54 ` Jan Kiszka
0 siblings, 0 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-08-29 20:54 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Markus Armbruster, qemu-devel, Luiz Capitulino
[-- 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 --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-08-26 14:48 [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Jan Kiszka
` (6 preceding siblings ...)
2011-08-29 19:22 ` [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Anthony Liguori
@ 2011-09-02 17:27 ` Luiz Capitulino
2011-09-06 14:48 ` Michael S. Tsirkin
8 siblings, 0 replies; 45+ messages in thread
From: Luiz Capitulino @ 2011-09-02 17:27 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Anthony Liguori, qemu-devel, Markus Armbruster
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?
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-08-26 14:48 [Qemu-devel] [PATCH 0/6] Device state visualization reloaded Jan Kiszka
` (7 preceding siblings ...)
2011-09-02 17:27 ` Luiz Capitulino
@ 2011-09-06 14:48 ` Michael S. Tsirkin
2011-09-06 15:45 ` Jan Kiszka
8 siblings, 1 reply; 45+ messages in thread
From: Michael S. Tsirkin @ 2011-09-06 14:48 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Luiz Capitulino, qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 14:48 ` Michael S. Tsirkin
@ 2011-09-06 15:45 ` Jan Kiszka
2011-09-06 15:51 ` Anthony Liguori
0 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-09-06 15:45 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony Liguori, Luiz Capitulino, qemu-devel, Markus Armbruster
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 15:45 ` Jan Kiszka
@ 2011-09-06 15:51 ` Anthony Liguori
2011-09-06 16:05 ` Jan Kiszka
2011-09-06 16:09 ` Michael S. Tsirkin
0 siblings, 2 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-09-06 15:51 UTC (permalink / raw)
To: Jan Kiszka
Cc: Markus Armbruster, Anthony Liguori, Luiz Capitulino, qemu-devel,
Michael S. Tsirkin
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
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 15:51 ` Anthony Liguori
@ 2011-09-06 16:05 ` Jan Kiszka
2011-09-06 16:08 ` Anthony Liguori
2011-09-06 16:09 ` Michael S. Tsirkin
1 sibling, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-09-06 16:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: Markus Armbruster, Anthony Liguori, Luiz Capitulino, qemu-devel,
Michael S. Tsirkin
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 16:05 ` Jan Kiszka
@ 2011-09-06 16:08 ` Anthony Liguori
2011-09-06 16:33 ` Jan Kiszka
0 siblings, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-09-06 16:08 UTC (permalink / raw)
To: Jan Kiszka
Cc: qemu-devel, Anthony Liguori, Michael S. Tsirkin,
Markus Armbruster, Luiz Capitulino
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
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 16:08 ` Anthony Liguori
@ 2011-09-06 16:33 ` Jan Kiszka
0 siblings, 0 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-09-06 16:33 UTC (permalink / raw)
To: Anthony Liguori
Cc: qemu-devel, Anthony Liguori, Michael S. Tsirkin,
Markus Armbruster, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 15:51 ` Anthony Liguori
2011-09-06 16:05 ` Jan Kiszka
@ 2011-09-06 16:09 ` Michael S. Tsirkin
2011-09-06 16:28 ` Anthony Liguori
2011-09-06 16:29 ` Jan Kiszka
1 sibling, 2 replies; 45+ messages in thread
From: Michael S. Tsirkin @ 2011-09-06 16:09 UTC (permalink / raw)
To: Anthony Liguori
Cc: Jan Kiszka, Anthony Liguori, qemu-devel, Markus Armbruster,
Luiz Capitulino
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
> >
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 16:09 ` Michael S. Tsirkin
@ 2011-09-06 16:28 ` Anthony Liguori
2011-09-06 17:05 ` Michael S. Tsirkin
2011-09-06 16:29 ` Jan Kiszka
1 sibling, 1 reply; 45+ messages in thread
From: Anthony Liguori @ 2011-09-06 16:28 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jan Kiszka, Anthony Liguori, Luiz Capitulino, qemu-devel,
Markus Armbruster
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
>>>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 16:28 ` Anthony Liguori
@ 2011-09-06 17:05 ` Michael S. Tsirkin
2011-09-07 9:37 ` Kevin Wolf
0 siblings, 1 reply; 45+ messages in thread
From: Michael S. Tsirkin @ 2011-09-06 17:05 UTC (permalink / raw)
To: Anthony Liguori
Cc: Jan Kiszka, Anthony Liguori, Luiz Capitulino, qemu-devel,
Markus Armbruster
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 17:05 ` Michael S. Tsirkin
@ 2011-09-07 9:37 ` Kevin Wolf
2011-09-07 13:06 ` Michael S. Tsirkin
0 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2011-09-07 9:37 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony Liguori, Jan Kiszka, Markus Armbruster, qemu-devel,
Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-07 9:37 ` Kevin Wolf
@ 2011-09-07 13:06 ` Michael S. Tsirkin
2011-09-07 13:13 ` Jan Kiszka
0 siblings, 1 reply; 45+ messages in thread
From: Michael S. Tsirkin @ 2011-09-07 13:06 UTC (permalink / raw)
To: Kevin Wolf
Cc: Anthony Liguori, Jan Kiszka, Markus Armbruster, qemu-devel,
Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-07 13:06 ` Michael S. Tsirkin
@ 2011-09-07 13:13 ` Jan Kiszka
2011-09-07 13:17 ` Michael S. Tsirkin
0 siblings, 1 reply; 45+ messages in thread
From: Jan Kiszka @ 2011-09-07 13:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Kevin Wolf, Anthony Liguori, Markus Armbruster, qemu-devel,
Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-07 13:13 ` Jan Kiszka
@ 2011-09-07 13:17 ` Michael S. Tsirkin
2011-09-07 13:23 ` Anthony Liguori
0 siblings, 1 reply; 45+ messages in thread
From: Michael S. Tsirkin @ 2011-09-07 13:17 UTC (permalink / raw)
To: Jan Kiszka
Cc: Kevin Wolf, Anthony Liguori, Markus Armbruster, qemu-devel,
Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-07 13:17 ` Michael S. Tsirkin
@ 2011-09-07 13:23 ` Anthony Liguori
2011-09-07 13:29 ` Jan Kiszka
2011-09-07 13:33 ` Michael S. Tsirkin
0 siblings, 2 replies; 45+ messages in thread
From: Anthony Liguori @ 2011-09-07 13:23 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Kevin Wolf, Anthony Liguori, qemu-devel, Jan Kiszka,
Markus Armbruster, Luiz Capitulino
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
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-07 13:23 ` Anthony Liguori
@ 2011-09-07 13:29 ` Jan Kiszka
2011-09-07 13:33 ` Michael S. Tsirkin
1 sibling, 0 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-09-07 13:29 UTC (permalink / raw)
To: Anthony Liguori
Cc: Kevin Wolf, Anthony Liguori, Michael S. Tsirkin,
Markus Armbruster, qemu-devel, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-07 13:23 ` Anthony Liguori
2011-09-07 13:29 ` Jan Kiszka
@ 2011-09-07 13:33 ` Michael S. Tsirkin
1 sibling, 0 replies; 45+ messages in thread
From: Michael S. Tsirkin @ 2011-09-07 13:33 UTC (permalink / raw)
To: Anthony Liguori
Cc: Kevin Wolf, Anthony Liguori, qemu-devel, Jan Kiszka,
Markus Armbruster, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Device state visualization reloaded
2011-09-06 16:09 ` Michael S. Tsirkin
2011-09-06 16:28 ` Anthony Liguori
@ 2011-09-06 16:29 ` Jan Kiszka
1 sibling, 0 replies; 45+ messages in thread
From: Jan Kiszka @ 2011-09-06 16:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Markus Armbruster, Anthony Liguori, qemu-devel, Luiz Capitulino
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
^ permalink raw reply [flat|nested] 45+ messages in thread