Commit 00ecec15 authored by Markus Armbruster's avatar Markus Armbruster
Browse files

qmp: Redo how the client requests out-of-band execution 



Commit cf869d53 "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, simpler 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: default avatarMarkus Armbruster <armbru@redhat.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-13-armbru@redhat.com>
[Commit message typo fixed]
parent 674ed722

docs/devel/qapi-code-gen.txt

+4 −6
Original line number Diff line number Diff line
@@ -649,13 +649,11 @@ example: 
 { 'command': 'migrate_recover',

   'data': { 'uri': 'str' }, 'allow-oob': true }



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:



 => { "execute": "command-support-oob",

      "arguments": { ... },

      "control": { "run-oob": true } }

 => { "exec-oob": "migrate-recover",

      "arguments": { "uri": "tcp:192.168.1.200:12345" } }

 <= { "return": { } }



Without it, even the commands that support out-of-band execution will

docs/interop/qmp-spec.txt

+8 −10
Original line number Diff line number Diff line
@@ -92,12 +92,16 @@ Currently supported capabilities are: 


The format for command execution is:



{ "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 }



 Where,



- 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.



2.3.1 Out-of-band execution

---------------------------
@@ -129,9 +130,6 @@ To be able to match responses back to their commands, the client needs 
to pass "id" with out-of-band commands.  Passing it with all commands

is recommended for clients that accept capability "oob".



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, "microseconds": 802384 }, 
3.7 Out-of-band execution

-------------------------



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-active state" } }

monitor.c

+3 −0
Original line number Diff line number Diff line
@@ -1300,6 +1300,9 @@ static bool qmp_cmd_oob_check(Monitor *mon, QDict *req, Error **errp) 
    QmpCommand *cmd;



    command = qdict_get_try_str(req, "execute");

    if (!command) {

        command = qdict_get_try_str(req, "exec-oob");

    }

    if (!command) {

        error_setg(errp, "Command field 'execute' missing");

        return false;

qapi/qmp-dispatch.c

+21 −30
Original line number Diff line number Diff line
@@ -23,11 +23,11 @@ 
QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob,

                              Error **errp)

{

    const char *exec_key = NULL;

    const QDictEntry *ent;

    const char *arg_name;

    const QObject *arg_obj;

    bool has_exec_key = false;

    QDict *dict = NULL;

    QDict *dict;



    dict = qobject_to(QDict, request);

    if (!dict) {
@@ -40,23 +40,23 @@ QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, 
        arg_name = qdict_entry_key(ent);

        arg_obj = qdict_entry_value(ent);



        if (!strcmp(arg_name, "execute")) {

        if (!strcmp(arg_name, "execute")

            || (!strcmp(arg_name, "exec-oob") && allow_oob)) {

            if (qobject_type(arg_obj) != 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 = true;

        } else if (!strcmp(arg_name, "arguments")) {

            if (qobject_type(arg_obj) != QTYPE_QDICT) {

                error_setg(errp,

                           "QMP input member 'arguments' must be an object");

            if (exec_key) {

                error_setg(errp, "QMP input member '%s' clashes with '%s'",

                           arg_name, exec_key);

                return NULL;

            }

        } else if (!strcmp(arg_name, "control") && allow_oob) {

            exec_key = arg_name;

        } else if (!strcmp(arg_name, "arguments")) {

            if (qobject_type(arg_obj) != QTYPE_QDICT) {

                error_setg(errp,

                           "QMP input member 'control' must be a dict");

                           "QMP input member 'arguments' must be an object");

                return NULL;

            }

        } else {
@@ -66,7 +66,7 @@ QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, 
        }

    }



    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, QObject *request, 
        return NULL;

    }



    command = qdict_get_str(dict, "execute");

    command = qdict_get_try_str(dict, "execute");

    if (!command) {

        assert(allow_oob);

        command = qdict_get_str(dict, "exec-oob");

    }

    cmd = qmp_find_command(cmds, command);

    if (cmd == NULL) {

        error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
@@ -137,25 +141,12 @@ QObject *qmp_build_error_object(Error *err) 
}



/*

 * 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 = qdict_get_qdict(dict, "control");

    if (!dict) {

        return false;

    }



    bool_obj = 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");

}



QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request,

tests/qmp-test.c

+2 −5
Original line number Diff line number Diff line
@@ -176,8 +176,7 @@ static void unblock_blocked_cmd(void) 


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);

}



static void recv_cmd_id(QTestState *s, const char *id)
@@ -229,9 +228,7 @@ static void test_qmp_oob(void) 
     * Try any command that does not support OOB but with OOB flag. We

     * should get failure.

     */

    resp = qtest_qmp(qts,

                     "{ 'execute': 'query-cpus',"

                     "  'control': { 'run-oob': true } }");

    resp = qtest_qmp(qts, "{ 'exec-oob': 'query-cpus' }");

    g_assert(qdict_haskey(resp, "error"));

    qobject_unref(resp);