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

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

[Luiz Capitulino]

The following changes since commit e5b3a24181ea0cebf1c5b20f44d016311b7048f0:

  Update version for v2.3.0 release (2015-04-24 15:05:06 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/qmp-unstable.git tags/for-upstream

for you to fetch changes up to 2d5a8346a484250934526a15b3a522bdba7e6772:

  qmp: Give saner messages related to qmp_capabilities misuse (2015-04-24 14:18:06 -0400)

----------------------------------------------------------------
Four little fixes

----------------------------------------------------------------
Eric Blake (2):
      qapi: Drop dead genlist parameter
      qmp: Give saner messages related to qmp_capabilities misuse

Luiz Capitulino (1):
      balloon: improve error msg when adding second device

Paolo Bonzini (1):
      qmp-commands: fix incorrect uses of ":O" specifier

 balloon.c                  |  1 -
 hw/virtio/virtio-balloon.c |  2 +-
 monitor.c                  | 23 +++++++++++++++++++----
 qmp-commands.hx            |  4 ++--
 scripts/qapi-visit.py      | 22 +++++++++-------------
 5 files changed, 31 insertions(+), 21 deletions(-)

[Qemu-devel] [PULL 1/4] balloon: improve error msg when adding second device

[Luiz Capitulino]

A VM supports only one balloon device, but due to several changes
in infrastructure the error message got messed up when trying
to add a second device. Fix it.

Before this fix

Command-line:

qemu-qmp: -device virtio-balloon-pci,id=balloon0: Another balloon device already registered
qemu-qmp: -device virtio-balloon-pci,id=balloon0: Adding balloon handler failed
qemu-qmp: -device virtio-balloon-pci,id=balloon0: Device 'virtio-balloon-pci' could not be initialized

HMP:

Another balloon device already registered
Adding balloon handler failed
Device 'virtio-balloon-pci' could not be initialized

QMP:

{ "execute": "device_add", "arguments": { "driver": "virtio-balloon-pci", "id": "balloon0" } }
{
	"error": {
		"class": "GenericError",
		"desc": "Adding balloon handler failed"
	}
}

After this fix

Command-line:

qemu-qmp: -device virtio-balloon-pci,id=balloon0: Only one balloon device is supported
qemu-qmp: -device virtio-balloon-pci,id=balloon0: Device 'virtio-balloon-pci' could not be initialized

HMP:

(qemu) device_add virtio-balloon-pci,id=balloon0
Only one balloon device is supported
Device 'virtio-balloon-pci' could not be initialized
(qemu)

QMP:

{ "execute": "device_add",
          "arguments": { "driver": "virtio-balloon-pci", "id": "balloon0" } }
{
    "error": {
        "class": "GenericError",
        "desc": "Only one balloon device is supported"
    }
}

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 balloon.c                  | 1 -
 hw/virtio/virtio-balloon.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/balloon.c b/balloon.c
index 70c00f5..c7033e3 100644
--- a/balloon.c
+++ b/balloon.c
@@ -58,7 +58,6 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
         /* We're already registered one balloon handler.  How many can
          * a guest really have?
          */
-        error_report("Another balloon device already registered");
         return -1;
     }
     balloon_event_fn = event_func;
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 95b0643..484c3c3 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -383,7 +383,7 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
                                    virtio_balloon_stat, s);
 
     if (ret < 0) {
-        error_setg(errp, "Adding balloon handler failed");
+        error_setg(errp, "Only one balloon device is supported");
         virtio_cleanup(vdev);
         return;
     }
-- 
1.9.3

[Qemu-devel] [PULL 2/4] qapi: Drop dead genlist parameter

[Luiz Capitulino]

From: Eric Blake <eblake@redhat.com>

Defaulting a parameter to True, then having all callers omit or
pass an explicit True for that parameter, is pointless. Looks
like it has been dead since introduction in commit 06d64c6, more
than 4 years ago.

Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi-visit.py | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 8f845a2..1be4d67 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -2,7 +2,7 @@
 # QAPI visitor generator
 #
 # Copyright IBM, Corp. 2011
-# Copyright (C) 2014 Red Hat, Inc.
+# Copyright (C) 2014-2015 Red Hat, Inc.
 #
 # Authors:
 #  Anthony Liguori <aliguori@us.ibm.com>
@@ -401,34 +401,31 @@ out:
 
     return ret
 
-def generate_declaration(name, members, genlist=True, builtin_type=False):
+def generate_declaration(name, members, builtin_type=False):
     ret = ""
     if not builtin_type:
         ret += mcgen('''
 
 void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp);
 ''',
-                    name=name)
+                     name=name)
 
-    if genlist:
-        ret += mcgen('''
+    ret += mcgen('''
 void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp);
 ''',
                  name=name)
 
     return ret
 
-def generate_enum_declaration(name, members, genlist=True):
-    ret = ""
-    if genlist:
-        ret += mcgen('''
+def generate_enum_declaration(name, members):
+    ret = mcgen('''
 void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp);
 ''',
-                     name=name)
+                name=name)
 
     return ret
 
