The current unit tests rely on monitor.o not being linked, such
that the monitor stubs get linked instead. Since error_vprintf
is in monitor.o this allows a stub error_vprintf impl to be used
that calls g_test_message.

This takes a different approach, with error_vprintf moving
back to error-report.c such that it is always linked into the
tests. The monitor_vprintf() stub is then changed to use
g_test_message if QTEST_SILENT_ERRORS is set, otherwise it will
return -1 and trigger error_vprintf to call vfprintf.

The end result is functionally equivalent for the purposes of
the unit tests.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/monitor.c    | 15 ---------------
 stubs/error-printf.c | 18 ------------------
 stubs/meson.build    |  1 -
 stubs/monitor-core.c | 14 +++++++++++++-
 util/error-report.c  | 15 +++++++++++++++
 5 files changed, 28 insertions(+), 35 deletions(-)
 delete mode 100644 stubs/error-printf.c

diff --git a/monitor/monitor.c b/monitor/monitor.c
index 627a59b23e..6dc5a7016d 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -268,21 +268,6 @@ void monitor_printc(Monitor *mon, int c)
     monitor_printf(mon, "'");
 }
 
-int error_vprintf(const char *fmt, va_list ap)
-{
-    Monitor *cur_mon = monitor_cur();
-    /*
-     * This will return -1 if 'cur_mon' is NULL, or is QMP.
-     * IOW this will only print if in HMP, otherwise we
-     * fallback to stderr for QMP / no-monitor scenarios.
-     */
-    int ret = monitor_vprintf(cur_mon, fmt, ap);
-    if (ret == -1) {
-        ret = vfprintf(stderr, fmt, ap);
-    }
-    return ret;
-}
-
 static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
     /* Limit guest-triggerable events to 1 per second */
     [QAPI_EVENT_RTC_CHANGE]        = { 1000 * SCALE_MS },
diff --git a/stubs/error-printf.c b/stubs/error-printf.c
deleted file mode 100644
index 1afa0f62ca..0000000000
--- a/stubs/error-printf.c
+++ /dev/null
@@ -1,18 +0,0 @@
-#include "qemu/osdep.h"
-#include "qemu/error-report.h"
-#include "monitor/monitor.h"
-
-int error_vprintf(const char *fmt, va_list ap)
-{
-    int ret;
-
-    if (g_test_initialized() && !g_test_subprocess() &&
-        getenv("QTEST_SILENT_ERRORS")) {
-        char *msg = g_strdup_vprintf(fmt, ap);
-        g_test_message("%s", msg);
-        ret = strlen(msg);
-        g_free(msg);
-        return ret;
-    }
-    return vfprintf(stderr, fmt, ap);
-}
diff --git a/stubs/meson.build b/stubs/meson.build
index 0b2778c568..3d77458a3f 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -3,7 +3,6 @@
 # below, so that it is clear who needs the stubbed functionality.
 
 stub_ss.add(files('cpu-get-clock.c'))
-stub_ss.add(files('error-printf.c'))
 stub_ss.add(files('fdset.c'))
 stub_ss.add(files('iothread-lock.c'))
 stub_ss.add(files('is-daemonized.c'))
diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c
index 1894cdfe1f..a7c32297c9 100644
--- a/stubs/monitor-core.c
+++ b/stubs/monitor-core.c
@@ -18,5 +18,17 @@ void qapi_event_emit(QAPIEvent event, QDict *qdict)
 
 int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 {
-    abort();
+    /*
+     * Pretend 'g_test_message' is our monitor console to
+     * stop the caller sending messages to stderr
+     */
+    if (g_test_initialized() && !g_test_subprocess() &&
+        getenv("QTEST_SILENT_ERRORS")) {
+        char *msg = g_strdup_vprintf(fmt, ap);
+        g_test_message("%s", msg);
+        size_t ret = strlen(msg);
+        g_free(msg);
+        return ret;
+    }
+    return -1;
 }
