Re: [Qemu-devel] [PATCH v1 21/21] error: ensure errno detail is printed with error_abort

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> On Thu, Mar 10, 2016 at 09:55:37AM +0100, Markus Armbruster wrote:
>> "Daniel P. Berrange"  writes:
>> 
>> > When _abort is passed in, the error reporting code
>> > will print the current error message and then abort() the
>> > process. Unfortunately at the time it aborts, we've not
>> > yet appended the errno detail. This makes debugging certain
>> > problems significantly harder as the log is incomplete.
>> >
>> > Signed-off-by: Daniel P. Berrange 
>> 
>> Reviewed-by: Markus Armbruster 
>> 
>> I can take this through my tree, but it's perhaps easier to let it flow
>> along with the rest of your series.
>
> Nah, it isn't critical to the rest of this series. Its just tacked
> on as I noticed it while debugging tests, so you might as well just
> take it through your tree as normal.

Okay, applied to error-next, thanks!

Re: [Qemu-devel] [PATCH v1 21/21] error: ensure errno detail is printed with error_abort

[Daniel P. Berrange]

On Thu, Mar 10, 2016 at 09:55:37AM +0100, Markus Armbruster wrote:
> "Daniel P. Berrange"  writes:
> 
> > When _abort is passed in, the error reporting code
> > will print the current error message and then abort() the
> > process. Unfortunately at the time it aborts, we've not
> > yet appended the errno detail. This makes debugging certain
> > problems significantly harder as the log is incomplete.
> >
> > Signed-off-by: Daniel P. Berrange 
> 
> Reviewed-by: Markus Armbruster 
> 
> I can take this through my tree, but it's perhaps easier to let it flow
> along with the rest of your series.

Nah, it isn't critical to the rest of this series. Its just tacked
on as I noticed it while debugging tests, so you might as well just
take it through your tree as normal.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v1 21/21] error: ensure errno detail is printed with error_abort

[Markus Armbruster]

"Daniel P. Berrange"  writes:

> When _abort is passed in, the error reporting code
> will print the current error message and then abort() the
> process. Unfortunately at the time it aborts, we've not
> yet appended the errno detail. This makes debugging certain
> problems significantly harder as the log is incomplete.
>
> Signed-off-by: Daniel P. Berrange 

Reviewed-by: Markus Armbruster 

I can take this through my tree, but it's perhaps easier to let it flow
along with the rest of your series.

Re: [Qemu-devel] [PATCH v1 21/21] error: ensure errno detail is printed with error_abort

[Paolo Bonzini]

Ccing error.c maintainer.

Paolo

On 09/03/2016 18:28, Daniel P. Berrange wrote:
> When _abort is passed in, the error reporting code
> will print the current error message and then abort() the
> process. Unfortunately at the time it aborts, we've not
> yet appended the errno detail. This makes debugging certain
> problems significantly harder as the log is incomplete.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  util/error.c | 40 +++-
>  1 file changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/util/error.c b/util/error.c
> index 471b8b3..47f93af 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -44,7 +44,8 @@ static void error_handle_fatal(Error **errp, Error *err)
>  
>  static void error_setv(Error **errp,
> const char *src, int line, const char *func,
> -   ErrorClass err_class, const char *fmt, va_list ap)
> +   ErrorClass err_class, const char *fmt, va_list ap,
> +   const char *suffix)
>  {
>  Error *err;
>  int saved_errno = errno;
> @@ -56,6 +57,11 @@ static void error_setv(Error **errp,
>  
>  err = g_malloc0(sizeof(*err));
>  err->msg = g_strdup_vprintf(fmt, ap);
> +if (suffix) {
> +char *msg = err->msg;
> +err->msg = g_strdup_printf("%s: %s", msg, suffix);
> +g_free(msg);
> +}
>  err->err_class = err_class;
>  err->src = src;
>  err->line = line;
> @@ -74,7 +80,7 @@ void error_set_internal(Error **errp,
>  va_list ap;
>  
>  va_start(ap, fmt);
> -error_setv(errp, src, line, func, err_class, fmt, ap);
> +error_setv(errp, src, line, func, err_class, fmt, ap, NULL);
>  va_end(ap);
>  }
>  
> @@ -85,7 +91,7 @@ void error_setg_internal(Error **errp,
>  va_list ap;
>  
>  va_start(ap, fmt);
> -error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap);
> +error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap, 
> NULL);
>  va_end(ap);
>  }
>  
> @@ -94,7 +100,6 @@ void error_setg_errno_internal(Error **errp,
> int os_errno, const char *fmt, ...)
>  {
>  va_list ap;
> -char *msg;
>  int saved_errno = errno;
>  
>  if (errp == NULL) {
> @@ -102,15 +107,10 @@ void error_setg_errno_internal(Error **errp,
>  }
>  
>  va_start(ap, fmt);
> -error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap);
> +error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap,
> +   os_errno != 0 ? strerror(os_errno) : NULL);
>  va_end(ap);
>  
> -if (os_errno != 0) {
> -msg = (*errp)->msg;
> -(*errp)->msg = g_strdup_printf("%s: %s", msg, strerror(os_errno));
> -g_free(msg);
> -}
> -
>  errno = saved_errno;
>  }
>  
> @@ -174,24 +174,22 @@ void error_setg_win32_internal(Error **errp,
> int win32_err, const char *fmt, ...)
>  {
>  va_list ap;
> -char *msg1, *msg2;
> +char *suffix = NULL;
>  
>  if (errp == NULL) {
>  return;
>  }
>  
> +if (win32_err != 0) {
> +suffix = g_win32_error_message(win32_err);
> +}
> +
>  va_start(ap, fmt);
> -error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap);
> +error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR,
> +   fmt, ap, suffix);
>  va_end(ap);
>  
> -if (win32_err != 0) {
> -msg1 = (*errp)->msg;
> -msg2 = g_win32_error_message(win32_err);
> -(*errp)->msg = g_strdup_printf("%s: %s (error: %x)", msg1, msg2,
> -   (unsigned)win32_err);
> -g_free(msg2);
> -g_free(msg1);
> -}
> +g_free(suffix);
>  }
>  
>  #endif
> 

