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

[PATCH 00/12] 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 more generally. We already honour
'-msg timestamp=on' in both (with duplicated code), and qemu_log
doesn't honour the '-msg guest-name=on' parameter, nor will it
include the binary name.

This series shouldn't be considered mutually exclusive with
Manos' backtrace suggestion above - it is complementary &
each proposal can be evaluated independently. 


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.

I'm not too happy with the constructor priority
logic, but I don't see a better option, as I don't
think it is practical to order .o files passed to
the linker in a way that nicely guarantees the order
of constructor execution.

Daniel P. Berrangé (12):
  include: define constant for early constructor priority
  monitor: initialize global data from a constructor
  system: unconditionally enable thread naming
  util: set the name for the 'main' thread
  util: add API to fetch the current thread id
  util: add API to fetch the current thread name
  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 +
 docs/about/deprecated.rst            |  7 +++
 include/qemu/compiler.h              |  8 +++
 include/qemu/error-report.h          |  4 --
 include/qemu/message.h               | 52 +++++++++++++++++
 include/qemu/thread.h                |  3 +-
 meson.build                          | 21 +++++++
 monitor/monitor.c                    | 14 +++--
 storage-daemon/qemu-storage-daemon.c |  6 ++
 system/vl.c                          | 30 +++++++---
 tests/qemu-iotests/041               |  2 +-
 tests/qemu-iotests/common.filter     |  2 +-
 tests/unit/test-error-report.c       |  6 +-
 util/error-report.c                  | 29 +---------
 util/log.c                           | 20 +++----
 util/meson.build                     |  1 +
 util/message.c                       | 69 ++++++++++++++++++++++
 util/qemu-thread-posix.c             | 60 ++++++++++++++-----
 util/qemu-thread-win32.c             | 86 ++++++++++++++++++++--------
 19 files changed, 321 insertions(+), 101 deletions(-)
 create mode 100644 include/qemu/message.h
 create mode 100644 util/message.c

-- 
2.50.1

[PATCH 01/12] 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.

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 02/12] 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.

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 03/12] 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.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/about/deprecated.rst |  7 +++++++
 include/qemu/thread.h     |  1 -
 system/vl.c               | 11 ++++++-----
 util/qemu-thread-posix.c  | 18 +-----------------
 util/qemu-thread-win32.c  | 27 ++++++---------------------
 5 files changed, 20 insertions(+), 44 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index d50645a071..bd6f865558 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -81,6 +81,13 @@ kernel since 2001. None of the board types QEMU supports need
 ``param_struct`` support, so this option has been deprecated and will
 be removed in a future QEMU version.
 
+``debug-threads`` option for ``-name``
+''''''''''''''''''''''''''''''''''''''
+
+Thread ``debug-threads`` option for the ``-name`` argument is now
+ignored. Thread naming is unconditionally enabled for all platforms
+where it is supported.
+
 User-mode emulator command line arguments
 -----------------------------------------
 
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 3b7057e6c6..a64fd90d4a 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -403,9 +403,7 @@ 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 = "DEPRECATED: thread names are always set where supported",
         },
         { /* End of list */ }
     },
@@ -554,9 +552,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

Re: [PATCH 03/12] system: unconditionally enable thread naming

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> 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.

Ah finally!

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

> 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.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  docs/about/deprecated.rst |  7 +++++++
>  include/qemu/thread.h     |  1 -
>  system/vl.c               | 11 ++++++-----
>  util/qemu-thread-posix.c  | 18 +-----------------
>  util/qemu-thread-win32.c  | 27 ++++++---------------------
>  5 files changed, 20 insertions(+), 44 deletions(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index d50645a071..bd6f865558 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -81,6 +81,13 @@ kernel since 2001. None of the board types QEMU supports need
>  ``param_struct`` support, so this option has been deprecated and will
>  be removed in a future QEMU version.
>  
> +``debug-threads`` option for ``-name``
> +''''''''''''''''''''''''''''''''''''''
> +
> +Thread ``debug-threads`` option for the ``-name`` argument is now
> +ignored. Thread naming is unconditionally enabled for all platforms
> +where it is supported.
> +
>  User-mode emulator command line arguments
>  -----------------------------------------
>  
> 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 3b7057e6c6..a64fd90d4a 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -403,9 +403,7 @@ 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 = "DEPRECATED: thread names are always set where supported",
>          },
>          { /* End of list */ }
>      },
> @@ -554,9 +552,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
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

