From nobody Tue Nov 4 18:40:45 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 1530609227228533.577813119618; Tue, 3 Jul 2018 02:13:47 -0700 (PDT) Received: from localhost ([::1]:39021 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1faHNY-0001DS-FU for importer@patchew.org; Tue, 03 Jul 2018 05:13:44 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53159) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1faH4b-0001VU-MO for qemu-devel@nongnu.org; Tue, 03 Jul 2018 04:54:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1faH4V-0007DW-7L for qemu-devel@nongnu.org; Tue, 03 Jul 2018 04:54:09 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:51084 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 1faH4V-0007Cz-1V for qemu-devel@nongnu.org; Tue, 03 Jul 2018 04:54:03 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8A799F68B2; Tue, 3 Jul 2018 08:54:02 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-126.ams2.redhat.com [10.36.116.126]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3E9B02027047; Tue, 3 Jul 2018 08:54:02 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id C09FF1132D3C; Tue, 3 Jul 2018 10:53:58 +0200 (CEST) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Tue, 3 Jul 2018 10:53:38 +0200 Message-Id: <20180703085358.13941-13-armbru@redhat.com> In-Reply-To: <20180703085358.13941-1-armbru@redhat.com> References: <20180703085358.13941-1-armbru@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 03 Jul 2018 08:54:02 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Tue, 03 Jul 2018 08:54:02 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.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 v2 12/32] qmp: Redo how the client requests out-of-band execution 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: mdroth@linux.vnet.ibm.com, 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" Commit cf869d53172 "qmp: support out-of-band (oob) execution" added a general mechanism for command-independent arguments just for an out-of-band flag: The "control" key is introduced to store this extra flag. "control" field is used to store arguments that are shared by all the commands, rather than command specific arguments. Let "run-oob" be the first. However, it failed to reject unknown members of "control". For instance, in QMP command {"execute": "query-name", "id": 42, "control": {"crap": true}} "crap" gets silently ignored. Instead of fixing this, revert the general "control" mechanism (because YAGNI), and do it the way I initially proposed, with key "exec-oob". Simpler code, simper interface. An out-of-band command {"execute": "migrate-pause", "id": 42, "control": {"run-oob": true}} becomes {"exec-oob": "migrate-pause", "id": 42} Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake --- docs/devel/qapi-code-gen.txt | 10 +++---- docs/interop/qmp-spec.txt | 18 ++++++------- monitor.c | 3 +++ qapi/qmp-dispatch.c | 51 +++++++++++++++--------------------- tests/qmp-test.c | 7 ++--- tests/test-qga.c | 3 +-- 6 files changed, 39 insertions(+), 53 deletions(-) diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt index 9625798d16..f020f6bab2 100644 --- a/docs/devel/qapi-code-gen.txt +++ b/docs/devel/qapi-code-gen.txt @@ -649,13 +649,11 @@ example: { 'command': 'migrate_recover', 'data': { 'uri': 'str' }, 'allow-oob': true } =20 -To execute a command with out-of-band priority, the client specifies -the "control" field in the request, with "run-oob" set to -true. Example: +To execute a command with out-of-band priority, the client uses key +"exec-oob" instead of "execute". Example: =20 - =3D> { "execute": "command-support-oob", - "arguments": { ... }, - "control": { "run-oob": true } } + =3D> { "exec-oob": "migrate-recover", + "arguments": { "uri": "tcp:192.168.1.200:12345" } } <=3D { "return": { } } =20 Without it, even the commands that support out-of-band execution will diff --git a/docs/interop/qmp-spec.txt b/docs/interop/qmp-spec.txt index a1d6f9ee06..1566b8ae5e 100644 --- a/docs/interop/qmp-spec.txt +++ b/docs/interop/qmp-spec.txt @@ -92,12 +92,16 @@ Currently supported capabilities are: =20 The format for command execution is: =20 -{ "execute": json-string, "arguments": json-object, "id": json-value, - "control": json-object } +{ "execute": json-string, "arguments": json-object, "id": json-value } + +or + +{ "exec-oob": json-string, "arguments": json-object, "id": json-value } =20 Where, =20 -- The "execute" member identifies the command to be executed by the Server +- The "execute" or "exec-oob" member identifies the command to be + executed by the server. The latter requests out-of-band execution. - The "arguments" member is used to pass any arguments required for the execution of the command, it is optional when no arguments are required. Each command documents what contents will be considered @@ -106,9 +110,6 @@ The format for command execution is: command execution, it is optional and will be part of the response if provided. The "id" member can be any json-value. A json-number incremented for each successive command works fine. -- The optional "control" member further specifies how the command is - to be executed. Currently, its only member is optional "run-oob". - See section "2.3.1 Out-of-band execution" for details. =20 2.3.1 Out-of-band execution --------------------------- @@ -129,9 +130,6 @@ To be able to match responses back to their commands, t= he client needs to pass "id" with out-of-band commands. Passing it with all commands is recommended for clients that accept capability "oob". =20 -To execute a command out-of-band, the client puts "run-oob": true into -execute's member "control". - If the client sends in-band commands faster than the server can execute them, the server will eventually drop commands to limit the queue length. The sever sends event COMMAND_DROPPED then. @@ -274,7 +272,7 @@ S: { "timestamp": { "seconds": 1258551470, "microsecond= s": 802384 }, 3.7 Out-of-band execution ------------------------- =20 -C: { "execute": "migrate-pause", "id": 42, "control": { "run-oob": true } } +C: { "exec-oob": "migrate-pause", "id": 42 } S: { "id": 42, "error": { "class": "GenericError", "desc": "migrate-pause is currently only supported during postcopy-a= ctive state" } } diff --git a/monitor.c b/monitor.c index 2c28b05ecb..8b0c29ce06 100644 --- a/monitor.c +++ b/monitor.c @@ -1299,6 +1299,9 @@ static bool qmp_cmd_oob_check(Monitor *mon, QDict *re= q, Error **errp) QmpCommand *cmd; =20 command =3D qdict_get_try_str(req, "execute"); + if (!command) { + command =3D qdict_get_try_str(req, "exec-oob"); + } if (!command) { error_setg(errp, "Command field 'execute' missing"); return false; diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 0ad0fab8ed..12be120fe7 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -23,11 +23,11 @@ QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, Error **errp) { + const char *exec_key =3D NULL; const QDictEntry *ent; const char *arg_name; const QObject *arg_obj; - bool has_exec_key =3D false; - QDict *dict =3D NULL; + QDict *dict; =20 dict =3D qobject_to(QDict, request); if (!dict) { @@ -40,25 +40,25 @@ QDict *qmp_dispatch_check_obj(const QObject *request, b= ool allow_oob, arg_name =3D qdict_entry_key(ent); arg_obj =3D qdict_entry_value(ent); =20 - if (!strcmp(arg_name, "execute")) { + if (!strcmp(arg_name, "execute") + || (!strcmp(arg_name, "exec-oob") && allow_oob)) { if (qobject_type(arg_obj) !=3D QTYPE_QSTRING) { - error_setg(errp, - "QMP input member 'execute' must be a string"); + error_setg(errp, "QMP input member '%s' must be a string", + arg_name); return NULL; } - has_exec_key =3D true; + if (exec_key) { + error_setg(errp, "QMP input member '%s' clashes with '%s'", + arg_name, exec_key); + return NULL; + } + exec_key =3D arg_name; } else if (!strcmp(arg_name, "arguments")) { if (qobject_type(arg_obj) !=3D QTYPE_QDICT) { error_setg(errp, "QMP input member 'arguments' must be an object= "); return NULL; } - } else if (!strcmp(arg_name, "control") && allow_oob) { - if (qobject_type(arg_obj) !=3D QTYPE_QDICT) { - error_setg(errp, - "QMP input member 'control' must be a dict"); - return NULL; - } } else { error_setg(errp, "QMP input member '%s' is unexpected", arg_name); @@ -66,7 +66,7 @@ QDict *qmp_dispatch_check_obj(const QObject *request, boo= l allow_oob, } } =20 - if (!has_exec_key) { + if (!exec_key) { error_setg(errp, "QMP input lacks member 'execute'"); return NULL; } @@ -88,7 +88,11 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QO= bject *request, return NULL; } =20 - command =3D qdict_get_str(dict, "execute"); + command =3D qdict_get_try_str(dict, "execute"); + if (!command) { + assert(allow_oob); + command =3D qdict_get_str(dict, "exec-oob"); + } cmd =3D qmp_find_command(cmds, command); if (cmd =3D=3D NULL) { error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, @@ -137,25 +141,12 @@ QObject *qmp_build_error_object(Error *err) } =20 /* - * Detect whether a request should be run out-of-band, by quickly - * peeking at whether we have: { "control": { "run-oob": true } }. By - * default commands are run in-band. + * Does @qdict look like a command to be run out-of-band? */ bool qmp_is_oob(QDict *dict) { - QBool *bool_obj; - - dict =3D qdict_get_qdict(dict, "control"); - if (!dict) { - return false; - } - - bool_obj =3D qobject_to(QBool, qdict_get(dict, "run-oob")); - if (!bool_obj) { - return false; - } - - return qbool_get_bool(bool_obj); + return qdict_haskey(dict, "exec-oob") + && !qdict_haskey(dict, "execute"); } =20 QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request, diff --git a/tests/qmp-test.c b/tests/qmp-test.c index d73730945a..3f4e43bd2b 100644 --- a/tests/qmp-test.c +++ b/tests/qmp-test.c @@ -177,8 +177,7 @@ static void unblock_blocked_cmd(void) =20 static void send_oob_cmd_that_fails(QTestState *s, const char *id) { - qtest_async_qmp(s, "{ 'execute': 'migrate-pause', 'id': %s," - " 'control': { 'run-oob': true } }", id); + qtest_async_qmp(s, "{ 'exec-oob': 'migrate-pause', 'id': %s }", id); } =20 static void recv_cmd_id(QTestState *s, const char *id) @@ -230,9 +229,7 @@ static void test_qmp_oob(void) * Try any command that does not support OOB but with OOB flag. We * should get failure. */ - resp =3D qtest_qmp(qts, - "{ 'execute': 'query-cpus'," - " 'control': { 'run-oob': true } }"); + resp =3D qtest_qmp(qts, "{ 'exec-oob': 'query-cpus' }"); g_assert(qdict_haskey(resp, "error")); qobject_unref(resp); =20 diff --git a/tests/test-qga.c b/tests/test-qga.c index febabc7ad5..daadf22ea3 100644 --- a/tests/test-qga.c +++ b/tests/test-qga.c @@ -249,8 +249,7 @@ static void test_qga_invalid_oob(gconstpointer fix) QDict *ret, *error; const char *class; =20 - ret =3D qmp_fd(fixture->fd, "{'execute': 'guest-ping'," - " 'control': {'run-oob': true}}"); + ret =3D qmp_fd(fixture->fd, "{'exec-oob': 'guest-ping'}"); g_assert_nonnull(ret); =20 error =3D qdict_get_qdict(ret, "error"); --=20 2.17.1