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

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Sep 19, 2025 at 10:20:51AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > 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>
> > 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 b2420732e1..7187ea15fa 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
> 
> Do you mean "The ``debug-threads`` option"?

Opps, yes indeed.

> 
> "option for" or "option of"?  Not a native speaker...

"of" is probably better.

> 
> > +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 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",
> 
> Please don't shout :)

Hmmm, yes.

> We rarely note deprecation notes in command line help texts.  I found
> two.
> 
> block/qcow.c:
> 
>         {
>             .name = BLOCK_OPT_ENCRYPT,
>             .type = QEMU_OPT_BOOL,
>             .help = "Encrypt the image with format 'aes'. (Deprecated "
>                     "in favor of " BLOCK_OPT_ENCRYPT_FORMAT "=aes)",
>         },
> 
> block/qcow2.c:
> 
>         {                                                               \
>             .name = BLOCK_OPT_ENCRYPT,                                  \
>             .type = QEMU_OPT_BOOL,                                      \
>             .help = "Encrypt the image with format 'aes'. (Deprecated " \
>                     "in favor of " BLOCK_OPT_ENCRYPT_FORMAT "=aes)",    \
>         },                                                              \
>         {                                                               \
> 
> Suggest something like
> 
>             .help = "Enable thread names"
>                     " (deprecated, always enabled where supported)",

Sure.

> 
> >          },
> >          { /* 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");
> 
> Not sure this is worth the trouble, but since you wrote and tested it
> already...

As a general rule I like there to be a printf() in any code path that
touches a deprecation, because we know  people often won't actually
read the docs. The message is helping them know what's coming soon


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