[Qemu-devel] [PATCH v1 21/21] error: ensure errno detail is printed with error_abort

[Daniel P. Berrange]

When _abort is passed in, the error reporting code
will print the current error message and then abort() the
process. Unfortunately at the time it aborts, we've not
yet appended the errno detail. This makes debugging certain
problems significantly harder as the log is incomplete.

Signed-off-by: Daniel P. Berrange 
---
 util/error.c | 40 +++-
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/util/error.c b/util/error.c
index 471b8b3..47f93af 100644
--- a/util/error.c
+++ b/util/error.c
@@ -44,7 +44,8 @@ static void error_handle_fatal(Error **errp, Error *err)
 
 static void error_setv(Error **errp,
const char *src, int line, const char *func,
-   ErrorClass err_class, const char *fmt, va_list ap)
+   ErrorClass err_class, const char *fmt, va_list ap,
+   const char *suffix)
 {
 Error *err;
 int saved_errno = errno;
@@ -56,6 +57,11 @@ static void error_setv(Error **errp,
 
 err = g_malloc0(sizeof(*err));
 err->msg = g_strdup_vprintf(fmt, ap);
+if (suffix) {
+char *msg = err->msg;
+err->msg = g_strdup_printf("%s: %s", msg, suffix);
+g_free(msg);
+}
 err->err_class = err_class;
 err->src = src;
 err->line = line;
@@ -74,7 +80,7 @@ void error_set_internal(Error **errp,
 va_list ap;
 
 va_start(ap, fmt);
-error_setv(errp, src, line, func, err_class, fmt, ap);
+error_setv(errp, src, line, func, err_class, fmt, ap, NULL);
 va_end(ap);
 }
 
@@ -85,7 +91,7 @@ void error_setg_internal(Error **errp,
 va_list ap;
 
 va_start(ap, fmt);
-error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap);
+error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap, 
NULL);
 va_end(ap);
 }
 
@@ -94,7 +100,6 @@ void error_setg_errno_internal(Error **errp,
int os_errno, const char *fmt, ...)
 {
 va_list ap;
-char *msg;
 int saved_errno = errno;
 
 if (errp == NULL) {
@@ -102,15 +107,10 @@ void error_setg_errno_internal(Error **errp,
 }
 
 va_start(ap, fmt);
-error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap);
+error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap,
+   os_errno != 0 ? strerror(os_errno) : NULL);
 va_end(ap);
 
-if (os_errno != 0) {
-msg = (*errp)->msg;
-(*errp)->msg = g_strdup_printf("%s: %s", msg, strerror(os_errno));
-g_free(msg);
-}
-
 errno = saved_errno;
 }
 
@@ -174,24 +174,22 @@ void error_setg_win32_internal(Error **errp,
int win32_err, const char *fmt, ...)
 {
 va_list ap;
-char *msg1, *msg2;
+char *suffix = NULL;
 
 if (errp == NULL) {
 return;
 }
 
+if (win32_err != 0) {
+suffix = g_win32_error_message(win32_err);
+}
+
 va_start(ap, fmt);
-error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR, fmt, ap);
+error_setv(errp, src, line, func, ERROR_CLASS_GENERIC_ERROR,
+   fmt, ap, suffix);
 va_end(ap);
 
-if (win32_err != 0) {
-msg1 = (*errp)->msg;
-msg2 = g_win32_error_message(win32_err);
-(*errp)->msg = g_strdup_printf("%s: %s (error: %x)", msg1, msg2,
-   (unsigned)win32_err);
-g_free(msg2);
-g_free(msg1);
-}
+g_free(suffix);
 }
 
 #endif
-- 
2.5.0