[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 05/22] docs/devel: update error handling guidance for HMP
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH v4 05/22] docs/devel: update error handling guidance for HMP commands |
Date: |
Thu, 28 Oct 2021 17:14:19 +0100 |
User-agent: |
Mutt/2.0.7 (2021-05-04) |
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> Best practice is to use the 'hmp_handle_error' function, not
> 'monitor_printf' or 'error_report_err'. This ensures that the
> message always gets an 'Error: ' prefix, distinguishing it
> from normal command output.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Yes OK; but that is potentially discouraging people from adding helpful
errors; certainly I'd want them to add text if they were calling
multiple qmp functions, so you knew what failed.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> docs/devel/writing-monitor-commands.rst | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/docs/devel/writing-monitor-commands.rst
> b/docs/devel/writing-monitor-commands.rst
> index a973c48f66..a381b52024 100644
> --- a/docs/devel/writing-monitor-commands.rst
> +++ b/docs/devel/writing-monitor-commands.rst
> @@ -293,9 +293,7 @@ Here's the implementation of the "hello-world" HMP
> command::
> Error *err = NULL;
>
> qmp_hello_world(!!message, message, &err);
> - if (err) {
> - monitor_printf(mon, "%s\n", error_get_pretty(err));
> - error_free(err);
> + if (hmp_handle_error(mon, err)) {
> return;
> }
> }
> @@ -307,9 +305,10 @@ There are three important points to be noticed:
> 1. The "mon" and "qdict" arguments are mandatory for all HMP functions. The
> former is the monitor object. The latter is how the monitor passes
> arguments entered by the user to the command implementation
> -2. hmp_hello_world() performs error checking. In this example we just print
> - the error description to the user, but we could do more, like taking
> - different actions depending on the error qmp_hello_world() returns
> +2. hmp_hello_world() performs error checking. In this example we just call
> + hmp_handle_error() which prints a message to the user, but we could do
> + more, like taking different actions depending on the error
> + qmp_hello_world() returns
> 3. The "err" variable must be initialized to NULL before performing the
> QMP call
>
> @@ -466,9 +465,7 @@ Here's the HMP counterpart of the query-alarm-clock
> command::
> Error *err = NULL;
>
> clock = qmp_query_alarm_clock(&err);
> - if (err) {
> - monitor_printf(mon, "Could not query alarm clock information\n");
> - error_free(err);
> + if (hmp_handle_error(mon, err)) {
> return;
> }
>
> @@ -607,9 +604,7 @@ has to traverse the list, it's shown below for reference::
> Error *err = NULL;
>
> method_list = qmp_query_alarm_methods(&err);
> - if (err) {
> - monitor_printf(mon, "Could not query alarm methods\n");
> - error_free(err);
> + if (hmp_handle_error(mon, err)) {
> return;
> }
>
> --
> 2.31.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
- [PATCH v4 00/22] monitor: explicitly permit QMP commands to be added for all use cases, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 01/22] monitor: remove 'info ioapic' HMP command, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 02/22] monitor: make hmp_handle_error return a boolean, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 03/22] docs/devel: rename file for writing monitor commands, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 04/22] docs/devel: tweak headings in monitor command docs, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 05/22] docs/devel: update error handling guidance for HMP commands, Daniel P . Berrangé, 2021/10/28
- Re: [PATCH v4 05/22] docs/devel: update error handling guidance for HMP commands,
Dr. David Alan Gilbert <=
- [PATCH v4 06/22] monitor: introduce HumanReadableText and HMP support, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 08/22] docs/devel: add example of command returning unstructured text, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 09/22] docs/devel: document expectations for HMP commands in the future, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 07/22] docs/devel: document expectations for QAPI data modelling for QMP, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 10/22] qapi: introduce x-query-roms QMP command, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 11/22] qapi: introduce x-query-profile QMP command, Daniel P . Berrangé, 2021/10/28
- [PATCH v4 12/22] qapi: introduce x-query-numa QMP command, Daniel P . Berrangé, 2021/10/28