* 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