diff --git a/util/error-report.c b/util/error-report.c
index 1b17c11de1..b262ad01cb 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -29,6 +29,21 @@ bool message_with_timestamp;
 bool error_with_guestname;
 const char *error_guest_name;
 
+int error_vprintf(const char *fmt, va_list ap)
+{
+    Monitor *cur_mon = monitor_cur();
+    /*
+     * This will return -1 if 'cur_mon' is NULL, or is QMP.
+     * IOW this will only print if in HMP, otherwise we
+     * fallback to stderr for QMP / no-monitor scenarios.
+     */
+    int ret = monitor_vprintf(cur_mon, fmt, ap);
+    if (ret == -1) {
+        ret = vfprintf(stderr, fmt, ap);
+    }
+    return ret;
+}
+
 int error_printf(const char *fmt, ...)
 {
     va_list ap;
-- 
2.52.0

Daniel P. Berrangé <berrange@redhat.com> writes:

> The current unit tests rely on monitor.o not being linked, such
> that the monitor stubs get linked instead. Since error_vprintf
> is in monitor.o this allows a stub error_vprintf impl to be used
> that calls g_test_message.
>
> This takes a different approach, with error_vprintf moving
> back to error-report.c such that it is always linked into the
> tests. The monitor_vprintf() stub is then changed to use
> g_test_message if QTEST_SILENT_ERRORS is set, otherwise it will
> return -1 and trigger error_vprintf to call vfprintf.
>
> The end result is functionally equivalent for the purposes of
> the unit tests.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  monitor/monitor.c    | 15 ---------------
>  stubs/error-printf.c | 18 ------------------
>  stubs/meson.build    |  1 -
>  stubs/monitor-core.c | 14 +++++++++++++-
>  util/error-report.c  | 15 +++++++++++++++
>  5 files changed, 28 insertions(+), 35 deletions(-)
>  delete mode 100644 stubs/error-printf.c
>
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 627a59b23e..6dc5a7016d 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -268,21 +268,6 @@ void monitor_printc(Monitor *mon, int c)
>      monitor_printf(mon, "'");
>  }
>  
> -int error_vprintf(const char *fmt, va_list ap)
> -{
> -    Monitor *cur_mon = monitor_cur();
> -    /*
> -     * This will return -1 if 'cur_mon' is NULL, or is QMP.
> -     * IOW this will only print if in HMP, otherwise we
> -     * fallback to stderr for QMP / no-monitor scenarios.
> -     */
> -    int ret = monitor_vprintf(cur_mon, fmt, ap);
> -    if (ret == -1) {
> -        ret = vfprintf(stderr, fmt, ap);
> -    }
> -    return ret;
> -}
> -
>  static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
>      /* Limit guest-triggerable events to 1 per second */
>      [QAPI_EVENT_RTC_CHANGE]        = { 1000 * SCALE_MS },
> diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> deleted file mode 100644
> index 1afa0f62ca..0000000000
> --- a/stubs/error-printf.c
> +++ /dev/null
> @@ -1,18 +0,0 @@
> -#include "qemu/osdep.h"
> -#include "qemu/error-report.h"
> -#include "monitor/monitor.h"
> -
> -int error_vprintf(const char *fmt, va_list ap)
> -{
> -    int ret;
> -
> -    if (g_test_initialized() && !g_test_subprocess() &&
> -        getenv("QTEST_SILENT_ERRORS")) {
> -        char *msg = g_strdup_vprintf(fmt, ap);
> -        g_test_message("%s", msg);
> -        ret = strlen(msg);
> -        g_free(msg);
> -        return ret;
> -    }
> -    return vfprintf(stderr, fmt, ap);
> -}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 0b2778c568..3d77458a3f 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -3,7 +3,6 @@
>  # below, so that it is clear who needs the stubbed functionality.
>  
>  stub_ss.add(files('cpu-get-clock.c'))
> -stub_ss.add(files('error-printf.c'))
>  stub_ss.add(files('fdset.c'))
>  stub_ss.add(files('iothread-lock.c'))
>  stub_ss.add(files('is-daemonized.c'))
> diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c
> index 1894cdfe1f..a7c32297c9 100644
> --- a/stubs/monitor-core.c
> +++ b/stubs/monitor-core.c
> @@ -18,5 +18,17 @@ void qapi_event_emit(QAPIEvent event, QDict *qdict)
>  
>  int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
>  {
> -    abort();
> +    /*
> +     * Pretend 'g_test_message' is our monitor console to
> +     * stop the caller sending messages to stderr
> +     */
> +    if (g_test_initialized() && !g_test_subprocess() &&
> +        getenv("QTEST_SILENT_ERRORS")) {
> +        char *msg = g_strdup_vprintf(fmt, ap);
> +        g_test_message("%s", msg);
> +        size_t ret = strlen(msg);
> +        g_free(msg);
> +        return ret;
> +    }
> +    return -1;
>  }
> diff --git a/util/error-report.c b/util/error-report.c
> index 1b17c11de1..b262ad01cb 100644
> --- a/util/error-report.c
> +++ b/util/error-report.c
> @@ -29,6 +29,21 @@ bool message_with_timestamp;
>  bool error_with_guestname;
>  const char *error_guest_name;
>  
> +int error_vprintf(const char *fmt, va_list ap)
> +{
> +    Monitor *cur_mon = monitor_cur();
> +    /*
> +     * This will return -1 if 'cur_mon' is NULL, or is QMP.
> +     * IOW this will only print if in HMP, otherwise we
> +     * fallback to stderr for QMP / no-monitor scenarios.
> +     */
> +    int ret = monitor_vprintf(cur_mon, fmt, ap);
> +    if (ret == -1) {
> +        ret = vfprintf(stderr, fmt, ap);
> +    }
> +    return ret;
> +}
> +
>  int error_printf(const char *fmt, ...)
>  {
>      va_list ap;

Without stubs, no change in behavior.

With both stubs, before the patch:

    monitor_vprintf() is not supposed to run, and aborts

    error_vprintf() calls g_test_message() for tests, else vfprintf()

afterwards:

    monitor_vprintf() calls g_test_message() and succeeds in tests, else
    fails

    error_vprintf() calls monitor_printf(), and when it fails falls back
    to vfprintf().

Alright, error_vprintf() behaves the same as before.

monitor_vprintf() no longer aborts.  Hmm.  What if we somehow acquire
calls?  In tests, they'll go to g_test_message(), which is fine, I
guess.  Outside tests, they'll fail.  So does the non-stub version
unless the current monitor is HMP.  Also fine, I guess.

Is it possible to link just one of the stubs?

On Tue, Jan 13, 2026 at 02:38:19PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > The current unit tests rely on monitor.o not being linked, such
> > that the monitor stubs get linked instead. Since error_vprintf
> > is in monitor.o this allows a stub error_vprintf impl to be used
> > that calls g_test_message.
> >
> > This takes a different approach, with error_vprintf moving
> > back to error-report.c such that it is always linked into the
> > tests. The monitor_vprintf() stub is then changed to use
> > g_test_message if QTEST_SILENT_ERRORS is set, otherwise it will
> > return -1 and trigger error_vprintf to call vfprintf.
> >
> > The end result is functionally equivalent for the purposes of
> > the unit tests.
> >
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  monitor/monitor.c    | 15 ---------------
> >  stubs/error-printf.c | 18 ------------------
> >  stubs/meson.build    |  1 -
> >  stubs/monitor-core.c | 14 +++++++++++++-
> >  util/error-report.c  | 15 +++++++++++++++
> >  5 files changed, 28 insertions(+), 35 deletions(-)
> >  delete mode 100644 stubs/error-printf.c
> >
> > diff --git a/monitor/monitor.c b/monitor/monitor.c
> > index 627a59b23e..6dc5a7016d 100644
> > --- a/monitor/monitor.c
> > +++ b/monitor/monitor.c
> > @@ -268,21 +268,6 @@ void monitor_printc(Monitor *mon, int c)
> >      monitor_printf(mon, "'");
> >  }
> >  
> > -int error_vprintf(const char *fmt, va_list ap)
> > -{
> > -    Monitor *cur_mon = monitor_cur();
> > -    /*
> > -     * This will return -1 if 'cur_mon' is NULL, or is QMP.
> > -     * IOW this will only print if in HMP, otherwise we
> > -     * fallback to stderr for QMP / no-monitor scenarios.
> > -     */
> > -    int ret = monitor_vprintf(cur_mon, fmt, ap);
> > -    if (ret == -1) {
> > -        ret = vfprintf(stderr, fmt, ap);
> > -    }
> > -    return ret;
> > -}
> > -
> >  static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
> >      /* Limit guest-triggerable events to 1 per second */
> >      [QAPI_EVENT_RTC_CHANGE]        = { 1000 * SCALE_MS },
> > diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> > deleted file mode 100644
> > index 1afa0f62ca..0000000000
> > --- a/stubs/error-printf.c
> > +++ /dev/null
> > @@ -1,18 +0,0 @@
> > -#include "qemu/osdep.h"
> > -#include "qemu/error-report.h"
> > -#include "monitor/monitor.h"
> > -
> > -int error_vprintf(const char *fmt, va_list ap)
> > -{
> > -    int ret;
> > -
> > -    if (g_test_initialized() && !g_test_subprocess() &&
> > -        getenv("QTEST_SILENT_ERRORS")) {
> > -        char *msg = g_strdup_vprintf(fmt, ap);
> > -        g_test_message("%s", msg);
> > -        ret = strlen(msg);
> > -        g_free(msg);
> > -        return ret;
> > -    }
> > -    return vfprintf(stderr, fmt, ap);
> > -}
> > diff --git a/stubs/meson.build b/stubs/meson.build
> > index 0b2778c568..3d77458a3f 100644
> > --- a/stubs/meson.build
> > +++ b/stubs/meson.build
> > @@ -3,7 +3,6 @@
> >  # below, so that it is clear who needs the stubbed functionality.
> >  
> >  stub_ss.add(files('cpu-get-clock.c'))
> > -stub_ss.add(files('error-printf.c'))
> >  stub_ss.add(files('fdset.c'))
> >  stub_ss.add(files('iothread-lock.c'))
> >  stub_ss.add(files('is-daemonized.c'))
> > diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c
> > index 1894cdfe1f..a7c32297c9 100644
> > --- a/stubs/monitor-core.c
> > +++ b/stubs/monitor-core.c
> > @@ -18,5 +18,17 @@ void qapi_event_emit(QAPIEvent event, QDict *qdict)
> >  
> >  int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
> >  {
> > -    abort();
> > +    /*
> > +     * Pretend 'g_test_message' is our monitor console to
> > +     * stop the caller sending messages to stderr
> > +     */
> > +    if (g_test_initialized() && !g_test_subprocess() &&
> > +        getenv("QTEST_SILENT_ERRORS")) {
> > +        char *msg = g_strdup_vprintf(fmt, ap);
> > +        g_test_message("%s", msg);
> > +        size_t ret = strlen(msg);
> > +        g_free(msg);
> > +        return ret;
> > +    }
> > +    return -1;
> >  }
> > diff --git a/util/error-report.c b/util/error-report.c
> > index 1b17c11de1..b262ad01cb 100644
> > --- a/util/error-report.c
> > +++ b/util/error-report.c
> > @@ -29,6 +29,21 @@ bool message_with_timestamp;
> >  bool error_with_guestname;
> >  const char *error_guest_name;
> >  
> > +int error_vprintf(const char *fmt, va_list ap)
> > +{
> > +    Monitor *cur_mon = monitor_cur();
> > +    /*
> > +     * This will return -1 if 'cur_mon' is NULL, or is QMP.
> > +     * IOW this will only print if in HMP, otherwise we
> > +     * fallback to stderr for QMP / no-monitor scenarios.
> > +     */
> > +    int ret = monitor_vprintf(cur_mon, fmt, ap);
> > +    if (ret == -1) {
> > +        ret = vfprintf(stderr, fmt, ap);
> > +    }
> > +    return ret;
> > +}
> > +
> >  int error_printf(const char *fmt, ...)
> >  {
> >      va_list ap;
> 
> Without stubs, no change in behavior.
> 
> With both stubs, before the patch:
> 
>     monitor_vprintf() is not supposed to run, and aborts
> 
>     error_vprintf() calls g_test_message() for tests, else vfprintf()
> 
> afterwards:
> 
>     monitor_vprintf() calls g_test_message() and succeeds in tests, else
>     fails
> 
>     error_vprintf() calls monitor_printf(), and when it fails falls back
>     to vfprintf().
> 
> Alright, error_vprintf() behaves the same as before.
> 
> monitor_vprintf() no longer aborts.  Hmm.  What if we somehow acquire
> calls?  In tests, they'll go to g_test_message(), which is fine, I
> guess.  Outside tests, they'll fail.  So does the non-stub version
> unless the current monitor is HMP.  Also fine, I guess.
> 
> Is it possible to link just one of the stubs?

There is only 1 stub after this patch - for monitor_vprintf().
error_vprintf() is never stubbed anymore.

The interesting scenario outside there tests is the non-system emulator
binaries.

Those will not have the monitor code, so will get the monitor_vprintf
stub. That will report g_test_initialized() == false, and so the stub
will return -1.  error_vprintf() will see this -1 return value and
thus call to vfprintf().

So the behaviour is again the same as before this patch AFAICT for all
linkage scenarios.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Jan 13, 2026 at 02:38:19PM +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > The current unit tests rely on monitor.o not being linked, such
>> > that the monitor stubs get linked instead. Since error_vprintf
>> > is in monitor.o this allows a stub error_vprintf impl to be used
>> > that calls g_test_message.
>> >
>> > This takes a different approach, with error_vprintf moving
>> > back to error-report.c such that it is always linked into the
>> > tests. The monitor_vprintf() stub is then changed to use
>> > g_test_message if QTEST_SILENT_ERRORS is set, otherwise it will
>> > return -1 and trigger error_vprintf to call vfprintf.
>> >
>> > The end result is functionally equivalent for the purposes of
>> > the unit tests.
>> >
>> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> > Reviewed-by: Eric Blake <eblake@redhat.com>
>> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

[...]

>> Without stubs, no change in behavior.
>> 
>> With both stubs, before the patch:
>> 
>>     monitor_vprintf() is not supposed to run, and aborts
>> 
>>     error_vprintf() calls g_test_message() for tests, else vfprintf()
>> 
>> afterwards:
>> 
>>     monitor_vprintf() calls g_test_message() and succeeds in tests, else
>>     fails
>> 
>>     error_vprintf() calls monitor_printf(), and when it fails falls back
>>     to vfprintf().
>> 
>> Alright, error_vprintf() behaves the same as before.
>> 
>> monitor_vprintf() no longer aborts.  Hmm.  What if we somehow acquire
>> calls?  In tests, they'll go to g_test_message(), which is fine, I
>> guess.  Outside tests, they'll fail.  So does the non-stub version
>> unless the current monitor is HMP.  Also fine, I guess.
>> 
>> Is it possible to link just one of the stubs?
>
> There is only 1 stub after this patch - for monitor_vprintf().
> error_vprintf() is never stubbed anymore.
>
> The interesting scenario outside there tests is the non-system emulator
> binaries.
>
> Those will not have the monitor code, so will get the monitor_vprintf
> stub. That will report g_test_initialized() == false, and so the stub
> will return -1.  error_vprintf() will see this -1 return value and
> thus call to vfprintf().
>
> So the behaviour is again the same as before this patch AFAICT for all
> linkage scenarios.

Thank you!

Reviewed-by: Markus Armbruster <armbru@redhat.com>