From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45728) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YyFUU-0002Fq-7K for qemu-devel@nongnu.org; Fri, 29 May 2015 04:18:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YyFUR-0001rW-1J for qemu-devel@nongnu.org; Fri, 29 May 2015 04:18:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50164) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YyFUQ-0001rR-Q6 for qemu-devel@nongnu.org; Fri, 29 May 2015 04:18:02 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 9253F2B9DDD for ; Fri, 29 May 2015 08:18:01 +0000 (UTC) From: Markus Armbruster References: <1432653655-30745-1-git-send-email-armbru@redhat.com> <1432653655-30745-14-git-send-email-armbru@redhat.com> <20150528153537.6ec3b1e0@redhat.com> Date: Fri, 29 May 2015 10:17:58 +0200 In-Reply-To: <20150528153537.6ec3b1e0@redhat.com> (Luiz Capitulino's message of "Thu, 28 May 2015 15:35:37 -0400") Message-ID: <87382fn6fd.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v2 13/20] monitor: Limit QError use to command handlers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: qemu-devel@nongnu.org Luiz Capitulino writes: > On Tue, 26 May 2015 17:20:48 +0200 > Markus Armbruster 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 >> Reviewed-by: Eric Blake >> --- >> 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). Oops. > 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). I'll do the latter. Thanks! >> qobject_decref(data); >> QDECREF(mon->error); >> mon->error = NULL;