[PATCH v4 00/23] util: sync error_report & qemu_log output more closely

[PATCH v4 00/23] util: sync error_report & qemu_log output more closely

[Daniel P. Berrangé]

This series is a tangent that came out of discussion in

   https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00903.html

In thinking about adding thread info to error_report, I
came to realize we should likely make qemu_log behave
consistently with error_report & friends. We already
honour '-msg timestamp=on', but don't honour 'guest-name=on'
and also don't include the binary name.

As an example of the current state, consider mixing error and
log output today:

- Default context:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
                       -d 'trace:qcrypto*'
  qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55ac6d97f700 dir=fish
  qcrypto_tls_creds_get_path TLS creds path creds=0x55ac6d97f700 filename=ca-cert.pem path=<none>
  qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory

- Full context:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
                       -d 'trace:qcrypto*' \
                       -msg guest-name=on,timestamp=on \
                       -name "fish food"
  2025-08-19T20:14:16.791413Z qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55e9a3458d10 dir=fish
  2025-08-19T20:14:16.791429Z qcrypto_tls_creds_get_path TLS creds path creds=0x55e9a3458d10 filename=ca-cert.pem path=<none>
  2025-08-19T20:14:16.791433Z fish food qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory

And after this series is complete:

- Default context:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
                      -d 'trace:qcrypto*'
  qemu-system-x86_64(1184284:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55a24ad5cb30 dir=fish
  qemu-system-x86_64(1184284:main): qcrypto_tls_creds_get_path TLS creds path creds=0x55a24ad5cb30 filename=ca-cert.pem path=<none>
  qemu-system-x86_64(1184284:main): Unable to access credentials fish/ca-cert.pem: No such file or directory

- Full context:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
                      -d 'trace:qcrypto*' \
                      -msg guest-name=on,timestamp=on \
                      -name "fish food"
  2025-08-19T20:12:50.211823Z [fish food] qemu-system-x86_64(1168876:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5582183d8760 dir=fish
  2025-08-19T20:12:50.211842Z [fish food] qemu-system-x86_64(1168876:main): qcrypto_tls_creds_get_path TLS creds path creds=0x5582183d8760 filename=ca-cert.pem
+path=<none>
  2025-08-19T20:12:50.211846Z [fish food] qemu-system-x86_64(1168876:main): Unable to access credentials fish/ca-cert.pem: No such file or directory

The main things to note:

 * error_report/warn_report/qemu_log share the same
   output format and -msg applies to both

 * -msg debug-threads=on is now unconditionally enabled
   and thus the param is deprecated & ignored

 * Thread ID and name are unconditionally enabled

 * Guest name is surrounded in [...] brackets

 * The default output lines are typically 15 chars
   wider given that we always include the thread
   ID + name now

 * This takes the liberty of assigning the new file
   to the existing error-report.c maintainer (Markus)
   Since splitting it off into message.c instead of
   putting it all in error-report.c felt slightly
   nicer.

One thing I didn't tackle is making the location
info get reported for qemu_log. This is used to
give context for error messages when parsing some
CLI args, and could be interesting for log messages
associated with those same CLI args.

Changes in v4:

 - Re-architected the way error_vprintf() operates
   to simplify monitor dependency
 - Fixed the move of error_vprintf from monitor.c
   to error-report.c so that unit tests can still
   get a stub to capture messages to g_test_message
 - Simplify changes to vreport() to avoid passing
   around function pointers
 - Add locking of stderr to vreport() to serialize
   incremental output
 - Fix thread naming logic changes on Windows
 - Change deprecation warning messages

Changes in v3:

 - Stop formatting a string in qmessage_context, instead
   output directly to a "FILE *" stream
 - Fix pre-existing bug interleaving qemu_log and
   vreport when the trace 'log' backend has mutex probes
   enabled

Changes in v2:

 - Re-use existing qemu_get_thread_id rather than
   re-inventing it as qemu_thread_get_id.
 - Expose qemu_thread_set_name and use it from all
   locations needing to set thread names
 - Fix qemu_log() to skip context prefix when
   emitting a log message in fragments across
   multiple calls
 - Skip allocating memory for empty context messages
 - Fix leak in win32 impl of qemu_thread_get_name
 - Use g_strlcpy where possible

Daniel P. Berrangé (23):
  include: define constant for early constructor priority
  monitor: initialize global data from a constructor
  system: unconditionally enable thread naming
  util: expose qemu_thread_set_name
  audio: make jackaudio use qemu_thread_set_name
  util: set the name for the 'main' thread
  util: add API to fetch the current thread name
  log: avoid repeated prefix on incremental qemu_log calls
  util: introduce some API docs for logging APIs
  ui/vnc: remove use of error_printf_unless_qmp()
  monitor: remove redundant error_[v]printf_unless_qmp
  monitor: refactor error_vprintf()
  monitor: move error_vprintf back to error-report.c
  monitor: introduce monitor_cur_is_hmp() helper
  util: don't skip error prefixes when QMP is active
  util: fix interleaving of error & trace output
  util: fix interleaving of error prefixes
  util: introduce common helper for error-report & log code
  util: convert error-report & log to message API for timestamp
  util: add support for formatting a workload name in messages
  util: add support for formatting a program name in messages
  util: add support for formatting thread info in messages
  util: add brackets around guest name in message context

 MAINTAINERS                          |  2 +
 audio/jackaudio.c                    | 30 +++++++--
 docs/about/deprecated.rst            |  7 ++
 include/monitor/monitor.h            |  4 +-
 include/qemu/compiler.h              |  8 +++
 include/qemu/error-report.h          |  4 --
 include/qemu/log-for-trace.h         | 35 +++++++++-
 include/qemu/log.h                   | 26 ++++++++
 include/qemu/message.h               | 40 ++++++++++++
 include/qemu/thread.h                |  3 +-
 meson.build                          | 21 ++++++
 monitor/monitor.c                    | 61 ++++++-----------
 rust/util/src/log.rs                 |  7 ++
 storage-daemon/qemu-storage-daemon.c |  6 ++
 stubs/error-printf.c                 | 23 -------
 stubs/meson.build                    |  1 -
 stubs/monitor-core.c                 | 20 +++++-
 system/vl.c                          | 31 ++++++---
 tests/qemu-iotests/041               |  2 +-
 tests/qemu-iotests/common.filter     |  2 +-
 tests/unit/test-error-report.c       |  6 +-
 tests/unit/test-util-sockets.c       |  1 +
 ui/vnc.c                             | 14 ++--
 util/error-report.c                  | 97 +++++++++++++++++-----------
 util/log.c                           | 28 ++++----
 util/meson.build                     |  1 +
 util/message.c                       | 47 ++++++++++++++
 util/qemu-thread-posix.c             | 77 +++++++++++++++-------
 util/qemu-thread-win32.c             | 87 +++++++++++++++++--------
 29 files changed, 485 insertions(+), 206 deletions(-)
 create mode 100644 include/qemu/message.h
 delete mode 100644 stubs/error-printf.c
 create mode 100644 util/message.c

-- 
2.50.1

[PATCH v4 01/23] include: define constant for early constructor priority

[Daniel P. Berrangé]

Functions marked with __attribute__((__constructor__)) will be
invoked in linker order. In theory this is well defined, but
in practice, it is hard to determine what this order will be
with the layers of indirection through meson, ninja and the
static libraries QEMU builds.

Notably, the order currently appears different between Linux
and Windows (as tested with Wine on Linux). This can cause
problems when certain QEMU constructors have a dependancy on
other QEMU constructors.

To address this define a QEMU_CONSTRUCTOR_EARLY constant which
provides a priority value that will run before other default
constructors. This is to be used for QEMU constructors that
are themselves self-contained, but may be relied upon by other
constructors.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/compiler.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 1c2b673c05..4c49f52eb0 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -194,6 +194,14 @@
 # define QEMU_USED
 #endif
 
+/*
+ * A priority for __attribute__((constructor(...))) that
+ * will run earlier than the default constructors. Must
+ * only be used for functions that have no dependency
+ * on global initialization of other QEMU subsystems.
+ */
+#define QEMU_CONSTRUCTOR_EARLY 101
+
 /*
  * Disable -ftrivial-auto-var-init on a local variable.
  *
-- 
2.50.1

[PATCH v4 02/23] monitor: initialize global data from a constructor

[Daniel P. Berrangé]

Some monitor functions, most notably, monitor_cur() rely on global
data being initialized by 'monitor_init_globals()'. The latter is
called relatively late in startup. If code triggers error_report()
before monitor_init_globals() is called, QEMU will abort when
accessing the uninitialized monitor mutex.

The critical monitor global data must be initialized from a
constructor function, to improve the guarantee that it is done
before any possible calls to monitor_cur(). Not only that, but
the constructor must be marked to run before the default
constructor in case any of them trigger error reporting.

Note in particular that the RCU constructor will spawn a background
thread so we might even have non-constructor QEMU code running
concurrently with other constructors.

As a general note, constructors should be extrememly careful
about what QEMU code they invoke, as it cannot be guaranteed that
the process is fully initialized and so not all normal QEMU API
rules apply.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Fixes: e69ee454b5f9 (monitor: Make current monitor a per-coroutine property)
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/monitor.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/monitor/monitor.c b/monitor/monitor.c
index c5a5d30877..da54e1b1ce 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -704,18 +704,22 @@ void monitor_cleanup(void)
     }
 }
 
-static void monitor_qapi_event_init(void)
+/*
+ * Initialize static vars that have no deps on external
+ * module initialization, and are required for external
+ * functions to call things like monitor_cur()
+ */
+static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
+monitor_init_static(void)
 {
+    qemu_mutex_init(&monitor_lock);
+    coroutine_mon = g_hash_table_new(NULL, NULL);
     monitor_qapi_event_state = g_hash_table_new(qapi_event_throttle_hash,
                                                 qapi_event_throttle_equal);
 }
 
 void monitor_init_globals(void)
 {
-    monitor_qapi_event_init();
-    qemu_mutex_init(&monitor_lock);
-    coroutine_mon = g_hash_table_new(NULL, NULL);
-
     /*
      * The dispatcher BH must run in the main loop thread, since we
      * have commands assuming that context.  It would be nice to get
-- 
2.50.1

[PATCH v4 03/23] system: unconditionally enable thread naming

[Daniel P. Berrangé]

When thread naming was introduced years ago, it was disabled by
default and put behind a command line flag:

  commit 8f480de0c91a18d550721f8d9af969ebfbda0793
  Author: Dr. David Alan Gilbert <dgilbert@redhat.com>
  Date:   Thu Jan 30 10:20:31 2014 +0000

    Add 'debug-threads' suboption to --name

This was done based on a concern that something might depend
on the historical thread naming. Thread names, however, were
never promised to be part of QEMU's public API. The defaults
will vary across platforms, so no assumptions should ever be
made about naming.

An opt-in behaviour is also unfortunately incompatible with
RCU which creates its thread from an constructor function
which is run before command line args are parsed. Thus the
RCU thread lacks any name.

libvirt has unconditionally enabled debug-threads=yes on all
VMs it creates for 10 years. Interestingly this DID expose a
bug in libvirt, as it parsed /proc/$PID/stat and could not
cope with a space in the thread name. This was a latent
pre-existing bug in libvirt though, and not a part of QEMU's
API.

Having thread names always available, will allow thread names
to be included in error reports and log messags QEMU prints
by default, which will improve ability to triage QEMU bugs.

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/about/deprecated.rst |  7 +++++++
 include/qemu/thread.h     |  1 -
 system/vl.c               | 12 +++++++-----
 util/qemu-thread-posix.c  | 18 +-----------------
 util/qemu-thread-win32.c  | 27 ++++++---------------------
 5 files changed, 21 insertions(+), 44 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index aa300bbd50..f920939c9a 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -68,6 +68,13 @@ configurations (e.g. -smp drawers=1,books=1,clusters=1 for x86 PC machine) is
 marked deprecated since 9.0, users have to ensure that all the topology members
 described with -smp are supported by the target machine.
 
+``debug-threads`` option for ``-name`` (since 10.2)
+'''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The ``debug-threads`` option of the ``-name`` argument is now
+ignored. Thread naming is unconditionally enabled for all platforms
+where it is supported.
+
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
 
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index f0302ed01f..3a286bb3ef 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -215,7 +215,6 @@ void *qemu_thread_join(QemuThread *thread);
 void qemu_thread_get_self(QemuThread *thread);
 bool qemu_thread_is_self(QemuThread *thread);
 G_NORETURN void qemu_thread_exit(void *retval);
-void qemu_thread_naming(bool enable);
 
 struct Notifier;
 /**
diff --git a/system/vl.c b/system/vl.c
index 00f3694725..e0884c1bb9 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -403,9 +403,8 @@ static QemuOptsList qemu_name_opts = {
         }, {
             .name = "debug-threads",
             .type = QEMU_OPT_BOOL,
-            .help = "When enabled, name the individual threads; defaults off.\n"
-                    "NOTE: The thread names are for debugging and not a\n"
-                    "stable API.",
+            .help = "Enable thread names"
+                    "(deprecated, always enabled where supported)",
         },
         { /* End of list */ }
     },
@@ -554,9 +553,12 @@ static int parse_name(void *opaque, QemuOpts *opts, Error **errp)
 {
     const char *proc_name;
 
-    if (qemu_opt_get(opts, "debug-threads")) {
-        qemu_thread_naming(qemu_opt_get_bool(opts, "debug-threads", false));
+    if (qemu_opt_get(opts, "debug-threads") &&
+        !qemu_opt_get_bool(opts, "debug-threads", false)) {
+        fprintf(stderr, "Ignoring deprecated 'debug-threads=no' option, " \
+                "thread naming is unconditionally enabled\n");
     }
+
     qemu_name = qemu_opt_get(opts, "guest");
 
     proc_name = qemu_opt_get(opts, "process");
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index ba725444ba..7c985b5d38 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -22,22 +22,6 @@
 #include <pthread_np.h>
 #endif
 
-static bool name_threads;
-
-void qemu_thread_naming(bool enable)
-{
-    name_threads = enable;
-
-#if !defined CONFIG_PTHREAD_SETNAME_NP_W_TID && \
-    !defined CONFIG_PTHREAD_SETNAME_NP_WO_TID && \
-    !defined CONFIG_PTHREAD_SET_NAME_NP
-    /* This is a debugging option, not fatal */
-    if (enable) {
-        fprintf(stderr, "qemu: thread naming not supported on this host\n");
-    }
-#endif
-}
-
 static void error_exit(int err, const char *msg)
 {
     fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
@@ -361,7 +345,7 @@ static void *qemu_thread_start(void *args)
     /* Attempt to set the threads name; note that this is for debug, so
      * we're not going to fail if we can't set it.
      */
-    if (name_threads && qemu_thread_args->name) {
+    if (qemu_thread_args->name) {
 # if defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
         pthread_setname_np(pthread_self(), qemu_thread_args->name);
 # elif defined(CONFIG_PTHREAD_SETNAME_NP_WO_TID)
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index ca2e0b512e..9595a5b090 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -17,8 +17,6 @@
 #include "qemu-thread-common.h"
 #include <process.h>
 
-static bool name_threads;
-
 typedef HRESULT (WINAPI *pSetThreadDescription) (HANDLE hThread,
                                                  PCWSTR lpThreadDescription);
 static pSetThreadDescription SetThreadDescriptionFunc;
@@ -44,16 +42,6 @@ static bool load_set_thread_description(void)
     return !!SetThreadDescriptionFunc;
 }
 
-void qemu_thread_naming(bool enable)
-{
-    name_threads = enable;
-
-    if (enable && !load_set_thread_description()) {
-        fprintf(stderr, "qemu: thread naming not supported on this host\n");
-        name_threads = false;
-    }
-}
-
 static void error_exit(int err, const char *msg)
 {
     char *pstr;
@@ -328,23 +316,20 @@ void *qemu_thread_join(QemuThread *thread)
     return ret;
 }
 
-static bool set_thread_description(HANDLE h, const char *name)
+static void set_thread_description(HANDLE h, const char *name)
 {
-    HRESULT hr;
     g_autofree wchar_t *namew = NULL;
 
     if (!load_set_thread_description()) {
-        return false;
+        return;
     }
 
     namew = g_utf8_to_utf16(name, -1, NULL, NULL, NULL);
     if (!namew) {
-        return false;
+        return;
     }
 
-    hr = SetThreadDescriptionFunc(h, namew);
-
-    return SUCCEEDED(hr);
+    SetThreadDescriptionFunc(h, namew);
 }
 
 void qemu_thread_create(QemuThread *thread, const char *name,
@@ -370,8 +355,8 @@ void qemu_thread_create(QemuThread *thread, const char *name,
     if (!hThread) {
         error_exit(GetLastError(), __func__);
     }
-    if (name_threads && name && !set_thread_description(hThread, name)) {
-        fprintf(stderr, "qemu: failed to set thread description: %s\n", name);
+    if (name) {
+        set_thread_description(hThread, name);
     }
     CloseHandle(hThread);
 
-- 
2.50.1

[PATCH v4 04/23] util: expose qemu_thread_set_name

[Daniel P. Berrangé]

The ability to set the thread name needs to be used in a number
of places, so expose the current impls as public methods.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/thread.h    |  1 +
 util/qemu-thread-posix.c | 26 ++++++++++++++++----------
 util/qemu-thread-win32.c | 13 ++++++++-----
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 3a286bb3ef..27b888ab0a 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -215,6 +215,7 @@ void *qemu_thread_join(QemuThread *thread);
 void qemu_thread_get_self(QemuThread *thread);
 bool qemu_thread_is_self(QemuThread *thread);
 G_NORETURN void qemu_thread_exit(void *retval);
+void qemu_thread_set_name(const char *name);
 
 struct Notifier;
 /**
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 7c985b5d38..b1c127dbe3 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -329,6 +329,21 @@ static void qemu_thread_atexit_notify(void *arg)
     notifier_list_notify(&thread_exit, NULL);
 }
 
+void qemu_thread_set_name(const char *name)
+{
+    /*
+     * Attempt to set the threads name; note that this is for debug, so
+     * we're not going to fail if we can't set it.
+     */
+# if defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
+    pthread_setname_np(pthread_self(), name);
+# elif defined(CONFIG_PTHREAD_SETNAME_NP_WO_TID)
+    pthread_setname_np(name);
+# elif defined(CONFIG_PTHREAD_SET_NAME_NP)
+    pthread_set_name_np(pthread_self(), name);
+# endif
+}
+
 typedef struct {
     void *(*start_routine)(void *);
     void *arg;
@@ -342,17 +357,8 @@ static void *qemu_thread_start(void *args)
     void *arg = qemu_thread_args->arg;
     void *r;
 
-    /* Attempt to set the threads name; note that this is for debug, so
-     * we're not going to fail if we can't set it.
-     */
     if (qemu_thread_args->name) {
-# if defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
-        pthread_setname_np(pthread_self(), qemu_thread_args->name);
-# elif defined(CONFIG_PTHREAD_SETNAME_NP_WO_TID)
-        pthread_setname_np(qemu_thread_args->name);
-# elif defined(CONFIG_PTHREAD_SET_NAME_NP)
-        pthread_set_name_np(pthread_self(), qemu_thread_args->name);
-# endif
+        qemu_thread_set_name(qemu_thread_args->name);
     }
     QEMU_TSAN_ANNOTATE_THREAD_NAME(qemu_thread_args->name);
     g_free(qemu_thread_args->name);
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 9595a5b090..4d2d663a9a 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -225,6 +225,7 @@ struct QemuThreadData {
     void             *arg;
     short             mode;
     NotifierList      exit;
+    char             *name;
 
     /* Only used for joinable threads. */
     bool              exited;
@@ -266,6 +267,10 @@ static unsigned __stdcall win32_start_routine(void *arg)
     void *(*start_routine)(void *) = data->start_routine;
     void *thread_arg = data->arg;
 
+    if (data->name) {
+        qemu_thread_set_name(data->name);
+        g_clear_pointer(&data->name, g_free);
+    }
     qemu_thread_data = data;
     qemu_thread_exit(start_routine(thread_arg));
     abort();
@@ -316,7 +321,7 @@ void *qemu_thread_join(QemuThread *thread)
     return ret;
 }
 
-static void set_thread_description(HANDLE h, const char *name)
+void qemu_thread_set_name(const char *name)
 {
     g_autofree wchar_t *namew = NULL;
 
@@ -329,7 +334,7 @@ static void set_thread_description(HANDLE h, const char *name)
         return;
     }
 
-    SetThreadDescriptionFunc(h, namew);
+    SetThreadDescriptionFunc(GetCurrentThread(), namew);
 }
 
 void qemu_thread_create(QemuThread *thread, const char *name,
@@ -344,6 +349,7 @@ void qemu_thread_create(QemuThread *thread, const char *name,
     data->arg = arg;
     data->mode = mode;
     data->exited = false;
+    data->name = g_strdup(name);
     notifier_list_init(&data->exit);
 
     if (data->mode != QEMU_THREAD_DETACHED) {
@@ -355,9 +361,6 @@ void qemu_thread_create(QemuThread *thread, const char *name,
     if (!hThread) {
         error_exit(GetLastError(), __func__);
     }
-    if (name) {
-        set_thread_description(hThread, name);
-    }
     CloseHandle(hThread);
 
     thread->data = data;
-- 
2.50.1

[PATCH v4 05/23] audio: make jackaudio use qemu_thread_set_name

[Daniel P. Berrangé]

This has greater portability than directly call pthread_setname_np,
which is only 1 out of 3 possible functions for pthreads that can
set the name.

The new API requires a trampoline function, since it can only set
the name of the current thread.

Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 audio/jackaudio.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index 974a3caad3..69dce3f302 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -629,18 +629,36 @@ static void qjack_enable_in(HWVoiceIn *hw, bool enable)
     ji->c.enabled = enable;
 }
 
-#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
+#if !defined(WIN32)
+struct QJackThreadData {
+    void *(*function)(void *);
+    void *arg;
+};
+
+static void *qjack_thread_trampoline(void *targ)
+{
+    struct QJackThreadData *data = targ;
+    void *(*function)(void *) = data->function;
+    void *arg = data->arg;
+
+    g_free(data);
+    qemu_thread_set_name("jack-client");
+
+    return function(arg);
+}
+
 static int qjack_thread_creator(jack_native_thread_t *thread,
     const pthread_attr_t *attr, void *(*function)(void *), void *arg)
 {
-    int ret = pthread_create(thread, attr, function, arg);
+    struct QJackThreadData *data = g_new0(struct QJackThreadData, 1);
+    data->function = function;
+    data->arg = arg;
+    int ret = pthread_create(thread, attr, qjack_thread_trampoline, data);
     if (ret != 0) {
+        g_free(data);
         return ret;
     }
 
-    /* set the name of the thread */
-    pthread_setname_np(*thread, "jack-client");
-
     return ret;
 }
 #endif
@@ -696,7 +714,7 @@ static void register_audio_jack(void)
 {
     qemu_mutex_init(&qjack_shutdown_lock);
     audio_driver_register(&jack_driver);
-#if !defined(WIN32) && defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
+#if !defined(WIN32)
     jack_set_thread_creator(qjack_thread_creator);
 #endif
     jack_set_error_function(qjack_error);
-- 
2.50.1

[PATCH v4 06/23] util: set the name for the 'main' thread

[Daniel P. Berrangé]

The default main thread name is undefined, so use a constructor to
explicitly set it to 'main'. This constructor is marked to run early
as the thread name is intended to be used in error reporting / logs
which may be triggered very early in QEMU execution.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/qemu-thread-posix.c | 6 ++++++
 util/qemu-thread-win32.c | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index b1c127dbe3..afeac9ecad 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -22,6 +22,12 @@
 #include <pthread_np.h>
 #endif
 
+static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
+qemu_thread_init(void)
+{
+    qemu_thread_set_name("main");
+}
+
 static void error_exit(int err, const char *msg)
 {
     fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 4d2d663a9a..8ca6429ad3 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -22,6 +22,12 @@ typedef HRESULT (WINAPI *pSetThreadDescription) (HANDLE hThread,
 static pSetThreadDescription SetThreadDescriptionFunc;
 static HMODULE kernel32_module;
 
+static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
+qemu_thread_init(void)
+{
+    qemu_thread_set_name("main");
+}
+
 static bool load_set_thread_description(void)
 {
     static gsize _init_once = 0;
-- 
2.50.1

[PATCH v4 07/23] util: add API to fetch the current thread name

[Daniel P. Berrangé]

This will be used to include the thread name in error reports
in a later patch. It returns a const string stored in a thread
local to avoid memory allocation when it is called repeatedly
in a single thread. The thread name should be set at the very
start of the thread execution, which is the case when using
qemu_thread_create.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/thread.h    |  1 +
 meson.build              | 21 +++++++++++++++++
 util/qemu-thread-posix.c | 33 ++++++++++++++++++++++++++-
 util/qemu-thread-win32.c | 49 ++++++++++++++++++++++++++++++++++++----
 4 files changed, 99 insertions(+), 5 deletions(-)

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 27b888ab0a..98cc5c41ac 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -216,6 +216,7 @@ void qemu_thread_get_self(QemuThread *thread);
 bool qemu_thread_is_self(QemuThread *thread);
 G_NORETURN void qemu_thread_exit(void *retval);
 void qemu_thread_set_name(const char *name);
+const char *qemu_thread_get_name(void);
 
 struct Notifier;
 /**
diff --git a/meson.build b/meson.build
index 72da97829a..0893005919 100644
--- a/meson.build
+++ b/meson.build
@@ -2912,6 +2912,27 @@ config_host_data.set('CONFIG_PTHREAD_SET_NAME_NP', cc.links(osdep_prefix + '''
     pthread_set_name_np(thread, "QEMU");
     return 0;
   }''', dependencies: threads))
+
+config_host_data.set('CONFIG_PTHREAD_GETNAME_NP', cc.links(osdep_prefix + '''
+  #include <pthread.h>
+
+  int main(void)
+  {
+    char buf[16];
+    pthread_getname_np(pthread_self(), buf, sizeof(buf));
+    return 0;
+  }''', dependencies: threads))
+config_host_data.set('CONFIG_PTHREAD_GET_NAME_NP', cc.links(osdep_prefix + '''
+  #include <pthread.h>
+  #include <pthread_np.h>
+
+  int main(void)
+  {
+    char buf[16];
+    pthread_get_name_np(pthread_self(), buf, sizeof(buf));
+    return 0;
+  }''', dependencies: threads))
+
 config_host_data.set('CONFIG_PTHREAD_CONDATTR_SETCLOCK', cc.links(osdep_prefix + '''
   #include <pthread.h>
 
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index afeac9ecad..05c7ea4c00 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -18,7 +18,7 @@
 #include "qemu/tsan.h"
 #include "qemu/bitmap.h"
 
-#ifdef CONFIG_PTHREAD_SET_NAME_NP
+#if defined(CONFIG_PTHREAD_SET_NAME_NP) || defined(CONFIG_PTHREAD_GET_NAME_NP)
 #include <pthread_np.h>
 #endif
 
@@ -532,3 +532,34 @@ void *qemu_thread_join(QemuThread *thread)
     }
     return ret;
 }
+
+/*
+ * This is not defined on Linux, but the man page indicates
+ * the buffer must be at least 16 bytes, including the NUL
+ * terminator
+ */
+#ifndef PTHREAD_MAX_NAMELEN_NP
+#define PTHREAD_MAX_NAMELEN_NP 16
+#endif
+
+static __thread char namebuf[PTHREAD_MAX_NAMELEN_NP];
+
+const char *qemu_thread_get_name(void)
+{
+    int rv;
+    if (namebuf[0] != '\0') {
+        return namebuf;
+    }
+
+# if defined(CONFIG_PTHREAD_GETNAME_NP)
+    rv = pthread_getname_np(pthread_self(), namebuf, sizeof(namebuf));
+# elif defined(CONFIG_PTHREAD_GET_NAME_NP)
+    rv = pthread_get_name_np(pthread_self(), namebuf, sizeof(namebuf));
+# else
+    rv = -1;
+# endif
+    if (rv != 0) {
+        strlcpy(namebuf, "unnamed", G_N_ELEMENTS(namebuf));
+    }
+    return namebuf;
+}
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 8ca6429ad3..20165c6f95 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -19,7 +19,10 @@
 
 typedef HRESULT (WINAPI *pSetThreadDescription) (HANDLE hThread,
                                                  PCWSTR lpThreadDescription);
+typedef HRESULT (WINAPI *pGetThreadDescription) (HANDLE hThread,
+                                                 PWSTR *lpThreadDescription);
 static pSetThreadDescription SetThreadDescriptionFunc;
+static pGetThreadDescription GetThreadDescriptionFunc;
 static HMODULE kernel32_module;
 
 static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
@@ -28,7 +31,7 @@ qemu_thread_init(void)
     qemu_thread_set_name("main");
 }
 
-static bool load_set_thread_description(void)
+static bool load_thread_description(void)
 {
     static gsize _init_once = 0;
 
@@ -38,14 +41,17 @@ static bool load_set_thread_description(void)
             SetThreadDescriptionFunc =
                 (pSetThreadDescription)GetProcAddress(kernel32_module,
                                                       "SetThreadDescription");
-            if (!SetThreadDescriptionFunc) {
+            GetThreadDescriptionFunc =
+                (pGetThreadDescription)GetProcAddress(kernel32_module,
+                                                      "GetThreadDescription");
+            if (!SetThreadDescriptionFunc || !GetThreadDescriptionFunc) {
                 FreeLibrary(kernel32_module);
             }
         }
         g_once_init_leave(&_init_once, 1);
     }
 
-    return !!SetThreadDescriptionFunc;
+    return !!(SetThreadDescriptionFunc && GetThreadDescriptionFunc);
 }
 
 static void error_exit(int err, const char *msg)
@@ -331,7 +337,7 @@ void qemu_thread_set_name(const char *name)
 {
     g_autofree wchar_t *namew = NULL;
 
-    if (!load_set_thread_description()) {
+    if (!load_thread_description()) {
         return;
     }
 
@@ -415,3 +421,38 @@ bool qemu_thread_is_self(QemuThread *thread)
 {
     return GetCurrentThreadId() == thread->tid;
 }
+
+static __thread char namebuf[64];
+
+const char *qemu_thread_get_name(void)
+{
+    HRESULT hr;
+    wchar_t *namew = NULL;
+    g_autofree char *name = NULL;
+
+    if (namebuf[0] != '\0') {
+        return namebuf;
+    }
+
+    if (!load_thread_description()) {
+        goto error;
+    }
+
+    hr = GetThreadDescriptionFunc(GetCurrentThread(), &namew);
+    if (!SUCCEEDED(hr)) {
+        goto error;
+    }
+
+    name = g_utf16_to_utf8(namew, -1, NULL, NULL, NULL);
+    LocalFree(namew);
+    if (!name) {
+        goto error;
+    }
+
+    g_strlcpy(namebuf, name, G_N_ELEMENTS(namebuf));
+    return namebuf;
+
+ error:
+    g_strlcpy(namebuf, "unnamed", G_N_ELEMENTS(namebuf));
+    return namebuf;
+}
-- 
2.50.1

[PATCH v4 08/23] log: avoid repeated prefix on incremental qemu_log calls

[Daniel P. Berrangé]

Some code makes multiple qemu_log calls to incrementally emit
a single message. Currently timestamps get prepended to all
qemu_log calls, even those continuing a previous incomplete
message.

This changes the qemu_log so it skips adding a new line prefix,
if the previous qemu_log call did NOT end with a newline.

Fixes: 012842c07552 (log: make '-msg timestamp=on' apply to all qemu_log usage)
Reported-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/log.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/util/log.c b/util/log.c
index abdcb6b311..2642a55c59 100644
--- a/util/log.c
+++ b/util/log.c
@@ -143,6 +143,12 @@ void qemu_log_unlock(FILE *logfile)
     }
 }
 
+/*
+ * 'true' if the previous log message lacked a trailing '\n',
+ * and thus the subsequent call must skip any prefix
+ */
+static __thread bool incomplete;
+
 void qemu_log(const char *fmt, ...)
 {
     FILE *f;
@@ -154,7 +160,7 @@ void qemu_log(const char *fmt, ...)
      * was emitted if we are delayed acquiring the
      * mutex
      */
-    if (message_with_timestamp) {
+    if (message_with_timestamp && !incomplete) {
         g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
         timestr = g_date_time_format_iso8601(dt);
     }
@@ -170,6 +176,7 @@ void qemu_log(const char *fmt, ...)
         va_start(ap, fmt);
         vfprintf(f, fmt, ap);
         va_end(ap);
+        incomplete = fmt[strlen(fmt) - 1] != '\n';
         qemu_log_unlock(f);
     }
 }
-- 
2.50.1

[PATCH v4 09/23] util: introduce some API docs for logging APIs

[Daniel P. Berrangé]

This addresses two key gotchas with the logging APIs:

 * Safely outputting a single line of text using
   multiple qemu_log() calls requires use of the
   qemu_log_trylock/unlock functions to avoid
   interleaving between threads

 * Directly outputting to the FILE object returned
   by qemu_log_trylock() must be discouraged because
   it prevents the inclusion of configured log message
   prefixes.

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/log-for-trace.h | 35 ++++++++++++++++++++++++++++++++++-
 include/qemu/log.h           | 26 ++++++++++++++++++++++++++
 rust/util/src/log.rs         |  7 +++++++
 3 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/include/qemu/log-for-trace.h b/include/qemu/log-for-trace.h
index d47c9cd446..4e05b2e26f 100644
--- a/include/qemu/log-for-trace.h
+++ b/include/qemu/log-for-trace.h
@@ -29,7 +29,40 @@ static inline bool qemu_loglevel_mask(int mask)
     return (qemu_loglevel & mask) != 0;
 }
 
-/* main logging function */
+/**
+ * qemu_log: report a log message
+ * @fmt: the format string for the message
+ * @...: the format string arguments
+ *
+ * This will emit a log message to the current output stream.
+ *
+ * The @fmt string should normally represent a complete line
+ * of text, ending with a newline character.
+ *
+ * If intending to call this function multiple times to
+ * incrementally construct a line of text, locking must
+ * be used to ensure that output from different threads
+ * is not interleaved.
+ *
+ * This is achieved by calling qemu_log_trylock() before
+ * starting the log line; calling qemu_log() multiple
+ * times with the last call having a newline at the end
+ * of @fmt; finishing with a call to qemu_log_unlock().
+ *
+ * The FILE object returned by qemu_log_trylock() does
+ * not need to be used for outputting text directly,
+ * it is merely used to associate the lock.
+ *
+ *    FILE *f = qemu_log_trylock()
+ *
+ *    qemu_log("Something");
+ *    qemu_log("Something");
+ *    qemu_log("Something");
+ *    qemu_log("The end\n");
+ *
+ *    qemu_log_unlock(f);
+ *
+ */
 void G_GNUC_PRINTF(1, 2) qemu_log(const char *fmt, ...);
 
 #endif
diff --git a/include/qemu/log.h b/include/qemu/log.h
index aae72985f0..867fe327e4 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -41,7 +41,33 @@ bool qemu_log_separate(void);
 
 /* Lock/unlock output. */
 
+/**
+ * Acquires a lock on the current log output stream.
+ * The returned FILE object must be passed to
+ * qemu_log_unlock() to later release the lock.
+ *
+ * This should be used to protect a sequence of calls
+ * to qemu_log(), if they are being used to incrementally
+ * output a single line of text. For qemu_log() calls which
+ * output a complete line of text it is not required to
+ * take explicit locks.
+ *
+ * The returned FILE object may be used to directly
+ * output log messages, however, doing so will prevent
+ * the inclusion of configured log message prefixes.
+ * It is thus recommended that this be used sparingly,
+ * only in cases where it is required to dump large
+ * data volumes. Use of qemu_log() is preferred for
+ * most output tasks.
+ *
+ * Returns: the current FILE if available, NULL on error
+ */
 FILE *qemu_log_trylock(void) G_GNUC_WARN_UNUSED_RESULT;
+
+/**
+ * Releases the lock on the log output, previously
+ * acquired by qemu_log_trylock().
+ */
 void qemu_log_unlock(FILE *fd);
 
 /* Logging functions: */
diff --git a/rust/util/src/log.rs b/rust/util/src/log.rs
index af9a3e9123..eaf493f0df 100644
--- a/rust/util/src/log.rs
+++ b/rust/util/src/log.rs
@@ -55,6 +55,13 @@ impl LogGuard {
     ///     writeln!(log, "test");
     /// }
     /// ```
+    ///
+    /// Note that directly writing to the log output will prevent the
+    /// inclusion of configured log prefixes. It is thus recommended
+    /// that this be used sparingly, only in cases where it is required
+    /// to dump large data volumes. Use of [`log_mask_ln!()`](crate::log_mask_ln)
+    /// macro() is preferred for most output tasks.
+
     pub fn new() -> Option<Self> {
         let f = unsafe { bindings::qemu_log_trylock() }.cast();
         NonNull::new(f).map(Self)
-- 
2.50.1

[PATCH v4 10/23] ui/vnc: remove use of error_printf_unless_qmp()

[Daniel P. Berrangé]

The error_printf_unless_qmp() will print to the monitor if the current
one is HMP, if it is QMP nothing will be printed, otherwise stderr
will be used.

This scenario is easily handled by checking !monitor_cur_is_qmp() and
then calling the error_printf() function.

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 ui/vnc.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 77c823bf2e..3cc5b4cfec 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3530,8 +3530,10 @@ int vnc_display_password(const char *id, const char *password)
         return -EINVAL;
     }
     if (vd->auth == VNC_AUTH_NONE) {
-        error_printf_unless_qmp("If you want use passwords please enable "
-                                "password auth using '-vnc ${dpy},password'.\n");
+        if (!monitor_cur_is_qmp()) {
+            error_printf("If you want to use passwords, please enable "
+                         "password auth using '-vnc ${dpy},password'.\n");
+        }
         return -EINVAL;
     }
 
@@ -3570,9 +3572,11 @@ static void vnc_display_print_local_addr(VncDisplay *vd)
         qapi_free_SocketAddress(addr);
         return;
     }
-    error_printf_unless_qmp("VNC server running on %s:%s\n",
-                            addr->u.inet.host,
-                            addr->u.inet.port);
+    if (!monitor_cur_is_qmp()) {
+        error_printf("VNC server running on %s:%s\n",
+                     addr->u.inet.host,
+                     addr->u.inet.port);
+    }
     qapi_free_SocketAddress(addr);
 }
 
-- 
2.50.1

[PATCH v4 11/23] monitor: remove redundant error_[v]printf_unless_qmp

[Daniel P. Berrangé]

These functions only had one caller which was easily converted to
the normal error_printf() function. Remove them as they don't add
sufficient value.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/monitor/monitor.h |  3 ---
 monitor/monitor.c         | 24 ------------------------
 stubs/error-printf.c      |  5 -----
 3 files changed, 32 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index c3740ec616..296690e1f1 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -59,7 +59,4 @@ void monitor_register_hmp(const char *name, bool info,
 void monitor_register_hmp_info_hrt(const char *name,
                                    HumanReadableText *(*handler)(Error **errp));
 
-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);
-
 #endif /* MONITOR_H */
diff --git a/monitor/monitor.c b/monitor/monitor.c
index da54e1b1ce..03dbe5d131 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -281,30 +281,6 @@ int error_vprintf(const char *fmt, va_list ap)
     return vfprintf(stderr, fmt, ap);
 }
 
-int error_vprintf_unless_qmp(const char *fmt, va_list ap)
-{
-    Monitor *cur_mon = monitor_cur();
-
-    if (!cur_mon) {
-        return vfprintf(stderr, fmt, ap);
-    }
-    if (!monitor_cur_is_qmp()) {
-        return monitor_vprintf(cur_mon, fmt, ap);
-    }
-    return -1;
-}
-
-int error_printf_unless_qmp(const char *fmt, ...)
-{
-    va_list ap;
-    int ret;
-
-    va_start(ap, fmt);
-    ret = error_vprintf_unless_qmp(fmt, ap);
-    va_end(ap);
-    return ret;
-}
-
 static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
     /* Limit guest-triggerable events to 1 per second */
     [QAPI_EVENT_RTC_CHANGE]        = { 1000 * SCALE_MS },
diff --git a/stubs/error-printf.c b/stubs/error-printf.c
index 0e326d8010..1afa0f62ca 100644
--- a/stubs/error-printf.c
+++ b/stubs/error-printf.c
@@ -16,8 +16,3 @@ int error_vprintf(const char *fmt, va_list ap)
     }
     return vfprintf(stderr, fmt, ap);
 }
-
-int error_vprintf_unless_qmp(const char *fmt, va_list ap)
-{
-    return error_vprintf(fmt, ap);
-}
-- 
2.50.1

[PATCH v4 12/23] monitor: refactor error_vprintf()

[Daniel P. Berrangé]

The monitor_vprintf() code will return -1 if either the monitor
is NULL, or the monitor is QMP. The error_vprintf() code can
take advantage of this to avoid having to duplicate the same
checks, and instead simply look at the return value.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/monitor.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/monitor/monitor.c b/monitor/monitor.c
index 03dbe5d131..c00f4aac5a 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -268,17 +268,19 @@ void monitor_printc(Monitor *mon, int c)
     monitor_printf(mon, "'");
 }
 
-/*
- * 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();
-
-    if (cur_mon && !monitor_cur_is_qmp()) {
-        return monitor_vprintf(cur_mon, fmt, ap);
+    /*
+     * This will return -1 if 'cur_mon' is NULL, or is QMP.
+     * IOW this will only print if in HMP, otherwise we
+     * fallback to stderr for QMP / no-monitor scenarios.
+     */
+    int ret = monitor_vprintf(cur_mon, fmt, ap);
+    if (ret == -1) {
+        ret = vfprintf(stderr, fmt, ap);
     }
-    return vfprintf(stderr, fmt, ap);
+    return ret;
 }
 
 static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
-- 
2.50.1

[PATCH v4 13/23] monitor: move error_vprintf back to error-report.c

[Daniel P. Berrangé]

The current unit tests rely on monitor.o not being linked, such
that the monitor stubs get linked instead. Since error_vprintf
is in monitor.o this allows a stub error_vprintf impl to be used
that calls g_test_message.

This takes a different approach, with error_vprintf moving
back to error-report.c such that it is always linked into the
tests. The monitor_vprintf() stub is then changed to use
g_test_message if QTEST_SLENT_ERROS is set, otherwise it will
return -1 and trigger error_vprintf to call vfprintf.

The end result is functionally equivalent for the purposes of
the unit tests.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/monitor.c    | 15 ---------------
 stubs/error-printf.c | 18 ------------------
 stubs/meson.build    |  1 -
 stubs/monitor-core.c | 14 +++++++++++++-
 util/error-report.c  | 15 +++++++++++++++
 5 files changed, 28 insertions(+), 35 deletions(-)
 delete mode 100644 stubs/error-printf.c

diff --git a/monitor/monitor.c b/monitor/monitor.c
index c00f4aac5a..e1e5dbfcbe 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -268,21 +268,6 @@ void monitor_printc(Monitor *mon, int c)
     monitor_printf(mon, "'");
 }
 
-int error_vprintf(const char *fmt, va_list ap)
-{
-    Monitor *cur_mon = monitor_cur();
-    /*
-     * This will return -1 if 'cur_mon' is NULL, or is QMP.
-     * IOW this will only print if in HMP, otherwise we
-     * fallback to stderr for QMP / no-monitor scenarios.
-     */
-    int ret = monitor_vprintf(cur_mon, fmt, ap);
-    if (ret == -1) {
-        ret = vfprintf(stderr, fmt, ap);
-    }
-    return ret;
-}
-
 static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
     /* Limit guest-triggerable events to 1 per second */
     [QAPI_EVENT_RTC_CHANGE]        = { 1000 * SCALE_MS },
diff --git a/stubs/error-printf.c b/stubs/error-printf.c
deleted file mode 100644
index 1afa0f62ca..0000000000
--- a/stubs/error-printf.c
+++ /dev/null
@@ -1,18 +0,0 @@
-#include "qemu/osdep.h"
-#include "qemu/error-report.h"
-#include "monitor/monitor.h"
-
-int error_vprintf(const char *fmt, va_list ap)
-{
-    int ret;
-
-    if (g_test_initialized() && !g_test_subprocess() &&
-        getenv("QTEST_SILENT_ERRORS")) {
-        char *msg = g_strdup_vprintf(fmt, ap);
-        g_test_message("%s", msg);
-        ret = strlen(msg);
-        g_free(msg);
-        return ret;
-    }
-    return vfprintf(stderr, fmt, ap);
-}
diff --git a/stubs/meson.build b/stubs/meson.build
index cef046e685..f58371f685 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -3,7 +3,6 @@
 # below, so that it is clear who needs the stubbed functionality.
 
 stub_ss.add(files('cpu-get-clock.c'))
-stub_ss.add(files('error-printf.c'))
 stub_ss.add(files('fdset.c'))
 stub_ss.add(files('iothread-lock.c'))
 stub_ss.add(files('is-daemonized.c'))
diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c
index 1894cdfe1f..a7c32297c9 100644
--- a/stubs/monitor-core.c
+++ b/stubs/monitor-core.c
@@ -18,5 +18,17 @@ void qapi_event_emit(QAPIEvent event, QDict *qdict)
 
 int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 {
-    abort();
+    /*
+     * Pretend 'g_test_message' is our monitor console to
+     * stop the caller sending messages to stderr
+     */
+    if (g_test_initialized() && !g_test_subprocess() &&
+        getenv("QTEST_SILENT_ERRORS")) {
+        char *msg = g_strdup_vprintf(fmt, ap);
+        g_test_message("%s", msg);
+        size_t ret = strlen(msg);
+        g_free(msg);
+        return ret;
+    }
+    return -1;
 }
diff --git a/util/error-report.c b/util/error-report.c
index 1b17c11de1..b262ad01cb 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -29,6 +29,21 @@ bool message_with_timestamp;
 bool error_with_guestname;
 const char *error_guest_name;
 
+int error_vprintf(const char *fmt, va_list ap)
+{
+    Monitor *cur_mon = monitor_cur();
+    /*
+     * This will return -1 if 'cur_mon' is NULL, or is QMP.
+     * IOW this will only print if in HMP, otherwise we
+     * fallback to stderr for QMP / no-monitor scenarios.
+     */
+    int ret = monitor_vprintf(cur_mon, fmt, ap);
+    if (ret == -1) {
+        ret = vfprintf(stderr, fmt, ap);
+    }
+    return ret;
+}
+
 int error_printf(const char *fmt, ...)
 {
     va_list ap;
-- 
2.50.1

[PATCH v4 14/23] monitor: introduce monitor_cur_is_hmp() helper

[Daniel P. Berrangé]

Note that this is not simply the inverse of monitor_cur_is_qmp(),
as both helpers require that monitor_cur() is first non-NULL.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/monitor/monitor.h      |  1 +
 monitor/monitor.c              | 10 ++++++++++
 stubs/monitor-core.c           |  6 ++++++
 tests/unit/test-util-sockets.c |  1 +
 4 files changed, 18 insertions(+)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 296690e1f1..9c71e6cf3c 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -16,6 +16,7 @@ extern QemuOptsList qemu_mon_opts;
 Monitor *monitor_cur(void);
 Monitor *monitor_set_cur(Coroutine *co, Monitor *mon);
 bool monitor_cur_is_qmp(void);
+bool monitor_cur_is_hmp(void);
 
 void monitor_init_globals(void);
 void monitor_init_globals_core(void);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index e1e5dbfcbe..27be5d7c73 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -116,6 +116,16 @@ bool monitor_cur_is_qmp(void)
     return cur_mon && monitor_is_qmp(cur_mon);
 }
 
+/**
+ * Is the current monitor, if any, a HMP monitor?
+ */
+bool monitor_cur_is_hmp(void)
+{
+    Monitor *cur_mon = monitor_cur();
+
+    return cur_mon && !monitor_is_qmp(cur_mon);
+}
+
 /**
  * Is @mon is using readline?
  * Note: not all HMP monitors use readline, e.g., gdbserver has a
diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c
index a7c32297c9..674211f48f 100644
--- a/stubs/monitor-core.c
+++ b/stubs/monitor-core.c
@@ -7,6 +7,12 @@ Monitor *monitor_cur(void)
     return NULL;
 }
 
+bool monitor_cur_is_hmp(void)
+{
+    /* since monitor_cur() above returns NULL, this can't be true */
+    return false;
+}
+
 Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)
 {
     return NULL;
diff --git a/tests/unit/test-util-sockets.c b/tests/unit/test-util-sockets.c
index ee66d727c3..4b7f408902 100644
--- a/tests/unit/test-util-sockets.c
+++ b/tests/unit/test-util-sockets.c
@@ -74,6 +74,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
 Monitor *monitor_cur(void) { return cur_mon; }
 Monitor *monitor_set_cur(Coroutine *co, Monitor *mon) { abort(); }
 int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) { abort(); }
+bool monitor_cur_is_hmp(void) { return false; }
 
 #ifndef _WIN32
 static void test_socket_fd_pass_name_good(void)
-- 
2.50.1

[PATCH v4 15/23] util: don't skip error prefixes when QMP is active

[Daniel P. Berrangé]

The vreport() function will print to HMP if available, otherwise
to stderr. In the event that vreport() is called during execution
of a QMP command, it will print to stderr, but mistakenly omit the
message prefixes (timestamp, guest name, program name).

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/error-report.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/util/error-report.c b/util/error-report.c
index b262ad01cb..872158ad90 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -159,7 +159,7 @@ static void print_loc(void)
     int i;
     const char *const *argp;
 
-    if (!monitor_cur() && g_get_prgname()) {
+    if (!monitor_cur_is_hmp() && g_get_prgname()) {
         error_printf("%s:", g_get_prgname());
         sep = " ";
     }
@@ -203,14 +203,14 @@ static void vreport(report_type type, const char *fmt, va_list ap)
 {
     gchar *timestr;
 
-    if (message_with_timestamp && !monitor_cur()) {
+    if (message_with_timestamp && !monitor_cur_is_hmp()) {
         timestr = real_time_iso8601();
         error_printf("%s ", timestr);
         g_free(timestr);
     }
 
     /* Only prepend guest name if -msg guest-name and -name guest=... are set */
-    if (error_with_guestname && error_guest_name && !monitor_cur()) {
+    if (error_with_guestname && error_guest_name && !monitor_cur_is_hmp()) {
         error_printf("%s ", error_guest_name);
     }
 
-- 
2.50.1

[PATCH v4 16/23] util: fix interleaving of error & trace output

[Daniel P. Berrangé]

The monitor_cur_hmp() function will acquire/release mutex locks, which
will trigger trace probes, which can in turn trigger qemu_log() calls.
vreport() calls monitor_cur() multiple times through its execution
both directly and indirectly via error_vprintf().

The result is that the prefix information printed by vreport() gets
interleaved with qemu_log() output, when run outside the context of
an HMP command dispatcher. This can be seen with:

 $ qemu-system-x86_64 \
     -msg timestamp=on,guest-name=on \
     -display none \
     -object tls-creds-x509,id=f,dir=fish \
     -name fish \
     -d trace:qemu_mutex*
   2025-09-10T16:30:42.514374Z qemu_mutex_unlock released mutex 0x560b0339b4c0 (/var/home/berrange/src/virt/qemu/include/qemu/lockable.h:56)
   2025-09-10T16:30:42.514400Z qemu_mutex_lock waiting on mutex 0x560b033983e0 (/var/home/berrange/src/virt/qemu/include/qemu/lockable.h:56)
   2025-09-10T16:30:42.514402Z qemu_mutex_locked taken mutex 0x560b033983e0 (/var/home/berrange/src/virt/qemu/include/qemu/lockable.h:56)
   2025-09-10T16:30:42.514404Z qemu_mutex_unlock released mutex 0x560b033983e0 (/var/home/berrange/src/virt/qemu/include/qemu/lockable.h:56)
   2025-09-10T16:30:42.516716Z qemu_mutex_lock waiting on mutex 0x560b03398560 (../monitor/monitor.c:91)
   2025-09-10T16:30:42.516723Z qemu_mutex_locked taken mutex 0x560b03398560 (../monitor/monitor.c:91)
   2025-09-10T16:30:42.516726Z qemu_mutex_unlock released mutex 0x560b03398560 (../monitor/monitor.c:96)
   2025-09-10T16:30:42.516728Z qemu_mutex_lock waiting on mutex 0x560b03398560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842057Z qemu_mutex_locked taken mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842058Z qemu_mutex_unlock released mutex 0x564f5e401560 (../monitor/monitor.c:96)
   2025-09-10T16:31:04.842055Z 2025-09-10T16:31:04.842060Z qemu_mutex_lock waiting on mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842061Z qemu_mutex_locked taken mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842062Z qemu_mutex_unlock released mutex 0x564f5e401560 (../monitor/monitor.c:96)
   2025-09-10T16:31:04.842064Z qemu_mutex_lock waiting on mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842065Z qemu_mutex_locked taken mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842066Z qemu_mutex_unlock released mutex 0x564f5e401560 (../monitor/monitor.c:96)
   fish 2025-09-10T16:31:04.842068Z qemu_mutex_lock waiting on mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842069Z qemu_mutex_locked taken mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842070Z qemu_mutex_unlock released mutex 0x564f5e401560 (../monitor/monitor.c:96)
   2025-09-10T16:31:04.842072Z qemu_mutex_lock waiting on mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842097Z qemu_mutex_locked taken mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842099Z qemu_mutex_unlock released mutex 0x564f5e401560 (../monitor/monitor.c:96)
   qemu-system-x86_64:2025-09-10T16:31:04.842100Z qemu_mutex_lock waiting on mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842102Z qemu_mutex_locked taken mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842103Z qemu_mutex_unlock released mutex 0x564f5e401560 (../monitor/monitor.c:96)
    2025-09-10T16:31:04.842105Z qemu_mutex_lock waiting on mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842106Z qemu_mutex_locked taken mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842107Z qemu_mutex_unlock released mutex 0x564f5e401560 (../monitor/monitor.c:96)
   Unable to access credentials fish/ca-cert.pem: No such file or directory2025-09-10T16:31:04.842109Z qemu_mutex_lock waiting on mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842110Z qemu_mutex_locked taken mutex 0x564f5e401560 (../monitor/monitor.c:91)
   2025-09-10T16:31:04.842111Z qemu_mutex_unlock released mutex 0x564f5e401560 (../monitor/monitor.c:96)

To avoid this interleaving (as well as reduce the huge number of
mutex lock/unlock calls) we need to ensure that monitor_cur_is_hmp()
is only called once at the start of vreport(), and if no HMP is
present, no further monitor APIs can be called.

This implies error_[v]printf() cannot be called from vreport(),
instead we must introduce error_[v]printf_mon() which accept a
pre-acquired Monitor object.

 $ qemu-system-x86_64 \
     -msg timestamp=on,guest-name=on \
     -display none \
     -object tls-creds-x509,id=f,dir=fish \
     -name fish \
     -d trace:qemu_mutex*
   2025-09-10T16:31:22.701691Z qemu_mutex_unlock released mutex 0x5626fd3b84c0 (/var/home/berrange/src/virt/qemu/include/qemu/lockable.h:56)
   2025-09-10T16:31:22.701728Z qemu_mutex_lock waiting on mutex 0x5626fd3b53e0 (/var/home/berrange/src/virt/qemu/include/qemu/lockable.h:56)
   2025-09-10T16:31:22.701730Z qemu_mutex_locked taken mutex 0x5626fd3b53e0 (/var/home/berrange/src/virt/qemu/include/qemu/lockable.h:56)
   2025-09-10T16:31:22.701732Z qemu_mutex_unlock released mutex 0x5626fd3b53e0 (/var/home/berrange/src/virt/qemu/include/qemu/lockable.h:56)
   2025-09-10T16:31:22.703989Z qemu_mutex_lock waiting on mutex 0x5626fd3b5560 (../monitor/monitor.c:91)
   2025-09-10T16:31:22.703996Z qemu_mutex_locked taken mutex 0x5626fd3b5560 (../monitor/monitor.c:91)
   2025-09-10T16:31:22.703999Z qemu_mutex_unlock released mutex 0x5626fd3b5560 (../monitor/monitor.c:96)
   2025-09-10T16:31:22.704000Z fish qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/error-report.c | 67 +++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 21 deletions(-)

diff --git a/util/error-report.c b/util/error-report.c
index 872158ad90..222c40d0d1 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -29,9 +29,12 @@ bool message_with_timestamp;
 bool error_with_guestname;
 const char *error_guest_name;
 
-int error_vprintf(const char *fmt, va_list ap)
+/*
+ * Print to current monitor if we have one, else to stderr.
+ */
+static int G_GNUC_PRINTF(2, 0)
+error_vprintf_mon(Monitor *cur_mon, const char *fmt, va_list ap)
 {
-    Monitor *cur_mon = monitor_cur();
     /*
      * This will return -1 if 'cur_mon' is NULL, or is QMP.
      * IOW this will only print if in HMP, otherwise we
@@ -44,13 +47,30 @@ int error_vprintf(const char *fmt, va_list ap)
     return ret;
 }
 
+static int G_GNUC_PRINTF(2, 3)
+error_printf_mon(Monitor *cur_mon, const char *fmt, ...)
+{
+    va_list ap;
+    int ret;
+
+    va_start(ap, fmt);
+    ret = error_vprintf_mon(cur_mon, fmt, ap);
+    va_end(ap);
+    return ret;
+}
+
+int error_vprintf(const char *fmt, va_list ap)
+{
+    return error_vprintf_mon(monitor_cur(), fmt, ap);
+}
+
 int error_printf(const char *fmt, ...)
 {
     va_list ap;
     int ret;
 
     va_start(ap, fmt);
-    ret = error_vprintf(fmt, ap);
+    ret = error_vprintf_mon(monitor_cur(), fmt, ap);
     va_end(ap);
     return ret;
 }
@@ -153,34 +173,34 @@ 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)
+static void print_loc(Monitor *cur)
 {
     const char *sep = "";
     int i;
     const char *const *argp;
 
-    if (!monitor_cur_is_hmp() && g_get_prgname()) {
-        error_printf("%s:", g_get_prgname());
+    if (!cur && g_get_prgname()) {
+        error_printf_mon(NULL, "%s:", g_get_prgname());
         sep = " ";
     }
     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]);
+            error_printf_mon(cur, "%s%s", sep, argp[i]);
             sep = " ";
         }
-        error_printf(": ");
+        error_printf_mon(cur, ": ");
         break;
     case LOC_FILE:
-        error_printf("%s:", (const char *)cur_loc->ptr);
+        error_printf_mon(cur, "%s:", (const char *)cur_loc->ptr);
         if (cur_loc->num) {
-            error_printf("%d:", cur_loc->num);
+            error_printf_mon(cur, "%d:", cur_loc->num);
         }
-        error_printf(" ");
+        error_printf_mon(cur, " ");
         break;
     default:
-        error_printf("%s", sep);
+        error_printf_mon(cur, "%s", sep);
     }
 }
 
@@ -201,34 +221,39 @@ real_time_iso8601(void)
 G_GNUC_PRINTF(2, 0)
 static void vreport(report_type type, const char *fmt, va_list ap)
 {
+    Monitor *cur = NULL;
     gchar *timestr;
 
-    if (message_with_timestamp && !monitor_cur_is_hmp()) {
+    if (monitor_cur_is_hmp()) {
+        cur = monitor_cur();
+    }
+
+    if (message_with_timestamp && !cur) {
         timestr = real_time_iso8601();
-        error_printf("%s ", timestr);
+        error_printf_mon(NULL, "%s ", timestr);
         g_free(timestr);
     }
 
     /* Only prepend guest name if -msg guest-name and -name guest=... are set */
-    if (error_with_guestname && error_guest_name && !monitor_cur_is_hmp()) {
-        error_printf("%s ", error_guest_name);
+    if (error_with_guestname && error_guest_name && !cur) {
+        error_printf_mon(NULL, "%s ", error_guest_name);
     }
 
-    print_loc();
+    print_loc(cur);
 
     switch (type) {
     case REPORT_TYPE_ERROR:
         break;
     case REPORT_TYPE_WARNING:
-        error_printf("warning: ");
+        error_printf_mon(cur, "warning: ");
         break;
     case REPORT_TYPE_INFO:
-        error_printf("info: ");
+        error_printf_mon(cur, "info: ");
         break;
     }
 
-    error_vprintf(fmt, ap);
-    error_printf("\n");
+    error_vprintf_mon(cur, fmt, ap);
+    error_printf_mon(cur, "\n");
 }
 
 /*
-- 
2.50.1

[PATCH v4 17/23] util: fix interleaving of error prefixes

[Daniel P. Berrangé]

The vreport() function will optionally emit an prefix for error
messages which is output to stderr incrementally. In the event
that two vreport() calls execute concurrently, there is a risk
that the prefix output will interleave. To address this it is
required to take a lock on 'stderr' when outputting errors.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/error-report.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/util/error-report.c b/util/error-report.c
index 222c40d0d1..b6aa20dbae 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -226,6 +226,8 @@ static void vreport(report_type type, const char *fmt, va_list ap)
 
     if (monitor_cur_is_hmp()) {
         cur = monitor_cur();
+    } else {
+        flockfile(stderr);
     }
 
     if (message_with_timestamp && !cur) {
@@ -254,6 +256,10 @@ static void vreport(report_type type, const char *fmt, va_list ap)
 
     error_vprintf_mon(cur, fmt, ap);
     error_printf_mon(cur, "\n");
+
+    if (!cur) {
+        funlockfile(stderr);
+    }
 }
 
 /*
-- 
2.50.1

[PATCH v4 18/23] util: introduce common helper for error-report & log code

[Daniel P. Berrangé]

The error-report and log code both have a need to add prefixes
to messages they are printing, with the current example being
a timestamp.

The format and configuration they use should be consistent, so
providing a common helper will ensure this is always the case.
Initially the helper only emits a timestamp, but future patches
will expand this.

This takes the liberty of assigning the new file to the same
maintainer as the existing error-report.c file, given it will
be extracting some functionality from the latter.

While vreport() dynamically changes between reporting to the
monitor vs stderr, depending on whether HMP is active or not,
message prefixes are only ever used in the non-HMP case. Thus
the helper API can take a FILE * object and not have to deal
with the monitor at all.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 MAINTAINERS            |  2 ++
 include/qemu/message.h | 28 ++++++++++++++++++++++++++++
 util/meson.build       |  1 +
 util/message.c         | 22 ++++++++++++++++++++++
 4 files changed, 53 insertions(+)
 create mode 100644 include/qemu/message.h
 create mode 100644 util/message.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 24b71a4fc5..e128a9d3be 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3173,9 +3173,11 @@ M: Markus Armbruster <armbru@redhat.com>
 S: Supported
 F: include/qapi/error.h
 F: include/qemu/error-report.h
+F: include/qemu/message.h
 F: qapi/error.json
 F: util/error.c
 F: util/error-report.c
+F: util/message.c
 F: scripts/coccinelle/err-bad-newline.cocci
 F: scripts/coccinelle/error-use-after-free.cocci
 F: scripts/coccinelle/error_propagate_null.cocci
diff --git a/include/qemu/message.h b/include/qemu/message.h
new file mode 100644
index 0000000000..0a06421f77
--- /dev/null
+++ b/include/qemu/message.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef QEMU_MESSAGE_H
+#define QEMU_MESSAGE_H
+
+enum QMessageFormatFlags {
+    QMESSAGE_FORMAT_TIMESTAMP = (1 << 0),
+};
+
+/**
+ * qmessage_set_format:
+ * @flags: the message information to emit
+ *
+ * Select which pieces of information to
+ * emit for messages
+ */
+void qmessage_set_format(int flags);
+
+/**
+ * qmessage_context_print:
+ * @fp: file to emit the prefix on
+ *
+ * Emit a message prefix with the information selected by
+ * an earlier call to qmessage_set_format.
+ */
+void qmessage_context_print(FILE *fp);
+
+#endif /* QEMU_MESSAGE_H */
diff --git a/util/meson.build b/util/meson.build
index 35029380a3..f5365e3b4f 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -40,6 +40,7 @@ util_ss.add(files('host-utils.c'))
 util_ss.add(files('bitmap.c', 'bitops.c'))
 util_ss.add(files('fifo8.c'))
 util_ss.add(files('cacheflush.c'))
+util_ss.add(files('message.c'))
 util_ss.add(files('error.c', 'error-report.c'))
 util_ss.add(files('qemu-print.c'))
 util_ss.add(files('id.c'))
diff --git a/util/message.c b/util/message.c
new file mode 100644
index 0000000000..643f10c06a
--- /dev/null
+++ b/util/message.c
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+
+#include "qemu/message.h"
+#include "monitor/monitor.h"
+
+static int message_format;
+
+void qmessage_set_format(int flags)
+{
+    message_format = flags;
+}
+
+void qmessage_context_print(FILE *fp)
+{
+    if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
+        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
+        g_autofree char *timestr = g_date_time_format_iso8601(dt);
+        fprintf(fp, "%s ", timestr);
+    }
+}
-- 
2.50.1

[PATCH v4 19/23] util: convert error-report & log to message API for timestamp

[Daniel P. Berrangé]

Both the error-report and log APIs can optional emit a timestamp
as a prefix on messages, with the '-msg timestamp=on' command
line flag is set.

Convert them to the new message API for formatting the context
prefix, guaranteeing they will have matching behaviour going
forward.

There is no change in output format for either logs or errors
with this conversion. There is a slight impact on logging code
in that recording the timestamp is now potentially delayed by
the need to acquire the logging mutex.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/error-report.h    |  1 -
 system/vl.c                    |  7 ++++++-
 tests/unit/test-error-report.c |  3 ++-
 util/error-report.c            | 18 ++----------------
 util/log.c                     | 21 ++++-----------------
 5 files changed, 14 insertions(+), 36 deletions(-)

diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index 3ae2357fda..c8000778ec 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -70,7 +70,6 @@ void error_init(const char *argv0);
                               fmt, ##__VA_ARGS__);      \
     })
 
-extern bool message_with_timestamp;
 extern bool error_with_guestname;
 extern const char *error_guest_name;
 
diff --git a/system/vl.c b/system/vl.c
index e0884c1bb9..12bfdb3431 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -26,6 +26,7 @@
 #include "qemu/help-texts.h"
 #include "qemu/datadir.h"
 #include "qemu/units.h"
+#include "qemu/message.h"
 #include "qemu/module.h"
 #include "qemu/target-info.h"
 #include "exec/cpu-common.h"
@@ -814,8 +815,12 @@ static void realtime_init(void)
 
 static void configure_msg(QemuOpts *opts)
 {
-    message_with_timestamp = qemu_opt_get_bool(opts, "timestamp", false);
+    int flags = 0;
+    if (qemu_opt_get_bool(opts, "timestamp", false)) {
+        flags |= QMESSAGE_FORMAT_TIMESTAMP;
+    }
     error_with_guestname = qemu_opt_get_bool(opts, "guest-name", false);
+    qmessage_set_format(flags);
 }
 
 
diff --git a/tests/unit/test-error-report.c b/tests/unit/test-error-report.c
index 54319c86c9..78f8b57660 100644
--- a/tests/unit/test-error-report.c
+++ b/tests/unit/test-error-report.c
@@ -13,6 +13,7 @@
 
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "qemu/message.h"
 
 static void
 test_error_report_simple(void)
@@ -90,7 +91,7 @@ static void
 test_error_report_timestamp(void)
 {
     if (g_test_subprocess()) {
-        message_with_timestamp = true;
+        qmessage_set_format(QMESSAGE_FORMAT_TIMESTAMP);
         warn_report("warn");
         error_report("err");
         return;
diff --git a/util/error-report.c b/util/error-report.c
index b6aa20dbae..c1146ba553 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "monitor/monitor.h"
 #include "qemu/error-report.h"
+#include "qemu/message.h"
 
 /*
  * @report_type is the type of message: error, warning or
@@ -24,8 +25,6 @@ typedef enum {
     REPORT_TYPE_INFO,
 } report_type;
 
-/* Prepend timestamp to messages */
-bool message_with_timestamp;
 bool error_with_guestname;
 const char *error_guest_name;
 
@@ -204,13 +203,6 @@ static void print_loc(Monitor *cur)
     }
 }
 
-static char *
-real_time_iso8601(void)
-{
-    g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
-    return g_date_time_format_iso8601(dt);
-}
-
 /*
  * Print a message to current monitor if we have one, else to stderr.
  * @report_type is the type of message: error, warning or informational.
@@ -222,18 +214,12 @@ G_GNUC_PRINTF(2, 0)
 static void vreport(report_type type, const char *fmt, va_list ap)
 {
     Monitor *cur = NULL;
-    gchar *timestr;
 
     if (monitor_cur_is_hmp()) {
         cur = monitor_cur();
     } else {
         flockfile(stderr);
-    }
-
-    if (message_with_timestamp && !cur) {
-        timestr = real_time_iso8601();
-        error_printf_mon(NULL, "%s ", timestr);
-        g_free(timestr);
+        qmessage_context_print(stderr);
     }
 
     /* Only prepend guest name if -msg guest-name and -name guest=... are set */
diff --git a/util/log.c b/util/log.c
index 2642a55c59..baa33ad8f1 100644
--- a/util/log.c
+++ b/util/log.c
@@ -22,6 +22,7 @@
 #include "qemu/range.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "qemu/message.h"
 #include "qemu/cutils.h"
 #include "trace/control.h"
 #include "qemu/thread.h"
@@ -151,26 +152,12 @@ static __thread bool incomplete;
 
 void qemu_log(const char *fmt, ...)
 {
-    FILE *f;
-    g_autofree const char *timestr = NULL;
-
-    /*
-     * Prepare the timestamp *outside* the logging
-     * lock so it better reflects when the message
-     * was emitted if we are delayed acquiring the
-     * mutex
-     */
-    if (message_with_timestamp && !incomplete) {
-        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
-        timestr = g_date_time_format_iso8601(dt);
-    }
-
-    f = qemu_log_trylock();
+    FILE *f = qemu_log_trylock();
     if (f) {
         va_list ap;
 
-        if (timestr) {
-            fprintf(f, "%s ", timestr);
+        if (!incomplete) {
+            qmessage_context_print(f);
         }
 
         va_start(ap, fmt);
-- 
2.50.1

[PATCH v4 20/23] util: add support for formatting a workload name in messages

[Daniel P. Berrangé]

The error_report function can include the guest name in any
messages it prints. The qemu_log function has no equivalent
behaviour.

This introduces support for a "workload name" in the new
messages API, which in the case of system emulators will
be the guest name. The possibility of defining a workload
name for other binaries is left as an exercise for the
future.

This change has no impact on the output of the error_report
function, but will change the qemu_log function. This can
be easily seen with the 'log' trace backend, and how it is
now more closely matching error_report output.

Before:

  # qemu-system-x86_64 -msg guest-name=on -name blah -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
  qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55b3af3fd870 dir=fish
  qcrypto_tls_creds_get_path TLS creds path creds=0x55b3af3fd870 filename=ca-cert.pem path=<none>
  blah qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory

After:

  # qemu-system-x86_64 -msg guest-name=on -name blah -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
  blah qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55b3af3fd870 dir=fish
  blah qcrypto_tls_creds_get_path TLS creds path creds=0x55b3af3fd870 filename=ca-cert.pem path=<none>
  blah qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/error-report.h |  3 ---
 include/qemu/message.h      | 10 ++++++++++
 system/vl.c                 |  6 ++++--
 util/error-report.c         |  7 -------
 util/message.c              | 11 +++++++++++
 5 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index c8000778ec..ffc305f828 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -70,7 +70,4 @@ void error_init(const char *argv0);
                               fmt, ##__VA_ARGS__);      \
     })
 
-extern bool error_with_guestname;
-extern const char *error_guest_name;
-
 #endif
diff --git a/include/qemu/message.h b/include/qemu/message.h
index 0a06421f77..cfc3c92648 100644
--- a/include/qemu/message.h
+++ b/include/qemu/message.h
@@ -5,6 +5,7 @@
 
 enum QMessageFormatFlags {
     QMESSAGE_FORMAT_TIMESTAMP = (1 << 0),
+    QMESSAGE_FORMAT_WORKLOAD_NAME = (1 << 1),
 };
 
 /**
@@ -16,6 +17,15 @@ enum QMessageFormatFlags {
  */
 void qmessage_set_format(int flags);
 
+/**
+ * qmessage_set_workload_name:
+ * @name: the name of the workload
+ *
+ * Set the workload name, which for a system emulator
+ * will be the guest VM name.
+ */
+void qmessage_set_workload_name(const char *name);
+
 /**
  * qmessage_context_print:
  * @fp: file to emit the prefix on
diff --git a/system/vl.c b/system/vl.c
index 12bfdb3431..56829880f6 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -819,7 +819,9 @@ static void configure_msg(QemuOpts *opts)
     if (qemu_opt_get_bool(opts, "timestamp", false)) {
         flags |= QMESSAGE_FORMAT_TIMESTAMP;
     }
-    error_with_guestname = qemu_opt_get_bool(opts, "guest-name", false);
+    if (qemu_opt_get_bool(opts, "guest-name", false)) {
+        flags |= QMESSAGE_FORMAT_WORKLOAD_NAME;
+    }
     qmessage_set_format(flags);
 }
 
@@ -3521,7 +3523,7 @@ void qemu_init(int argc, char **argv)
                     exit(1);
                 }
                 /* Capture guest name if -msg guest-name is used later */
-                error_guest_name = qemu_opt_get(opts, "guest");
+                qmessage_set_workload_name(qemu_opt_get(opts, "guest"));
                 break;
             case QEMU_OPTION_prom_env:
                 if (nb_prom_envs >= MAX_PROM_ENVS) {
diff --git a/util/error-report.c b/util/error-report.c
index c1146ba553..edb04585aa 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -25,8 +25,6 @@ typedef enum {
     REPORT_TYPE_INFO,
 } report_type;
 
-bool error_with_guestname;
-const char *error_guest_name;
 
 /*
  * Print to current monitor if we have one, else to stderr.
@@ -222,11 +220,6 @@ static void vreport(report_type type, const char *fmt, va_list ap)
         qmessage_context_print(stderr);
     }
 
-    /* Only prepend guest name if -msg guest-name and -name guest=... are set */
-    if (error_with_guestname && error_guest_name && !cur) {
-        error_printf_mon(NULL, "%s ", error_guest_name);
-    }
-
     print_loc(cur);
 
     switch (type) {
diff --git a/util/message.c b/util/message.c
index 643f10c06a..a91c008e08 100644
--- a/util/message.c
+++ b/util/message.c
@@ -6,12 +6,18 @@
 #include "monitor/monitor.h"
 
 static int message_format;
+static char *message_workloadname;
 
 void qmessage_set_format(int flags)
 {
     message_format = flags;
 }
 
+void qmessage_set_workload_name(const char *name)
+{
+    message_workloadname = g_strdup(name);
+}
+
 void qmessage_context_print(FILE *fp)
 {
     if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
@@ -19,4 +25,9 @@ void qmessage_context_print(FILE *fp)
         g_autofree char *timestr = g_date_time_format_iso8601(dt);
         fprintf(fp, "%s ", timestr);
     }
+
+    if ((message_format & QMESSAGE_FORMAT_WORKLOAD_NAME) &&
+        message_workloadname) {
+        fprintf(fp, "%s ", message_workloadname);
+    }
 }
-- 
2.50.1

[PATCH v4 21/23] util: add support for formatting a program name in messages

[Daniel P. Berrangé]

The error_report function can include the program name in any
messages it prints. The qemu_log function has no equivalent
behaviour.

This introduces support for a "program name" in the new
messages API, which will be included by default for all
binaries.

This change tweaks the output of the error_report function,
adding a space between the program name and the location
info. The qemu_log function will gain the program name. This
can be easily seen with the 'log' trace backend, and how it
is now more closely matching error_report output.

Before:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
  qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5584e13937f0 dir=fish
  qcrypto_tls_creds_get_path TLS creds path creds=0x5584e13937f0 filename=ca-cert.pem path=<none>
  qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory

After:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
  qemu-system-x86_64: qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5584e13937f0 dir=fish
  qemu-system-x86_64: qcrypto_tls_creds_get_path TLS creds path creds=0x5584e13937f0 filename=ca-cert.pem path=<none>
  qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/message.h         | 1 +
 system/vl.c                    | 2 +-
 tests/unit/test-error-report.c | 5 +++--
 util/error-report.c            | 4 ----
 util/message.c                 | 9 ++++++++-
 5 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/include/qemu/message.h b/include/qemu/message.h
index cfc3c92648..b8518d06be 100644
--- a/include/qemu/message.h
+++ b/include/qemu/message.h
@@ -6,6 +6,7 @@
 enum QMessageFormatFlags {
     QMESSAGE_FORMAT_TIMESTAMP = (1 << 0),
     QMESSAGE_FORMAT_WORKLOAD_NAME = (1 << 1),
+    QMESSAGE_FORMAT_PROGRAM_NAME = (1 << 2),
 };
 
 /**
diff --git a/system/vl.c b/system/vl.c
index 56829880f6..0ac1b11caf 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -815,7 +815,7 @@ static void realtime_init(void)
 
 static void configure_msg(QemuOpts *opts)
 {
-    int flags = 0;
+    int flags = QMESSAGE_FORMAT_PROGRAM_NAME;
     if (qemu_opt_get_bool(opts, "timestamp", false)) {
         flags |= QMESSAGE_FORMAT_TIMESTAMP;
     }
diff --git a/tests/unit/test-error-report.c b/tests/unit/test-error-report.c
index 78f8b57660..3987a8ed25 100644
--- a/tests/unit/test-error-report.c
+++ b/tests/unit/test-error-report.c
@@ -48,7 +48,7 @@ test_error_report_loc(void)
     g_test_trap_subprocess(NULL, 0, 0);
     g_test_trap_assert_passed();
     g_test_trap_assert_stderr("\
-test-error-report:some-file.c:7717: test error1*\
+test-error-report: some-file.c:7717: test error1*\
 test-error-report: test error2*\
 ");
 }
@@ -91,7 +91,8 @@ static void
 test_error_report_timestamp(void)
 {
     if (g_test_subprocess()) {
-        qmessage_set_format(QMESSAGE_FORMAT_TIMESTAMP);
+        qmessage_set_format(QMESSAGE_FORMAT_TIMESTAMP |
+                            QMESSAGE_FORMAT_PROGRAM_NAME);
         warn_report("warn");
         error_report("err");
         return;
diff --git a/util/error-report.c b/util/error-report.c
index edb04585aa..b010f97bda 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -176,10 +176,6 @@ static void print_loc(Monitor *cur)
     int i;
     const char *const *argp;
 
-    if (!cur && g_get_prgname()) {
-        error_printf_mon(NULL, "%s:", g_get_prgname());
-        sep = " ";
-    }
     switch (cur_loc->kind) {
     case LOC_CMDLINE:
         argp = cur_loc->ptr;
diff --git a/util/message.c b/util/message.c
index a91c008e08..1db9254733 100644
--- a/util/message.c
+++ b/util/message.c
@@ -5,7 +5,7 @@
 #include "qemu/message.h"
 #include "monitor/monitor.h"
 
-static int message_format;
+static int message_format = QMESSAGE_FORMAT_PROGRAM_NAME;
 static char *message_workloadname;
 
 void qmessage_set_format(int flags)
@@ -30,4 +30,11 @@ void qmessage_context_print(FILE *fp)
         message_workloadname) {
         fprintf(fp, "%s ", message_workloadname);
     }
+
+    if (message_format & QMESSAGE_FORMAT_PROGRAM_NAME) {
+        const char *pgnamestr = g_get_prgname();
+        if (pgnamestr) {
+            fprintf(fp, "%s: ", pgnamestr);
+        }
+    }
 }
-- 
2.50.1

[PATCH v4 22/23] util: add support for formatting thread info in messages

[Daniel P. Berrangé]

The message context is now extended to be able to include the
thread ID and thread name, after the program name. On Linux
the thread ID will match the process TID visible in /proc,
while on other platforms it will merely be an integer repr
of the system thread object address/ID.

This changes the output for both error_report and qemu_log,
when running under the system emulators or the QEMU storage
daemon. Other programs omit the thread information since
they are largely single threaded, though potentially it
would be useful to enable in all of them, given that the RCU
thread will always get spawned by a constructor function.

Before:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
  qemu-system-x86_64: qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x560db818e080 dir=fish
  qemu-system-x86_64: qcrypto_tls_creds_get_path TLS creds path creds=0x560db818e080 filename=ca-cert.pem path=<none>
  qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory

After:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
  qemu-system-x86_64: (772366:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x560db818e080 dir=fish
  qemu-system-x86_64: (772366:main): qcrypto_tls_creds_get_path TLS creds path creds=0x560db818e080 filename=ca-cert.pem path=<none>
  qemu-system-x86_64: (772366:main): Unable to access credentials fish/ca-cert.pem: No such file or directory

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/message.h               | 1 +
 storage-daemon/qemu-storage-daemon.c | 6 ++++++
 system/vl.c                          | 8 ++++++--
 tests/qemu-iotests/041               | 2 +-
 tests/qemu-iotests/common.filter     | 2 +-
 util/message.c                       | 7 +++++++
 6 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/qemu/message.h b/include/qemu/message.h
index b8518d06be..2cc092c993 100644
--- a/include/qemu/message.h
+++ b/include/qemu/message.h
@@ -7,6 +7,7 @@ enum QMessageFormatFlags {
     QMESSAGE_FORMAT_TIMESTAMP = (1 << 0),
     QMESSAGE_FORMAT_WORKLOAD_NAME = (1 << 1),
     QMESSAGE_FORMAT_PROGRAM_NAME = (1 << 2),
+    QMESSAGE_FORMAT_THREAD_INFO = (1 << 3),
 };
 
 /**
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index eb72561358..cc44ed7848 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -47,6 +47,7 @@
 #include "qemu/cutils.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include "qemu/message.h"
 #include "qemu/help_option.h"
 #include "qemu/job.h"
 #include "qemu/log.h"
@@ -65,6 +66,10 @@ static const char *pid_file;
 static char *pid_file_realpath;
 static volatile bool exit_requested = false;
 
+#define QMESSAGE_FORMAT_DEFAULT \
+    (QMESSAGE_FORMAT_PROGRAM_NAME | \
+     QMESSAGE_FORMAT_THREAD_INFO)
+
 void qemu_system_killed(int signal, pid_t pid)
 {
     exit_requested = true;
@@ -399,6 +404,7 @@ int main(int argc, char *argv[])
 #endif
 
     error_init(argv[0]);
+    qmessage_set_format(QMESSAGE_FORMAT_DEFAULT);
     qemu_init_exec_dir(argv[0]);
     os_setup_signal_handling();
 
diff --git a/system/vl.c b/system/vl.c
index 0ac1b11caf..fb8cc41876 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -145,6 +145,10 @@
 
 #define MAX_VIRTIO_CONSOLES 1
 
+#define QMESSAGE_FORMAT_DEFAULT \
+    (QMESSAGE_FORMAT_PROGRAM_NAME | \
+     QMESSAGE_FORMAT_THREAD_INFO)
+
 typedef struct BlockdevOptionsQueueEntry {
     BlockdevOptions *bdo;
     Location loc;
@@ -812,10 +816,9 @@ static void realtime_init(void)
     }
 }
 
-
 static void configure_msg(QemuOpts *opts)
 {
-    int flags = QMESSAGE_FORMAT_PROGRAM_NAME;
+    int flags = QMESSAGE_FORMAT_DEFAULT;
     if (qemu_opt_get_bool(opts, "timestamp", false)) {
         flags |= QMESSAGE_FORMAT_TIMESTAMP;
     }
@@ -2889,6 +2892,7 @@ void qemu_init(int argc, char **argv)
     module_call_init(MODULE_INIT_OPTS);
 
     error_init(argv[0]);
+    qmessage_set_format(QMESSAGE_FORMAT_DEFAULT);
     qemu_init_exec_dir(argv[0]);
 
     os_setup_limits();
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 8452845f44..dc2666afe7 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1102,7 +1102,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
         self.vm.shutdown()
         log = iotests.filter_qtest(self.vm.get_log())
         log = re.sub(r'^Formatting.*\n', '', log)
-        log = re.sub(r'^%s: ' % os.path.basename(iotests.qemu_prog), '', log)
+        log = re.sub(r'^%s: \(\d+:\w+\): ' % os.path.basename(iotests.qemu_prog), '', log)
 
         self.assertEqual(log,
                          "Can no longer replace 'img1' by 'repair0', because " +
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 511a55b1e8..84f352d30b 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -81,7 +81,7 @@ _filter_qemu_io()
 # replace occurrences of QEMU_PROG with "qemu"
 _filter_qemu()
 {
-    gsed -e "s#\\(^\\|(qemu) \\)$(basename $QEMU_PROG):#\1QEMU_PROG:#" \
+    gsed -e "s#\\(^\\|(qemu) \\)$(basename $QEMU_PROG): ([0-9]\+:[-_a-zA-Z]\+):#\1QEMU_PROG:#" \
         -e 's#^QEMU [0-9]\+\.[0-9]\+\.[0-9]\+ monitor#QEMU X.Y.Z monitor#' \
         -e $'s#\r##' # QEMU monitor uses \r\n line endings
 }
diff --git a/util/message.c b/util/message.c
index 1db9254733..814ca0316e 100644
--- a/util/message.c
+++ b/util/message.c
@@ -37,4 +37,11 @@ void qmessage_context_print(FILE *fp)
             fprintf(fp, "%s: ", pgnamestr);
         }
     }
+
+    if (message_format & QMESSAGE_FORMAT_THREAD_INFO) {
+        int thid = qemu_get_thread_id();
+        const char *thname = qemu_thread_get_name();
+
+        fprintf(fp, "(%d:%s): ", thid, thname);
+    }
 }
-- 
2.50.1

[PATCH v4 23/23] util: add brackets around guest name in message context

[Daniel P. Berrangé]

The message context can optionally include the guest name if the
argument '-msg guest-name=on' is given. The formatting, however,
does not look good if the guest name contains whitespace. Change
the output to include square brackets to demarcate the name.

Before:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*' -msg guest-name=on -name "fish food"
  fish food qemu-system-x86_64: (1146846:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55657e94e690 dir=fish
  fish food qemu-system-x86_64: (1146846:main): qcrypto_tls_creds_get_path TLS creds path creds=0x55657e94e690 filename=ca-cert.pem path=<none>
  fish food qemu-system-x86_64: (1146846:main): Unable to access credentials fish/ca-cert.pem: No such file or directory

After:

  # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*' -msg guest-name=on -name "fish food"
  [fish food] qemu-system-x86_64: (1144713:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5604ada2c950 dir=fish
  [fish food] qemu-system-x86_64: (1144713:main): qcrypto_tls_creds_get_path TLS creds path creds=0x5604ada2c950 filename=ca-cert.pem path=<none>
  [fish food] qemu-system-x86_64: (1144713:main): Unable to access credentials fish/ca-cert.pem: No such file or directory

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/message.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/message.c b/util/message.c
index 814ca0316e..7d94b9d970 100644
--- a/util/message.c
+++ b/util/message.c
@@ -28,7 +28,7 @@ void qmessage_context_print(FILE *fp)
 
     if ((message_format & QMESSAGE_FORMAT_WORKLOAD_NAME) &&
         message_workloadname) {
-        fprintf(fp, "%s ", message_workloadname);
+        fprintf(fp, "[%s] ", message_workloadname);
     }
 
     if (message_format & QMESSAGE_FORMAT_PROGRAM_NAME) {
-- 
2.50.1

Re: [PATCH v4 04/23] util: expose qemu_thread_set_name

[Philippe Mathieu-Daudé]

On 25/9/25 11:44, Daniel P. Berrangé wrote:
> The ability to set the thread name needs to be used in a number
> of places, so expose the current impls as public methods.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qemu/thread.h    |  1 +
>   util/qemu-thread-posix.c | 26 ++++++++++++++++----------
>   util/qemu-thread-win32.c | 13 ++++++++-----
>   3 files changed, 25 insertions(+), 15 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 17/23] util: fix interleaving of error prefixes

[Philippe Mathieu-Daudé]

On 25/9/25 11:44, Daniel P. Berrangé wrote:
> The vreport() function will optionally emit an prefix for error
> messages which is output to stderr incrementally. In the event
> that two vreport() calls execute concurrently, there is a risk
> that the prefix output will interleave. To address this it is
> required to take a lock on 'stderr' when outputting errors.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   util/error-report.c | 6 ++++++
>   1 file changed, 6 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 20/23] util: add support for formatting a workload name in messages

[Philippe Mathieu-Daudé]

On 25/9/25 11:44, Daniel P. Berrangé wrote:
> The error_report function can include the guest name in any
> messages it prints. The qemu_log function has no equivalent
> behaviour.
> 
> This introduces support for a "workload name" in the new
> messages API, which in the case of system emulators will
> be the guest name. The possibility of defining a workload
> name for other binaries is left as an exercise for the
> future.
> 
> This change has no impact on the output of the error_report
> function, but will change the qemu_log function. This can
> be easily seen with the 'log' trace backend, and how it is
> now more closely matching error_report output.
> 
> Before:
> 
>    # qemu-system-x86_64 -msg guest-name=on -name blah -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55b3af3fd870 dir=fish
>    qcrypto_tls_creds_get_path TLS creds path creds=0x55b3af3fd870 filename=ca-cert.pem path=<none>
>    blah qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> After:
> 
>    # qemu-system-x86_64 -msg guest-name=on -name blah -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    blah qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55b3af3fd870 dir=fish
>    blah qcrypto_tls_creds_get_path TLS creds path creds=0x55b3af3fd870 filename=ca-cert.pem path=<none>
>    blah qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qemu/error-report.h |  3 ---
>   include/qemu/message.h      | 10 ++++++++++
>   system/vl.c                 |  6 ++++--
>   util/error-report.c         |  7 -------
>   util/message.c              | 11 +++++++++++
>   5 files changed, 25 insertions(+), 12 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 21/23] util: add support for formatting a program name in messages

[Philippe Mathieu-Daudé]

On 25/9/25 11:44, Daniel P. Berrangé wrote:
> The error_report function can include the program name in any
> messages it prints. The qemu_log function has no equivalent
> behaviour.
> 
> This introduces support for a "program name" in the new
> messages API, which will be included by default for all
> binaries.
> 
> This change tweaks the output of the error_report function,
> adding a space between the program name and the location
> info. The qemu_log function will gain the program name. This
> can be easily seen with the 'log' trace backend, and how it
> is now more closely matching error_report output.
> 
> Before:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5584e13937f0 dir=fish
>    qcrypto_tls_creds_get_path TLS creds path creds=0x5584e13937f0 filename=ca-cert.pem path=<none>
>    qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> After:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    qemu-system-x86_64: qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5584e13937f0 dir=fish
>    qemu-system-x86_64: qcrypto_tls_creds_get_path TLS creds path creds=0x5584e13937f0 filename=ca-cert.pem path=<none>
>    qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qemu/message.h         | 1 +
>   system/vl.c                    | 2 +-
>   tests/unit/test-error-report.c | 5 +++--
>   util/error-report.c            | 4 ----
>   util/message.c                 | 9 ++++++++-
>   5 files changed, 13 insertions(+), 8 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 22/23] util: add support for formatting thread info in messages

[Philippe Mathieu-Daudé]

On 25/9/25 11:44, Daniel P. Berrangé wrote:
> The message context is now extended to be able to include the
> thread ID and thread name, after the program name. On Linux
> the thread ID will match the process TID visible in /proc,
> while on other platforms it will merely be an integer repr
> of the system thread object address/ID.
> 
> This changes the output for both error_report and qemu_log,
> when running under the system emulators or the QEMU storage
> daemon. Other programs omit the thread information since
> they are largely single threaded, though potentially it
> would be useful to enable in all of them, given that the RCU
> thread will always get spawned by a constructor function.
> 
> Before:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    qemu-system-x86_64: qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x560db818e080 dir=fish
>    qemu-system-x86_64: qcrypto_tls_creds_get_path TLS creds path creds=0x560db818e080 filename=ca-cert.pem path=<none>
>    qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> After:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    qemu-system-x86_64: (772366:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x560db818e080 dir=fish
>    qemu-system-x86_64: (772366:main): qcrypto_tls_creds_get_path TLS creds path creds=0x560db818e080 filename=ca-cert.pem path=<none>
>    qemu-system-x86_64: (772366:main): Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qemu/message.h               | 1 +
>   storage-daemon/qemu-storage-daemon.c | 6 ++++++
>   system/vl.c                          | 8 ++++++--
>   tests/qemu-iotests/041               | 2 +-
>   tests/qemu-iotests/common.filter     | 2 +-
>   util/message.c                       | 7 +++++++
>   6 files changed, 22 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 23/23] util: add brackets around guest name in message context

[Philippe Mathieu-Daudé]

On 25/9/25 11:44, Daniel P. Berrangé wrote:
> The message context can optionally include the guest name if the
> argument '-msg guest-name=on' is given. The formatting, however,
> does not look good if the guest name contains whitespace. Change
> the output to include square brackets to demarcate the name.
> 
> Before:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*' -msg guest-name=on -name "fish food"
>    fish food qemu-system-x86_64: (1146846:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55657e94e690 dir=fish
>    fish food qemu-system-x86_64: (1146846:main): qcrypto_tls_creds_get_path TLS creds path creds=0x55657e94e690 filename=ca-cert.pem path=<none>
>    fish food qemu-system-x86_64: (1146846:main): Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> After:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*' -msg guest-name=on -name "fish food"
>    [fish food] qemu-system-x86_64: (1144713:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5604ada2c950 dir=fish
>    [fish food] qemu-system-x86_64: (1144713:main): qcrypto_tls_creds_get_path TLS creds path creds=0x5604ada2c950 filename=ca-cert.pem path=<none>
>    [fish food] qemu-system-x86_64: (1144713:main): Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   util/message.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/message.c b/util/message.c
> index 814ca0316e..7d94b9d970 100644
> --- a/util/message.c
> +++ b/util/message.c
> @@ -28,7 +28,7 @@ void qmessage_context_print(FILE *fp)
>   
>       if ((message_format & QMESSAGE_FORMAT_WORKLOAD_NAME) &&
>           message_workloadname) {
> -        fprintf(fp, "%s ", message_workloadname);
> +        fprintf(fp, "[%s] ", message_workloadname);

Squash with patch #20 "util: add support for formatting a workload
name in messages"? Anyhow,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v4 00/23] util: sync error_report & qemu_log output more closely

[Philippe Mathieu-Daudé]

Hi Daniel,

On 25/9/25 11:44, Daniel P. Berrangé wrote:
> This series is a tangent that came out of discussion in
> 
>     https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00903.html
> 
> In thinking about adding thread info to error_report, I
> came to realize we should likely make qemu_log behave
> consistently with error_report & friends. We already
> honour '-msg timestamp=on', but don't honour 'guest-name=on'
> and also don't include the binary name.
> 
> As an example of the current state, consider mixing error and
> log output today:
> 
> - Default context:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
>                         -d 'trace:qcrypto*'
>    qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55ac6d97f700 dir=fish
>    qcrypto_tls_creds_get_path TLS creds path creds=0x55ac6d97f700 filename=ca-cert.pem path=<none>
>    qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> - Full context:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
>                         -d 'trace:qcrypto*' \
>                         -msg guest-name=on,timestamp=on \
>                         -name "fish food"
>    2025-08-19T20:14:16.791413Z qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55e9a3458d10 dir=fish
>    2025-08-19T20:14:16.791429Z qcrypto_tls_creds_get_path TLS creds path creds=0x55e9a3458d10 filename=ca-cert.pem path=<none>
>    2025-08-19T20:14:16.791433Z fish food qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> And after this series is complete:
> 
> - Default context:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
>                        -d 'trace:qcrypto*'
>    qemu-system-x86_64(1184284:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55a24ad5cb30 dir=fish
>    qemu-system-x86_64(1184284:main): qcrypto_tls_creds_get_path TLS creds path creds=0x55a24ad5cb30 filename=ca-cert.pem path=<none>
>    qemu-system-x86_64(1184284:main): Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> - Full context:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
>                        -d 'trace:qcrypto*' \
>                        -msg guest-name=on,timestamp=on \
>                        -name "fish food"
>    2025-08-19T20:12:50.211823Z [fish food] qemu-system-x86_64(1168876:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5582183d8760 dir=fish
>    2025-08-19T20:12:50.211842Z [fish food] qemu-system-x86_64(1168876:main): qcrypto_tls_creds_get_path TLS creds path creds=0x5582183d8760 filename=ca-cert.pem
> +path=<none>
>    2025-08-19T20:12:50.211846Z [fish food] qemu-system-x86_64(1168876:main): Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> The main things to note:
> 
>   * error_report/warn_report/qemu_log share the same
>     output format and -msg applies to both
> 
>   * -msg debug-threads=on is now unconditionally enabled
>     and thus the param is deprecated & ignored
> 
>   * Thread ID and name are unconditionally enabled
> 
>   * Guest name is surrounded in [...] brackets
> 
>   * The default output lines are typically 15 chars
>     wider given that we always include the thread
>     ID + name now
> 
>   * This takes the liberty of assigning the new file
>     to the existing error-report.c maintainer (Markus)
>     Since splitting it off into message.c instead of
>     putting it all in error-report.c felt slightly
>     nicer.
> 
> One thing I didn't tackle is making the location
> info get reported for qemu_log. This is used to
> give context for error messages when parsing some
> CLI args, and could be interesting for log messages
> associated with those same CLI args.

Testing with this change on top ...:

-- >8 --
diff --git a/include/qemu/message.h b/include/qemu/message.h
index 2cc092c993b..97fd2a94fbd 100644
--- a/include/qemu/message.h
+++ b/include/qemu/message.h
@@ -10,2 +10,3 @@ enum QMessageFormatFlags {
      QMESSAGE_FORMAT_THREAD_INFO = (1 << 3),
+    QMESSAGE_FORMAT_VCPU_ID = (1 << 4),
  };
diff --git a/system/vl.c b/system/vl.c
index bf9f80491f2..2f43a075be9 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -149,3 +149,4 @@
      (QMESSAGE_FORMAT_PROGRAM_NAME | \
-     QMESSAGE_FORMAT_THREAD_INFO)
+     QMESSAGE_FORMAT_THREAD_INFO | \
+     QMESSAGE_FORMAT_VCPU_ID)

diff --git a/util/message.c b/util/message.c
index 7d94b9d970d..19d1331403a 100644
--- a/util/message.c
+++ b/util/message.c
@@ -6,2 +6,3 @@
  #include "monitor/monitor.h"
+#include "hw/core/cpu.h"

@@ -46,2 +47,8 @@ void qmessage_context_print(FILE *fp)
      }
+
+    if (message_format & QMESSAGE_FORMAT_VCPU_ID) {
+        if (current_cpu) {
+            fprintf(fp, "(cpuid:%d): ", current_cpu->cpu_index);
+        }
+    }
  }
---

... on a branch where I test heavy vcpu context switching.

When using tracing + stderr AND the stdio console, I sometimes get
the trace events to disappear at some point. Not sure why (or if related
to my branch) yet.

When redirecting traces to a file ('-D foo.log'), all traces are
collected (no hang).

In both cases I notice a high performance slow down (when tracing is
used).

Suspicions:
- flocking
- new formatting

Regards,

Phil.

Re: [PATCH v4 00/23] util: sync error_report & qemu_log output more closely

[Daniel P. Berrangé]

On Thu, Sep 25, 2025 at 12:22:39PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
> 
> On 25/9/25 11:44, Daniel P. Berrangé wrote:
> > This series is a tangent that came out of discussion in
> > 
> >     https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00903.html
> > 
> > In thinking about adding thread info to error_report, I
> > came to realize we should likely make qemu_log behave
> > consistently with error_report & friends. We already
> > honour '-msg timestamp=on', but don't honour 'guest-name=on'
> > and also don't include the binary name.
> > 
> > As an example of the current state, consider mixing error and
> > log output today:
> > 
> > - Default context:
> > 
> >    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
> >                         -d 'trace:qcrypto*'
> >    qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55ac6d97f700 dir=fish
> >    qcrypto_tls_creds_get_path TLS creds path creds=0x55ac6d97f700 filename=ca-cert.pem path=<none>
> >    qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> > 
> > - Full context:
> > 
> >    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
> >                         -d 'trace:qcrypto*' \
> >                         -msg guest-name=on,timestamp=on \
> >                         -name "fish food"
> >    2025-08-19T20:14:16.791413Z qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55e9a3458d10 dir=fish
> >    2025-08-19T20:14:16.791429Z qcrypto_tls_creds_get_path TLS creds path creds=0x55e9a3458d10 filename=ca-cert.pem path=<none>
> >    2025-08-19T20:14:16.791433Z fish food qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> > 
> > And after this series is complete:
> > 
> > - Default context:
> > 
> >    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
> >                        -d 'trace:qcrypto*'
> >    qemu-system-x86_64(1184284:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55a24ad5cb30 dir=fish
> >    qemu-system-x86_64(1184284:main): qcrypto_tls_creds_get_path TLS creds path creds=0x55a24ad5cb30 filename=ca-cert.pem path=<none>
> >    qemu-system-x86_64(1184284:main): Unable to access credentials fish/ca-cert.pem: No such file or directory
> > 
> > - Full context:
> > 
> >    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish \
> >                        -d 'trace:qcrypto*' \
> >                        -msg guest-name=on,timestamp=on \
> >                        -name "fish food"
> >    2025-08-19T20:12:50.211823Z [fish food] qemu-system-x86_64(1168876:main): qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5582183d8760 dir=fish
> >    2025-08-19T20:12:50.211842Z [fish food] qemu-system-x86_64(1168876:main): qcrypto_tls_creds_get_path TLS creds path creds=0x5582183d8760 filename=ca-cert.pem
> > +path=<none>
> >    2025-08-19T20:12:50.211846Z [fish food] qemu-system-x86_64(1168876:main): Unable to access credentials fish/ca-cert.pem: No such file or directory
> > 
> > The main things to note:
> > 
> >   * error_report/warn_report/qemu_log share the same
> >     output format and -msg applies to both
> > 
> >   * -msg debug-threads=on is now unconditionally enabled
> >     and thus the param is deprecated & ignored
> > 
> >   * Thread ID and name are unconditionally enabled
> > 
> >   * Guest name is surrounded in [...] brackets
> > 
> >   * The default output lines are typically 15 chars
> >     wider given that we always include the thread
> >     ID + name now
> > 
> >   * This takes the liberty of assigning the new file
> >     to the existing error-report.c maintainer (Markus)
> >     Since splitting it off into message.c instead of
> >     putting it all in error-report.c felt slightly
> >     nicer.
> > 
> > One thing I didn't tackle is making the location
> > info get reported for qemu_log. This is used to
> > give context for error messages when parsing some
> > CLI args, and could be interesting for log messages
> > associated with those same CLI args.
> 
> Testing with this change on top ...:
> 
> -- >8 --
> diff --git a/include/qemu/message.h b/include/qemu/message.h
> index 2cc092c993b..97fd2a94fbd 100644
> --- a/include/qemu/message.h
> +++ b/include/qemu/message.h
> @@ -10,2 +10,3 @@ enum QMessageFormatFlags {
>      QMESSAGE_FORMAT_THREAD_INFO = (1 << 3),
> +    QMESSAGE_FORMAT_VCPU_ID = (1 << 4),
>  };
> diff --git a/system/vl.c b/system/vl.c
> index bf9f80491f2..2f43a075be9 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -149,3 +149,4 @@
>      (QMESSAGE_FORMAT_PROGRAM_NAME | \
> -     QMESSAGE_FORMAT_THREAD_INFO)
> +     QMESSAGE_FORMAT_THREAD_INFO | \
> +     QMESSAGE_FORMAT_VCPU_ID)
> 
> diff --git a/util/message.c b/util/message.c
> index 7d94b9d970d..19d1331403a 100644
> --- a/util/message.c
> +++ b/util/message.c
> @@ -6,2 +6,3 @@
>  #include "monitor/monitor.h"
> +#include "hw/core/cpu.h"
> 
> @@ -46,2 +47,8 @@ void qmessage_context_print(FILE *fp)
>      }
> +
> +    if (message_format & QMESSAGE_FORMAT_VCPU_ID) {
> +        if (current_cpu) {
> +            fprintf(fp, "(cpuid:%d): ", current_cpu->cpu_index);
> +        }
> +    }
>  }
> ---
> 
> ... on a branch where I test heavy vcpu context switching.
> 
> When using tracing + stderr AND the stdio console, I sometimes get
> the trace events to disappear at some point. Not sure why (or if related
> to my branch) yet.
> 
> When redirecting traces to a file ('-D foo.log'), all traces are
> collected (no hang).
> 
> In both cases I notice a high performance slow down (when tracing is
> used).
> 
> Suspicions:
> - flocking

The logging code (and thus tracing) already used flockfile in
qemu_log, so that should not be new. Only error_report gained
new flockfile calls in this series.

> - new formatting

Seems the more likely option.  If practical, a git bisect across the
series should show which part introduced any slowdown.

Assuming you've not given any -msg option to QEMU though, the finger
probably points towards the thread name code, as the bit that is
new and enabled by default.

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 :|

Re: [PATCH v4 00/23] util: sync error_report & qemu_log output more closely

[Philippe Mathieu-Daudé]

On 25/9/25 12:31, Daniel P. Berrangé wrote:
> On Thu, Sep 25, 2025 at 12:22:39PM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Daniel,
>>
>> On 25/9/25 11:44, Daniel P. Berrangé wrote:
>>> This series is a tangent that came out of discussion in
>>>
>>>      https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg00903.html
>>>
>>> In thinking about adding thread info to error_report, I
>>> came to realize we should likely make qemu_log behave
>>> consistently with error_report & friends. We already
>>> honour '-msg timestamp=on', but don't honour 'guest-name=on'
>>> and also don't include the binary name.


>>> The main things to note:
>>>
>>>    * error_report/warn_report/qemu_log share the same
>>>      output format and -msg applies to both
>>>
>>>    * -msg debug-threads=on is now unconditionally enabled
>>>      and thus the param is deprecated & ignored
>>>
>>>    * Thread ID and name are unconditionally enabled
>>>
>>>    * Guest name is surrounded in [...] brackets
>>>
>>>    * The default output lines are typically 15 chars
>>>      wider given that we always include the thread
>>>      ID + name now
>>>
>>>    * This takes the liberty of assigning the new file
>>>      to the existing error-report.c maintainer (Markus)
>>>      Since splitting it off into message.c instead of
>>>      putting it all in error-report.c felt slightly
>>>      nicer.
>>>
>>> One thing I didn't tackle is making the location
>>> info get reported for qemu_log. This is used to
>>> give context for error messages when parsing some
>>> CLI args, and could be interesting for log messages
>>> associated with those same CLI args.
>>
>> Testing with this change on top ...:
>>
>> -- >8 --
>> diff --git a/include/qemu/message.h b/include/qemu/message.h
>> index 2cc092c993b..97fd2a94fbd 100644
>> --- a/include/qemu/message.h
>> +++ b/include/qemu/message.h
>> @@ -10,2 +10,3 @@ enum QMessageFormatFlags {
>>       QMESSAGE_FORMAT_THREAD_INFO = (1 << 3),
>> +    QMESSAGE_FORMAT_VCPU_ID = (1 << 4),
>>   };
>> diff --git a/system/vl.c b/system/vl.c
>> index bf9f80491f2..2f43a075be9 100644
>> --- a/system/vl.c
>> +++ b/system/vl.c
>> @@ -149,3 +149,4 @@
>>       (QMESSAGE_FORMAT_PROGRAM_NAME | \
>> -     QMESSAGE_FORMAT_THREAD_INFO)
>> +     QMESSAGE_FORMAT_THREAD_INFO | \
>> +     QMESSAGE_FORMAT_VCPU_ID)
>>
>> diff --git a/util/message.c b/util/message.c
>> index 7d94b9d970d..19d1331403a 100644
>> --- a/util/message.c
>> +++ b/util/message.c
>> @@ -6,2 +6,3 @@
>>   #include "monitor/monitor.h"
>> +#include "hw/core/cpu.h"
>>
>> @@ -46,2 +47,8 @@ void qmessage_context_print(FILE *fp)
>>       }
>> +
>> +    if (message_format & QMESSAGE_FORMAT_VCPU_ID) {
>> +        if (current_cpu) {
>> +            fprintf(fp, "(cpuid:%d): ", current_cpu->cpu_index);
>> +        }
>> +    }
>>   }
>> ---
>>
>> ... on a branch where I test heavy vcpu context switching.
>>
>> When using tracing + stderr AND the stdio console, I sometimes get
>> the trace events to disappear at some point. Not sure why (or if related
>> to my branch) yet.
>>
>> When redirecting traces to a file ('-D foo.log'), all traces are
>> collected (no hang).
>>
>> In both cases I notice a high performance slow down (when tracing is
>> used).
>>
>> Suspicions:
>> - flocking
> 
> The logging code (and thus tracing) already used flockfile in
> qemu_log, so that should not be new. Only error_report gained
> new flockfile calls in this series.
> 
>> - new formatting
> 
> Seems the more likely option.  If practical, a git bisect across the
> series should show which part introduced any slowdown.

This is planned.

> Assuming you've not given any -msg option to QEMU though, the finger

Indeed.

> probably points towards the thread name code, as the bit that is
> new and enabled by default.

I thought about that then noticed the thread name is const (preformatted
once in each qemu_thread_get_name() implementation).

> 
> With regards,
> Daniel

Re: [PATCH v4 21/23] util: add support for formatting a program name in messages

[Philippe Mathieu-Daudé]

On 25/9/25 11:44, Daniel P. Berrangé wrote:
> The error_report function can include the program name in any
> messages it prints. The qemu_log function has no equivalent
> behaviour.
> 
> This introduces support for a "program name" in the new
> messages API, which will be included by default for all
> binaries.
> 
> This change tweaks the output of the error_report function,
> adding a space between the program name and the location
> info. The qemu_log function will gain the program name. This
> can be easily seen with the 'log' trace backend, and how it
> is now more closely matching error_report output.
> 
> Before:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5584e13937f0 dir=fish
>    qcrypto_tls_creds_get_path TLS creds path creds=0x5584e13937f0 filename=ca-cert.pem path=<none>
>    qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> After:
> 
>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    qemu-system-x86_64: qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5584e13937f0 dir=fish
>    qemu-system-x86_64: qcrypto_tls_creds_get_path TLS creds path creds=0x5584e13937f0 filename=ca-cert.pem path=<none>
>    qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qemu/message.h         | 1 +
>   system/vl.c                    | 2 +-
>   tests/unit/test-error-report.c | 5 +++--
>   util/error-report.c            | 4 ----
>   util/message.c                 | 9 ++++++++-
>   5 files changed, 13 insertions(+), 8 deletions(-)


> diff --git a/util/message.c b/util/message.c
> index a91c008e08..1db9254733 100644
> --- a/util/message.c
> +++ b/util/message.c
> @@ -5,7 +5,7 @@
>   #include "qemu/message.h"
>   #include "monitor/monitor.h"
>   
> -static int message_format;
> +static int message_format = QMESSAGE_FORMAT_PROGRAM_NAME;
>   static char *message_workloadname;
>   
>   void qmessage_set_format(int flags)
> @@ -30,4 +30,11 @@ void qmessage_context_print(FILE *fp)
>           message_workloadname) {
>           fprintf(fp, "%s ", message_workloadname);
>       }
> +
> +    if (message_format & QMESSAGE_FORMAT_PROGRAM_NAME) {
> +        const char *pgnamestr = g_get_prgname();
> +        if (pgnamestr) {
> +            fprintf(fp, "%s: ", pgnamestr);

No formatting needed, use fputs()?

> +        }
> +    }
>   }

Re: [PATCH v4 20/23] util: add support for formatting a workload name in messages

[Philippe Mathieu-Daudé]

On 25/9/25 11:44, Daniel P. Berrangé wrote:
> The error_report function can include the guest name in any
> messages it prints. The qemu_log function has no equivalent
> behaviour.
> 
> This introduces support for a "workload name" in the new
> messages API, which in the case of system emulators will
> be the guest name. The possibility of defining a workload
> name for other binaries is left as an exercise for the
> future.
> 
> This change has no impact on the output of the error_report
> function, but will change the qemu_log function. This can
> be easily seen with the 'log' trace backend, and how it is
> now more closely matching error_report output.
> 
> Before:
> 
>    # qemu-system-x86_64 -msg guest-name=on -name blah -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55b3af3fd870 dir=fish
>    qcrypto_tls_creds_get_path TLS creds path creds=0x55b3af3fd870 filename=ca-cert.pem path=<none>
>    blah qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> After:
> 
>    # qemu-system-x86_64 -msg guest-name=on -name blah -object tls-creds-x509,id=t0,dir=fish -d 'trace:qcrypto*'
>    blah qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55b3af3fd870 dir=fish
>    blah qcrypto_tls_creds_get_path TLS creds path creds=0x55b3af3fd870 filename=ca-cert.pem path=<none>
>    blah qemu-system-x86_64: Unable to access credentials fish/ca-cert.pem: No such file or directory
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/qemu/error-report.h |  3 ---
>   include/qemu/message.h      | 10 ++++++++++
>   system/vl.c                 |  6 ++++--
>   util/error-report.c         |  7 -------
>   util/message.c              | 11 +++++++++++
>   5 files changed, 25 insertions(+), 12 deletions(-)


>   void qmessage_context_print(FILE *fp)
>   {
>       if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
> @@ -19,4 +25,9 @@ void qmessage_context_print(FILE *fp)
>           g_autofree char *timestr = g_date_time_format_iso8601(dt);
>           fprintf(fp, "%s ", timestr);
>       }
> +
> +    if ((message_format & QMESSAGE_FORMAT_WORKLOAD_NAME) &&
> +        message_workloadname) {
> +        fprintf(fp, "%s ", message_workloadname);

Here also no formatting used; use fputs + fputc?

> +    }
>   }

Re: [PATCH v4 23/23] util: add brackets around guest name in message context

[Philippe Mathieu-Daudé]

On 25/9/25 12:01, Philippe Mathieu-Daudé wrote:
> On 25/9/25 11:44, Daniel P. Berrangé wrote:
>> The message context can optionally include the guest name if the
>> argument '-msg guest-name=on' is given. The formatting, however,
>> does not look good if the guest name contains whitespace. Change
>> the output to include square brackets to demarcate the name.
>>
>> Before:
>>
>>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 
>> 'trace:qcrypto*' -msg guest-name=on -name "fish food"
>>    fish food qemu-system-x86_64: (1146846:main): 
>> qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x55657e94e690 
>> dir=fish
>>    fish food qemu-system-x86_64: (1146846:main): 
>> qcrypto_tls_creds_get_path TLS creds path creds=0x55657e94e690 
>> filename=ca-cert.pem path=<none>
>>    fish food qemu-system-x86_64: (1146846:main): Unable to access 
>> credentials fish/ca-cert.pem: No such file or directory
>>
>> After:
>>
>>    # qemu-system-x86_64 -object tls-creds-x509,id=t0,dir=fish -d 
>> 'trace:qcrypto*' -msg guest-name=on -name "fish food"
>>    [fish food] qemu-system-x86_64: (1144713:main): 
>> qcrypto_tls_creds_x509_load TLS creds x509 load creds=0x5604ada2c950 
>> dir=fish
>>    [fish food] qemu-system-x86_64: (1144713:main): 
>> qcrypto_tls_creds_get_path TLS creds path creds=0x5604ada2c950 
>> filename=ca-cert.pem path=<none>
>>    [fish food] qemu-system-x86_64: (1144713:main): Unable to access 
>> credentials fish/ca-cert.pem: No such file or directory
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   util/message.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/util/message.c b/util/message.c
>> index 814ca0316e..7d94b9d970 100644
>> --- a/util/message.c
>> +++ b/util/message.c
>> @@ -28,7 +28,7 @@ void qmessage_context_print(FILE *fp)
>>       if ((message_format & QMESSAGE_FORMAT_WORKLOAD_NAME) &&
>>           message_workloadname) {
>> -        fprintf(fp, "%s ", message_workloadname);
>> +        fprintf(fp, "[%s] ", message_workloadname);

No formatting -> fputs?

> 
> Squash with patch #20 "util: add support for formatting a workload
> name in messages"? Anyhow,
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

Re: [PATCH v4 14/23] monitor: introduce monitor_cur_is_hmp() helper

[Richard Henderson]

On 9/25/25 02:44, Daniel P. Berrangé wrote:
> Note that this is not simply the inverse of monitor_cur_is_qmp(),
> as both helpers require that monitor_cur() is first non-NULL.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   include/monitor/monitor.h      |  1 +
>   monitor/monitor.c              | 10 ++++++++++
>   stubs/monitor-core.c           |  6 ++++++
>   tests/unit/test-util-sockets.c |  1 +
>   4 files changed, 18 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v4 07/23] util: add API to fetch the current thread name

[Eric Blake]

On Thu, Sep 25, 2025 at 10:44:25AM +0100, Daniel P. Berrangé wrote:
> This will be used to include the thread name in error reports
> in a later patch. It returns a const string stored in a thread
> local to avoid memory allocation when it is called repeatedly
> in a single thread. The thread name should be set at the very
> start of the thread execution, which is the case when using
> qemu_thread_create.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  include/qemu/thread.h    |  1 +
>  meson.build              | 21 +++++++++++++++++
>  util/qemu-thread-posix.c | 33 ++++++++++++++++++++++++++-
>  util/qemu-thread-win32.c | 49 ++++++++++++++++++++++++++++++++++++----
>  4 files changed, 99 insertions(+), 5 deletions(-)
> 

> +++ b/util/qemu-thread-posix.c

> +const char *qemu_thread_get_name(void)

> +# endif
> +    if (rv != 0) {
> +        strlcpy(namebuf, "unnamed", G_N_ELEMENTS(namebuf));

Should this be g_strlcpy() instead of strlcpy(), for consistency with
the Windows half of the patch, and since g_strlcpy() is much more
frequent in the QEMU codebase?

> +++ b/util/qemu-thread-win32.c

> @@ -38,14 +41,17 @@ static bool load_set_thread_description(void)
>              SetThreadDescriptionFunc =
>                  (pSetThreadDescription)GetProcAddress(kernel32_module,
>                                                        "SetThreadDescription");
> -            if (!SetThreadDescriptionFunc) {
> +            GetThreadDescriptionFunc =
> +                (pGetThreadDescription)GetProcAddress(kernel32_module,
> +                                                      "GetThreadDescription");
> +            if (!SetThreadDescriptionFunc || !GetThreadDescriptionFunc) {
>                  FreeLibrary(kernel32_module);
>              }
>          }
>          g_once_init_leave(&_init_once, 1);
>      }
>  
> -    return !!SetThreadDescriptionFunc;
> +    return !!(SetThreadDescriptionFunc && GetThreadDescriptionFunc);

Pre-patch, the !! was necessary for converting a pointer to a bool.
But && also converts to a bool, and "!!(bool)" is redundant compared
to just "bool".

But overall, the patch makes sense.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v4 09/23] util: introduce some API docs for logging APIs

[Eric Blake]

On Thu, Sep 25, 2025 at 10:44:27AM +0100, Daniel P. Berrangé wrote:
> This addresses two key gotchas with the logging APIs:
> 
>  * Safely outputting a single line of text using
>    multiple qemu_log() calls requires use of the
>    qemu_log_trylock/unlock functions to avoid
>    interleaving between threads
> 
>  * Directly outputting to the FILE object returned
>    by qemu_log_trylock() must be discouraged because
>    it prevents the inclusion of configured log message
>    prefixes.
> 
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---

> +++ b/include/qemu/log-for-trace.h
> @@ -29,7 +29,40 @@ static inline bool qemu_loglevel_mask(int mask)
>      return (qemu_loglevel & mask) != 0;
>  }
>  
> -/* main logging function */
> +/**
> + * qemu_log: report a log message

> + *
> + * The FILE object returned by qemu_log_trylock() does
> + * not need to be used for outputting text directly,
> + * it is merely used to associate the lock.
> + *
> + *    FILE *f = qemu_log_trylock()
> + *
> + *    qemu_log("Something");
> + *    qemu_log("Something");
> + *    qemu_log("Something");
> + *    qemu_log("The end\n");
> + *
> + *    qemu_log_unlock(f);

Should this example include spaces, as in qemu_log("Something ");, so
that copy-and-paste coding is less likely to
ResultInCompressedSentences across the log calls?

Adjusting a comment is trivial, so:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v4 13/23] monitor: move error_vprintf back to error-report.c

[Eric Blake]

On Thu, Sep 25, 2025 at 10:44:31AM +0100, Daniel P. Berrangé wrote:
> The current unit tests rely on monitor.o not being linked, such
> that the monitor stubs get linked instead. Since error_vprintf
> is in monitor.o this allows a stub error_vprintf impl to be used
> that calls g_test_message.
> 
> This takes a different approach, with error_vprintf moving
> back to error-report.c such that it is always linked into the
> tests. The monitor_vprintf() stub is then changed to use
> g_test_message if QTEST_SLENT_ERROS is set, otherwise it will

Double typo; should be QTEST_SILENT_ERRORS...

> return -1 and trigger error_vprintf to call vfprintf.
> 
> The end result is functionally equivalent for the purposes of
> the unit tests.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---

> +++ b/stubs/monitor-core.c
> @@ -18,5 +18,17 @@ void qapi_event_emit(QAPIEvent event, QDict *qdict)
>  
>  int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
>  {
> -    abort();
> +    /*
> +     * Pretend 'g_test_message' is our monitor console to
> +     * stop the caller sending messages to stderr
> +     */
> +    if (g_test_initialized() && !g_test_subprocess() &&
> +        getenv("QTEST_SILENT_ERRORS")) {

...to match the code.

With that fixed,

Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org