[Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue)

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue, 14 Jan 2014 17:44:51 +0100
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 14.01.2014 um 04:38 hat Edgar E. Iglesias geschrieben:
> > On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
> > > Ping,
> > > 
> > > Has this one been forgotten or are there issues? PMM had a small
> > > comment, but he waived it AFAICT.
> > 
> > Pong,
> > 
> > I've merged it now, thanks!
> 
> I believe it's something in this pull requests that breaks make check.

And you're right. But first, let me confirm that we're talking about the
same breakage. This is what I'm getting:

make tests/check-qom-interface
libqemuutil.a(qemu-error.o): In function `error_vprintf':
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:23: undefined reference to `cur_mon'
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `cur_mon'
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `monitor_vprintf'
libqemuutil.a(qemu-error.o): In function `error_printf_unless_qmp':
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:47: undefined reference to `monitor_cur_is_qmp'
libqemuutil.a(qemu-error.o): In function `error_print_loc':
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:174: undefined reference to `cur_mon'
collect2: error: ld returned 1 exit status
make: *** [tests/check-qom-interface] Error 1

I tried bisecting it, but git bisect weren't capable of finding the
culprit. So debugged it, and the problem was introduced by:

  commit 594278718323ca7bffaab0fb7fc6c82fa2c1cd5f
  Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
  Date:   Wed Jan 1 18:49:52 2014 -0800
  
      qerror: Remove assert_no_error()

There isn't nothing really wrong with this commit. The problem seems to
be that the tests link against libqemuutil.a and this library pulls in
everything from util/. The commit above changed util/error.c to call
error_report(), which depends on 'cur_mon', which is only made available
by monitor.o.

I don't think we want to mess up with including monitor.o on libqemuutil.a.
Besides, I now find it a bit weird to call error_report() from an error
reporting function. So it's better to just call fprintf(stderr,) instead.

Peter, Markus, are you ok with this patch?

PS: I do run make check before sending a pull request, and did run this
    time too. Not sure how I didn't catch this. Thanks for the report
    Kevin!

diff --git a/util/error.c b/util/error.c
index f11f1d5..7c7650c 100644
--- a/util/error.c
+++ b/util/error.c
@@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
     err->err_class = err_class;
 
     if (errp == &error_abort) {
-        error_report("%s", error_get_pretty(err));
+        fprintf(stderr, "%s", error_get_pretty(err));
         abort();
     }
 
@@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
     err->err_class = err_class;
 
     if (errp == &error_abort) {
-        error_report("%s", error_get_pretty(err));
+        fprintf(stderr, "%s", error_get_pretty(err));
         abort();
     }
 
@@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
     err->err_class = err_class;
 
     if (errp == &error_abort) {
-        error_report("%s", error_get_pretty(err));
+        fprintf(stderr, "%s", error_get_pretty(err));
         abort();
     }
 
@@ -171,7 +171,7 @@ void error_free(Error *err)
 void error_propagate(Error **dst_err, Error *local_err)
 {
     if (local_err && dst_err == &error_abort) {
-        error_report("%s", error_get_pretty(local_err));
+        fprintf(stderr, "%s", error_get_pretty(local_err));
         abort();
     } else if (dst_err && !*dst_err) {
         *dst_err = local_err;