On Tue, Jan 13, 2026 at 02:38:19PM +0100, Markus Armbruster wrote: > Daniel P. Berrangé <[email protected]> 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 <[email protected]> > > Reviewed-by: Eric Blake <[email protected]> > > Signed-off-by: Daniel P. Berrangé <[email protected]> > > --- > > 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 :|
