Re: [PATCH v2 1/2] util/log: Derive thread id from getpid() on hosts w/o gettid() syscall

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

On Wed, Oct 19, 2022 at 05:16:50PM +0200, Greg Kurz wrote:
> A subsequent patch needs to be able to differentiate the main QEMU
> thread from other threads. An obvious way to do so is to compare
> log_thread_id() and getpid(), based on the fact that they are equal
> for the main thread on systems that have the gettid() syscall (e.g.
> linux).
> 
> Adapt the fallback code for systems without gettid() to provide the
> same assumption.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  util/log.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/util/log.c b/util/log.c
> index d6eb0378c3a3..e1c2535cfcd2 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -72,8 +72,13 @@ static int log_thread_id(void)
>  #elif defined(SYS_gettid)
>      return syscall(SYS_gettid);
>  #else
> +    static __thread int my_id = -1;
>      static int counter;
> -    return qatomic_fetch_inc(&counter);
> +
> +    if (my_id == -1) {
> +        my_id = getpid() + qatomic_fetch_inc(&counter);
> +    }
> +    return my_id;

This doesn't look safe for linux-user when we fork, but don't exec.

The getpid() will change after the fork, but counter won't be
reset, so a thread in the parent could clash with a thread
in the forked child.

I feel like if we want to check for the main thread, we should
be using pthread_self(), and compare result against the value
cached from main. Or cache in a __constructor__ function in
log.c to keep it isolated from main().


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