[Qemu-devel] [FOR 0.12 0/7]: More QMP related fixes

[Qemu-devel] [FOR 0.12 0/7]: More QMP related fixes

[Luiz Capitulino]

Apart form that there are two important changes:

1. New output for success response
2. A new argument type

 Thanks.

[Qemu-devel] [PATCH 1/7] QMP: Only handle converted commands

[Luiz Capitulino]

Looks like I dropped this check when addressing the 'query-'
commands request.

QMP should only handle converted commands, obviously.

Reported-by: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index ebd0282..1455258 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4070,7 +4070,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
                       qobject_from_jsonf("{ 'item': %s }", info_item));
     } else {
         cmd = monitor_find_command(cmd_name);
-        if (!cmd) {
+        if (!cmd || !monitor_handler_ported(cmd)) {
             qemu_error_new(QERR_COMMAND_NOT_FOUND, cmd_name);
             goto err_input;
         }
-- 
1.6.6.rc3

[Qemu-devel] [PATCH 2/7] QMP: Return an empty dict by default

[Luiz Capitulino]

Currently, when a regular command doesn't have any data to output,
QMP will emit:

{ "return": "OK" }

Returning an empty dict is better though, because dicts can support
some protocol changes in a compatible way.

So, with this commit we will return:

{ "return": {} }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 1455258..d238660 100644
--- a/monitor.c
+++ b/monitor.c
@@ -286,7 +286,8 @@ static void monitor_protocol_emitter(Monitor *mon, QObject *data)
             qobject_incref(data);
             qdict_put_obj(qmp, "return", data);
         } else {
-            qdict_put(qmp, "return", qstring_from_str("OK"));
+            /* return an empty QDict by default */
+            qdict_put(qmp, "return", qdict_new());
         }
     } else {
         /* error response */
-- 
1.6.6.rc3

[Qemu-devel] [PATCH 3/7] QMP: Assure that returned data is a QDict

[Luiz Capitulino]

This is for debug purposes only.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index d238660..8ef1582 100644
--- a/monitor.c
+++ b/monitor.c
@@ -283,6 +283,7 @@ static void monitor_protocol_emitter(Monitor *mon, QObject *data)
     if (!monitor_has_error(mon)) {
         /* success response */
         if (data) {
+            assert(qobject_type(data) == QTYPE_QDICT);
             qobject_incref(data);
             qdict_put_obj(qmp, "return", data);
         } else {
-- 
1.6.6.rc3

[Qemu-devel] [PATCH 4/7] QMP: Update README file

[Luiz Capitulino]

- Fix output description
- Fix command-line usage notes
- Minor improvements

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/README |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/QMP/README b/QMP/README
index 50c31f2..09e7053 100644
--- a/QMP/README
+++ b/QMP/README
@@ -4,45 +4,57 @@
 Introduction
 -------------
 
-The QEMU Monitor Protocol (QMP) is a JSON[1] based protocol for QEMU.
+The QEMU Monitor Protocol (QMP) allows applications to communicate with
+QEMU's Monitor.
 
-By using it applications can control QEMU in reliable and "parseable" way,
-QMP also provides asynchronous events support.
+QMP is JSON[1] based and has the following features:
+
+- Lightweight, text-based, easy to parse data format
+- Asynchronous events support 
+- Stability
 
 For more information, please, refer to the following files:
 
-o qmp-spec.txt    QEMU Monitor Protocol current draft specification
+o qmp-spec.txt    QEMU Monitor Protocol current specification
 o qmp-events.txt  List of available asynchronous events
 
 There are also two simple Python scripts available:
 
 o qmp-shell       A shell
-o vm-info         Show some informations about the Virtal Machine
+o vm-info         Show some information about the Virtual Machine
 
 [1] http://www.json.org
 
 Usage
 -----
 
-To enable QMP, QEMU has to be started in "control mode". This is done
-by passing the flag "control" to the "-monitor" command-line option.
+To enable QMP, QEMU has to be started in "control mode". There are
+two ways of doing this, the simplest one is using the the '-qmp'
+command-line option.
 
 For example:
 
-$ qemu [...] -monitor control,tcp:localhost:4444,server
+$ qemu [...] -qmp tcp:localhost:4444,server
 
 Will start QEMU in control mode, waiting for a client TCP connection
 on localhost port 4444.
 
-To manually test it you can connect with telnet and issue commands:
+It is also possible to use the '-mon' command-line option to have
+more complex combinations. Please, refer to the QEMU's manpage for
+more information.
+
+Simple Testing
+--------------
+
+To manually test QMP one can connect with telnet and issue commands:
 
 $ telnet localhost 4444
-Trying ::1...
+Trying 127.0.0.1...
 Connected to localhost.
 Escape character is '^]'.
 {"QMP": {"capabilities": []}}
 { "execute": "query-version" }
-{"return": "0.11.50"}
+{"return": {"qemu": "0.11.50", "package": ""}}
 
 Contact
 -------
-- 
1.6.6.rc3

[Qemu-devel] [PATCH 5/7] QMP: Update spec file

[Luiz Capitulino]

- Remove "draft" status
- Change default success response to be json-object
- Change error and event data member to be a json-object
- Update examples
- Add new section "Compatibility Considerations"
- Other fixes and clarifications

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp-spec.txt |   69 +++++++++++++++++++++++++++++++++--------------------
 1 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 1cbd21c..3d25156 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -1,4 +1,4 @@
-           QEMU Monitor Protocol Draft Specification - Version 0.1
+           QEMU Monitor Protocol Specification - Version 0.1
 
 1. Introduction
 ===============
@@ -27,9 +27,9 @@ the JSON standard:
 
 http://www.ietf.org/rfc/rfc4627.txt
 
-For convenience, json-objects mentioned in this document will have its members
-in a certain order. However, in real protocol usage json-objects members can
-be in ANY order, thus no particular order should be assumed.
+For convenience, json-object members and json-array elements mentioned in
+this document will be in a certain order. However, in real protocol usage
+they can be in ANY order, thus no particular order should be assumed.
 
 2.1 General Definitions
 -----------------------
@@ -85,12 +85,13 @@ without errors.
 
 The format is:
 
-{ "return": json-value, "id": json-value }
+{ "return": json-object, "id": json-value }
 
  Where,
 
 - The "return" member contains the command returned data, which is defined
-  in a per-command basis or "OK" if the command does not return data
+  in a per-command basis or an empty json-object if the command does not
+  return data
 - The "id" member contains the transaction identification associated
   with the command execution (if issued by the Client)
 
@@ -102,7 +103,7 @@ completed because of an error condition.
 
 The format is:
 
-{ "error": { "class": json-string, "data": json-value, "desc": json-string },
+{ "error": { "class": json-string, "data": json-object, "desc": json-string },
   "id": json-value }
 
  Where,
@@ -110,7 +111,7 @@ The format is:
 - The "class" member contains the error class name (eg. "ServiceUnavailable")
 - The "data" member contains specific error data and is defined in a
   per-command basis, it will be an empty json-object if the error has no data
-- The "desc" member is a human-readable error message.  Clients should
+- The "desc" member is a human-readable error message. Clients should
   not attempt to parse this message.
 - The "id" member contains the transaction identification associated with
   the command execution (if issued by the Client)
@@ -127,7 +128,7 @@ to the Client at any time. They are called 'asynchronous events'.
 
 The format is:
 
-{ "event": json-string, "data": json-value,
+{ "event": json-string, "data": json-object,
   "timestamp": { "seconds": json-number, "microseconds": json-number } }
 
  Where,
@@ -135,7 +136,7 @@ The format is:
 - The "event" member contains the event's name
 - The "data" member contains event specific data, which is defined in a
   per-event basis, it is optional
-- The "timestamp" member contains the exact time of when the event ocurred
+- The "timestamp" member contains the exact time of when the event occurred
   in the Server. It is a fixed json-object with time in seconds and
   microseconds
 
@@ -157,19 +158,20 @@ S: {"QMP": {"capabilities": []}}
 ---------------------------
 
 C: { "execute": "stop" }
-S: {"return": "OK"}
+S: {"return": {}}
 
 3.3 KVM information
 -------------------
 
-C: {"execute": "query-kvm", "id": "example"}
-S: {"return": "enabled", "id": "example"}
+C: { "execute": "query-kvm", "id": "example" }
+S: {"return": {"enabled": true, "present": true}, "id": "example"}
 
 3.4 Parsing error
 ------------------
 
 C: { "execute": }
-S: {"error": {"class": "JSONParsing", "data": {}}}
+S: {"error": {"class": "JSONParsing", "desc": "Invalid JSON syntax", "data":
+{}}}
 
 3.5 Powerdown event
 -------------------
@@ -177,19 +179,34 @@ S: {"error": {"class": "JSONParsing", "data": {}}}
 S: {"timestamp": {"seconds": 1258551470, "microseconds": 802384}, "event":
 "POWERDOWN"}
 
-4. Notes to Client implementors
--------------------------------
+4. Compatibility Considerations
+--------------------------------
 
-4.1 It is recommended to always start the Server in pause mode, thus the
-    Client is able to perform any setup procedure without the risk of
-    race conditions and related problems
+In order to achieve maximum compatibility between versions, the following
+changes are forbidden in newer versions of the Server:
 
-4.2 It is recommended to always check the capabilities json-array, issued
-    with the greeting message, at connection time
+- Removal of commands
+- Removal of command arguments
+- Addition of extra mandatory arguments for commands
+- Modification of arguments types
+- Modification of arguments, commands, events or error names
+- Modification of arguments in replies, events or errors
 
-4.3 Json-objects or json-arrays mentioned in this document are not fixed
-    and no particular size or number of members/elements should be assumed.
-    New members/elements can be added at any time.
+Likewise, Clients must not assume any particular:
 
-4.4 No particular order of json-objects members should be assumed, they
-    can change at any time
+- Size of json-objects or length of json-arrays
+- Order of json-object members or json-array elements
+- Amount of errors generated by a command, that is, new errors can be added
+  to any existing command in newer versions of the Server
+
+Additionally, Clients should always:
+
+- Check the capabilities json-array at connection time
+- Check the availability of commands with 'query-commands' before issuing them
+
+5. Recommendations to Client implementors
+-----------------------------------------
+
+5.1 The Server should be always started in pause mode, thus the Client is
+    able to perform any setup procedure without the risk of race conditions
+    and related problems
-- 
1.6.6.rc3

[Qemu-devel] [PATCH 6/7] monitor: Introduce 'M' argument type

[Luiz Capitulino]

This is a target long value in megabytes which should be
converted to bytes.

It will be used by handlers which accept a megabyte value
when in "user mode".

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 8ef1582..f779680 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3501,6 +3501,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
             break;
         case 'i':
         case 'l':
+        case 'M':
             {
                 int64_t val;
 
@@ -3531,6 +3532,8 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     monitor_printf(mon, "\'%s\' has failed: ", cmdname);
                     monitor_printf(mon, "integer is for 32-bit values\n");
                     goto fail;
+                } else if (c == 'M') {
+                    val <<= 20;
                 }
                 qdict_put(qdict, key, qint_from_int(val));
             }
@@ -3935,6 +3938,7 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
         }
         case 'i':
         case 'l':
+        case 'M':
             if (qobject_type(value) != QTYPE_QINT) {
                 qemu_error_new(QERR_INVALID_PARAMETER_TYPE, name, "int");
                 return -1;
-- 
1.6.6.rc3

[Qemu-devel] [PATCH 7/7] monitor: do_balloon(): Use 'M' argument type

[Luiz Capitulino]

This makes do_balloon() accept megabyte values from the user
Monitor while accepting byte values for QMP.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |    3 +--
 qemu-monitor.hx |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index f779680..6f5b90d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2083,8 +2083,7 @@ static void do_balloon(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     if (balloon_get_value()) {
         /* ballooning is active */
-        ram_addr_t value = qdict_get_int(qdict, "value");
-        qemu_balloon(value << 20);
+        qemu_balloon(qdict_get_int(qdict, "value"));
     }
 }
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 338e30e..aac0f3e 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -887,7 +887,7 @@ ETEXI
 
     {
         .name       = "balloon",
-        .args_type  = "value:i",
+        .args_type  = "value:M",
         .params     = "target",
         .help       = "request VM to change it's memory allocation (in MB)",
         .user_print = monitor_user_noop,
-- 
1.6.6.rc3

Re: [Qemu-devel] [PATCH 5/7] QMP: Update spec file

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

[...]
> -4. Notes to Client implementors
> --------------------------------
> +4. Compatibility Considerations
> +--------------------------------
>  
> -4.1 It is recommended to always start the Server in pause mode, thus the
> -    Client is able to perform any setup procedure without the risk of
> -    race conditions and related problems
> +In order to achieve maximum compatibility between versions, the following
> +changes are forbidden in newer versions of the Server:
>  
> -4.2 It is recommended to always check the capabilities json-array, issued
> -    with the greeting message, at connection time
> +- Removal of commands
> +- Removal of command arguments
> +- Addition of extra mandatory arguments for commands
> +- Modification of arguments types
> +- Modification of arguments, commands, events or error names
> +- Modification of arguments in replies, events or errors

While I think these promises are appropriate for a mature version of the
protocol, I do not think we should make them for 0.12.

We've just dreamed up version 0.1 of the protocol.  It hasn't been used
in anger.  Yes, we put some serious thought in it, and we even have
prototype code using it in libvirt, but let's face it, we're not
infallible: we *will* have to evolve stuff.

Without a real user, there is no real need to constrict evolution of the
protocol in such a harsh way.  All it'll buy is is compatibility cruft.
Passage of time will bring us plenty of cruft without us setting
ourselves up for extras.

Let's cut ourselves some slack here, please.

[...]

Re: [Qemu-devel] [FOR 0.12 0/7]: More QMP related fixes

[Markus Armbruster]

Apart from the compatibility promise I challenged [PATCH 5/7], the
series is fine, and I badly want it in 0.12.

Re: [Qemu-devel] [PATCH 5/7] QMP: Update spec file

[Anthony Liguori]

Markus Armbruster wrote:
> While I think these promises are appropriate for a mature version of the
> protocol, I do not think we should make them for 0.12.
>
> We've just dreamed up version 0.1 of the protocol.  It hasn't been used
> in anger.  Yes, we put some serious thought in it, and we even have
> prototype code using it in libvirt, but let's face it, we're not
> infallible: we *will* have to evolve stuff.
>
> Without a real user, there is no real need to constrict evolution of the
> protocol in such a harsh way.  All it'll buy is is compatibility cruft.
> Passage of time will bring us plenty of cruft without us setting
> ourselves up for extras.
>
> Let's cut ourselves some slack here, please.
>   

I've been working on the release notes and I was intending on announcing 
the QMP support in 0.12 as a "preview" with full support in 0.13.

The idea being that we would try to maintain compatibility but "preview" 
gives us enough slack that if we break it, we can at least claim that it 
was just a preview ;-)

> [...]
>   


-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 5/7] QMP: Update spec file

[Luiz Capitulino]

On Fri, 18 Dec 2009 11:44:27 -0600
Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:

> Markus Armbruster wrote:
> > While I think these promises are appropriate for a mature version of the
> > protocol, I do not think we should make them for 0.12.
> >
> > We've just dreamed up version 0.1 of the protocol.  It hasn't been used
> > in anger.  Yes, we put some serious thought in it, and we even have
> > prototype code using it in libvirt, but let's face it, we're not
> > infallible: we *will* have to evolve stuff.
> >
> > Without a real user, there is no real need to constrict evolution of the
> > protocol in such a harsh way.  All it'll buy is is compatibility cruft.
> > Passage of time will bring us plenty of cruft without us setting
> > ourselves up for extras.
> >
> > Let's cut ourselves some slack here, please.
> >   
> 
> I've been working on the release notes and I was intending on announcing 
> the QMP support in 0.12 as a "preview" with full support in 0.13.
> 
> The idea being that we would try to maintain compatibility but "preview" 
> gives us enough slack that if we break it, we can at least claim that it 
> was just a preview ;-)

 Should I update the spec then?

Re: [Qemu-devel] [PATCH 5/7] QMP: Update spec file

[Luiz Capitulino]

On Fri, 18 Dec 2009 18:20:52 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> [...]
> > -4. Notes to Client implementors
> > --------------------------------
> > +4. Compatibility Considerations
> > +--------------------------------
> >  
> > -4.1 It is recommended to always start the Server in pause mode, thus the
> > -    Client is able to perform any setup procedure without the risk of
> > -    race conditions and related problems
> > +In order to achieve maximum compatibility between versions, the following
> > +changes are forbidden in newer versions of the Server:
> >  
> > -4.2 It is recommended to always check the capabilities json-array, issued
> > -    with the greeting message, at connection time
> > +- Removal of commands
> > +- Removal of command arguments
> > +- Addition of extra mandatory arguments for commands
> > +- Modification of arguments types
> > +- Modification of arguments, commands, events or error names
> > +- Modification of arguments in replies, events or errors
> 
> While I think these promises are appropriate for a mature version of the
> protocol, I do not think we should make them for 0.12.
> 
> We've just dreamed up version 0.1 of the protocol.  It hasn't been used
> in anger.  Yes, we put some serious thought in it, and we even have
> prototype code using it in libvirt, but let's face it, we're not
> infallible: we *will* have to evolve stuff.

 While I agree with your arguments, I think this will happen to any
QMP version or any stable protocol/API.

 You have the point that QMP is immature at this point, but:

1. Given the current Monitor design, if you want QMP to be perfect to be
useful then we're going to have it around QEMU version 10.0

2. I don't think we're going to get any serious user until we
declare it stable

Re: [Qemu-devel] [PATCH 5/7] QMP: Update spec file

[Markus Armbruster]

Anthony Liguori <aliguori@linux.vnet.ibm.com> writes:

> Markus Armbruster wrote:
>> While I think these promises are appropriate for a mature version of the
>> protocol, I do not think we should make them for 0.12.
>>
>> We've just dreamed up version 0.1 of the protocol.  It hasn't been used
>> in anger.  Yes, we put some serious thought in it, and we even have
>> prototype code using it in libvirt, but let's face it, we're not
>> infallible: we *will* have to evolve stuff.
>>
>> Without a real user, there is no real need to constrict evolution of the
>> protocol in such a harsh way.  All it'll buy is is compatibility cruft.
>> Passage of time will bring us plenty of cruft without us setting
>> ourselves up for extras.
>>
>> Let's cut ourselves some slack here, please.
>>   
>
> I've been working on the release notes and I was intending on
> announcing the QMP support in 0.12 as a "preview" with full support in
> 0.13.
>
> The idea being that we would try to maintain compatibility but
> "preview" gives us enough slack that if we break it, we can at least
> claim that it was just a preview ;-)

Works for me.  But then the unconditional promise in this patch is
misleading.  If we care, we should amend it to spell out "this is
preview, all promises are null and void".

Re: [Qemu-devel] [PATCH 5/7] QMP: Update spec file

[Luiz Capitulino]

On Fri, 18 Dec 2009 19:06:18 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Anthony Liguori <aliguori@linux.vnet.ibm.com> writes:
> 
> > Markus Armbruster wrote:
> >> While I think these promises are appropriate for a mature version of the
> >> protocol, I do not think we should make them for 0.12.
> >>
> >> We've just dreamed up version 0.1 of the protocol.  It hasn't been used
> >> in anger.  Yes, we put some serious thought in it, and we even have
> >> prototype code using it in libvirt, but let's face it, we're not
> >> infallible: we *will* have to evolve stuff.
> >>
> >> Without a real user, there is no real need to constrict evolution of the
> >> protocol in such a harsh way.  All it'll buy is is compatibility cruft.
> >> Passage of time will bring us plenty of cruft without us setting
> >> ourselves up for extras.
> >>
> >> Let's cut ourselves some slack here, please.
> >>   
> >
> > I've been working on the release notes and I was intending on
> > announcing the QMP support in 0.12 as a "preview" with full support in
> > 0.13.
> >
> > The idea being that we would try to maintain compatibility but
> > "preview" gives us enough slack that if we break it, we can at least
> > claim that it was just a preview ;-)
> 
> Works for me.  But then the unconditional promise in this patch is
> misleading.  If we care, we should amend it to spell out "this is
> preview, all promises are null and void".

 I'd prefer to just drop this part.