[PATCH 04/12] 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.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 util/qemu-thread-posix.c | 12 ++++++++++++
 util/qemu-thread-win32.c |  6 ++++++
 2 files changed, 18 insertions(+)

diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 7c985b5d38..121d7ed69b 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -22,6 +22,18 @@
 #include <pthread_np.h>
 #endif
 
+static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
+qemu_thread_init(void)
+{
+# if defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
+    pthread_setname_np(pthread_self(), "main");
+# elif defined(CONFIG_PTHREAD_SETNAME_NP_WO_TID)
+    pthread_setname_np("main");
+# elif defined(CONFIG_PTHREAD_SET_NAME_NP)
+    pthread_set_name_np(pthread_self(), "main");
+# endif
+}
+
 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 9595a5b090..5e6ca0c12f 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -332,6 +332,12 @@ static void set_thread_description(HANDLE h, const char *name)
     SetThreadDescriptionFunc(h, namew);
 }
 
+static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
+qemu_thread_init(void)
+{
+    set_thread_description(GetCurrentThread(), "main");
+}
+
 void qemu_thread_create(QemuThread *thread, const char *name,
                        void *(*start_routine)(void *),
                        void *arg, int mode)
-- 
2.50.1

Re: [PATCH 04/12] util: set the name for the 'main' thread

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> 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.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  util/qemu-thread-posix.c | 12 ++++++++++++
>  util/qemu-thread-win32.c |  6 ++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 7c985b5d38..121d7ed69b 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -22,6 +22,18 @@
>  #include <pthread_np.h>
>  #endif
>  
> +static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
> +qemu_thread_init(void)
> +{
> +# if defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
> +    pthread_setname_np(pthread_self(), "main");
> +# elif defined(CONFIG_PTHREAD_SETNAME_NP_WO_TID)
> +    pthread_setname_np("main");
> +# elif defined(CONFIG_PTHREAD_SET_NAME_NP)
> +    pthread_set_name_np(pthread_self(), "main");
> +# endif

Should this three way ifdef not be broken out somewhere;
it's already in qemu_thread_start() (and it looks like qjack_thread_creator
should use it)
(and the setname/set_name variety is hilarious).

Other than,


Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

> +}
> +
>  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 9595a5b090..5e6ca0c12f 100644
> --- a/util/qemu-thread-win32.c
> +++ b/util/qemu-thread-win32.c
> @@ -332,6 +332,12 @@ static void set_thread_description(HANDLE h, const char *name)
>      SetThreadDescriptionFunc(h, namew);
>  }
>  
> +static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
> +qemu_thread_init(void)
> +{
> +    set_thread_description(GetCurrentThread(), "main");
> +}
> +
>  void qemu_thread_create(QemuThread *thread, const char *name,
>                         void *(*start_routine)(void *),
>                         void *arg, int mode)
> -- 
> 2.50.1
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 04/12] util: set the name for the 'main' thread

[Daniel P. Berrangé]

On Tue, Aug 19, 2025 at 11:18:00PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > 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.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  util/qemu-thread-posix.c | 12 ++++++++++++
> >  util/qemu-thread-win32.c |  6 ++++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> > index 7c985b5d38..121d7ed69b 100644
> > --- a/util/qemu-thread-posix.c
> > +++ b/util/qemu-thread-posix.c
> > @@ -22,6 +22,18 @@
> >  #include <pthread_np.h>
> >  #endif
> >  
> > +static void __attribute__((__constructor__(QEMU_CONSTRUCTOR_EARLY)))
> > +qemu_thread_init(void)
> > +{
> > +# if defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
> > +    pthread_setname_np(pthread_self(), "main");
> > +# elif defined(CONFIG_PTHREAD_SETNAME_NP_WO_TID)
> > +    pthread_setname_np("main");
> > +# elif defined(CONFIG_PTHREAD_SET_NAME_NP)
> > +    pthread_set_name_np(pthread_self(), "main");
> > +# endif
> 
> Should this three way ifdef not be broken out somewhere;
> it's already in qemu_thread_start() (and it looks like qjack_thread_creator
> should use it)

