| Message ID | 20190125172229.23025-2-cfergeau@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6,1/2] qemu-io: Use error_[gs]et_progname() | expand |
On Fri, Jan 25, 2019 at 06:22:29PM +0100, Christophe Fergeau wrote: > This commit adds a qemu_init_logging() helper which calls > g_log_set_default_handler() so that glib logs (g_log, g_warning, ...) > are handled similarly to other QEMU logs. This means they will get a > timestamp if timestamps are enabled, and they will go through the > HMP monitor if one is configured. > This commit also adds a call to qemu_init_logging() to the binaries > installed by QEMU. > glib debug messages are enabled through G_MESSAGES_DEBUG similarly to > glib default log handler. > > At the moment, this change will mostly impact SPICE logging if your > spice version is >= 0.14.1. With older spice versions, this is not going > to work as expected, but will not have any ill effect, so this call is > not conditional on the SPICE version. > > Signed-off-by: Christophe Fergeau <cfergeau@redhat.com> > --- > bsd-user/main.c | 2 ++ > include/qemu/error-report.h | 3 ++- > linux-user/main.c | 2 ++ > qemu-img.c | 2 +- > qemu-io.c | 2 +- > qemu-nbd.c | 2 +- > scsi/qemu-pr-helper.c | 1 + > util/qemu-error.c | 48 ++++++++++++++++++++++++++++++++++++- > vl.c | 2 +- > 9 files changed, 58 insertions(+), 6 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Christophe Fergeau <cfergeau@redhat.com> writes: > This commit adds a qemu_init_logging() helper which calls > g_log_set_default_handler() so that glib logs (g_log, g_warning, ...) > are handled similarly to other QEMU logs. This means they will get a > timestamp if timestamps are enabled, and they will go through the > HMP monitor if one is configured. > This commit also adds a call to qemu_init_logging() to the binaries > installed by QEMU. > glib debug messages are enabled through G_MESSAGES_DEBUG similarly to > glib default log handler. > > At the moment, this change will mostly impact SPICE logging if your > spice version is >= 0.14.1. With older spice versions, this is not going > to work as expected, but will not have any ill effect, so this call is > not conditional on the SPICE version. > > Signed-off-by: Christophe Fergeau <cfergeau@redhat.com> > --- > bsd-user/main.c | 2 ++ > include/qemu/error-report.h | 3 ++- > linux-user/main.c | 2 ++ > qemu-img.c | 2 +- > qemu-io.c | 2 +- > qemu-nbd.c | 2 +- > scsi/qemu-pr-helper.c | 1 + > util/qemu-error.c | 48 ++++++++++++++++++++++++++++++++++++- > vl.c | 2 +- > 9 files changed, 58 insertions(+), 6 deletions(-) > > diff --git a/bsd-user/main.c b/bsd-user/main.c > index 0d3156974c..8fd8ae4127 100644 > --- a/bsd-user/main.c > +++ b/bsd-user/main.c > @@ -24,6 +24,7 @@ > #include "qapi/error.h" > #include "qemu.h" > #include "qemu/config-file.h" > +#include "qemu/error-report.h" > #include "qemu/path.h" > #include "qemu/help_option.h" > #include "cpu.h" > @@ -743,6 +744,7 @@ int main(int argc, char **argv) > if (argc <= 1) > usage(); > > + error_init(argv[0]); > module_call_init(MODULE_INIT_TRACE); > qemu_init_cpu_list(); > module_call_init(MODULE_INIT_QOM); This changes output of error_report() & friends in this program. Probably in a good way. Should it be a separate commit? If not, it should at least be mentioned in the commit message. > diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h > index 0a8d9cc9ea..ce43c02314 100644 > --- a/include/qemu/error-report.h > +++ b/include/qemu/error-report.h > @@ -34,7 +34,6 @@ void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); > void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2); > void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); > void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2); > -void error_set_progname(const char *argv0); > > void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); > void warn_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); > @@ -49,6 +48,8 @@ bool error_report_once_cond(bool *printed, const char *fmt, ...) > bool warn_report_once_cond(bool *printed, const char *fmt, ...) > GCC_FMT_ATTR(2, 3); > > +void error_init(const char *argv0); > + > /* > * Similar to error_report(), except it prints the message just once. > * Return true when it prints, false otherwise. > diff --git a/linux-user/main.c b/linux-user/main.c > index a0aba9cb1e..f9efe9ff6e 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -27,6 +27,7 @@ > #include "qemu/path.h" > #include "qemu/config-file.h" > #include "qemu/cutils.h" > +#include "qemu/error-report.h" > #include "qemu/help_option.h" > #include "cpu.h" > #include "exec/exec-all.h" > @@ -600,6 +601,7 @@ int main(int argc, char **argv, char **envp) > int ret; > int execfd; > > + error_init(argv[0]); > module_call_init(MODULE_INIT_TRACE); > qemu_init_cpu_list(); > module_call_init(MODULE_INIT_QOM); Likewise. > diff --git a/qemu-img.c b/qemu-img.c > index ad04f59565..0af9cac244 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -4912,8 +4912,8 @@ int main(int argc, char **argv) > signal(SIGPIPE, SIG_IGN); > #endif > > + error_init(argv[0]); > module_call_init(MODULE_INIT_TRACE); > - error_set_progname(argv[0]); > qemu_init_exec_dir(argv[0]); > > if (qemu_init_main_loop(&local_error)) { > diff --git a/qemu-io.c b/qemu-io.c > index 2c52ac0ade..8d5d5911cb 100644 > --- a/qemu-io.c > +++ b/qemu-io.c > @@ -522,8 +522,8 @@ int main(int argc, char **argv) > signal(SIGPIPE, SIG_IGN); > #endif > > + error_init(argv[0]); > module_call_init(MODULE_INIT_TRACE); > - error_set_progname(argv[0]); > qemu_init_exec_dir(argv[0]); > > qcrypto_init(&error_fatal); > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 1f7b2a03f5..39849f1692 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -685,8 +685,8 @@ int main(int argc, char **argv) > signal(SIGPIPE, SIG_IGN); > #endif > > + error_init(argv[0]); > module_call_init(MODULE_INIT_TRACE); > - error_set_progname(argv[0]); > qcrypto_init(&error_fatal); > > module_call_init(MODULE_INIT_QOM); > diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c > index e7af637232..2541fbbd1b 100644 > --- a/scsi/qemu-pr-helper.c > +++ b/scsi/qemu-pr-helper.c > @@ -895,6 +895,7 @@ int main(int argc, char **argv) > > signal(SIGPIPE, SIG_IGN); > > + error_init(argv[0]); > module_call_init(MODULE_INIT_TRACE); > module_call_init(MODULE_INIT_QOM); > qemu_add_opts(&qemu_trace_opts); Likewise. > diff --git a/util/qemu-error.c b/util/qemu-error.c > index fcbe8a1f74..2684870683 100644 > --- a/util/qemu-error.c > +++ b/util/qemu-error.c > @@ -142,7 +142,7 @@ static const char *progname; > /* > * Set the program name for error_print_loc(). > */ > -void error_set_progname(const char *argv0) > +static void error_set_progname(const char *argv0) > { > const char *p = strrchr(argv0, '/'); > progname = p ? p + 1 : argv0; > @@ -345,3 +345,49 @@ bool warn_report_once_cond(bool *printed, const char *fmt, ...) > va_end(ap); > return true; > } > + > +static char *qemu_glog_domains; > + > +static void qemu_log_func(const gchar *log_domain, > + GLogLevelFlags log_level, > + const gchar *message, > + gpointer user_data) > +{ > + switch (log_level & G_LOG_LEVEL_MASK) { > + case G_LOG_LEVEL_DEBUG: > + case G_LOG_LEVEL_INFO: > + /* Use same G_MESSAGES_DEBUG logic as glib to enable/disable debug > + * messages */ Let's wing this comment. > + if (qemu_glog_domains == NULL) { > + break; > + } > + if (strcmp(qemu_glog_domains, "all") != 0 && > + (log_domain == NULL || !strstr(qemu_glog_domains, log_domain))) { > + break; > + } > + /* Fall through */ > + case G_LOG_LEVEL_MESSAGE: > + info_report("%s: %s", log_domain?:"", message); Spaces around ?:, please. If log_domain is null, the message will look like info: : MESSAGE text which is ugly. What about info_report("%s%s%s", log_domain ? ": " : "", log_domain ?: "", message); More of the same below. > + break; > + case G_LOG_LEVEL_WARNING: > + warn_report("%s: %s", log_domain?:"", message); > + break; > + case G_LOG_LEVEL_CRITICAL: > + /* Fall through */ > + case G_LOG_LEVEL_ERROR: > + error_report("%s: %s", log_domain?:"", message); > + break; > + } > +} > + > +void error_init(const char *argv0) > +{ > + /* Set the program name for error_print_loc(). */ > + error_set_progname(argv0); > + > + /* This sets up glib logging so libraries using it also print their logs > + * through error_report(), warn_report(), info_report(). */ Let's wing this comment. > + 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")); > +} > diff --git a/vl.c b/vl.c > index bc9fbec654..bfef38c3a3 100644 > --- a/vl.c > +++ b/vl.c > @@ -3039,6 +3039,7 @@ int main(int argc, char **argv, char **envp) > QSIMPLEQ_HEAD(, BlockdevOptions_queue) bdo_queue > = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue); > > + error_init(argv[0]); > module_call_init(MODULE_INIT_TRACE); > > qemu_init_cpu_list(); > @@ -3047,7 +3048,6 @@ int main(int argc, char **argv, char **envp) > qemu_mutex_lock_iothread(); > > atexit(qemu_run_exit_notifiers); > - error_set_progname(argv[0]); > qemu_init_exec_dir(argv[0]); > > module_call_init(MODULE_INIT_QOM); Almost ready.
diff --git a/bsd-user/main.c b/bsd-user/main.c index 0d3156974c..8fd8ae4127 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -24,6 +24,7 @@ #include "qapi/error.h" #include "qemu.h" #include "qemu/config-file.h" +#include "qemu/error-report.h" #include "qemu/path.h" #include "qemu/help_option.h" #include "cpu.h" @@ -743,6 +744,7 @@ int main(int argc, char **argv) if (argc <= 1) usage(); + error_init(argv[0]); module_call_init(MODULE_INIT_TRACE); qemu_init_cpu_list(); module_call_init(MODULE_INIT_QOM); diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h index 0a8d9cc9ea..ce43c02314 100644 --- a/include/qemu/error-report.h +++ b/include/qemu/error-report.h @@ -34,7 +34,6 @@ void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2); void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2); -void error_set_progname(const char *argv0); void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void warn_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); @@ -49,6 +48,8 @@ bool error_report_once_cond(bool *printed, const char *fmt, ...) bool warn_report_once_cond(bool *printed, const char *fmt, ...) GCC_FMT_ATTR(2, 3); +void error_init(const char *argv0); + /* * Similar to error_report(), except it prints the message just once. * Return true when it prints, false otherwise. diff --git a/linux-user/main.c b/linux-user/main.c index a0aba9cb1e..f9efe9ff6e 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -27,6 +27,7 @@ #include "qemu/path.h" #include "qemu/config-file.h" #include "qemu/cutils.h" +#include "qemu/error-report.h" #include "qemu/help_option.h" #include "cpu.h" #include "exec/exec-all.h" @@ -600,6 +601,7 @@ int main(int argc, char **argv, char **envp) int ret; int execfd; + error_init(argv[0]); module_call_init(MODULE_INIT_TRACE); qemu_init_cpu_list(); module_call_init(MODULE_INIT_QOM); diff --git a/qemu-img.c b/qemu-img.c index ad04f59565..0af9cac244 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -4912,8 +4912,8 @@ int main(int argc, char **argv) signal(SIGPIPE, SIG_IGN); #endif + error_init(argv[0]); module_call_init(MODULE_INIT_TRACE); - error_set_progname(argv[0]); qemu_init_exec_dir(argv[0]); if (qemu_init_main_loop(&local_error)) { diff --git a/qemu-io.c b/qemu-io.c index 2c52ac0ade..8d5d5911cb 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -522,8 +522,8 @@ int main(int argc, char **argv) signal(SIGPIPE, SIG_IGN); #endif + error_init(argv[0]); module_call_init(MODULE_INIT_TRACE); - error_set_progname(argv[0]); qemu_init_exec_dir(argv[0]); qcrypto_init(&error_fatal); diff --git a/qemu-nbd.c b/qemu-nbd.c index 1f7b2a03f5..39849f1692 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -685,8 +685,8 @@ int main(int argc, char **argv) signal(SIGPIPE, SIG_IGN); #endif + error_init(argv[0]); module_call_init(MODULE_INIT_TRACE); - error_set_progname(argv[0]); qcrypto_init(&error_fatal); module_call_init(MODULE_INIT_QOM); diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c index e7af637232..2541fbbd1b 100644 --- a/scsi/qemu-pr-helper.c +++ b/scsi/qemu-pr-helper.c @@ -895,6 +895,7 @@ int main(int argc, char **argv) signal(SIGPIPE, SIG_IGN); + error_init(argv[0]); module_call_init(MODULE_INIT_TRACE); module_call_init(MODULE_INIT_QOM); qemu_add_opts(&qemu_trace_opts); diff --git a/util/qemu-error.c b/util/qemu-error.c index fcbe8a1f74..2684870683 100644 --- a/util/qemu-error.c +++ b/util/qemu-error.c @@ -142,7 +142,7 @@ static const char *progname; /* * Set the program name for error_print_loc(). */ -void error_set_progname(const char *argv0) +static void error_set_progname(const char *argv0) { const char *p = strrchr(argv0, '/'); progname = p ? p + 1 : argv0; @@ -345,3 +345,49 @@ bool warn_report_once_cond(bool *printed, const char *fmt, ...) va_end(ap); return true; } + +static char *qemu_glog_domains; + +static void qemu_log_func(const gchar *log_domain, + GLogLevelFlags log_level, + const gchar *message, + gpointer user_data) +{ + switch (log_level & G_LOG_LEVEL_MASK) { + case G_LOG_LEVEL_DEBUG: + case G_LOG_LEVEL_INFO: + /* Use same G_MESSAGES_DEBUG logic as glib to enable/disable debug + * messages */ + if (qemu_glog_domains == NULL) { + break; + } + if (strcmp(qemu_glog_domains, "all") != 0 && + (log_domain == NULL || !strstr(qemu_glog_domains, log_domain))) { + break; + } + /* Fall through */ + case G_LOG_LEVEL_MESSAGE: + info_report("%s: %s", log_domain?:"", message); + break; + case G_LOG_LEVEL_WARNING: + warn_report("%s: %s", log_domain?:"", message); + break; + case G_LOG_LEVEL_CRITICAL: + /* Fall through */ + case G_LOG_LEVEL_ERROR: + error_report("%s: %s", log_domain?:"", message); + break; + } +} + +void error_init(const char *argv0) +{ + /* Set the program name for error_print_loc(). */ + error_set_progname(argv0); + + /* This sets up glib logging so libraries using it also print their logs + * through error_report(), warn_report(), info_report(). */ + 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")); +} diff --git a/vl.c b/vl.c index bc9fbec654..bfef38c3a3 100644 --- a/vl.c +++ b/vl.c @@ -3039,6 +3039,7 @@ int main(int argc, char **argv, char **envp) QSIMPLEQ_HEAD(, BlockdevOptions_queue) bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue); + error_init(argv[0]); module_call_init(MODULE_INIT_TRACE); qemu_init_cpu_list(); @@ -3047,7 +3048,6 @@ int main(int argc, char **argv, char **envp) qemu_mutex_lock_iothread(); atexit(qemu_run_exit_notifiers); - error_set_progname(argv[0]); qemu_init_exec_dir(argv[0]); module_call_init(MODULE_INIT_QOM);
This commit adds a qemu_init_logging() helper which calls g_log_set_default_handler() so that glib logs (g_log, g_warning, ...) are handled similarly to other QEMU logs. This means they will get a timestamp if timestamps are enabled, and they will go through the HMP monitor if one is configured. This commit also adds a call to qemu_init_logging() to the binaries installed by QEMU. glib debug messages are enabled through G_MESSAGES_DEBUG similarly to glib default log handler. At the moment, this change will mostly impact SPICE logging if your spice version is >= 0.14.1. With older spice versions, this is not going to work as expected, but will not have any ill effect, so this call is not conditional on the SPICE version. Signed-off-by: Christophe Fergeau <cfergeau@redhat.com> --- bsd-user/main.c | 2 ++ include/qemu/error-report.h | 3 ++- linux-user/main.c | 2 ++ qemu-img.c | 2 +- qemu-io.c | 2 +- qemu-nbd.c | 2 +- scsi/qemu-pr-helper.c | 1 + util/qemu-error.c | 48 ++++++++++++++++++++++++++++++++++++- vl.c | 2 +- 9 files changed, 58 insertions(+), 6 deletions(-)