Re: [Qemu-devel] [PATCH 5/7] QMP: Update spec file

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Fri, 18 Dec 2009 18:20:52 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>> 
>> [...]
>> > -4. Notes to Client implementors
>> > --------------------------------
>> > +4. Compatibility Considerations
>> > +--------------------------------
>> >  
>> > -4.1 It is recommended to always start the Server in pause mode, thus the
>> > -    Client is able to perform any setup procedure without the risk of
>> > -    race conditions and related problems
>> > +In order to achieve maximum compatibility between versions, the following
>> > +changes are forbidden in newer versions of the Server:
>> >  
>> > -4.2 It is recommended to always check the capabilities json-array, issued
>> > -    with the greeting message, at connection time
>> > +- Removal of commands
>> > +- Removal of command arguments
>> > +- Addition of extra mandatory arguments for commands
>> > +- Modification of arguments types
>> > +- Modification of arguments, commands, events or error names
>> > +- Modification of arguments in replies, events or errors
>> 
>> While I think these promises are appropriate for a mature version of the
>> protocol, I do not think we should make them for 0.12.
>> 
>> We've just dreamed up version 0.1 of the protocol.  It hasn't been used
>> in anger.  Yes, we put some serious thought in it, and we even have
>> prototype code using it in libvirt, but let's face it, we're not
>> infallible: we *will* have to evolve stuff.
>
>  While I agree with your arguments, I think this will happen to any
> QMP version or any stable protocol/API.
>
>  You have the point that QMP is immature at this point, but:
>
> 1. Given the current Monitor design, if you want QMP to be perfect to be
> useful then we're going to have it around QEMU version 10.0

