* [RFC 1/4] util: Introduce LogOutput
2025-09-02 10:30 [RFC 0/4] util: qmessage_context followup Richard Henderson
@ 2025-09-02 10:30 ` Richard Henderson
2025-09-02 10:30 ` [RFC 2/4] util: Drop QMESSAGE_CONTEXT_SKIP_MONITOR Richard Henderson
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2025-09-02 10:30 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/monitor/monitor.h | 4 +++
include/qemu/log-output.h | 14 ++++++++++
monitor/monitor.c | 54 +++++++++++++++++++++++++++++++--------
stubs/error-printf.c | 11 ++++++++
util/log.c | 7 +++++
5 files changed, 79 insertions(+), 11 deletions(-)
create mode 100644 include/qemu/log-output.h
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index c3740ec616..8c4d73592f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -4,6 +4,7 @@
#include "block/block.h"
#include "qapi/qapi-types-misc.h"
#include "qemu/readline.h"
+#include "qemu/log-output.h"
#include "exec/hwaddr.h"
typedef struct MonitorHMP MonitorHMP;
@@ -62,4 +63,7 @@ void monitor_register_hmp_info_hrt(const char *name,
int error_vprintf_unless_qmp(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
int error_printf_unless_qmp(const char *fmt, ...) G_GNUC_PRINTF(1, 2);
+const LogOutput *error_log_output(void **popaque);
+const LogOutput *error_log_output_unless_qmp(void **popaque);
+
#endif /* MONITOR_H */
diff --git a/include/qemu/log-output.h b/include/qemu/log-output.h
new file mode 100644
index 0000000000..1d502aae77
--- /dev/null
+++ b/include/qemu/log-output.h
@@ -0,0 +1,14 @@
+#ifndef QEMU_LOG_OUTPUT_H
+#define QEMU_LOG_OUTPUT_H 1
+
+typedef int LogOutputVararg(void *, const char *, ...) G_GNUC_PRINTF(2, 3);
+typedef int LogOutputVaList(void *, const char *, va_list) G_GNUC_PRINTF(2, 0);
+
+typedef struct LogOutput {
+ LogOutputVararg *print;
+ LogOutputVaList *vprint;
+} LogOutput;
+
+extern const LogOutput log_output_stdio;
+
+#endif
diff --git a/monitor/monitor.c b/monitor/monitor.c
index da54e1b1ce..71a3d62e0f 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -268,28 +268,60 @@ void monitor_printc(Monitor *mon, int c)
monitor_printf(mon, "'");
}
+static const LogOutput log_output_monitor = {
+ /* Abuse the Monitor* argument of monitor_printf as void* opaque. */
+ (LogOutputVararg *)monitor_printf,
+ (LogOutputVaList *)monitor_vprintf,
+};
+
+const LogOutput *error_log_output(void **popaque)
+{
+ Monitor *cur_mon = monitor_cur();
+
+ if (cur_mon && !monitor_cur_is_qmp()) {
+ *popaque = cur_mon;
+ return &log_output_monitor;
+ }
+
+ *popaque = stderr;
+ return &log_output_stdio;
+}
+
+const LogOutput *error_log_output_unless_qmp(void **popaque)
+{
+ Monitor *cur_mon = monitor_cur();
+
+ if (!cur_mon) {
+ *popaque = stderr;
+ return &log_output_stdio;
+ }
+ if (!monitor_cur_is_qmp()) {
+ *popaque = cur_mon;
+ return &log_output_monitor;
+ }
+
+ *popaque = NULL;
+ return NULL;
+}
+
/*
* Print to current monitor if we have one, else to stderr.
*/
int error_vprintf(const char *fmt, va_list ap)
{
- Monitor *cur_mon = monitor_cur();
+ void *opaque;
+ const LogOutput *l = error_log_output(&opaque);
- if (cur_mon && !monitor_cur_is_qmp()) {
- return monitor_vprintf(cur_mon, fmt, ap);
- }
- return vfprintf(stderr, fmt, ap);
+ return l->vprint(opaque, fmt, ap);
}
int error_vprintf_unless_qmp(const char *fmt, va_list ap)
{
- Monitor *cur_mon = monitor_cur();
+ void *opaque;
+ const LogOutput *l = error_log_output_unless_qmp(&opaque);
- if (!cur_mon) {
- return vfprintf(stderr, fmt, ap);
- }
- if (!monitor_cur_is_qmp()) {
- return monitor_vprintf(cur_mon, fmt, ap);
+ if (l) {
+ return l->vprint(opaque, fmt, ap);
}
return -1;
}
diff --git a/stubs/error-printf.c b/stubs/error-printf.c
index 0e326d8010..82e4756bda 100644
--- a/stubs/error-printf.c
+++ b/stubs/error-printf.c
@@ -21,3 +21,14 @@ int error_vprintf_unless_qmp(const char *fmt, va_list ap)
{
return error_vprintf(fmt, ap);
}
+
+const LogOutput *error_log_output(void **popaque)
+{
+ *popaque = stderr;
+ return &log_output_stdio;
+}
+
+const LogOutput *error_log_output_unless_qmp(void **popaque)
+{
+ return error_log_output(popaque);
+}
diff --git a/util/log.c b/util/log.c
index fc900cde0c..4b5953dcc7 100644
--- a/util/log.c
+++ b/util/log.c
@@ -28,6 +28,7 @@
#include "qemu/thread.h"
#include "qemu/lockable.h"
#include "qemu/rcu.h"
+#include "qemu/log-output.h"
#ifdef CONFIG_LINUX
#include <sys/syscall.h>
#endif
@@ -592,3 +593,9 @@ ssize_t rust_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream)
return ret < 0 ? -errno : 0;
}
#endif
+
+const LogOutput log_output_stdio = {
+ /* Abuse the FILE* argument of fprintf as void* opaque. */
+ (LogOutputVararg *)fprintf,
+ (LogOutputVaList *)vfprintf,
+};
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [RFC 2/4] util: Drop QMESSAGE_CONTEXT_SKIP_MONITOR
2025-09-02 10:30 [RFC 0/4] util: qmessage_context followup Richard Henderson
2025-09-02 10:30 ` [RFC 1/4] util: Introduce LogOutput Richard Henderson
@ 2025-09-02 10:30 ` Richard Henderson
2025-09-02 10:30 ` [RFC 3/4] util/message: Use LogOutput Richard Henderson
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2025-09-02 10:30 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/message.h | 11 +----------
util/error-report.c | 9 +++++----
util/log.c | 2 +-
util/message.c | 7 +------
4 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/include/qemu/message.h b/include/qemu/message.h
index 6dbb068ca9..68b08f0ea7 100644
--- a/include/qemu/message.h
+++ b/include/qemu/message.h
@@ -28,25 +28,16 @@ void qmessage_set_format(int flags);
*/
void qmessage_set_workload_name(const char *name);
-enum QMessageContextFlags {
- QMESSAGE_CONTEXT_SKIP_MONITOR = (1 << 0),
-};
-
/**
* qmessage_context:
- * @flags: the message formatting control flags
*
* Format a message prefix with the information
* previously selected by a call to
* qmessage_set_format.
*
- * If @flags contains QMESSAGE_CONTEXT_SKIP_MONITOR
- * an empty string will be returned if running in
- * the context of a HMP command
- *
* Returns: a formatted message prefix, or empty string;
* to be freed by the caller.
*/
-char *qmessage_context(int flags);
+char *qmessage_context(void);
#endif /* QEMU_MESSAGE_H */
diff --git a/util/error-report.c b/util/error-report.c
index 2e58ee1c50..fa34019dad 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -172,10 +172,11 @@ static void print_loc(void)
G_GNUC_PRINTF(2, 0)
static void vreport(report_type type, const char *fmt, va_list ap)
{
- g_autofree gchar *context = qmessage_context(QMESSAGE_CONTEXT_SKIP_MONITOR);
-
- if (context != NULL) {
- error_printf("%s", context);
+ if (!monitor_cur()) {
+ g_autofree gchar *context = qmessage_context();
+ if (context) {
+ error_printf("%s", context);
+ }
}
print_loc();
diff --git a/util/log.c b/util/log.c
index 4b5953dcc7..b129634708 100644
--- a/util/log.c
+++ b/util/log.c
@@ -161,7 +161,7 @@ void qemu_log(const char *fmt, ...)
* acquiring the mutex
*/
g_autofree const char *context =
- incomplete ? NULL : qmessage_context(0);
+ incomplete ? NULL : qmessage_context();
f = qemu_log_trylock();
if (f) {
diff --git a/util/message.c b/util/message.c
index 6d3580b7be..8deba3940c 100644
--- a/util/message.c
+++ b/util/message.c
@@ -20,17 +20,12 @@ void qmessage_set_workload_name(const char *name)
}
-char *qmessage_context(int flags)
+char *qmessage_context(void)
{
g_autofree char *timestr = NULL;
const char *wknamestr = NULL;
const char *pgnamestr = NULL;
- if ((flags & QMESSAGE_CONTEXT_SKIP_MONITOR) &&
- monitor_cur()) {
- return NULL;
- }
-
if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
timestr = g_date_time_format_iso8601(dt);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [RFC 3/4] util/message: Use LogOutput
2025-09-02 10:30 [RFC 0/4] util: qmessage_context followup Richard Henderson
2025-09-02 10:30 ` [RFC 1/4] util: Introduce LogOutput Richard Henderson
2025-09-02 10:30 ` [RFC 2/4] util: Drop QMESSAGE_CONTEXT_SKIP_MONITOR Richard Henderson
@ 2025-09-02 10:30 ` Richard Henderson
2025-09-02 10:30 ` [RFC 4/4] util/error-report: Use LogOutput in vreport Richard Henderson
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2025-09-02 10:30 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/qemu/message.h | 12 +++++-------
util/error-report.c | 8 ++++----
util/log.c | 16 ++--------------
util/message.c | 40 ++++++++++++++++++----------------------
4 files changed, 29 insertions(+), 47 deletions(-)
diff --git a/include/qemu/message.h b/include/qemu/message.h
index 68b08f0ea7..ac285a0021 100644
--- a/include/qemu/message.h
+++ b/include/qemu/message.h
@@ -3,6 +3,8 @@
#ifndef QEMU_MESSAGE_H
#define QEMU_MESSAGE_H
+#include "qemu/log-output.h"
+
enum QMessageFormatFlags {
QMESSAGE_FORMAT_TIMESTAMP = (1 << 0),
QMESSAGE_FORMAT_WORKLOAD_NAME = (1 << 1),
@@ -31,13 +33,9 @@ void qmessage_set_workload_name(const char *name);
/**
* qmessage_context:
*
- * Format a message prefix with the information
- * previously selected by a call to
- * qmessage_set_format.
- *
- * Returns: a formatted message prefix, or empty string;
- * to be freed by the caller.
+ * Format a message prefix with the information previously selected
+ * by a call to qmessage_set_format.
*/
-char *qmessage_context(void);
+void qmessage_context(const LogOutput *l, void *opaque);
#endif /* QEMU_MESSAGE_H */
diff --git a/util/error-report.c b/util/error-report.c
index fa34019dad..6ef556af5f 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -173,10 +173,10 @@ G_GNUC_PRINTF(2, 0)
static void vreport(report_type type, const char *fmt, va_list ap)
{
if (!monitor_cur()) {
- g_autofree gchar *context = qmessage_context();
- if (context) {
- error_printf("%s", context);
- }
+ void *opaque;
+ const LogOutput *l = error_log_output(&opaque);
+
+ qmessage_context(l, opaque);
}
print_loc();
diff --git a/util/log.c b/util/log.c
index b129634708..8bbd8e5dda 100644
--- a/util/log.c
+++ b/util/log.c
@@ -153,23 +153,11 @@ static __thread bool incomplete;
void qemu_log(const char *fmt, ...)
{
- FILE *f;
- /*
- * Prepare the context *outside* the logging
- * lock so any timestamp better reflects when
- * the message was emitted if we are delayed
- * acquiring the mutex
- */
- g_autofree const char *context =
- incomplete ? NULL : qmessage_context();
-
- f = qemu_log_trylock();
+ FILE *f = qemu_log_trylock();
if (f) {
va_list ap;
- if (context != NULL) {
- fwrite(context, 1, strlen(context), f);
- }
+ qmessage_context(&log_output_stdio, f);
va_start(ap, fmt);
vfprintf(f, fmt, ap);
diff --git a/util/message.c b/util/message.c
index 8deba3940c..0c63d128fc 100644
--- a/util/message.c
+++ b/util/message.c
@@ -20,7 +20,7 @@ void qmessage_set_workload_name(const char *name)
}
-char *qmessage_context(void)
+void qmessage_context(const LogOutput *l, void *opaque)
{
g_autofree char *timestr = NULL;
const char *wknamestr = NULL;
@@ -43,26 +43,22 @@ char *qmessage_context(void)
int thid = qemu_get_thread_id();
const char *thname = qemu_thread_get_name();
- return g_strdup_printf("%s%s%s%s%s%s(%d:%s): ",
- timestr ? timestr : "",
- timestr ? " " : "",
- wknamestr ? "[" : "",
- wknamestr ? wknamestr : "",
- wknamestr ? "] " : "",
- pgnamestr ? pgnamestr : "",
- thid, thname);
- } else {
- if (!timestr && !wknamestr && !pgnamestr) {
- return NULL;
- }
-
- return g_strdup_printf("%s%s%s%s%s%s%s",
- timestr ? timestr : "",
- timestr ? " " : "",
- wknamestr ? "[" : "",
- wknamestr ? wknamestr : "",
- wknamestr ? "] " : "",
- pgnamestr ? pgnamestr : "",
- pgnamestr ? ": " : "");
+ l->print(opaque, "%s%s%s%s%s%s(%d:%s): ",
+ timestr ? timestr : "",
+ timestr ? " " : "",
+ wknamestr ? "[" : "",
+ wknamestr ? wknamestr : "",
+ wknamestr ? "] " : "",
+ pgnamestr ? pgnamestr : "",
+ thid, thname);
+ } else if (timestr || wknamestr || pgnamestr) {
+ l->print(opaque, "%s%s%s%s%s%s%s",
+ timestr ? timestr : "",
+ timestr ? " " : "",
+ wknamestr ? "[" : "",
+ wknamestr ? wknamestr : "",
+ wknamestr ? "] " : "",
+ pgnamestr ? pgnamestr : "",
+ pgnamestr ? ": " : "");
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [RFC 4/4] util/error-report: Use LogOutput in vreport
2025-09-02 10:30 [RFC 0/4] util: qmessage_context followup Richard Henderson
` (2 preceding siblings ...)
2025-09-02 10:30 ` [RFC 3/4] util/message: Use LogOutput Richard Henderson
@ 2025-09-02 10:30 ` Richard Henderson
2025-09-02 16:47 ` [RFC 0/4] util: qmessage_context followup Daniel P. Berrangé
2025-09-10 18:05 ` Daniel P. Berrangé
5 siblings, 0 replies; 7+ messages in thread
From: Richard Henderson @ 2025-09-02 10:30 UTC (permalink / raw)
To: qemu-devel; +Cc: berrange
Merge print_loc. Use the LogOutput callbacks instead of
error_printf and error_vprintf.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
util/error-report.c | 72 +++++++++++++++++++++------------------------
1 file changed, 34 insertions(+), 38 deletions(-)
diff --git a/util/error-report.c b/util/error-report.c
index 6ef556af5f..db5b2c1608 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -132,36 +132,6 @@ void loc_set_file(const char *fname, int lno)
}
}
-/*
- * Print current location to current monitor if we have one, else to stderr.
- */
-static void print_loc(void)
-{
- const char *sep = "";
- int i;
- const char *const *argp;
-
- switch (cur_loc->kind) {
- case LOC_CMDLINE:
- argp = cur_loc->ptr;
- for (i = 0; i < cur_loc->num; i++) {
- error_printf("%s%s", sep, argp[i]);
- sep = " ";
- }
- error_printf(": ");
- break;
- case LOC_FILE:
- error_printf("%s:", (const char *)cur_loc->ptr);
- if (cur_loc->num) {
- error_printf("%d:", cur_loc->num);
- }
- error_printf(" ");
- break;
- default:
- error_printf("%s", sep);
- }
-}
-
/*
* Print a message to current monitor if we have one, else to stderr.
* @report_type is the type of message: error, warning or informational.
@@ -172,28 +142,54 @@ static void print_loc(void)
G_GNUC_PRINTF(2, 0)
static void vreport(report_type type, const char *fmt, va_list ap)
{
- if (!monitor_cur()) {
- void *opaque;
- const LogOutput *l = error_log_output(&opaque);
+ void *opaque;
+ const LogOutput *l = error_log_output(&opaque);
+ if (!monitor_cur()) {
qmessage_context(l, opaque);
}
- print_loc();
+ switch (cur_loc->kind) {
+ case LOC_CMDLINE:
+ {
+ const char *const *argp = cur_loc->ptr;
+ const char *sep = "";
+
+ for (int i = 0; i < cur_loc->num; i++) {
+ l->print(opaque, "%s%s", sep, argp[i]);
+ sep = " ";
+ }
+ l->print(opaque, ": ");
+ }
+ break;
+ case LOC_FILE:
+ {
+ const char *file = cur_loc->ptr;
+
+ if (cur_loc->num) {
+ error_printf("%s:%d: ", file, cur_loc->num);
+ } else {
+ error_printf("%s: ", file);
+ }
+ }
+ break;
+ case LOC_NONE:
+ break;
+ }
switch (type) {
case REPORT_TYPE_ERROR:
break;
case REPORT_TYPE_WARNING:
- error_printf("warning: ");
+ l->print(opaque, "warning: ");
break;
case REPORT_TYPE_INFO:
- error_printf("info: ");
+ l->print(opaque, "info: ");
break;
}
- error_vprintf(fmt, ap);
- error_printf("\n");
+ l->vprint(opaque, fmt, ap);
+ l->print(opaque, "\n");
}
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [RFC 0/4] util: qmessage_context followup
2025-09-02 10:30 [RFC 0/4] util: qmessage_context followup Richard Henderson
` (3 preceding siblings ...)
2025-09-02 10:30 ` [RFC 4/4] util/error-report: Use LogOutput in vreport Richard Henderson
@ 2025-09-02 16:47 ` Daniel P. Berrangé
2025-09-10 18:05 ` Daniel P. Berrangé
5 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2025-09-02 16:47 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Tue, Sep 02, 2025 at 12:30:06PM +0200, Richard Henderson wrote:
> Hi Daniel,
>
> I'm still not keen on qmessage_context allocating a string.
>
> If we *did* allocate a string, it should be a GString so that we can
> easily append to that. The only benefit I see from this is that we
> would collect all of the output and it would reach stderr atomically.
>
> However, I think it's better to not collect the message and just
> output the pieces. Something like the following. The names are
> horrible and I didn't document the patches well.
Thanks for the suggestion, I'll have a look and see about merging
this, or something like it, into my series for a new posting.
>
>
> r~
>
>
> Richard Henderson (4):
> util: Introduce LogOutput
> util: Drop QMESSAGE_CONTEXT_SKIP_MONITOR
> util/message: Use LogOutput
> util/error-report: Use LogOutput in vreport
>
> include/monitor/monitor.h | 4 +++
> include/qemu/log-output.h | 14 ++++++++
> include/qemu/message.h | 21 +++--------
> monitor/monitor.c | 54 +++++++++++++++++++++++------
> stubs/error-printf.c | 11 ++++++
> util/error-report.c | 73 +++++++++++++++++++--------------------
> util/log.c | 23 +++++-------
> util/message.c | 45 ++++++++++--------------
> 8 files changed, 139 insertions(+), 106 deletions(-)
> create mode 100644 include/qemu/log-output.h
>
> --
> 2.43.0
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFC 0/4] util: qmessage_context followup
2025-09-02 10:30 [RFC 0/4] util: qmessage_context followup Richard Henderson
` (4 preceding siblings ...)
2025-09-02 16:47 ` [RFC 0/4] util: qmessage_context followup Daniel P. Berrangé
@ 2025-09-10 18:05 ` Daniel P. Berrangé
5 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2025-09-10 18:05 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Tue, Sep 02, 2025 at 12:30:06PM +0200, Richard Henderson wrote:
> Hi Daniel,
>
> I'm still not keen on qmessage_context allocating a string.
>
> If we *did* allocate a string, it should be a GString so that we can
> easily append to that. The only benefit I see from this is that we
> would collect all of the output and it would reach stderr atomically.
>
> However, I think it's better to not collect the message and just
> output the pieces. Something like the following. The names are
> horrible and I didn't document the patches well.
I've posted a v3 of my series which incorporates the conceptual
idea you show here. I managed to simplify it somewhat as I came
to realize the qmessage_context code never needs to write to
anything other than a "FILE *", because we always skip the context
when writing to HMP. IOW, we could simply use fprintf() directly
throughout
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread