[GEDI] [PATCH v3 04/25] error: auto propagated local_err

Vladimir Sementsov-Ogievskiy vsementsov at virtuozzo.com
Tue Sep 24 20:08:41 UTC 2019


Here is introduced ERRP_FUNCTION_BEGIN macro, to be used at start of
functions with errp parameter.

It has three goals:

1. Fix issue with error_fatal & error_append_hint: user can't see these
hints, because exit() happens in error_setg earlier than hint is
appended. [Reported by Greg Kurz]

2. Fix issue with error_abort & error_propagate: when we wrap
error_abort by local_err+error_propagate, resulting coredump will
refer to error_propagate and not to the place where error happened.
(the macro itself doesn't fix the issue, but it allows to [3.] drop all
local_err+error_propagate pattern, which will definitely fix the issue)
[Reported by Kevin Wolf]

3. Drop local_err+error_propagate pattern, which is used to workaround
void functions with errp parameter, when caller wants to know resulting
status. (Note: actually these functions could be merely updated to
return int error code).

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov at virtuozzo.com>
---

CC: John Snow <jsnow at redhat.com>
CC: Kevin Wolf <kwolf at redhat.com>
CC: Max Reitz <mreitz at redhat.com>
CC: Fam Zheng <fam at euphon.net>
CC: Jeff Cody <codyprime at gmail.com>
CC: "Marc-André Lureau" <marcandre.lureau at redhat.com>
CC: Paolo Bonzini <pbonzini at redhat.com>
CC: Greg Kurz <groug at kaod.org>
CC: Subbaraya Sundeep <sundeep.lkml at gmail.com>
CC: Peter Maydell <peter.maydell at linaro.org>
CC: Paul Burton <pburton at wavecomp.com>
CC: Aleksandar Rikalo <arikalo at wavecomp.com>
CC: "Michael S. Tsirkin" <mst at redhat.com>
CC: Marcel Apfelbaum <marcel.apfelbaum at gmail.com>
CC: Mark Cave-Ayland <mark.cave-ayland at ilande.co.uk>
CC: David Gibson <david at gibson.dropbear.id.au>
CC: Yuval Shaia <yuval.shaia at oracle.com>
CC: Cornelia Huck <cohuck at redhat.com>
CC: Eric Farman <farman at linux.ibm.com>
CC: Richard Henderson <rth at twiddle.net>
CC: David Hildenbrand <david at redhat.com>
CC: Halil Pasic <pasic at linux.ibm.com>
CC: Christian Borntraeger <borntraeger at de.ibm.com>
CC: Gerd Hoffmann <kraxel at redhat.com>
CC: Alex Williamson <alex.williamson at redhat.com>
CC: Markus Armbruster <armbru at redhat.com>
CC: Michael Roth <mdroth at linux.vnet.ibm.com>
CC: Juan Quintela <quintela at redhat.com>
CC: "Dr. David Alan Gilbert" <dgilbert at redhat.com>
CC: Eric Blake <eblake at redhat.com>
CC: Jason Wang <jasowang at redhat.com>
CC: "Daniel P. Berrangé" <berrange at redhat.com>
CC: Eduardo Habkost <ehabkost at redhat.com>
CC: qemu-block at nongnu.org
CC: qemu-devel at nongnu.org
CC: integration at gluster.org
CC: qemu-arm at nongnu.org
CC: qemu-ppc at nongnu.org
CC: qemu-s390x at nongnu.org

 include/qapi/error.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 9376f59c35..fb41f7a790 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -322,6 +322,41 @@ void error_set_internal(Error **errp,
                         ErrorClass err_class, const char *fmt, ...)
     GCC_FMT_ATTR(6, 7);
 
+typedef struct ErrorPropagator {
+    Error *local_err;
+    Error **errp;
+} ErrorPropagator;
+
+static inline void error_propagator_cleanup(ErrorPropagator *prop)
+{
+    error_propagate(prop->errp, prop->local_err);
+}
+
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(ErrorPropagator, error_propagator_cleanup);
+
+/*
+ * ERRP_FUNCTION_BEGIN
+ *
+ * This macro is created to be the first line of a function with Error **errp
+ * parameter.
+ *
+ * If errp is NULL or points to error_fatal, it is rewritten to point to a
+ * local Error object, which will be automatically propagated to the original
+ * errp on function exit (see error_propagator_cleanup).
+ *
+ * After invocation of this macro it is always safe to dereference errp
+ * (as it's not NULL anymore) and to append hints (by error_append_hint)
+ * (as, if it was error_fatal, we swapped it with a local_error to be
+ * propagated on cleanup).
+ *
+ * Note: we don't wrap the error_abort case, as we want resulting coredump
+ * to point to the place where the error happened, not to error_propagate.
+ */
+#define ERRP_FUNCTION_BEGIN() \
+g_auto(ErrorPropagator) __auto_errp_prop = {.errp = errp}; \
+errp = ((errp == NULL || *errp == error_fatal) ? \
+    &__auto_errp_prop.local_err : errp)
+
 /*
  * Special error destination to abort on error.
  * See error_setg() and error_propagate() for details.
-- 
2.21.0



More information about the integration mailing list