Re: [Qemu-devel] [PATCH v3 ] doc: Introduce coding style for errors

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2557 bytes --]

On 01/15/2016 06:54 AM, Lluís Vilanova wrote:
> Gives some general guidelines for reporting errors in QEMU.
> 
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  HACKING |   36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 

> +7.1. Errors in user inputs
> +
> +QEMU provides the functions in "include/qemu/error-report.h" to report errors
> +related to inputs provided by the user (e.g., command line arguments or
> +configuration files).
> +
> +These functions generate error messages with a uniform format that can reference
> +a location on the offending input.

s/on/in/

> +
> +7.2. Other errors
> +
> +QEMU provides the functions in "include/qapi/error.h" to report other types of
> +errors (i.e., not triggered by command line arguments or configuration files).

Maybe: "not directly triggered". After all, we DO have places where
Error is used which can ultimately be traced to a user command (such as
in QMP commands), but where the local code can also be called
internally; the use of Error at the local level then lets us leave it up
to the caller whether to report a message (because the caller has more
context).

> +
> +Functions in this header are used to accumulate error messages in an 'Error'
> +object, which can be propagated up the call chain where it is finally reported.
> +
> +In its simplest form, you can immediately report an error with:
> +
> +    error_setg(&error_fatal, "Error with %s", "arguments");

This paradigm doesn't appear anywhere in the current code base
(hw/ppc/spapr*.c has a few cases of error_setg(&error_abort), but
nothing directly passes error_fatal).  It's a bit odd to document
something that isn't actually used.

> +
> +See the "include/qapi/error.h" header for additional convenience functions and
> +special arguments. Specially, see 'error_fatal' and 'error_abort' to show errors

s/Specially/Specifically/

> +and immediately terminate QEMU.
> +
> +WARNING: Do *not* use 'error_fatal' or 'error_abort' for errors that are (or can
> +be) triggered by guest code (e.g., some unimplimented corner case in guest code

s/unimplimented/unimplemented/

> +translation or device code). Otherwise that can be abused by guest code to
> +terminate QEMU. Instead, you should use the 'error_report()' routine.

But a definite improvement over v2. I think we're getting closer to a
good summary.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]