[Qemu-devel] [PATCH for-2.12 1/8] qmp: fix qmp_capabilities error regression

Peter Xu posted 8 patches 7 years, 10 months ago
[Qemu-devel] [PATCH for-2.12 1/8] qmp: fix qmp_capabilities error regression
Posted by Peter Xu 7 years, 10 months ago
When someone sents a command before QMP handshake, error was like this:

 {"execute": "query-cpus"}
 {"error": {"class": "CommandNotFound", "desc":
            "Expecting capabilities negotiation with 'qmp_capabilities'"}}

While after cf869d5317 it becomes:

 {"execute": "query-cpus"}
 {"error": {"class": "CommandNotFound", "desc":
            "The command query-cpus has not been found"}}

Fix it back to the nicer one.

Fixes: cf869d5317 ("qmp: support out-of-band (oob) execution", 2018-03-19)
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 monitor.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/monitor.c b/monitor.c
index 77f4c41cfa..849fa23bf9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1203,8 +1203,14 @@ static bool qmp_cmd_oob_check(Monitor *mon, QDict *req, Error **errp)
 
     cmd = qmp_find_command(mon->qmp.commands, command);
     if (!cmd) {
-        error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
-                  "The command %s has not been found", command);
+        if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
+            error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+                      "Expecting capabilities negotiation "
+                      "with 'qmp_capabilities'");
+        } else {
+            error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+                      "The command %s has not been found", command);
+        }
         return false;
     }
 
@@ -4027,7 +4033,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
 {
     Monitor *mon, *old_mon;
     QObject *req, *rsp = NULL, *id;
-    QDict *qdict = NULL;
     bool need_resume;
 
     req = req_obj->req;
@@ -4050,18 +4055,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
 
     cur_mon = old_mon;
 
-    if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
-        qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error");
-        if (qdict
-            && !g_strcmp0(qdict_get_try_str(qdict, "class"),
-                    QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) {
-            /* Provide a more useful error message */
-            qdict_del(qdict, "desc");
-            qdict_put_str(qdict, "desc", "Expecting capabilities negotiation"
-                          " with 'qmp_capabilities'");
-        }
-    }
-
     /* Respond if necessary */
     monitor_qmp_respond(mon, rsp, NULL, id);
 
-- 
2.14.3


Re: [Qemu-devel] [PATCH for-2.12 1/8] qmp: fix qmp_capabilities error regression
Posted by Marc-André Lureau 7 years, 10 months ago
On Mon, Mar 26, 2018 at 8:38 AM, Peter Xu <peterx@redhat.com> wrote:
> When someone sents a command before QMP handshake, error was like this:
>
>  {"execute": "query-cpus"}
>  {"error": {"class": "CommandNotFound", "desc":
>             "Expecting capabilities negotiation with 'qmp_capabilities'"}}
>
> While after cf869d5317 it becomes:
>
>  {"execute": "query-cpus"}
>  {"error": {"class": "CommandNotFound", "desc":
>             "The command query-cpus has not been found"}}
>
> Fix it back to the nicer one.
>
> Fixes: cf869d5317 ("qmp: support out-of-band (oob) execution", 2018-03-19)
> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  monitor.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 77f4c41cfa..849fa23bf9 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1203,8 +1203,14 @@ static bool qmp_cmd_oob_check(Monitor *mon, QDict *req, Error **errp)
>
>      cmd = qmp_find_command(mon->qmp.commands, command);
>      if (!cmd) {
> -        error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> -                  "The command %s has not been found", command);
> +        if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
> +            error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> +                      "Expecting capabilities negotiation "
> +                      "with 'qmp_capabilities'");
> +        } else {
> +            error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> +                      "The command %s has not been found", command);
> +        }
>          return false;
>      }
>
> @@ -4027,7 +4033,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
>  {
>      Monitor *mon, *old_mon;
>      QObject *req, *rsp = NULL, *id;
> -    QDict *qdict = NULL;
>      bool need_resume;
>
>      req = req_obj->req;
> @@ -4050,18 +4055,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
>
>      cur_mon = old_mon;
>
> -    if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
> -        qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error");
> -        if (qdict
> -            && !g_strcmp0(qdict_get_try_str(qdict, "class"),
> -                    QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) {
> -            /* Provide a more useful error message */
> -            qdict_del(qdict, "desc");
> -            qdict_put_str(qdict, "desc", "Expecting capabilities negotiation"
> -                          " with 'qmp_capabilities'");
> -        }
> -    }
> -
>      /* Respond if necessary */
>      monitor_qmp_respond(mon, rsp, NULL, id);
>
> --
> 2.14.3
>

Re: [Qemu-devel] [PATCH for-2.12 1/8] qmp: fix qmp_capabilities error regression
Posted by Eric Blake 7 years, 10 months ago
On 03/26/2018 01:38 AM, Peter Xu wrote:
> When someone sents a command before QMP handshake, error was like this:

s/sents/sends/
s/error was/the error used to be/

> 
>   {"execute": "query-cpus"}
>   {"error": {"class": "CommandNotFound", "desc":
>              "Expecting capabilities negotiation with 'qmp_capabilities'"}}
> 
> While after cf869d5317 it becomes:
> 
>   {"execute": "query-cpus"}
>   {"error": {"class": "CommandNotFound", "desc":
>              "The command query-cpus has not been found"}}
> 
> Fix it back to the nicer one.
> 
> Fixes: cf869d5317 ("qmp: support out-of-band (oob) execution", 2018-03-19)
> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   monitor.c | 23 ++++++++---------------
>   1 file changed, 8 insertions(+), 15 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org