From: Marc-André Lureau <marcandre.lureau@redhat.com>
error_vprintf() is implemented in monitor.c, which overrides the
default implementation from stubs/, while avoiding a direct dependency
to the monitor from error-report.c.
However, the stub solution isn't working when moving error-report.c and
stubs/error-printf.c in a common library. Linking with such library
creates conflicts for the error_vprintf() implementations (and weak
symbols aren't great either). Instead, use the "traditional" approach to
provide overidable callbacks.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
include/monitor/monitor.h | 1 +
include/qemu/error-report.h | 6 ++++--
bsd-user/main.c | 2 +-
linux-user/main.c | 2 +-
monitor/monitor.c | 2 +-
qemu-img.c | 2 +-
qemu-io.c | 2 +-
qemu-nbd.c | 2 +-
scsi/qemu-pr-helper.c | 2 +-
softmmu/vl.c | 2 +-
storage-daemon/qemu-storage-daemon.c | 2 +-
stubs/error-printf.c | 18 ------------------
util/error-report.c | 27 ++++++++++++++++++++++++---
stubs/meson.build | 1 -
14 files changed, 38 insertions(+), 33 deletions(-)
delete mode 100644 stubs/error-printf.c
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 44653e195b45..e94ab2e74889 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -16,6 +16,7 @@ extern QemuOptsList qemu_mon_opts;
Monitor *monitor_cur(void);
Monitor *monitor_set_cur(Coroutine *co, Monitor *mon);
bool monitor_cur_is_qmp(void);
+int monitor_error_vprintf(const char *fmt, va_list ap);
void monitor_init_globals(void);
void monitor_init_globals_core(void);
diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index e2e630f207f0..a2bc030b4bfe 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -14,6 +14,7 @@
#define QEMU_ERROR_REPORT_H
typedef bool (*ErrorReportDetailedFunc)(void);
+typedef int (*ErrorReportVPrintfFunc)(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
typedef struct Location {
/* all members are private to qemu-error.c */
@@ -32,7 +33,6 @@ void loc_set_none(void);
void loc_set_cmdline(char **argv, int idx, int cnt);
void loc_set_file(const char *fname, int lno);
-int error_vprintf(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
int error_printf(const char *fmt, ...) G_GNUC_PRINTF(1, 2);
void error_vreport(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
@@ -48,7 +48,9 @@ bool error_report_once_cond(bool *printed, const char *fmt, ...)
bool warn_report_once_cond(bool *printed, const char *fmt, ...)
G_GNUC_PRINTF(2, 3);
-void error_init(const char *argv0, ErrorReportDetailedFunc detailed_fn);
+void error_init(const char *argv0,
+ ErrorReportDetailedFunc detailed_fn,
+ ErrorReportVPrintfFunc vprintf_fn);
/*
* Similar to error_report(), except it prints the message just once.
diff --git a/bsd-user/main.c b/bsd-user/main.c
index d5f8fca863d7..1cc1ba9b2e6e 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -292,7 +292,7 @@ int main(int argc, char **argv)
save_proc_pathname(argv[0]);
- error_init(argv[0], NULL);
+ error_init(argv[0], NULL, NULL);
module_call_init(MODULE_INIT_TRACE);
qemu_init_cpu_list();
module_call_init(MODULE_INIT_QOM);
diff --git a/linux-user/main.c b/linux-user/main.c
index 84f380bd366d..75f72099739d 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -646,7 +646,7 @@ int main(int argc, char **argv, char **envp)
unsigned long max_reserved_va;
bool preserve_argv0;
- error_init(argv[0], NULL);
+ error_init(argv[0], NULL, NULL);
module_call_init(MODULE_INIT_TRACE);
qemu_init_cpu_list();
module_call_init(MODULE_INIT_QOM);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index ba4c1716a48a..490e7babd895 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -263,7 +263,7 @@ int monitor_printf(Monitor *mon, const char *fmt, ...)
/*
* Print to current monitor if we have one, else to stderr.
*/
-int error_vprintf(const char *fmt, va_list ap)
+int monitor_error_vprintf(const char *fmt, va_list ap)
{
Monitor *cur_mon = monitor_cur();
diff --git a/qemu-img.c b/qemu-img.c
index 1f27a9fc70f6..00383f48f7bc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -5396,7 +5396,7 @@ int main(int argc, char **argv)
#endif
socket_init();
- error_init(argv[0], NULL);
+ error_init(argv[0], NULL, NULL);
module_call_init(MODULE_INIT_TRACE);
qemu_init_exec_dir(argv[0]);
diff --git a/qemu-io.c b/qemu-io.c
index b5cdc7c922a7..09794cd781be 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -539,7 +539,7 @@ int main(int argc, char **argv)
#endif
socket_init();
- error_init(argv[0], NULL);
+ error_init(argv[0], NULL, NULL);
module_call_init(MODULE_INIT_TRACE);
qemu_init_exec_dir(argv[0]);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 6bc632c93611..112303674cfb 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -587,7 +587,7 @@ int main(int argc, char **argv)
#endif
socket_init();
- error_init(argv[0], NULL);
+ error_init(argv[0], NULL, NULL);
module_call_init(MODULE_INIT_TRACE);
qcrypto_init(&error_fatal);
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 8d80e58d4498..d265d11b6261 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -910,7 +910,7 @@ int main(int argc, char **argv)
signal(SIGPIPE, SIG_IGN);
- error_init(argv[0], NULL);
+ error_init(argv[0], NULL, NULL);
module_call_init(MODULE_INIT_TRACE);
module_call_init(MODULE_INIT_QOM);
qemu_add_opts(&qemu_trace_opts);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 3b46fc9c1fc5..ef54af0efd6f 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2639,7 +2639,7 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_add_opts(&qemu_action_opts);
module_call_init(MODULE_INIT_OPTS);
- error_init(argv[0], error_is_detailed);
+ error_init(argv[0], error_is_detailed, monitor_error_vprintf);
qemu_init_exec_dir(argv[0]);
qemu_init_arch_modules();
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 7e4d5030a045..0e0893695628 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -379,7 +379,7 @@ int main(int argc, char *argv[])
signal(SIGPIPE, SIG_IGN);
#endif
- error_init(argv[0], error_is_detailed);
+ error_init(argv[0], error_is_detailed, monitor_error_vprintf);
qemu_init_exec_dir(argv[0]);
os_setup_signal_handling();
diff --git a/stubs/error-printf.c b/stubs/error-printf.c
deleted file mode 100644
index 1afa0f62ca26..000000000000
--- 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/util/error-report.c b/util/error-report.c
index c2181f80a83d..1452047cd2e8 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -23,11 +23,14 @@ typedef enum {
REPORT_TYPE_INFO,
} report_type;
+static int error_vprintf(const char *fmt, va_list ap);
+
/* Prepend timestamp to messages */
bool message_with_timestamp;
bool error_with_guestname;
const char *error_guest_name;
ErrorReportDetailedFunc detailed_fn = NULL;
+ErrorReportVPrintfFunc vprintf_fn = error_vprintf;
int error_printf(const char *fmt, ...)
{
@@ -35,7 +38,7 @@ int error_printf(const char *fmt, ...)
int ret;
va_start(ap, fmt);
- ret = error_vprintf(fmt, ap);
+ ret = vprintf_fn(fmt, ap);
va_end(ap);
return ret;
}
@@ -222,7 +225,7 @@ static void vreport(report_type type, const char *fmt, va_list ap)
break;
}
- error_vprintf(fmt, ap);
+ vprintf_fn(fmt, ap);
error_printf("\n");
}
@@ -387,7 +390,24 @@ static void qemu_log_func(const gchar *log_domain,
}
}
-void error_init(const char *argv0, ErrorReportDetailedFunc detailed)
+static 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);
+}
+
+void error_init(const char *argv0,
+ ErrorReportDetailedFunc detailed,
+ ErrorReportVPrintfFunc vprintf)
{
const char *p = strrchr(argv0, '/');
@@ -403,4 +423,5 @@ void error_init(const char *argv0, ErrorReportDetailedFunc detailed)
qemu_glog_domains = g_strdup(g_getenv("G_MESSAGES_DEBUG"));
detailed_fn = detailed;
+ vprintf_fn = vprintf ?: error_vprintf;
}
diff --git a/stubs/meson.build b/stubs/meson.build
index d8f3fd5c44f2..498b6ee0466e 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -9,7 +9,6 @@ stub_ss.add(files('cpus-get-virtual-clock.c'))
stub_ss.add(files('qemu-timer-notify-cb.c'))
stub_ss.add(files('icount.c'))
stub_ss.add(files('dump.c'))
-stub_ss.add(files('error-printf.c'))
stub_ss.add(files('fdset.c'))
stub_ss.add(files('gdbstub.c'))
stub_ss.add(files('get-vm-name.c'))
--
2.37.0.rc0
marcandre.lureau@redhat.com writes: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > error_vprintf() is implemented in monitor.c, which overrides the > default implementation from stubs/, while avoiding a direct dependency > to the monitor from error-report.c. > > However, the stub solution isn't working when moving error-report.c and > stubs/error-printf.c in a common library. Linking with such library > creates conflicts for the error_vprintf() implementations I'm feeling dense today: how? Why would the linker pull in error-printf.o when the symbols it defines are already provided by .o the linker processed before the library containing error-printf.o? > (and weak > symbols aren't great either). Weak symbols are great, except Windows isn't. > Instead, use the "traditional" approach to > provide overidable callbacks. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi On Thu, Jul 7, 2022 at 4:18 PM Markus Armbruster <armbru@redhat.com> wrote: > marcandre.lureau@redhat.com writes: > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > error_vprintf() is implemented in monitor.c, which overrides the > > default implementation from stubs/, while avoiding a direct dependency > > to the monitor from error-report.c. > > > > However, the stub solution isn't working when moving error-report.c and > > stubs/error-printf.c in a common library. Linking with such library > > creates conflicts for the error_vprintf() implementations > > I'm feeling dense today: how? > If I try to overwrite a symbol in qemu when linking with a static library from a subproject, the linker fails: FAILED: storage-daemon/qemu-storage-daemon cc -m64 -mcx16 -o storage-daemon/qemu-storage-daemon storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-introspect.c.o storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-types.c.o storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-visit.c.o storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-commands.c.o storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-init-commands.c.o storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-events.c.o storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-emit-events.c.o storage-daemon/qemu-storage-daemon.p/qemu-storage-daemon.c.o -Wl,--as-needed -Wl,--no-undefined -pie -Wl,--whole-archive libblockdev.fa libblock.fa libcrypto.fa libauthz.fa libqom.fa libio.fa -Wl,--start-group libevent-loop-base.a libchardev.fa libqmp.fa -Wl,--no-whole-archive -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -fstack-protector-strong -Wl,-rpath,/usr/lib64/iscsi -Wl,-rpath-link,/usr/lib64/iscsi libqemuutil.a subprojects/libvhost-user/libvhost-user-glib.a subprojects/libvhost-user/libvhost-user.a subprojects/qemu-common/libqemu-common.a libblockdev.fa subprojects/libvduse/libvduse.a libblock.fa libcrypto.fa libauthz.fa libqom.fa libio.fa libchardev.fa libqmp.fa @block.syms /usr/lib64/libgnutls.so /usr/lib64/liburing.so /usr/lib64/libgio-2.0.so /usr/lib64/libgobject-2.0.so /usr/lib64/libglib-2.0.so -Wl,--export-dynamic -pthread -lgmodule-2.0 -lglib-2.0 -lglib-2.0 -lm /usr/lib64/libpixman-1.so -lgmodule-2.0 -lglib-2.0 -lglib-2.0 -lgmodule-2.0 -lglib-2.0 -lglib-2.0 -lgmodule-2.0 -lglib-2.0 -lglib-2.0 /usr/lib64/libfuse3.so -lpthread -lgmodule-2.0 -lglib-2.0 -lglib-2.0 /usr/lib64/libzstd.so /usr/lib64/libz.so /usr/lib64/iscsi/libiscsi.so -laio -lpam -lutil -Wl,--end-group /usr/bin/ld: subprojects/qemu-common/libqemu-common.a.p/src_error-report.c.o: in function `error_init': /home/elmarco/src/qemu/build/../subprojects/qemu-common/src/error-report.c:413: multiple definition of `error_init'; libqmp.fa.p/monitor_qmp.c.o:/home/elmarco/src/qemu/build/../monitor/qmp.c:40: first defined here But I can't really explain why it works when overwriting symbols from libqemuutil.a, I am a bit puzzled here. Am I missing something obvious? (this is a bit of dark linker magic going on) > Why would the linker pull in error-printf.o when the symbols it defines > are already provided by .o the linker processed before the library > containing error-printf.o? > > > (and weak > > symbols aren't great either). > > Weak symbols are great, except Windows isn't. > > > Instead, use the "traditional" approach to > > provide overidable callbacks. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > -- Marc-André Lureau
Hi On Thu, Jul 7, 2022 at 10:05 PM Marc-André Lureau < marcandre.lureau@gmail.com> wrote: > Hi > > On Thu, Jul 7, 2022 at 4:18 PM Markus Armbruster <armbru@redhat.com> > wrote: > >> marcandre.lureau@redhat.com writes: >> >> > From: Marc-André Lureau <marcandre.lureau@redhat.com> >> > >> > error_vprintf() is implemented in monitor.c, which overrides the >> > default implementation from stubs/, while avoiding a direct dependency >> > to the monitor from error-report.c. >> > >> > However, the stub solution isn't working when moving error-report.c and >> > stubs/error-printf.c in a common library. Linking with such library >> > creates conflicts for the error_vprintf() implementations >> >> I'm feeling dense today: how? >> > > If I try to overwrite a symbol in qemu when linking with a static library > from a subproject, the linker fails: > > FAILED: storage-daemon/qemu-storage-daemon > cc -m64 -mcx16 -o storage-daemon/qemu-storage-daemon > storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-introspect.c.o > storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-types.c.o > storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-visit.c.o > storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-commands.c.o > storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-init-commands.c.o > storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-events.c.o > storage-daemon/qemu-storage-daemon.p/meson-generated_.._qapi_qapi-emit-events.c.o > storage-daemon/qemu-storage-daemon.p/qemu-storage-daemon.c.o > -Wl,--as-needed -Wl,--no-undefined -pie -Wl,--whole-archive libblockdev.fa > libblock.fa libcrypto.fa libauthz.fa libqom.fa libio.fa -Wl,--start-group > libevent-loop-base.a libchardev.fa libqmp.fa -Wl,--no-whole-archive > -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -fstack-protector-strong > -Wl,-rpath,/usr/lib64/iscsi -Wl,-rpath-link,/usr/lib64/iscsi libqemuutil.a > subprojects/libvhost-user/libvhost-user-glib.a > subprojects/libvhost-user/libvhost-user.a > subprojects/qemu-common/libqemu-common.a libblockdev.fa > subprojects/libvduse/libvduse.a libblock.fa libcrypto.fa libauthz.fa > libqom.fa libio.fa libchardev.fa libqmp.fa @block.syms > /usr/lib64/libgnutls.so /usr/lib64/liburing.so /usr/lib64/libgio-2.0.so > /usr/lib64/libgobject-2.0.so /usr/lib64/libglib-2.0.so > -Wl,--export-dynamic -pthread -lgmodule-2.0 -lglib-2.0 -lglib-2.0 -lm > /usr/lib64/libpixman-1.so -lgmodule-2.0 -lglib-2.0 -lglib-2.0 -lgmodule-2.0 > -lglib-2.0 -lglib-2.0 -lgmodule-2.0 -lglib-2.0 -lglib-2.0 > /usr/lib64/libfuse3.so -lpthread -lgmodule-2.0 -lglib-2.0 -lglib-2.0 > /usr/lib64/libzstd.so /usr/lib64/libz.so /usr/lib64/iscsi/libiscsi.so -laio > -lpam -lutil -Wl,--end-group > /usr/bin/ld: > subprojects/qemu-common/libqemu-common.a.p/src_error-report.c.o: in > function `error_init': > /home/elmarco/src/qemu/build/../subprojects/qemu-common/src/error-report.c:413: > multiple definition of `error_init'; > libqmp.fa.p/monitor_qmp.c.o:/home/elmarco/src/qemu/build/../monitor/qmp.c:40: > first defined here > > But I can't really explain why it works when overwriting symbols from > libqemuutil.a, I am a bit puzzled here. Am I missing something obvious? > (this is a bit of dark linker magic going on) > > After playing with this for a few hours ... I managed to get the symbol override work, by having a single overridable function per unit. No idea why in qemutil.a/stubs we manage to have several. That's a mystery. Anyway, I will send a new version where I also introduce the subproject, to demonstrate it works. thanks > >> Why would the linker pull in error-printf.o when the symbols it defines >> are already provided by .o the linker processed before the library >> containing error-printf.o? >> >> > (and weak >> > symbols aren't great either). >> >> Weak symbols are great, except Windows isn't. >> >> > Instead, use the "traditional" approach to >> > provide overidable callbacks. >> > >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> >> > > -- > Marc-André Lureau > -- Marc-André Lureau
© 2016 - 2026 Red Hat, Inc.