diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h index 303a15ba84..514bfc45b0 100644 --- a/include/qapi/qmp/dispatch.h +++ b/include/qapi/qmp/dispatch.h @@ -48,8 +48,6 @@ bool qmp_command_is_enabled(const QmpCommand *cmd); const char *qmp_command_name(const QmpCommand *cmd); bool qmp_has_success_response(const QmpCommand *cmd); QObject *qmp_build_error_object(Error *err); -QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, - Error **errp); QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request, bool allow_oob); bool qmp_is_oob(QDict *dict); diff --git a/monitor.c b/monitor.c index c49214cd8f..be2a856d1c 100644 --- a/monitor.c +++ b/monitor.c @@ -1290,48 +1290,6 @@ static void qmp_caps_apply(Monitor *mon, QMPCapabilityList *list) } } -/* - * Return true if check successful, or false otherwise. When false is - * returned, detailed error will be in errp if provided. - */ -static bool qmp_cmd_oob_check(Monitor *mon, QDict *req, Error **errp) -{ - const char *command; - 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; - } - - cmd = qmp_find_command(mon->qmp.commands, command); - if (!cmd) { - 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; - } - - 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) { @@ -4171,6 +4129,7 @@ static void monitor_qmp_dispatch(Monitor *mon, QObject *req, QObject *id) { Monitor *old_mon; QObject *rsp; + QDict *error; old_mon = cur_mon; cur_mon = mon; @@ -4179,6 +4138,19 @@ static void monitor_qmp_dispatch(Monitor *mon, QObject *req, QObject *id) cur_mon = old_mon; + if (mon->qmp.commands == &qmp_cap_negotiation_commands) { + error = 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 negotiation" + " with 'qmp_capabilities'"); + } + } + + /* Respond if necessary */ monitor_qmp_respond(mon, rsp, NULL, qobject_ref(id)); } @@ -4256,7 +4228,9 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) error_setg(&err, QERR_JSON_PARSING); } if (err) { - goto err; + assert(!req); + monitor_qmp_respond(mon, NULL, err, NULL); + return; } qdict = qobject_to(QDict, req); @@ -4271,18 +4245,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) qobject_unref(req_json); } - /* Check against the request in general layout */ - qdict = 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) ?: ""); @@ -4336,12 +4299,6 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) /* 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); } 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" -QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, - Error **errp) +static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, + Error **errp) { const char *exec_key = NULL; const QDictEntry *ent; @@ -78,6 +78,7 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, bool allow_oob, Error **errp) { Error *local_err = NULL; + bool oob; const char *command; QDict *args, *dict; QmpCommand *cmd; @@ -89,9 +90,11 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, } command = qdict_get_try_str(dict, "execute"); + oob = false; if (!command) { assert(allow_oob); command = qdict_get_str(dict, "exec-oob"); + oob = true; } cmd = qmp_find_command(cmds, command); if (cmd == 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; + } if (runstate_check(RUN_STATE_PRECONFIG) && !(cmd->options & QCO_ALLOW_PRECONFIG)) { diff --git a/tests/qmp-test.c b/tests/qmp-test.c index 9eb20a1a18..ceaf4a6789 100644 --- a/tests/qmp-test.c +++ b/tests/qmp-test.c @@ -242,12 +242,12 @@ static void test_qmp_oob(void) recv_cmd_id(qts, "ib-blocks-1"); recv_cmd_id(qts, "ib-quick-1"); - /* 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(); qtest_quit(qts);