[Qemu-devel] [PULL v2 09/32] qmp qemu-ga: Revert change that accidentally made qemu-ga accept "id"

[Markus Armbruster]

Commit cf869d53172 "qmp: support out-of-band (oob) execution" changed
how we check "id":

    Note that in the patch I exported qmp_dispatch_check_obj() to be
    used to check the request earlier, and at the same time allowed
    "id" field to be there since actually we always allow that.

The part after "and" is ill-advised: it makes qemu-ga accept and
ignore "id".  Revert.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
---
 monitor.c           | 14 ++++++++------
 qapi/qmp-dispatch.c |  2 --
 tests/test-qga.c    | 13 +++++--------
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/monitor.c b/monitor.c
index 0dd6e22463..7245ee37ce 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4264,7 +4264,7 @@ static void monitor_qmp_bh_dispatcher(void *data)
 static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
 {
     QObject *req, *id = NULL;
-    QDict *qdict = NULL;
+    QDict *qdict;
     MonitorQMP *mon_qmp = container_of(parser, MonitorQMP, parser);
     Monitor *mon = container_of(mon_qmp, Monitor, qmp);
     Error *err = NULL;
@@ -4279,6 +4279,12 @@ static void handle_qmp_command(JSONMessageParser 
*parser, GQueue *tokens)
         goto err;
     }
 
+    qdict = qobject_to(QDict, req);
+    if (qdict) {
+        id = qobject_ref(qdict_get(qdict, "id"));
+        qdict_del(qdict, "id");
+    } /* else will fail qmp_dispatch() */
+
     /* Check against the request in general layout */
     qdict = qmp_dispatch_check_obj(req, &err);
     if (!qdict) {
@@ -4290,16 +4296,12 @@ static void handle_qmp_command(JSONMessageParser 
*parser, GQueue *tokens)
         goto err;
     }
 
-    id = qdict_get(qdict, "id");
-
     req_obj = g_new0(QMPRequest, 1);
     req_obj->mon = mon;
-    req_obj->id = qobject_ref(id);
+    req_obj->id = id;
     req_obj->req = req;
     req_obj->need_resume = false;
 
-    qdict_del(qdict, "id");
-
     if (qmp_is_oob(qdict)) {
         /* Out-of-band (OOB) requests are executed directly in parser. */
         trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(req_obj->id)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 935f9e159c..3d5d5e110f 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -52,8 +52,6 @@ QDict *qmp_dispatch_check_obj(const QObject *request, Error 
**errp)
                            "QMP input member 'arguments' must be an object");
                 return NULL;
             }
-        } else if (!strcmp(arg_name, "id")) {
-            continue;
         } else if (!strcmp(arg_name, "control")) {
             if (qobject_type(arg_obj) != QTYPE_QDICT) {
                 error_setg(errp,
diff --git a/tests/test-qga.c b/tests/test-qga.c
index 6b632e3da4..564a4594b5 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -229,19 +229,16 @@ static void test_qga_ping(gconstpointer fix)
 
 static void test_qga_invalid_id(gconstpointer fix)
 {
-    /*
-     * FIXME "id" is ignored; it should either be repeated in the
-     * reply, or rejected on input
-     */
     const TestFixture *fixture = fix;
-    QDict *ret, *val;
+    QDict *ret, *error;
+    const char *class;
 
     ret = qmp_fd(fixture->fd, "{'execute': 'guest-ping', 'id': 1}");
     g_assert_nonnull(ret);
-    qmp_assert_no_error(ret);
 
-    val = qdict_get_qdict(ret, "return");
-    g_assert(!qdict_haskey(val, "id"));
+    error = qdict_get_qdict(ret, "error");
+    class = qdict_get_try_str(error, "class");
+    g_assert_cmpstr(class, ==, "GenericError");
 
     qobject_unref(ret);
 }
-- 
2.17.1