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

[Lluís Vilanova]

Gives some general guidelines for reporting errors in QEMU.


Signed-off-by: Lluís Vilanova <address@hidden>
---
 HACKING              |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/qapi/error.h |   12 ++++++++++++
 util/error.c         |   24 +++++++++++++++++------
 3 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/HACKING b/HACKING
index 12fbc8a..af36009 100644
--- a/HACKING
+++ b/HACKING
@@ -157,3 +157,55 @@ painful. These are:
  * you may assume that integers are 2s complement representation
  * you may assume that right shift of a signed integer duplicates
    the sign bit (ie it is an arithmetic shift, not a logical shift)
+
+7. Error reporting
+
+QEMU provides two different mechanisms for reporting errors. You should use one
+of these mechanisms instead of manually reporting them (i.e., do not use
+'printf', 'exit' or 'abort').
+
+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.
+
+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).
+
+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_now, "Error with %s", "arguments");
+
+For convenience, you can also use 'error_setg_errno' and 'error_setg_win32' to
+append a message for OS-specific errors, and 'error_setg_file_open' for errors
+when opening files.
+
+7.3. Errors with an immediate exit/abort
+
+There are two convenience functions to report errors that immediately terminate
+QEMU:
+
+* 'error_setg(&error_fatal, msg, ...)'
+
+  Reports a fatal error with the given error message and exits QEMU.
+
+* 'error_setg(&error_abort, msg, ...)'
+
+  Reports a programming error with the given error message and aborts QEMU.
+
+In general, you should report errors but *not* terminate QEMU if the errors are
+(or can be) triggered by guest code (e.g., some unimplemented corner case).
+This also applies to hardware emulation (it is OK to leave the device in a
+non-operational state until next reboot, though). Otherwise that can be abused
+by guest code to terminate QEMU.
+
+In such cases you should use the argument 'error_now'.
diff --git a/include/qapi/error.h b/include/qapi/error.h
index c69dddb..e2bfc19 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -53,6 +53,9 @@
  * Call a function treating errors as fatal:
  *     foo(arg, &error_fatal);
  *
+ * Call a function immediately showing all errors:
+ *     foo(arg, &error_now);
+ *
  * Receive an error and pass it on to the caller:
  *     Error *err = NULL;
  *     foo(arg, &err);
@@ -104,6 +107,7 @@ ErrorClass error_get_class(const Error *err);
  * then.
  * If @errp is &error_abort, print a suitable message and abort().
  * If @errp is &error_fatal, print a suitable message and exit(1).
+ * If @errp is &error_now, print a suitable message.
  * If @errp is anything else, address@hidden must be NULL.
  * The new error's class is ERROR_CLASS_GENERIC_ERROR, and its
  * human-readable error message is made from printf-style @fmt, ...
@@ -154,6 +158,7 @@ void error_setg_win32_internal(Error **errp,
  * abort().
  * Else, if @dst_errp is &error_fatal, print a suitable message and
  * exit(1).
+ * Else, if @dst_errp is &error_now, print a suitable message.
  * Else, if @dst_errp already contains an error, ignore this one: free
  * the error object.
  * Else, move the error object from @local_err to address@hidden
@@ -217,4 +222,11 @@ extern Error *error_abort;
  */
 extern Error *error_fatal;
 
+/*
+ * Pass to error_setg() & friends to immediately show an error.
+ *
+ * Has the same formatting of #error_abort, but does not terminate QEMU.
+ */
+extern Error *error_now;
+
 #endif
diff --git a/util/error.c b/util/error.c
index 8b86490..e507039 100644
--- a/util/error.c
+++ b/util/error.c
@@ -27,6 +27,7 @@ struct Error
 
 Error *error_abort;
 Error *error_fatal;
+Error *error_now;
 
 static void error_handle_fatal(Error **errp, Error *err)
 {
@@ -62,9 +63,14 @@ static void error_setv(Error **errp,
     err->func = func;
 
     error_handle_fatal(errp, err);
-    *errp = err;
-
-    errno = saved_errno;
+    if (errp == &error_now) {
+        fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
+                err->func, err->src, err->line);
+        error_report_err(err);
+    } else {
+        *errp = err;
+        errno = saved_errno;
+    }
 }
 
 void error_set_internal(Error **errp,
@@ -226,9 +232,15 @@ void error_propagate(Error **dst_errp, Error *local_err)
         return;
     }
     error_handle_fatal(dst_errp, local_err);
-    if (dst_errp && !*dst_errp) {
-        *dst_errp = local_err;
+    if (dst_errp == &error_now) {
+        fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
+                local_err->func, local_err->src, local_err->line);
+        error_report_err(local_err);
     } else {
-        error_free(local_err);
+        if (dst_errp && !*dst_errp) {
+            *dst_errp = local_err;
+        } else {
+            error_free(local_err);
+        }
     }
 }