[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 13/20] monitor: Limit QError use to command h
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 13/20] monitor: Limit QError use to command handlers |
Date: |
Fri, 29 May 2015 10:17:58 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Luiz Capitulino <address@hidden> writes:
> On Tue, 26 May 2015 17:20:48 +0200
> Markus Armbruster <address@hidden> 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 <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> ---
>> 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;
- Re: [Qemu-devel] [PATCH v2 04/20] monitor: Convert client_migrate_info to QAPI, (continued)
- [Qemu-devel] [PATCH v2 03/20] monitor: Improve and document client_migrate_info protocol error, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 05/20] monitor: Use traditional command interface for HMP drive_del, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 14/20] monitor: Rename handle_user_command() to handle_hmp_command(), Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 08/20] monitor: Drop unused "new" HMP command interface, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 10/20] monitor: Propagate errors through qmp_check_input_obj(), Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 12/20] monitor: Inline monitor_has_error() into its only caller, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 13/20] monitor: Limit QError use to command handlers, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 06/20] monitor: Use traditional command interface for HMP device_add, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 09/20] monitor: Propagate errors through qmp_check_client_args(), Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 11/20] monitor: Wean monitor_protocol_emitter() off mon->error, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 17/20] monitor: Drop do_qmp_capabilities()'s superfluous QMP check, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 19/20] monitor: Rename monitor_ctrl_mode() to monitor_is_qmp(), Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 07/20] monitor: Use trad. command interface for HMP pcie_aer_inject_error, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 20/20] monitor: Change return type of monitor_cur_is_qmp() to bool, Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 15/20] monitor: Rename monitor_control_read(), monitor_control_event(), Markus Armbruster, 2015/05/26
- [Qemu-devel] [PATCH v2 16/20] monitor: Unbox Monitor member mc and rename to qmp, Markus Armbruster, 2015/05/26