No human creation will ever be perfect.

> 2. I don't think we're going to get any serious user until we
> declare it stable

You're quite right about that.  But right now QMP doesn't cover enough
ground for serious use anyway.  Once it does, we'll soon be compelled to
declare it stable, and live with the consequences.

Re: [Qemu-devel] [PATCH 3/7] QMP: Assure that returned data is a QDict

[Nathan Baum]

On Fri, 2009-12-18 at 13:25 -0200, Luiz Capitulino wrote:
> This is for debug purposes only.

This breaks quite a lot of commands where the returned data is a QList,
e.g. query-commands, query-mice, query-cpus. Is the assert wrong, or are
such commands meant to be returning a QDict?

> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  monitor.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index d238660..8ef1582 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -283,6 +283,7 @@ static void monitor_protocol_emitter(Monitor *mon, QObject *data)
>      if (!monitor_has_error(mon)) {
>          /* success response */
>          if (data) {
> +            assert(qobject_type(data) == QTYPE_QDICT);
>              qobject_incref(data);
>              qdict_put_obj(qmp, "return", data);
>          } else {

Re: [Qemu-devel] [PATCH 3/7] QMP: Assure that returned data is a QDict

[Luiz Capitulino]

On Mon, 21 Dec 2009 19:21:18 +0000
Nathan Baum <nathan@parenthephobia.org.uk> wrote:

> On Fri, 2009-12-18 at 13:25 -0200, Luiz Capitulino wrote:
> > This is for debug purposes only.
> 
> This breaks quite a lot of commands where the returned data is a QList,
> e.g. query-commands, query-mice, query-cpus. Is the assert wrong, or are
> such commands meant to be returning a QDict?

 The assert is wrong, as we've defined that returning a QList of
QDicts is ok.

 We could check for a QList too and check its contents but I think that
only dropping the assert is ok for now.

 Will submit a patch and thanks for testing QMP.