[PATCH v5 05/24] util: expose qemu_thread_set_name

Daniel P. Berrangé posted 24 patches 3 weeks, 3 days ago
There is a newer version of this series
[PATCH v5 05/24] util: expose qemu_thread_set_name
Posted by Daniel P. Berrangé 3 weeks, 3 days ago
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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.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.52.0


Re: [PATCH v5 05/24] util: expose qemu_thread_set_name
Posted by Markus Armbruster via Devel 2 weeks, 5 days ago
Daniel P. Berrangé <berrange@redhat.com> writes:

> 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>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.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);

qemu_thread_set_name() is factored out.  No change in behavior.

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

This delays setting the thread name until the thread runs.  Fine, I
guess, but I'd mention it in the commit message.

The new QemuThreadData member @name is non-null only between thread
creation and thread name setting, unlike the other members under
/* Passed to win32_start_routine.  */  Worth a comment?  Keep it alive
until the thread dies?  Entirely up to you.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Re: [PATCH v5 05/24] util: expose qemu_thread_set_name
Posted by Daniel P. Berrangé 2 weeks, 5 days ago
On Tue, Jan 13, 2026 at 10:16:06AM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > 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>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.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);
> 
> qemu_thread_set_name() is factored out.  No change in behavior.
> 
> > 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;
> 
> This delays setting the thread name until the thread runs.  Fine, I
> guess, but I'd mention it in the commit message.
> 
> The new QemuThreadData member @name is non-null only between thread
> creation and thread name setting, unlike the other members under
> /* Passed to win32_start_routine.  */  Worth a comment?  Keep it alive
> until the thread dies?  Entirely up to you.

win32_start_routine never returns as it calls into qemu_thread_exit
so it is simpler to just free immediately after use. I'll comment
the struct.

> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

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