* [Qemu-devel] [RFC] qapi/error: Optional error propagation backtrace
@ 2013-09-09 13:53 Max Reitz
2013-10-08 9:53 ` Max Reitz
0 siblings, 1 reply; 2+ messages in thread
From: Max Reitz @ 2013-09-09 13:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Max Reitz
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 <mreitz@redhat.com>
---
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
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [RFC] qapi/error: Optional error propagation backtrace
2013-09-09 13:53 [Qemu-devel] [RFC] qapi/error: Optional error propagation backtrace Max Reitz
@ 2013-10-08 9:53 ` Max Reitz
0 siblings, 0 replies; 2+ messages in thread
From: Max Reitz @ 2013-10-08 9:53 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
On 2013-09-09 15:53, Max Reitz wrote:
> 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 <mreitz@redhat.com>
Ping – any comments on this?
> ---
> 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
> }
> }
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-10-08 9:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-09 13:53 [Qemu-devel] [RFC] qapi/error: Optional error propagation backtrace Max Reitz
2013-10-08 9:53 ` Max Reitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).