diff mbox

[PULL,06/14] qmp: fix qmp_capabilities error regression

Message ID 20180327143014.1767669-7-eblake@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Blake March 27, 2018, 2:30 p.m. UTC
From: Peter Xu <peterx@redhat.com>

When someone sends a command before QMP handshake, the error used to be
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>
Message-Id: <20180326063901.27425-2-peterx@redhat.com>
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: commit message grammar tweaks]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 monitor.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

Comments

Marc-André Lureau March 27, 2018, 3:01 p.m. UTC | #1
On Tue, Mar 27, 2018 at 4:30 PM, Eric Blake <eblake@redhat.com> wrote:
> From: Peter Xu <peterx@redhat.com>
>
> When someone sends a command before QMP handshake, the error used to be
> 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>
> Message-Id: <20180326063901.27425-2-peterx@redhat.com>
> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by (not x2 Reported-by ;)

> Reviewed-by: Eric Blake <eblake@redhat.com>
> [eblake: commit message grammar tweaks]
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  monitor.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index de709fc2e5d..3b1ef34711b 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
>
>
Eric Blake March 27, 2018, 3:16 p.m. UTC | #2
On 03/27/2018 10:01 AM, Marc-André Lureau wrote:
> On Tue, Mar 27, 2018 at 4:30 PM, Eric Blake <eblake@redhat.com> wrote:
>> From: Peter Xu <peterx@redhat.com>
>>
>> When someone sends a command before QMP handshake, the error used to be
>> 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>
>> Message-Id: <20180326063901.27425-2-peterx@redhat.com>
>> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Reviewed-by (not x2 Reported-by ;)
> 

Shoot.  Want me to send a v2 pull request?
Marc-André Lureau March 27, 2018, 3:31 p.m. UTC | #3
On Tue, Mar 27, 2018 at 5:16 PM, Eric Blake <eblake@redhat.com> wrote:
> On 03/27/2018 10:01 AM, Marc-André Lureau wrote:
>>
>> On Tue, Mar 27, 2018 at 4:30 PM, Eric Blake <eblake@redhat.com> wrote:
>>>
>>> From: Peter Xu <peterx@redhat.com>
>>>
>>> When someone sends a command before QMP handshake, the error used to be
>>> 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>
>>> Message-Id: <20180326063901.27425-2-peterx@redhat.com>
>>> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>>
>> Reviewed-by (not x2 Reported-by ;)
>>
>
> Shoot.  Want me to send a v2 pull request?

no need ;)
diff mbox

Patch

diff --git a/monitor.c b/monitor.c
index de709fc2e5d..3b1ef34711b 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);