On Tue, 26 May 2015 17:20:48 +0200 Markus Armbruster <arm...@redhat.com> wrote:
> The previous commits narrowed use of QError to handle_qmp_command() > and its helpers monitor_protocol_emitter(), build_qmp_error_dict(). > Narrow it further to just the command handler call: instead of > converting Error to QError throughout handle_qmp_command(), convert > the QError gotten from the command handler to Error, and switch the > helpers from QError to Error. > > Signed-off-by: Markus Armbruster <arm...@redhat.com> > Reviewed-by: Eric Blake <ebl...@redhat.com> > --- > monitor.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/monitor.c b/monitor.c > index f7e8fdf..1ed8462 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -391,19 +391,19 @@ static void monitor_json_emitter(Monitor *mon, const > QObject *data) > QDECREF(json); > } > > -static QDict *build_qmp_error_dict(const QError *err) > +static QDict *build_qmp_error_dict(Error *err) > { > QObject *obj; > > - obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %p } }", > - ErrorClass_lookup[err->err_class], > - qerror_human(err)); > + obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %s } }", > + ErrorClass_lookup[error_get_class(err)], > + error_get_pretty(err)); > > return qobject_to_qdict(obj); > } > > static void monitor_protocol_emitter(Monitor *mon, QObject *data, > - QError *err) > + Error *err) > { > QDict *qmp; > > @@ -4982,13 +4982,12 @@ static void handle_qmp_command(JSONMessageParser > *parser, QList *tokens) > obj = json_parser_parse(tokens, NULL); > if (!obj) { > // FIXME: should be triggered in json_parser_parse() > - qerror_report(QERR_JSON_PARSING); > + error_set(&local_err, QERR_JSON_PARSING); > goto err_out; > } > > input = qmp_check_input_obj(obj, &local_err); > if (!input) { > - qerror_report_err(local_err); > qobject_decref(obj); > goto err_out; > } > @@ -5000,8 +4999,8 @@ static void handle_qmp_command(JSONMessageParser > *parser, QList *tokens) > trace_handle_qmp_command(mon, cmd_name); > cmd = qmp_find_cmd(cmd_name); > if (!cmd) { > - qerror_report(ERROR_CLASS_COMMAND_NOT_FOUND, > - "The command %s has not been found", cmd_name); > + error_set(&local_err, ERROR_CLASS_COMMAND_NOT_FOUND, > + "The command %s has not been found", cmd_name); > goto err_out; > } > if (invalid_qmp_mode(mon, cmd)) { > @@ -5018,7 +5017,6 @@ static void handle_qmp_command(JSONMessageParser > *parser, QList *tokens) > > qmp_check_client_args(cmd, args, &local_err); > if (local_err) { > - qerror_report_err(local_err); > goto err_out; > } > > @@ -5026,12 +5024,16 @@ static void handle_qmp_command(JSONMessageParser > *parser, QList *tokens) > /* Command failed... */ > if (!mon->error) { > /* ... without setting an error, so make one up */ > - qerror_report(QERR_UNDEFINED_ERROR); > + error_set(&local_err, QERR_UNDEFINED_ERROR); > } > } > + if (mon->error) { > + error_set(&local_err, mon->error->err_class, "%s", > + mon->error->err_msg); > + } > > err_out: > - monitor_protocol_emitter(mon, data, mon->error); > + monitor_protocol_emitter(mon, data, local_err); This breaks error reporting from invalid_qmp_mode(). The end result is that every command succeeds in capability negotiation mode and qmp_capabilities never fails (even in command mode). There are two simple ways to fix it: just propagate mon->error to local_err when invalid_qmp_mode() fails, or change invalid_qmp_mode() to take an Error object (preferable). > qobject_decref(data); > QDECREF(mon->error); > mon->error = NULL;