Oh dear, I missed that jack-audio wasn't using our thread creation
wrapper :-(  Yes, we could split this into a helper function.


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

[PATCH 05/12] util: add API to fetch the current thread id

[Daniel P. Berrangé]

For logging it is useful to include the current thread id. On
POSIX there is no standard API for this, so try the Linux gettid()
syscall preferentially, otherwise fallback to casting the result
of thread_self() to an integer.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/thread.h    |  1 +
 util/qemu-thread-posix.c | 11 +++++++++++
 util/qemu-thread-win32.c |  5 +++++
 3 files changed, 17 insertions(+)

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 3a286bb3ef..96eee82679 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);
+uint64_t qemu_thread_get_id(void);
 
 struct Notifier;
 /**
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 121d7ed69b..f240a93632 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -532,3 +532,14 @@ void *qemu_thread_join(QemuThread *thread)
     }
     return ret;
 }
+
+uint64_t qemu_thread_get_id(void)
+{
+#ifdef CONFIG_GETTID
+    return (uint64_t)gettid();
+#elif defined(SYS_gettid)
+    return (uint64_t)syscall(SYS_gettid);
+#else
+    return (uint64_t)pthread_self();
+#endif
+}
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 5e6ca0c12f..f6956717b2 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -412,3 +412,8 @@ bool qemu_thread_is_self(QemuThread *thread)
 {
     return GetCurrentThreadId() == thread->tid;
 }
+
+uint64_t qemu_thread_get_id(void)
+{
+    return (uint64_t)GetCurrentThreadId();
+}
-- 
2.50.1

Re: [PATCH 05/12] util: add API to fetch the current thread id

[Richard Henderson]

On 8/20/25 06:27, Daniel P. Berrangé wrote:
> +uint64_t qemu_thread_get_id(void)
> +{
> +#ifdef CONFIG_GETTID
> +    return (uint64_t)gettid();
> +#elif defined(SYS_gettid)
> +    return (uint64_t)syscall(SYS_gettid);

No cast required here; extension to uint64_t will happen automatically.

> +#else
> +    return (uint64_t)pthread_self();

Incorrect cast here: you want uintptr_t.


r~

Re: [PATCH 05/12] util: add API to fetch the current thread id

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> For logging it is useful to include the current thread id. On
> POSIX there is no standard API for this, so try the Linux gettid()
> syscall preferentially, otherwise fallback to casting the result
> of thread_self() to an integer.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  include/qemu/thread.h    |  1 +
>  util/qemu-thread-posix.c | 11 +++++++++++
>  util/qemu-thread-win32.c |  5 +++++
>  3 files changed, 17 insertions(+)
> 
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index 3a286bb3ef..96eee82679 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);
> +uint64_t qemu_thread_get_id(void);
>  
>  struct Notifier;
>  /**
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 121d7ed69b..f240a93632 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -532,3 +532,14 @@ void *qemu_thread_join(QemuThread *thread)
>      }
>      return ret;
>  }
> +
> +uint64_t qemu_thread_get_id(void)
> +{
> +#ifdef CONFIG_GETTID
> +    return (uint64_t)gettid();
> +#elif defined(SYS_gettid)
> +    return (uint64_t)syscall(SYS_gettid);
> +#else
> +    return (uint64_t)pthread_self();

My reading of the manpage is that is not guaranteed to be castable.
It also explicitly says it's not the same as gettid.  Albeit probably is.

Dave

> +#endif
> +}
> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
> index 5e6ca0c12f..f6956717b2 100644
> --- a/util/qemu-thread-win32.c
> +++ b/util/qemu-thread-win32.c
> @@ -412,3 +412,8 @@ bool qemu_thread_is_self(QemuThread *thread)
>  {
>      return GetCurrentThreadId() == thread->tid;
>  }
> +
> +uint64_t qemu_thread_get_id(void)
> +{
> +    return (uint64_t)GetCurrentThreadId();
> +}
> -- 
> 2.50.1
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 05/12] util: add API to fetch the current thread id

[Daniel P. Berrangé]

On Tue, Aug 19, 2025 at 11:35:26PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > For logging it is useful to include the current thread id. On
> > POSIX there is no standard API for this, so try the Linux gettid()
> > syscall preferentially, otherwise fallback to casting the result
> > of thread_self() to an integer.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  include/qemu/thread.h    |  1 +
> >  util/qemu-thread-posix.c | 11 +++++++++++
> >  util/qemu-thread-win32.c |  5 +++++
> >  3 files changed, 17 insertions(+)
> > 
> > diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> > index 3a286bb3ef..96eee82679 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);
> > +uint64_t qemu_thread_get_id(void);
> >  
> >  struct Notifier;
> >  /**
> > diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> > index 121d7ed69b..f240a93632 100644
> > --- a/util/qemu-thread-posix.c
> > +++ b/util/qemu-thread-posix.c
> > @@ -532,3 +532,14 @@ void *qemu_thread_join(QemuThread *thread)
> >      }
> >      return ret;
> >  }
> > +
> > +uint64_t qemu_thread_get_id(void)
> > +{
> > +#ifdef CONFIG_GETTID
> > +    return (uint64_t)gettid();
> > +#elif defined(SYS_gettid)
> > +    return (uint64_t)syscall(SYS_gettid);
> > +#else
> > +    return (uint64_t)pthread_self();
> 
> My reading of the manpage is that is not guaranteed to be castable.
> It also explicitly says it's not the same as gettid.  Albeit probably is.

Yeah, technically pthread_t could be a struct. Guess it depends how much
we care about strict standards portability, vs portability to our tested
set of platforms (macOS, *BSD)[1]. We could hardcode to 0 as an alternative.

With regards,
Daniel

[1] admittedly i've not actually compile tested this yet on non-Linux,
    aside from Windows which uses the non-pthreads code.
-- 
|: 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 :|

[PATCH 06/12] 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 on POSIX platforms, at the
cost of having to store an allocated string pointer on Windows.

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

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 96eee82679..0366cb743d 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);
 uint64_t qemu_thread_get_id(void);
+const char *qemu_thread_get_name(void);
 
 struct Notifier;
 /**
diff --git a/meson.build b/meson.build
index 50c774a195..ebdcb5d65d 100644
--- a/meson.build
+++ b/meson.build
@@ -2920,6 +2920,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 f240a93632..282802bc39 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
 
@@ -543,3 +543,26 @@ uint64_t qemu_thread_get_id(void)
     return (uint64_t)pthread_self();
 #endif
 }
+
+#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 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) {
+        strncpy(namebuf, "unnamed", sizeof(namebuf) - 1);
+        namebuf[sizeof(namebuf) - 1] = '\0';
+    }
+    return namebuf;
+}
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index f6956717b2..c31a8a6bc8 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -19,10 +19,13 @@
 
 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 bool load_set_thread_description(void)
+static bool load_thread_description(void)
 {
     static gsize _init_once = 0;
 
@@ -32,14 +35,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)
@@ -320,7 +326,7 @@ static void set_thread_description(HANDLE h, const char *name)
 {
     g_autofree wchar_t *namew = NULL;
 
-    if (!load_set_thread_description()) {
+    if (!load_thread_description()) {
         return;
     }
 
@@ -417,3 +423,39 @@ uint64_t qemu_thread_get_id(void)
 {
     return (uint64_t)GetCurrentThreadId();
 }
+
+static __thread char *namebuf;
+
+const char *qemu_thread_get_name(void)
+{
+    HRESULT hr;
+    wchar_t *namew = NULL;
+    g_autofree char *name = NULL;
+
+    if (!load_thread_description()) {
+        goto error;
+    }
+
+    hr = GetThreadDescriptionFunc(GetCurrentThread(), &namew);
+    if (!SUCCEEDED(hr)) {
+        goto error;
+    }
+
+    g_free(namebuf);
+    namebuf = g_utf16_to_utf8(namew, -1, NULL, NULL, NULL);
+    if (!namebuf) {
+        goto error;
+    }
+
+ cleanup:
+    if (namew) {
+        LocalFree(namew);
+    }
+    return namebuf;
+
+ error:
+    if (!namebuf) {
+        namebuf = g_strdup("unnamed");
+    }
+    goto cleanup;
+}
-- 
2.50.1

Re: [PATCH 06/12] util: add API to fetch the current thread name

[Richard Henderson]

On 8/20/25 06:27, Daniel P. Berrangé wrote:
> +        strncpy(namebuf, "unnamed", sizeof(namebuf) - 1);
> +        namebuf[sizeof(namebuf) - 1] = '\0';

g_strlcpy?

r~

Re: [PATCH 06/12] util: add API to fetch the current thread name

[Daniel P. Berrangé]

On Wed, Aug 20, 2025 at 07:50:05AM +1000, Richard Henderson wrote:
> On 8/20/25 06:27, Daniel P. Berrangé wrote:
> > +        strncpy(namebuf, "unnamed", sizeof(namebuf) - 1);
> > +        namebuf[sizeof(namebuf) - 1] = '\0';
> 
> g_strlcpy?

Sure, I always forget about that.


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

[PATCH 07/12] 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.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 MAINTAINERS            |  2 ++
 include/qemu/message.h | 40 ++++++++++++++++++++++++++++++++++++++++
 util/meson.build       |  1 +
 util/message.c         | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+)
 create mode 100644 include/qemu/message.h
 create mode 100644 util/message.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a07086ed76..3cc6c0b409 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3169,9 +3169,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..160bee8417
--- /dev/null
+++ b/include/qemu/message.h
@@ -0,0 +1,40 @@
+/* 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);
+
+enum QMessageContextFlags {
+    QMESSAGE_CONTEXT_SKIP_MONITOR = (1 << 0),
+};
+
+/*
+ * qmessage_context:
+ * @flags: the message formatting control flags
+ *
+ * Format a message prefix with the information
+ * previously selected by a call to
+ * qmessage_set_format.
+ *
+ * If @flags contains QMESSAGE_CONTEXT_SKIP_MONITOR
+ * an empty string will be returned if running in
+ * the context of a HMP command
+ *
+ * Returns: a formatted message prefix, or empty string;
+ * to be freed by the caller.
+ */
+char *qmessage_context(int flags);
+
+#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..4c7eeb75e2
--- /dev/null
+++ b/util/message.c
@@ -0,0 +1,32 @@
+/* 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;
+}
+
+char *qmessage_context(int flags)
+{
+    g_autofree char *timestr = NULL;
+
+    if ((flags & QMESSAGE_CONTEXT_SKIP_MONITOR) &&
+        monitor_cur()) {
+        return g_strdup("");
+    }
+
+    if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
+        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
+        timestr = g_date_time_format_iso8601(dt);
+    }
+
+    return g_strdup_printf("%s%s",
+                           timestr ? timestr : "",
+                           timestr ? " " : "");
+}
-- 
2.50.1

Re: [PATCH 07/12] util: introduce common helper for error-report & log code

[Richard Henderson]

On 8/20/25 06:27, Daniel P. Berrangé wrote:
> +char *qmessage_context(int flags)
> +{
> +    g_autofree char *timestr = NULL;
> +
> +    if ((flags & QMESSAGE_CONTEXT_SKIP_MONITOR) &&
> +        monitor_cur()) {
> +        return g_strdup("");

In the event we want no context, we really should avoid memory allocation too.


> +    }
> +
> +    if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
> +        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
> +        timestr = g_date_time_format_iso8601(dt);
> +    }
> +
> +    return g_strdup_printf("%s%s",
> +                           timestr ? timestr : "",
> +                           timestr ? " " : "");

g_strdup_printf isn't the nicest string concatenation tool.
And again, there's a no-context path.


r~

Re: [PATCH 07/12] util: introduce common helper for error-report & log code

[Daniel P. Berrangé]

On Wed, Aug 20, 2025 at 07:57:47AM +1000, Richard Henderson wrote:
> On 8/20/25 06:27, Daniel P. Berrangé wrote:
> > +char *qmessage_context(int flags)
> > +{
> > +    g_autofree char *timestr = NULL;
> > +
> > +    if ((flags & QMESSAGE_CONTEXT_SKIP_MONITOR) &&
> > +        monitor_cur()) {
> > +        return g_strdup("");
> 
> In the event we want no context, we really should avoid memory allocation too.

Yes, won't not be too hard to check for NULL in the caller.

> > +    }
> > +
> > +    if (message_format & QMESSAGE_FORMAT_TIMESTAMP) {
> > +        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
> > +        timestr = g_date_time_format_iso8601(dt);
> > +    }
> > +
> > +    return g_strdup_printf("%s%s",
> > +                           timestr ? timestr : "",
> > +                           timestr ? " " : "");
> 
> g_strdup_printf isn't the nicest string concatenation tool.

A GString would make it easier. Though typically that would involve
an allocation of GSTring struct, I guess we could be sneaky and stack
allocate the struct.

> And again, there's a no-context path.



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

[PATCH 08/12] 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, guaranteeign they will have matching behaviour going
forward.

There is no change in output format for either logs or errors
with this conversion.

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                     | 20 +++++++-------------
 5 files changed, 18 insertions(+), 31 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 a64fd90d4a..696dd92669 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"
@@ -813,8 +814,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 1b17c11de1..932af5d9f5 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;
 
@@ -169,13 +168,6 @@ static void print_loc(void)
     }
 }
 
-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.
@@ -186,13 +178,9 @@ real_time_iso8601(void)
 G_GNUC_PRINTF(2, 0)
 static void vreport(report_type type, const char *fmt, va_list ap)
 {
-    gchar *timestr;
+    g_autofree gchar *context = qmessage_context(QMESSAGE_CONTEXT_SKIP_MONITOR);
 
-    if (message_with_timestamp && !monitor_cur()) {
-        timestr = real_time_iso8601();
-        error_printf("%s ", timestr);
-        g_free(timestr);
-    }
+    error_printf("%s", context);
 
     /* Only prepend guest name if -msg guest-name and -name guest=... are set */
     if (error_with_guestname && error_guest_name && !monitor_cur()) {
diff --git a/util/log.c b/util/log.c
index abdcb6b311..435d1c7135 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"
@@ -146,26 +147,19 @@ void qemu_log_unlock(FILE *logfile)
 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
+     * Prepare the context *outside* the logging
+     * lock so any timestamp better reflects when
+     * the message was emitted if we are delayed
+     * acquiring the mutex
      */
-    if (message_with_timestamp) {
-        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
-        timestr = g_date_time_format_iso8601(dt);
-    }
+    g_autofree const char *context = qmessage_context(0);
 
     f = qemu_log_trylock();
     if (f) {
         va_list ap;
 
-        if (timestr) {
-            fprintf(f, "%s ", timestr);
-        }
+        fwrite(context, 1, strlen(context), f);
 
         va_start(ap, fmt);
         vfprintf(f, fmt, ap);
-- 
2.50.1

Re: [PATCH 08/12] util: convert error-report & log to message API for timestamp

[Richard Henderson]

On 8/20/25 06:27, Daniel P. Berrangé wrote:
>   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
> +     * Prepare the context*outside* the logging
> +     * lock so any timestamp better reflects when
> +     * the message was emitted if we are delayed
> +     * acquiring the mutex
>        */
> -    if (message_with_timestamp) {
> -        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
> -        timestr = g_date_time_format_iso8601(dt);
> -    }

Something that really ought to be handled better is that we currently use qemu_log to 
print pieces of a larger message.  E.g. looping over the general registers to print the 
whole cpu state.

(1) We don't want timestamps in the middle, and
(2) we don't want unnecessary memory allocation in the middle.


r~

Re: [PATCH 08/12] util: convert error-report & log to message API for timestamp

[Daniel P. Berrangé]

On Wed, Aug 20, 2025 at 08:02:14AM +1000, Richard Henderson wrote:
> On 8/20/25 06:27, Daniel P. Berrangé wrote:
> >   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
> > +     * Prepare the context*outside* the logging
> > +     * lock so any timestamp better reflects when
> > +     * the message was emitted if we are delayed
> > +     * acquiring the mutex
> >        */
> > -    if (message_with_timestamp) {
> > -        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
> > -        timestr = g_date_time_format_iso8601(dt);
> > -    }
> 
> Something that really ought to be handled better is that we currently use
> qemu_log to print pieces of a larger message.  E.g. looping over the general
> registers to print the whole cpu state.
> 
> (1) We don't want timestamps in the middle, and
> (2) we don't want unnecessary memory allocation in the middle.

Interestingly I found that for CPU_LOG_TB_CPU we don't use the qemu_log()
function at all. TCG calls qemu_log_try_lock() to acquire the raw FILE *
handle and writes directly to it, so we miss all prefix info entirely.
If other parts of QEMU that do big multi-line log dumps do the same,
then we're ok.

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 08/12] util: convert error-report & log to message API for timestamp

[Daniel P. Berrangé]

On Wed, Aug 20, 2025 at 08:02:14AM +1000, Richard Henderson wrote:
> On 8/20/25 06:27, Daniel P. Berrangé wrote:
> >   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
> > +     * Prepare the context*outside* the logging
> > +     * lock so any timestamp better reflects when
> > +     * the message was emitted if we are delayed
> > +     * acquiring the mutex
> >        */
> > -    if (message_with_timestamp) {
> > -        g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
> > -        timestr = g_date_time_format_iso8601(dt);
> > -    }
> 
> Something that really ought to be handled better is that we currently use
> qemu_log to print pieces of a larger message.  E.g. looping over the general
> registers to print the whole cpu state.
> 
> (1) We don't want timestamps in the middle, and
> (2) we don't want unnecessary memory allocation in the middle.

This turned out to be surprisingly simple to fix - don't emit the
prefix if the previous qemu_log() call lacked a terminating newline
character.

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

[PATCH 09/12] 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

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qemu/error-report.h |  3 ---
 include/qemu/message.h      | 14 ++++++++++++--
 system/vl.c                 |  6 ++++--
 util/error-report.c         |  7 -------
 util/message.c              | 19 +++++++++++++++++--
 5 files changed, 33 insertions(+), 16 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 160bee8417..95fab69424 100644
--- a/include/qemu/message.h
+++ b/include/qemu/message.h
@@ -5,9 +5,10 @@
 
 enum QMessageFormatFlags {
     QMESSAGE_FORMAT_TIMESTAMP = (1 << 0),
+    QMESSAGE_FORMAT_WORKLOAD_NAME = (1 << 1),
 };
 
-/*
+/**
  * qmessage_set_format:
  * @flags: the message information to emit
  *
@@ -16,11 +17,20 @@ 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);
+
 enum QMessageContextFlags {
     QMESSAGE_CONTEXT_SKIP_MONITOR = (1 << 0),
 };
 
-/*
+/**
  * qmessage_context:
  * @flags: the message formatting control flags
  *
diff --git a/system/vl.c b/system/vl.c
index 696dd92669..fee6fdf7b1 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -818,7 +818,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);
 }
 
@@ -3520,7 +3522,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 932af5d9f5..765381dc37 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;
 
 int error_printf(const char *fmt, ...)
 {
@@ -182,11 +180,6 @@ static void vreport(report_type type, const char *fmt, va_list ap)
 
     error_printf("%s", context);
 
-    /* Only prepend guest name if -msg guest-name and -name guest=... are set */
-    if (error_with_guestname && error_guest_name && !monitor_cur()) {
-        error_printf("%s ", error_guest_name);
-    }
-
     print_loc();
 
     switch (type) {
diff --git a/util/message.c b/util/message.c
index 4c7eeb75e2..1ce3a34928 100644
--- a/util/message.c
+++ b/util/message.c
@@ -6,15 +6,24 @@
 #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);
+}
+
+
 char *qmessage_context(int flags)
 {
     g_autofree char *timestr = NULL;
+    const char *wknamestr = NULL;
 
     if ((flags & QMESSAGE_CONTEXT_SKIP_MONITOR) &&
         monitor_cur()) {
@@ -26,7 +35,13 @@ char *qmessage_context(int flags)
         timestr = g_date_time_format_iso8601(dt);
     }
 
-    return g_strdup_printf("%s%s",
+    if (message_format & QMESSAGE_FORMAT_WORKLOAD_NAME) {
+        wknamestr = message_workloadname;
+    }
+
+    return g_strdup_printf("%s%s%s%s",
                            timestr ? timestr : "",
-                           timestr ? " " : "");
+                           timestr ? " " : "",
+                           wknamestr ? wknamestr : "",
+                           wknamestr ? " " : "");
 }
-- 
2.50.1

[PATCH 10/12] 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

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                 | 13 ++++++++++---
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/include/qemu/message.h b/include/qemu/message.h
index 95fab69424..4550ce8ed7 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 fee6fdf7b1..9030212c50 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -814,7 +814,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 765381dc37..65262d29c4 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -141,10 +141,6 @@ static void print_loc(void)
     int i;
     const char *const *argp;
 
-    if (!monitor_cur() && g_get_prgname()) {
-        error_printf("%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 1ce3a34928..f1c4f92e7b 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)
@@ -24,6 +24,7 @@ char *qmessage_context(int flags)
 {
     g_autofree char *timestr = NULL;
     const char *wknamestr = NULL;
+    const char *pgnamestr = NULL;
 
     if ((flags & QMESSAGE_CONTEXT_SKIP_MONITOR) &&
         monitor_cur()) {
@@ -39,9 +40,15 @@ char *qmessage_context(int flags)
         wknamestr = message_workloadname;
     }
 
-    return g_strdup_printf("%s%s%s%s",
+    if (message_format & QMESSAGE_FORMAT_PROGRAM_NAME) {
+        pgnamestr = g_get_prgname();
+    }
+
+    return g_strdup_printf("%s%s%s%s%s%s",
                            timestr ? timestr : "",
                            timestr ? " " : "",
                            wknamestr ? wknamestr : "",
-                           wknamestr ? " " : "");
+                           wknamestr ? " " : "",
+                           pgnamestr ? pgnamestr : "",
+                           pgnamestr ? ": " : "");
 }
-- 
2.50.1

[PATCH 11/12] 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                       | 27 ++++++++++++++++++++-------
 6 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/include/qemu/message.h b/include/qemu/message.h
index 4550ce8ed7..6dbb068ca9 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 9030212c50..20b655a7bc 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;
@@ -811,10 +815,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;
     }
@@ -2888,6 +2891,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..d9954da42b 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\(.*\): ' % 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..ecf74c223e 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)(.*):#\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 f1c4f92e7b..300dc10a4f 100644
--- a/util/message.c
+++ b/util/message.c
@@ -44,11 +44,24 @@ char *qmessage_context(int flags)
         pgnamestr = g_get_prgname();
     }
 
-    return g_strdup_printf("%s%s%s%s%s%s",
-                           timestr ? timestr : "",
-                           timestr ? " " : "",
-                           wknamestr ? wknamestr : "",
-                           wknamestr ? " " : "",
-                           pgnamestr ? pgnamestr : "",
-                           pgnamestr ? ": " : "");
+    if (message_format & QMESSAGE_FORMAT_THREAD_INFO) {
+        uint64_t thid = qemu_thread_get_id();
+        const char *thname = qemu_thread_get_name();
+
+        return g_strdup_printf("%s%s%s%s%s(%" PRIu64 ":%s): ",
+                               timestr ? timestr : "",
+                               timestr ? " " : "",
+                               wknamestr ? wknamestr : "",
+                               wknamestr ? " " : "",
+                               pgnamestr ? pgnamestr : "",
+                               thid, thname);
+    } else {
+        return g_strdup_printf("%s%s%s%s%s%s",
+                               timestr ? timestr : "",
+                               timestr ? " " : "",
+                               wknamestr ? wknamestr : "",
+                               wknamestr ? " " : "",
+                               pgnamestr ? pgnamestr : "",
+                               pgnamestr ? ": " : "");
+    }
 }