-def generate_decl_enum(name, members, genlist=True):
+def generate_decl_enum(name, members):
     return mcgen('''
 
 void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp);
@@ -542,8 +539,7 @@ exprs = parse_schema(input_file)
 # for built-in types in our header files and simply guard them
 fdecl.write(guardstart("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
 for typename in builtin_types:
-    fdecl.write(generate_declaration(typename, None, genlist=True,
-                                     builtin_type=True))
+    fdecl.write(generate_declaration(typename, None, builtin_type=True))
 fdecl.write(guardend("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
 
 # ...this doesn't work for cases where we link in multiple objects that
-- 
1.9.3

[Qemu-devel] [PULL 3/4] qmp-commands: fix incorrect uses of ":O" specifier

[Luiz Capitulino]

From: Paolo Bonzini <pbonzini@redhat.com>

As far as the QMP parser is concerned, neither the 'O' nor the 'q' format specifiers
put any constraint on the command.  However, there are two differences:

1) from a documentation point of view 'O' says that this command takes
a dictionary.  The dictionary will be converted to QemuOpts in the
handler to match the corresponding HMP command.

2) 'O' sets QMP_ACCEPT_UNKNOWNS, resulting in the command accepting invalid
extra arguments.  For example the following is accepted:

   { "execute": "send-key",
        "arguments": { "keys": [ { "type": "qcode", "data": "ctrl" },
                                 { "type": "qcode", "data": "alt" },
                                 { "type": "qcode", "data": "delete" } ], "foo": "bar" } }

Neither send-key nor migrate-set-capabilities take a QemuOpts-like
dictionary; they take an array of dictionaries.  And neither command
really wants to have extra unknown arguments.  Thus, the right
specifier to use in this case is 'q'; with this patch the above
command fails with

   {"error": {"class": "GenericError", "desc": "Invalid parameter 'foo'"}}

as intended.

Reported-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qmp-commands.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3a42ad0..09f48ba 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -332,7 +332,7 @@ EQMP
 
     {
         .name       = "send-key",
-        .args_type  = "keys:O,hold-time:i?",
+        .args_type  = "keys:q,hold-time:i?",
         .mhandler.cmd_new = qmp_marshal_input_send_key,
     },
 
@@ -3288,7 +3288,7 @@ EQMP
 
     {
         .name       = "migrate-set-capabilities",
-        .args_type  = "capabilities:O",
+        .args_type  = "capabilities:q",
         .params     = "capability:s,state:b",
 	.mhandler.cmd_new = qmp_marshal_input_migrate_set_capabilities,
     },
-- 
1.9.3

[Qemu-devel] [PULL 4/4] qmp: Give saner messages related to qmp_capabilities misuse

[Luiz Capitulino]

From: Eric Blake <eblake@redhat.com>

Pretending that QMP doesn't understand a command merely because
we are not in the right mode doesn't help first-time users figure
out what to do to correct things.  Although the documentation for
QMP calls out capabilities negotiation, we should also make it
clear in our error messages what we were expecting.  With this
patch, I now get the following transcript:

$ ./x86_64-softmmu/qemu-system-x86_64 -qmp stdio -nodefaults
{"QMP": {"version": {"qemu": {"micro": 93, "minor": 2, "major": 2}, "package": ""}, "capabilities": []}}
{"execute":"huh"}
{"error": {"class": "CommandNotFound", "desc": "The command huh has not been found"}}
{"execute":"quit"}
{"error": {"class": "CommandNotFound", "desc": "Expecting capabilities negotiation with 'qmp_capabilities' before command 'quit'"}}
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"qmp_capabilities"}
{"error": {"class": "CommandNotFound", "desc": "Capabilities negotiation is already complete, command 'qmp_capabilities' ignored"}}
{"execute":"quit"}
{"return": {}}
{"timestamp": {"seconds": 1429110729, "microseconds": 181935}, "event": "SHUTDOWN"}

Signed-off-by: Eric Blake <eblake@redhat.com>
Tested-By: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Paulo Vital <paulo.vital@profitbricks.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 68873ec..9f37700 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4783,10 +4783,22 @@ static int monitor_can_read(void *opaque)
     return (mon->suspend_cnt == 0) ? 1 : 0;
 }
 
-static int invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd)
+static bool invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd)
 {
-    int is_cap = cmd->mhandler.cmd_new == do_qmp_capabilities;
-    return (qmp_cmd_mode(mon) ? is_cap : !is_cap);
+    bool is_cap = cmd->mhandler.cmd_new == do_qmp_capabilities;
+    if (is_cap && qmp_cmd_mode(mon)) {
+        qerror_report(ERROR_CLASS_COMMAND_NOT_FOUND,
+                      "Capabilities negotiation is already complete, command "
+                      "'%s' ignored", cmd->name);
+        return true;
+    }
+    if (!is_cap && !qmp_cmd_mode(mon)) {
+        qerror_report(ERROR_CLASS_COMMAND_NOT_FOUND,
+                      "Expecting capabilities negotiation with "
+                      "'qmp_capabilities' before command '%s'", cmd->name);
+        return true;
+    }
+    return false;
 }
 
 /*
@@ -5080,11 +5092,14 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     cmd_name = qdict_get_str(input, "execute");
     trace_handle_qmp_command(mon, cmd_name);
     cmd = qmp_find_cmd(cmd_name);
-    if (!cmd || invalid_qmp_mode(mon, cmd)) {
+    if (!cmd) {
         qerror_report(ERROR_CLASS_COMMAND_NOT_FOUND,
                       "The command %s has not been found", cmd_name);
         goto err_out;
     }
+    if (invalid_qmp_mode(mon, cmd)) {
+        goto err_out;
+    }
 
     obj = qdict_get(input, "arguments");
     if (!obj) {
-- 
1.9.3

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

[Peter Maydell]

On 24 April 2015 at 20:01, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> The following changes since commit e5b3a24181ea0cebf1c5b20f44d016311b7048f0:
>
>   Update version for v2.3.0 release (2015-04-24 15:05:06 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/qmp-unstable.git tags/for-upstream
>
> for you to fetch changes up to 2d5a8346a484250934526a15b3a522bdba7e6772:
>
>   qmp: Give saner messages related to qmp_capabilities misuse (2015-04-24 14:18:06 -0400)
>
> ----------------------------------------------------------------
> Four little fixes
>
> ----------------------------------------------------------------


Applied, thanks.

-- PMM