Re: [Qemu-devel] [RFC] qapi/error: Optional error propagation backtrace

qemu-devel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC] qapi/error: Optional error propagation backtrace

From:	Max Reitz
Subject:	Re: [Qemu-devel] [RFC] qapi/error: Optional error propagation backtrace
Date:	Tue, 08 Oct 2013 11:53:14 +0200
User-agent:	Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130923 Thunderbird/17.0.9

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 <address@hidden>


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
      }
  }

[Prev in Thread]

Current Thread

[Next in Thread]

Re: [Qemu-devel] [RFC] qapi/error: Optional error propagation backtrace, Max Reitz <=

Prev by Date: Re: [Qemu-devel] [PATCH v2 14/17] qemu-iotests: Check autodel behaviour for device_del
Next by Date: [Qemu-devel] [PATCH] osdep: initialize glib threads in all QEMU tools
Previous by thread: Re: [Qemu-devel] BUG: RTC issue when Windows guest is idle
Next by thread: [Qemu-devel] [PATCH] osdep: initialize glib threads in all QEMU tools
Index(es):
- Date
- Thread