-- 
2.50.1

[PATCH 12/12] 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

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

diff --git a/util/message.c b/util/message.c
index 300dc10a4f..9ce2e27f5b 100644
--- a/util/message.c
+++ b/util/message.c
@@ -48,19 +48,21 @@ char *qmessage_context(int flags)
         uint64_t thid = qemu_thread_get_id();
         const char *thname = qemu_thread_get_name();
 
-        return g_strdup_printf("%s%s%s%s%s(%" PRIu64 ":%s): ",
+        return g_strdup_printf("%s%s%s%s%s%s(%" PRIu64 ":%s): ",
                                timestr ? timestr : "",
                                timestr ? " " : "",
+                               wknamestr ? "[" : "",
                                wknamestr ? wknamestr : "",
-                               wknamestr ? " " : "",
+                               wknamestr ? "] " : "",
                                pgnamestr ? pgnamestr : "",
                                thid, thname);
     } else {
-        return g_strdup_printf("%s%s%s%s%s%s",
+        return g_strdup_printf("%s%s%s%s%s%s%s",
                                timestr ? timestr : "",
                                timestr ? " " : "",
+                               wknamestr ? "[" : "",
                                wknamestr ? wknamestr : "",
-                               wknamestr ? " " : "",
+                               wknamestr ? "] " : "",
                                pgnamestr ? pgnamestr : "",
                                pgnamestr ? ": " : "");
     }
-- 
2.50.1