From patchwork Mon Sep 9 13:53:13 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Max Reitz X-Patchwork-Id: 273578 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id B73D52C012C for ; Mon, 9 Sep 2013 23:53:56 +1000 (EST) Received: from localhost ([::1]:51476 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJ1uc-0001sM-BK for incoming@patchwork.ozlabs.org; Mon, 09 Sep 2013 09:53:54 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33161) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJ1uB-0001s1-EH for qemu-devel@nongnu.org; Mon, 09 Sep 2013 09:53:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VJ1u5-00083K-CW for qemu-devel@nongnu.org; Mon, 09 Sep 2013 09:53:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41573) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VJ1u5-00083C-3x for qemu-devel@nongnu.org; Mon, 09 Sep 2013 09:53:21 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r89DrKSX016444 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 9 Sep 2013 09:53:20 -0400 Received: from localhost (dhcp-200-247.str.redhat.com [10.33.200.247]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r89DrIUm028977 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Mon, 9 Sep 2013 09:53:19 -0400 From: Max Reitz To: qemu-devel@nongnu.org Date: Mon, 9 Sep 2013 15:53:13 +0200 Message-Id: <1378734793-17818-1-git-send-email-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Kevin Wolf , Max Reitz Subject: [Qemu-devel] [RFC] qapi/error: Optional error propagation backtrace X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Add a configure switch which enables an error propagation backtrace. This results in the error_set function prepending every message by the source file name, function and line in which it was called, as well as error_propagate appending this information to the propagated message, resulting in a backtrace. Signed-off-by: Max Reitz --- Since this obviously breaks existing tests (and cannot really be used for new tests since it includes a line number, which is a rather volatile output) and is generally not very useful to normal users, the switch is disabled by default. This functionality is intended for developers tracking down error paths. Since I do not know whether I am the only one considering it useful in the first place, this is just an RFC for now. Furthermore, I am not sure whether a configure switch is really the right way to implement this. --- configure | 12 +++++++++++ include/qapi/error.h | 40 +++++++++++++++++++++++++++--------- util/error.c | 57 +++++++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 90 insertions(+), 19 deletions(-) diff --git a/configure b/configure index e989609..4e43f7b 100755 --- a/configure +++ b/configure @@ -243,6 +243,7 @@ gtk="" gtkabi="2.0" tpm="no" libssh2="" +error_backtrace="no" # parse CC options first for opt do @@ -949,6 +950,10 @@ for opt do ;; --enable-libssh2) libssh2="yes" ;; + --enable-error-bt) error_backtrace="yes" + ;; + --disable-error-bt) error_backtrace="no" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac @@ -1168,6 +1173,8 @@ echo " --gcov=GCOV use specified gcov [$gcov_tool]" echo " --enable-tpm enable TPM support" echo " --disable-libssh2 disable ssh block device support" echo " --enable-libssh2 enable ssh block device support" +echo " --disable-error-bt disable backtraces on internal errors" +echo " --enable-error-bt enable backtraces on internal errors" echo "" echo "NOTE: The object files are built at the place where configure is launched" exit 1 @@ -3650,6 +3657,7 @@ echo "gcov $gcov_tool" echo "gcov enabled $gcov" echo "TPM support $tpm" echo "libssh2 support $libssh2" +echo "error backtraces $error_backtrace" echo "TPM passthrough $tpm_passthrough" echo "QOM debugging $qom_cast_debug" @@ -4044,6 +4052,10 @@ if test "$libssh2" = "yes" ; then echo "CONFIG_LIBSSH2=y" >> $config_host_mak fi +if test "$error_backtrace" = "yes" ; then + echo "CONFIG_ERROR_BACKTRACE=y" >> $config_host_mak +fi + if test "$virtio_blk_data_plane" = "yes" ; then echo 'CONFIG_VIRTIO_BLK_DATA_PLANE=$(CONFIG_VIRTIO)' >> $config_host_mak fi diff --git a/include/qapi/error.h b/include/qapi/error.h index ffd1cea..d3cfe35 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -25,16 +25,28 @@ typedef struct Error Error; /** * Set an indirect pointer to an error given a ErrorClass value and a * printf-style human message. This function is not meant to be used outside - * of QEMU. + * of QEMU. The message will be prepended by the file/function/line information + * if CONFIG_ERROR_BACKTRACE is defined. */ -void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(3, 4); +void error_set_bt(const char *file, const char *func, int line, + Error **err, ErrorClass err_class, const char *fmt, ...) + GCC_FMT_ATTR(6, 7); + +#define error_set(...) \ + error_set_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) /** * Set an indirect pointer to an error given a ErrorClass value and a * printf-style human message, followed by a strerror() string if - * @os_error is not zero. + * @os_error is not zero. The message will be prepended by the + * file/function/line information if CONFIG_ERROR_BACKTRACE is defined. */ -void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(4, 5); +void error_set_errno_bt(const char *file, const char *func, int line, + Error **err, int os_error, ErrorClass err_class, + const char *fmt, ...) GCC_FMT_ATTR(7, 8); + +#define error_set_errno(...) \ + error_set_errno_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) /** * Same as error_set(), but sets a generic error @@ -47,7 +59,11 @@ void error_set_errno(Error **err, int os_error, ErrorClass err_class, const char /** * Helper for open() errors */ -void error_setg_file_open(Error **errp, int os_errno, const char *filename); +void error_setg_file_open_bt(const char *file, const char *func, int line, + Error **errp, int os_errno, const char *filename); + +#define error_setg_file_open(...) \ + error_setg_file_open_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) /** * Returns true if an indirect pointer to an error is pointing to a valid @@ -71,11 +87,17 @@ Error *error_copy(const Error *err); const char *error_get_pretty(Error *err); /** - * Propagate an error to an indirect pointer to an error. This function will - * always transfer ownership of the error reference and handles the case where - * dst_err is NULL correctly. Errors after the first are discarded. + * Propagate an error to an indirect pointer to an error, appending the given + * file/function/line information to the message (as a backtrace) if + * CONFIG_ERROR_BACKTRACE is defined. This function will always transfer + * ownership of the error reference and handles the case where dst_err is NULL + * correctly. Errors after the first are discarded. */ -void error_propagate(Error **dst_err, Error *local_err); +void error_propagate_bt(const char *file, const char *func, int line, + Error **dest_err, Error *local_err); + +#define error_propagate(...) \ + error_propagate_bt(__FILE__, __func__, __LINE__, __VA_ARGS__) /** * Free an error object. diff --git a/util/error.c b/util/error.c index 53b0435..a07f4c7 100644 --- a/util/error.c +++ b/util/error.c @@ -23,9 +23,11 @@ struct Error ErrorClass err_class; }; -void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) +void error_set_bt(const char *file, const char *func, int line, + Error **errp, ErrorClass err_class, const char *fmt, ...) { Error *err; + char *msg1; va_list ap; if (errp == NULL) { @@ -36,15 +38,22 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) err = g_malloc0(sizeof(*err)); va_start(ap, fmt); - err->msg = g_strdup_vprintf(fmt, ap); + msg1 = g_strdup_vprintf(fmt, ap); +#ifdef CONFIG_ERROR_BACKTRACE + err->msg = g_strdup_printf("%s:%i (in %s): %s", file, line, func, msg1); + g_free(msg1); +#else + err->msg = msg1; +#endif va_end(ap); err->err_class = err_class; *errp = err; } -void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, - const char *fmt, ...) +void error_set_errno_bt(const char *file, const char *func, int line, + Error **errp, int os_errno, ErrorClass err_class, + const char *fmt, ...) { Error *err; char *msg1; @@ -59,21 +68,34 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, va_start(ap, fmt); msg1 = g_strdup_vprintf(fmt, ap); +#ifdef CONFIG_ERROR_BACKTRACE + if (os_errno != 0) { + err->msg = g_strdup_printf("%s:%i (in %s): %s: %s", file, line, func, + msg1, strerror(os_errno)); + } else { + err->msg = g_strdup_printf("%s:%i (in %s): %s", file, line, func, msg1); + } + g_free(msg1); +#else if (os_errno != 0) { err->msg = g_strdup_printf("%s: %s", msg1, strerror(os_errno)); g_free(msg1); } else { err->msg = msg1; } +#endif va_end(ap); err->err_class = err_class; *errp = err; } -void error_setg_file_open(Error **errp, int os_errno, const char *filename) +void error_setg_file_open_bt(const char *file, const char *func, int line, + Error **errp, int os_errno, const char *filename) { - error_setg_errno(errp, os_errno, "Could not open '%s'", filename); + error_set_errno_bt(file, func, line, errp, os_errno, + ERROR_CLASS_GENERIC_ERROR, "Could not open '%s'", + filename); } Error *error_copy(const Error *err) @@ -110,11 +132,26 @@ void error_free(Error *err) } } -void error_propagate(Error **dst_err, Error *local_err) + +void error_propagate_bt(const char *file, const char *func, int line, + Error **dst_err, Error *local_err) { - if (dst_err && !*dst_err) { - *dst_err = local_err; - } else if (local_err) { + if (local_err) { +#ifdef CONFIG_ERROR_BACKTRACE + if (dst_err && !*dst_err) { + Error *err = g_malloc0(sizeof(*err)); + err->msg = g_strdup_printf("%s\n from %s:%i (in %s)", + local_err->msg, file, line, func); + err->err_class = local_err->err_class; + *dst_err = err; + } error_free(local_err); +#else + if (dst_err && !*dst_err) { + *dst_err = local_err; + } else { + error_free(local_err); + } +#endif } }