From nobody Tue Nov 4 19:05:51 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 153054995134369.39426041159584; Mon, 2 Jul 2018 09:45:51 -0700 (PDT) Received: from localhost ([::1]:34215 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fa1xT-0005Ji-3I for importer@patchew.org; Mon, 02 Jul 2018 12:45:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42607) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fa1au-0004Sr-GV for qemu-devel@nongnu.org; Mon, 02 Jul 2018 12:22:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fa1aq-0006eG-VW for qemu-devel@nongnu.org; Mon, 02 Jul 2018 12:22:28 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:47586 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fa1aq-0006cn-JW for qemu-devel@nongnu.org; Mon, 02 Jul 2018 12:22:24 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 248767CBBA for ; Mon, 2 Jul 2018 16:22:24 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-58.ams2.redhat.com [10.36.116.58]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 770322166B5D; Mon, 2 Jul 2018 16:22:23 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 9CD941132CAA; Mon, 2 Jul 2018 18:22:18 +0200 (CEST) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Mon, 2 Jul 2018 18:22:03 +0200 Message-Id: <20180702162218.13678-18-armbru@redhat.com> In-Reply-To: <20180702162218.13678-1-armbru@redhat.com> References: <20180702162218.13678-1-armbru@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Mon, 02 Jul 2018 16:22:24 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Mon, 02 Jul 2018 16:22:24 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'armbru@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH 17/32] qmp: Don't let malformed in-band commands jump the queue X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: stefanha@redhat.com, peterx@redhat.com, dgilbert@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" handle_qmp_command() reports certain errors right away. This is wrong when OOB is enabled, because the errors can "jump the queue" then, as the previous commit demonstrates. To fix, we need to delay errors until dispatch. Do that for semantic errors, mostly by reverting ill-advised parts of commit cf869d53172 "qmp: support out-of-band (oob) execution". Bonus: doesn't run qmp_dispatch_check_obj() twice, once in handle_qmp_command(), and again in do_qmp_dispatch(). That's also due to commit cf869d53172. The next commit will fix queue jumping for syntax errors. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake qmp.commands, command); - if (!cmd) { - if (mon->qmp.commands =3D=3D &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; - } - - if (qmp_is_oob(req)) { - if (!(cmd->options & QCO_ALLOW_OOB)) { - error_setg(errp, "The command %s does not support OOB", - command); - return false; - } - } - - return true; -} - void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable, Error **errp) { @@ -4170,6 +4128,7 @@ static void monitor_qmp_dispatch(Monitor *mon, QObjec= t *req, QObject *id) { Monitor *old_mon; QObject *rsp; + QDict *error; =20 old_mon =3D cur_mon; cur_mon =3D mon; @@ -4178,6 +4137,19 @@ static void monitor_qmp_dispatch(Monitor *mon, QObje= ct *req, QObject *id) =20 cur_mon =3D old_mon; =20 + if (mon->qmp.commands =3D=3D &qmp_cap_negotiation_commands) { + error =3D qdict_get_qdict(qobject_to(QDict, rsp), "error"); + if (error + && !g_strcmp0(qdict_get_try_str(error, "class"), + QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) { + /* Provide a more useful error message */ + qdict_del(error, "desc"); + qdict_put_str(error, "desc", "Expecting capabilities negotiati= on" + " with 'qmp_capabilities'"); + } + } + + /* Respond if necessary */ monitor_qmp_respond(mon, rsp, NULL, qobject_ref(id)); } =20 @@ -4255,7 +4227,9 @@ static void handle_qmp_command(JSONMessageParser *par= ser, GQueue *tokens) error_setg(&err, QERR_JSON_PARSING); } if (err) { - goto err; + assert(!req); + monitor_qmp_respond(mon, NULL, err, NULL); + return; } =20 qdict =3D qobject_to(QDict, req); @@ -4270,18 +4244,7 @@ static void handle_qmp_command(JSONMessageParser *pa= rser, GQueue *tokens) qobject_unref(req_json); } =20 - /* Check against the request in general layout */ - qdict =3D qmp_dispatch_check_obj(req, qmp_oob_enabled(mon), &err); - if (!qdict) { - goto err; - } - - /* Check against OOB specific */ - if (!qmp_cmd_oob_check(mon, qdict, &err)) { - goto err; - } - - if (qmp_is_oob(qdict)) { + if (qdict && 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(id) ?: ""); @@ -4335,12 +4298,6 @@ static void handle_qmp_command(JSONMessageParser *pa= rser, GQueue *tokens) =20 /* Kick the dispatcher routine */ qemu_bh_schedule(mon_global.qmp_dispatcher_bh); - return; - -err: - /* FIXME overtakes queued in-band commands, wrong when !qmp_is_oob() */ - monitor_qmp_respond(mon, NULL, err, NULL); - qobject_unref(req); } =20 static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size) diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 12be120fe7..6d78f3e9f6 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -20,8 +20,8 @@ #include "qapi/qmp/qbool.h" #include "sysemu/sysemu.h" =20 -QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, - Error **errp) +static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oo= b, + Error **errp) { const char *exec_key =3D NULL; const QDictEntry *ent; @@ -78,6 +78,7 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QOb= ject *request, bool allow_oob, Error **errp) { Error *local_err =3D NULL; + bool oob; const char *command; QDict *args, *dict; QmpCommand *cmd; @@ -89,9 +90,11 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QO= bject *request, } =20 command =3D qdict_get_try_str(dict, "execute"); + oob =3D false; if (!command) { assert(allow_oob); command =3D qdict_get_str(dict, "exec-oob"); + oob =3D true; } cmd =3D qmp_find_command(cmds, command); if (cmd =3D=3D NULL) { @@ -104,6 +107,11 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, = QObject *request, command); return NULL; } + if (oob && !(cmd->options & QCO_ALLOW_OOB)) { + error_setg(errp, "The command %s does not support OOB", + command); + return false; + } =20 if (runstate_check(RUN_STATE_PRECONFIG) && !(cmd->options & QCO_ALLOW_PRECONFIG)) { diff --git a/tests/qmp-test.c b/tests/qmp-test.c index fe5e5b548a..3f29c6c305 100644 --- a/tests/qmp-test.c +++ b/tests/qmp-test.c @@ -240,12 +240,12 @@ static void test_qmp_oob(void) recv_cmd_id(qts, "ib-blocks-1"); recv_cmd_id(qts, "ib-quick-1"); =20 - /* FIXME certain in-band errors overtake slow in-band command */ + /* Even malformed in-band command fails in-band */ send_cmd_that_blocks(qts, "blocks-2"); qtest_async_qmp(qts, "{ 'id': 'err-2' }"); - recv_cmd_id(qts, NULL); unblock_blocked_cmd(); recv_cmd_id(qts, "blocks-2"); + recv_cmd_id(qts, "err-2"); cleanup_blocking_cmd(); =20 qtest_quit(qts); --=20 2.17.1