Hi On Thu, Jul 7, 2022 at 4:13 PM Markus Armbruster <[email protected]> wrote:
> [email protected] writes: > > > From: Marc-André Lureau <[email protected]> > > > > Remove monitor dependency from error printing code, by allowing programs > > to set a callback for when to use "detailed" reporting or not. > > > > Signed-off-by: Marc-André Lureau <[email protected]> > > --- > > include/qemu/error-report.h | 4 +++- > > bsd-user/main.c | 2 +- > > linux-user/main.c | 2 +- > > qemu-img.c | 2 +- > > qemu-io.c | 2 +- > > qemu-nbd.c | 2 +- > > scsi/qemu-pr-helper.c | 2 +- > > softmmu/vl.c | 7 ++++++- > > storage-daemon/qemu-storage-daemon.c | 7 ++++++- > > util/error-report.c | 8 +++++--- > > 10 files changed, 26 insertions(+), 12 deletions(-) > > > > diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h > > index 3ae2357fda54..e2e630f207f0 100644 > > --- a/include/qemu/error-report.h > > +++ b/include/qemu/error-report.h > > @@ -13,6 +13,8 @@ > > #ifndef QEMU_ERROR_REPORT_H > > #define QEMU_ERROR_REPORT_H > > > > +typedef bool (*ErrorReportDetailedFunc)(void); > > + > > typedef struct Location { > > /* all members are private to qemu-error.c */ > > enum { LOC_NONE, LOC_CMDLINE, LOC_FILE } kind; > > @@ -46,7 +48,7 @@ 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); > > +void error_init(const char *argv0, ErrorReportDetailedFunc detailed_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 6f09180d6541..d5f8fca863d7 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]); > > + error_init(argv[0], NULL); > > module_call_init(MODULE_INIT_TRACE); > > qemu_init_cpu_list(); > > module_call_init(MODULE_INIT_QOM); > [More such calls of error_init()...] > > diff --git a/softmmu/vl.c b/softmmu/vl.c > > index 54e920ada1a1..3b46fc9c1fc5 100644 > > --- a/softmmu/vl.c > > +++ b/softmmu/vl.c > > @@ -2590,6 +2590,11 @@ void qmp_x_exit_preconfig(Error **errp) > > } > > } > > > > +static bool error_is_detailed(void) > > +{ > > + return !monitor_cur(); > > +} > > + > > void qemu_init(int argc, char **argv, char **envp) > > { > > QemuOpts *opts; > > @@ -2634,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_init(argv[0], error_is_detailed); > > 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 c104817cdddc..7e4d5030a045 100644 > > --- a/storage-daemon/qemu-storage-daemon.c > > +++ b/storage-daemon/qemu-storage-daemon.c > > @@ -368,13 +368,18 @@ static void pid_file_init(void) > > atexit(pid_file_cleanup); > > } > > > > +static bool error_is_detailed(void) > > +{ > > + return !monitor_cur(); > > +} > > + > > int main(int argc, char *argv[]) > > { > > #ifdef CONFIG_POSIX > > signal(SIGPIPE, SIG_IGN); > > #endif > > > > - error_init(argv[0]); > > + error_init(argv[0], error_is_detailed); > > qemu_init_exec_dir(argv[0]); > > os_setup_signal_handling(); > > > > diff --git a/util/error-report.c b/util/error-report.c > > index c43227a975e2..c2181f80a83d 100644 > > --- a/util/error-report.c > > +++ b/util/error-report.c > > @@ -11,7 +11,6 @@ > > */ > > > > #include "qemu/osdep.h" > > -#include "monitor/monitor.h" > > #include "qemu/error-report.h" > > > > /* > > @@ -28,6 +27,7 @@ typedef enum { > > bool message_with_timestamp; > > bool error_with_guestname; > > const char *error_guest_name; > > +ErrorReportDetailedFunc detailed_fn = NULL; > > > > int error_printf(const char *fmt, ...) > > { > > @@ -195,7 +195,7 @@ real_time_iso8601(void) > > */ > > static void vreport(report_type type, const char *fmt, va_list ap) > > { > > - bool detailed = !monitor_cur(); > > + bool detailed = detailed_fn ? detailed_fn() : TRUE; > > gchar *timestr; > > > > if (message_with_timestamp && detailed) { > > @@ -387,7 +387,7 @@ static void qemu_log_func(const gchar *log_domain, > > } > > } > > > > -void error_init(const char *argv0) > > +void error_init(const char *argv0, ErrorReportDetailedFunc detailed) > > { > > const char *p = strrchr(argv0, '/'); > > > > @@ -401,4 +401,6 @@ void error_init(const char *argv0) > > g_log_set_default_handler(qemu_log_func, NULL); > > g_warn_if_fail(qemu_glog_domains == NULL); > > qemu_glog_domains = g_strdup(g_getenv("G_MESSAGES_DEBUG")); > > + > > + detailed_fn = detailed; > > } > > A callback works, but note that each program's function is fixed. Why > not use the linker to resolve it? Have a .o in libqemuutil.a that > defines error_is_detailed() to return false always. Have another > error_is_detailed() that returns !monitor_cur() in monitor.c. A program > that links the monitor gets the latter, all the others the former. > > What do you think? > Yes, if we manage to overwrite symbols from a static library in a subproject. See the other thread for discussion. -- Marc-André Lureau
