On Mon, Jan 14, 2019 at 12:54:20PM +0100, Christophe Fergeau wrote: > On Fri, Jan 04, 2019 at 03:40:34PM +0000, Stefan Hajnoczi wrote: > > On Thu, Jan 03, 2019 at 03:14:55PM +0100, Christophe Fergeau wrote: > > > Hey, > > > > > > On Thu, Jan 03, 2019 at 10:54:25AM +0000, Stefan Hajnoczi wrote: > > > > On Fri, Dec 14, 2018 at 11:56:42AM +0100, Christophe Fergeau wrote: > > > > > +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: > > > > > + /* 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_INFO: > > > > > + /* Fall through */ > > > > > + case G_LOG_LEVEL_MESSAGE: > > > > > + info_report("%s: %s", log_domain, message); > > > > > > > > QEMU itself uses glib, so what happens if *_report() emit more log > > > > messages? Can this result in an infinite loop? > > > > > > If *_report try to output messages through the g_log API, glib will > > > catch it and will abort, so not an infinite loop, but not a > > > desirable outcome either. > > > > > > Some low-level logging functions which are going to dump the message > > > without glib (or using g_log_default_handler()) are needed. > > > The timestamping/prepending of 'error:', 'warning:' to the error message > > > is done right below *_report (in vreport) so this looked like a good > > > place for that. > > > > If you're confident that QEMU will never (indirectly) emit a glib log > > message from the *_report() code path, then I'm happy. > > In these codepaths, I'm confident up to > void monitor_vfprintf(FILE *stream, const char *fmt, va_list ap) > { > if (cur_mon && !monitor_cur_is_qmp()) { > monitor_vprintf(cur_mon, fmt, ap); > } else { > vfprintf(stream, fmt, ap); > } > } > > From there, it was not obvious to me whether the monitor_vprintf > codepath can be triggered from info/warn/error_report or not. If they > can, I'd feel less confident that monitor_vprintf will never indirectly > call glib log functions.
I think we're okay because otherwise monitor_vprintf() -> info/warn/error_report() would already be an infinite loop today. Stefan
signature.asc
Description: PGP signature