From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:40699) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1goXxb-0001yY-2M for qemu-devel@nongnu.org; Tue, 29 Jan 2019 13:18:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1goXxZ-0001et-Ov for qemu-devel@nongnu.org; Tue, 29 Jan 2019 13:18:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57663) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1goXxZ-0001Yl-Fo for qemu-devel@nongnu.org; Tue, 29 Jan 2019 13:18:09 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2F5828666E for ; Tue, 29 Jan 2019 18:17:57 +0000 (UTC) From: Markus Armbruster References: <20190125172229.23025-1-cfergeau@redhat.com> <20190125172229.23025-2-cfergeau@redhat.com> Date: Tue, 29 Jan 2019 19:17:51 +0100 In-Reply-To: <20190125172229.23025-2-cfergeau@redhat.com> (Christophe Fergeau's message of "Fri, 25 Jan 2019 18:22:29 +0100") Message-ID: <87k1inpbu8.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v6 2/2] log: Make glib logging go through QEMU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christophe Fergeau Cc: qemu-devel@nongnu.org, Stefan Hajnoczi , Max Reitz Christophe Fergeau 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 > --- > 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.