Message ID | CAJ+F1CJ4w5q3j6EHeicPTbmPHE2hjndzqqr577DZDTpfKftd3Q@mail.gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Thu, Mar 22, 2018 at 11:22:14AM +0100, Marc-André Lureau wrote: > Hi > > On Fri, Mar 9, 2018 at 10:00 AM, Peter Xu <peterx@redhat.com> wrote: > > Having "allow-oob" to true for a command does not mean that this command > > will always be run in out-of-band mode. The out-of-band quick path will > > only be executed if we specify the extra "run-oob" flag when sending the > > QMP request: > > > > { "execute": "command-that-allows-oob", > > "arguments": { ... }, > > "control": { "run-oob": true } } > > > > 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. > > > > 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. > > > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > > Signed-off-by: Peter Xu <peterx@redhat.com> > > --- > > include/qapi/qmp/dispatch.h | 2 ++ > > monitor.c | 84 ++++++++++++++++++++++++++++++++++++++++----- > > qapi/qmp-dispatch.c | 33 +++++++++++++++++- > > trace-events | 2 ++ > > 4 files changed, 111 insertions(+), 10 deletions(-) > > > > diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h > > index 26eb13ff41..ffb4652f71 100644 > > --- a/include/qapi/qmp/dispatch.h > > +++ b/include/qapi/qmp/dispatch.h > > @@ -48,6 +48,8 @@ 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, Error **errp); > > +bool qmp_is_oob(QDict *dict); > > > > typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque); > > > > diff --git a/monitor.c b/monitor.c > > index 4d57a8d341..5c8afe9f50 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -1110,6 +1110,44 @@ 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) { > > + error_setg(errp, "Command field 'execute' missing"); > > + return false; > > + } > > + > > + cmd = qmp_find_command(mon->qmp.commands, command); > > + if (!cmd) { > > + error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, > > + "The command %s has not been found", command); > > + return false; > > + } > > + > > + if (qmp_is_oob(req)) { > > + if (!qmp_oob_enabled(mon)) { > > + error_setg(errp, "Please enable Out-Of-Band first " > > + "for the session during capabilities negociation"); > > + return false; > > + } > > + 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) > > { > > @@ -4001,6 +4039,7 @@ static void monitor_qmp_bh_dispatcher(void *data) > > QMPRequest *req_obj = monitor_qmp_requests_pop_one(); > > > > if (req_obj) { > > + trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: ""); > > monitor_qmp_dispatch_one(req_obj); > > /* Reschedule instead of looping so the main loop stays responsive */ > > qemu_bh_schedule(mon_global.qmp_dispatcher_bh); > > @@ -4024,17 +4063,31 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) > > error_setg(&err, QERR_JSON_PARSING); > > } > > if (err) { > > - monitor_qmp_respond(mon, NULL, err, NULL); > > - qobject_decref(req); > > - return; > > + goto err; > > + } > > + > > + /* Check against the request in general layout */ > > + qdict = qmp_dispatch_check_obj(req, &err); > > + if (!qdict) { > > + goto err; > > } > > > > - qdict = qobject_to_qdict(req); > > - if (qdict) { > > - id = qdict_get(qdict, "id"); > > - qobject_incref(id); > > - qdict_del(qdict, "id"); > > - } /* else will fail qmp_dispatch() */ > > + /* Check against OOB specific */ > > + if (!qmp_cmd_oob_check(mon, qdict, &err)) { > > + goto err; > > + } > > This check happens before monitor_qmp_dispatch_one(), where there is > now dead code for COMMAND_NOT_FOUND error handling (changing the error > description), since it will error here. > > A solution is to move the error manipulation code to monitor_qmp_respond(): So yes, now it'll be checked earlier now since the allow-oob is bound to per command, then we'll need that info earlier here. While I kept the other check untouched since there'll be other places that calls qmp_dispatch() too. I'm just wondering would there be any problem even without below change? Thanks, > > > diff --git a/monitor.c b/monitor.c > index 52b4c77ac3..df09a1657d 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -3997,6 +3997,18 @@ static void monitor_qmp_respond(Monitor *mon, > QObject *rsp, > qdict_put_obj(qobject_to(QDict, rsp), "id", id); > } > > + if (mon->qmp.commands == &qmp_cap_negotiation_commands) { > + qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error"); > + if (qdict > + && !g_strcmp0(qdict_get_try_str(qdict, "class"), > + > QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) { > + /* Provide a more useful error message */ > + qdict_del(qdict, "desc"); > + qdict_put_str(qdict, "desc", "Expecting capabilities > negotiation" > + " with 'qmp_capabilities'"); > + } > + } > + > monitor_json_emitter(mon, rsp); > } > > @@ -4022,7 +4034,6 @@ static void monitor_qmp_dispatch_one(QMPRequest > *req_obj, bool oob_cmd) > { > Monitor *mon, *old_mon; > QObject *req, *rsp = NULL, *id; > - QDict *qdict = NULL; > bool need_resume; > > req = req_obj->req; > @@ -4045,18 +4056,6 @@ static void monitor_qmp_dispatch_one(QMPRequest > *req_obj, bool oob_cmd) > > cur_mon = old_mon; > > - if (mon->qmp.commands == &qmp_cap_negotiation_commands) { > - qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error"); > - if (qdict > - && !g_strcmp0(qdict_get_try_str(qdict, "class"), > - QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) { > - /* Provide a more useful error message */ > - qdict_del(qdict, "desc"); > - qdict_put_str(qdict, "desc", "Expecting capabilities negotiation" > - " with 'qmp_capabilities'"); > - } > - } > - > /* Respond if necessary */ > monitor_qmp_respond(mon, rsp, NULL, id); > > > > + > > + id = qdict_get(qdict, "id"); > > + > > + /* When OOB is enabled, the "id" field is mandatory. */ > > + if (qmp_oob_enabled(mon) && !id) { > > + error_setg(&err, "Out-Of-Band capability requires that " > > + "every command contains an 'id' field"); > > + goto err; > > + } > > + > > + qobject_incref(id); > > + qdict_del(qdict, "id"); > > > > req_obj = g_new0(QMPRequest, 1); > > req_obj->mon = mon; > > @@ -4042,6 +4095,14 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) > > req_obj->req = req; > > req_obj->need_resume = false; > > > > + 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) > > + ?: ""); > > + monitor_qmp_dispatch_one(req_obj); > > + return; > > + } > > + > > /* Protect qmp_requests and fetching its length. */ > > qemu_mutex_lock(&mon->qmp.qmp_queue_lock); > > > > @@ -4078,6 +4139,11 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) > > > > /* Kick the dispatcher routine */ > > qemu_bh_schedule(mon_global.qmp_dispatcher_bh); > > + return; > > + > > +err: > > + monitor_qmp_respond(mon, NULL, err, NULL); > > + qobject_decref(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 e31ac4be1f..9a2e18c768 100644 > > --- a/qapi/qmp-dispatch.c > > +++ b/qapi/qmp-dispatch.c > > @@ -17,8 +17,9 @@ > > #include "qapi/qmp/json-parser.h" > > #include "qapi/qmp/qdict.h" > > #include "qapi/qmp/qjson.h" > > +#include "qapi/qmp/qbool.h" > > > > -static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) > > +QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) > > { > > const QDictEntry *ent; > > const char *arg_name; > > @@ -50,6 +51,14 @@ static 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, > > + "QMP input member 'control' must be a dict"); > > + return NULL; > > + } > > } else { > > error_setg(errp, "QMP input member '%s' is unexpected", > > arg_name); > > @@ -120,6 +129,28 @@ QObject *qmp_build_error_object(Error *err) > > error_get_pretty(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. > > + */ > > +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); > > +} > > + > > QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request) > > { > > Error *err = NULL; > > diff --git a/trace-events b/trace-events > > index fe10c3b487..bd036da278 100644 > > --- a/trace-events > > +++ b/trace-events > > @@ -48,6 +48,8 @@ monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) "event= > > handle_hmp_command(void *mon, const char *cmdline) "mon %p cmdline: %s" > > handle_qmp_command(void *mon, const char *req) "mon %p req: %s" > > monitor_suspend(void *ptr, int cnt) "mon %p: %d" > > +monitor_qmp_cmd_in_band(const char *id) "%s" > > +monitor_qmp_cmd_out_of_band(const char *id) "%s" > > > > # dma-helpers.c > > dma_blk_io(void *dbs, void *bs, int64_t offset, bool to_dev) "dbs=%p bs=%p offset=%" PRId64 " to_dev=%d" > > -- > > 2.14.3 > > > > > > > > -- > Marc-André Lureau
Hi On Fri, Mar 23, 2018 at 6:18 AM, Peter Xu <peterx@redhat.com> wrote: > On Thu, Mar 22, 2018 at 11:22:14AM +0100, Marc-André Lureau wrote: >> Hi >> >> On Fri, Mar 9, 2018 at 10:00 AM, Peter Xu <peterx@redhat.com> wrote: >> > Having "allow-oob" to true for a command does not mean that this command >> > will always be run in out-of-band mode. The out-of-band quick path will >> > only be executed if we specify the extra "run-oob" flag when sending the >> > QMP request: >> > >> > { "execute": "command-that-allows-oob", >> > "arguments": { ... }, >> > "control": { "run-oob": true } } >> > >> > 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. >> > >> > 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. >> > >> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> >> > Signed-off-by: Peter Xu <peterx@redhat.com> >> > --- >> > include/qapi/qmp/dispatch.h | 2 ++ >> > monitor.c | 84 ++++++++++++++++++++++++++++++++++++++++----- >> > qapi/qmp-dispatch.c | 33 +++++++++++++++++- >> > trace-events | 2 ++ >> > 4 files changed, 111 insertions(+), 10 deletions(-) >> > >> > diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h >> > index 26eb13ff41..ffb4652f71 100644 >> > --- a/include/qapi/qmp/dispatch.h >> > +++ b/include/qapi/qmp/dispatch.h >> > @@ -48,6 +48,8 @@ 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, Error **errp); >> > +bool qmp_is_oob(QDict *dict); >> > >> > typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque); >> > >> > diff --git a/monitor.c b/monitor.c >> > index 4d57a8d341..5c8afe9f50 100644 >> > --- a/monitor.c >> > +++ b/monitor.c >> > @@ -1110,6 +1110,44 @@ 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) { >> > + error_setg(errp, "Command field 'execute' missing"); >> > + return false; >> > + } >> > + >> > + cmd = qmp_find_command(mon->qmp.commands, command); >> > + if (!cmd) { >> > + error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, >> > + "The command %s has not been found", command); >> > + return false; >> > + } >> > + >> > + if (qmp_is_oob(req)) { >> > + if (!qmp_oob_enabled(mon)) { >> > + error_setg(errp, "Please enable Out-Of-Band first " >> > + "for the session during capabilities negociation"); >> > + return false; >> > + } >> > + 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) >> > { >> > @@ -4001,6 +4039,7 @@ static void monitor_qmp_bh_dispatcher(void *data) >> > QMPRequest *req_obj = monitor_qmp_requests_pop_one(); >> > >> > if (req_obj) { >> > + trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: ""); >> > monitor_qmp_dispatch_one(req_obj); >> > /* Reschedule instead of looping so the main loop stays responsive */ >> > qemu_bh_schedule(mon_global.qmp_dispatcher_bh); >> > @@ -4024,17 +4063,31 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) >> > error_setg(&err, QERR_JSON_PARSING); >> > } >> > if (err) { >> > - monitor_qmp_respond(mon, NULL, err, NULL); >> > - qobject_decref(req); >> > - return; >> > + goto err; >> > + } >> > + >> > + /* Check against the request in general layout */ >> > + qdict = qmp_dispatch_check_obj(req, &err); >> > + if (!qdict) { >> > + goto err; >> > } >> > >> > - qdict = qobject_to_qdict(req); >> > - if (qdict) { >> > - id = qdict_get(qdict, "id"); >> > - qobject_incref(id); >> > - qdict_del(qdict, "id"); >> > - } /* else will fail qmp_dispatch() */ >> > + /* Check against OOB specific */ >> > + if (!qmp_cmd_oob_check(mon, qdict, &err)) { >> > + goto err; >> > + } >> >> This check happens before monitor_qmp_dispatch_one(), where there is >> now dead code for COMMAND_NOT_FOUND error handling (changing the error >> description), since it will error here. >> >> A solution is to move the error manipulation code to monitor_qmp_respond(): > > So yes, now it'll be checked earlier now since the allow-oob is bound > to per command, then we'll need that info earlier here. While I kept > the other check untouched since there'll be other places that calls > qmp_dispatch() too. > > I'm just wondering would there be any problem even without below change? Not a problem, just less descriptive error. And we should remove the dead code. It's best imho to apply the below change. I will send a patch. > Thanks, > >> >> >> diff --git a/monitor.c b/monitor.c >> index 52b4c77ac3..df09a1657d 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -3997,6 +3997,18 @@ static void monitor_qmp_respond(Monitor *mon, >> QObject *rsp, >> qdict_put_obj(qobject_to(QDict, rsp), "id", id); >> } >> >> + if (mon->qmp.commands == &qmp_cap_negotiation_commands) { >> + qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error"); >> + if (qdict >> + && !g_strcmp0(qdict_get_try_str(qdict, "class"), >> + >> QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) { >> + /* Provide a more useful error message */ >> + qdict_del(qdict, "desc"); >> + qdict_put_str(qdict, "desc", "Expecting capabilities >> negotiation" >> + " with 'qmp_capabilities'"); >> + } >> + } >> + >> monitor_json_emitter(mon, rsp); >> } >> >> @@ -4022,7 +4034,6 @@ static void monitor_qmp_dispatch_one(QMPRequest >> *req_obj, bool oob_cmd) >> { >> Monitor *mon, *old_mon; >> QObject *req, *rsp = NULL, *id; >> - QDict *qdict = NULL; >> bool need_resume; >> >> req = req_obj->req; >> @@ -4045,18 +4056,6 @@ static void monitor_qmp_dispatch_one(QMPRequest >> *req_obj, bool oob_cmd) >> >> cur_mon = old_mon; >> >> - if (mon->qmp.commands == &qmp_cap_negotiation_commands) { >> - qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error"); >> - if (qdict >> - && !g_strcmp0(qdict_get_try_str(qdict, "class"), >> - QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) { >> - /* Provide a more useful error message */ >> - qdict_del(qdict, "desc"); >> - qdict_put_str(qdict, "desc", "Expecting capabilities negotiation" >> - " with 'qmp_capabilities'"); >> - } >> - } >> - >> /* Respond if necessary */ >> monitor_qmp_respond(mon, rsp, NULL, id); >> >> >> > + >> > + id = qdict_get(qdict, "id"); >> > + >> > + /* When OOB is enabled, the "id" field is mandatory. */ >> > + if (qmp_oob_enabled(mon) && !id) { >> > + error_setg(&err, "Out-Of-Band capability requires that " >> > + "every command contains an 'id' field"); >> > + goto err; >> > + } >> > + >> > + qobject_incref(id); >> > + qdict_del(qdict, "id"); >> > >> > req_obj = g_new0(QMPRequest, 1); >> > req_obj->mon = mon; >> > @@ -4042,6 +4095,14 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) >> > req_obj->req = req; >> > req_obj->need_resume = false; >> > >> > + 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) >> > + ?: ""); >> > + monitor_qmp_dispatch_one(req_obj); >> > + return; >> > + } >> > + >> > /* Protect qmp_requests and fetching its length. */ >> > qemu_mutex_lock(&mon->qmp.qmp_queue_lock); >> > >> > @@ -4078,6 +4139,11 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) >> > >> > /* Kick the dispatcher routine */ >> > qemu_bh_schedule(mon_global.qmp_dispatcher_bh); >> > + return; >> > + >> > +err: >> > + monitor_qmp_respond(mon, NULL, err, NULL); >> > + qobject_decref(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 e31ac4be1f..9a2e18c768 100644 >> > --- a/qapi/qmp-dispatch.c >> > +++ b/qapi/qmp-dispatch.c >> > @@ -17,8 +17,9 @@ >> > #include "qapi/qmp/json-parser.h" >> > #include "qapi/qmp/qdict.h" >> > #include "qapi/qmp/qjson.h" >> > +#include "qapi/qmp/qbool.h" >> > >> > -static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) >> > +QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) >> > { >> > const QDictEntry *ent; >> > const char *arg_name; >> > @@ -50,6 +51,14 @@ static 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, >> > + "QMP input member 'control' must be a dict"); >> > + return NULL; >> > + } >> > } else { >> > error_setg(errp, "QMP input member '%s' is unexpected", >> > arg_name); >> > @@ -120,6 +129,28 @@ QObject *qmp_build_error_object(Error *err) >> > error_get_pretty(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. >> > + */ >> > +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); >> > +} >> > + >> > QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request) >> > { >> > Error *err = NULL; >> > diff --git a/trace-events b/trace-events >> > index fe10c3b487..bd036da278 100644 >> > --- a/trace-events >> > +++ b/trace-events >> > @@ -48,6 +48,8 @@ monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) "event= >> > handle_hmp_command(void *mon, const char *cmdline) "mon %p cmdline: %s" >> > handle_qmp_command(void *mon, const char *req) "mon %p req: %s" >> > monitor_suspend(void *ptr, int cnt) "mon %p: %d" >> > +monitor_qmp_cmd_in_band(const char *id) "%s" >> > +monitor_qmp_cmd_out_of_band(const char *id) "%s" >> > >> > # dma-helpers.c >> > dma_blk_io(void *dbs, void *bs, int64_t offset, bool to_dev) "dbs=%p bs=%p offset=%" PRId64 " to_dev=%d" >> > -- >> > 2.14.3 >> > >> > >> >> >> >> -- >> Marc-André Lureau > > -- > Peter Xu
diff --git a/monitor.c b/monitor.c index 52b4c77ac3..df09a1657d 100644 --- a/monitor.c +++ b/monitor.c @@ -3997,6 +3997,18 @@ static void monitor_qmp_respond(Monitor *mon, QObject *rsp, qdict_put_obj(qobject_to(QDict, rsp), "id", id); } + if (mon->qmp.commands == &qmp_cap_negotiation_commands) { + qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error"); + if (qdict + && !g_strcmp0(qdict_get_try_str(qdict, "class"), + QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) { + /* Provide a more useful error message */ + qdict_del(qdict, "desc"); + qdict_put_str(qdict, "desc", "Expecting capabilities negotiation" + " with 'qmp_capabilities'"); + } + } +