At least the simple trace backend works by spawning a helper thread,
and setting up an atexit() handler that coordinates completion with
the helper thread.  But since atexit registrations survive fork() but
helper threads do not, this means that qemu-nbd configured to use the
simple trace will deadlock waiting for a thread that no longer exists
when it has daemonized.

Better is to follow the example of vl.c: don't call any setup
functions that might spawn helper threads until we are in the final
process that will be doing the work worth tracing.

Tested by configuring with --enable-trace-backends=simple, then running
  qemu-nbd --fork --trace=nbd_\*,file=qemu-nbd.trace -f raw -r README.rst
followed by `nbdinfo nbd://localhost`, and observing that the trace
file is now created without hanging.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-nbd.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 05b61da51ea..ed5895861bb 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -852,10 +852,6 @@ int main(int argc, char **argv)
         export_name = "";
     }

-    if (!trace_init_backends()) {
-        exit(1);
-    }
-    trace_init_file();
     qemu_set_log(LOG_TRACE, &error_fatal);

     socket_activation = check_socket_activation();
@@ -1045,6 +1041,18 @@ int main(int argc, char **argv)
 #endif /* WIN32 */
     }

+    /*
+     * trace_init must be done after daemonization.  Why? Because at
+     * least the simple backend spins up a helper thread as well as an
+     * atexit() handler that waits on that thread, but the helper
+     * thread won't survive a fork, leading to deadlock in the child
+     * if we initialized pre-fork.
+     */
+    if (!trace_init_backends()) {
+        exit(1);
+    }
+    trace_init_file();
+
     if (opts.device != NULL && sockpath == NULL) {
         sockpath = g_malloc(128);
         snprintf(sockpath, 128, SOCKET_PATH, basename(opts.device));
-- 
2.48.1

On 27/02/2025 23.06, Eric Blake wrote:
> At least the simple trace backend works by spawning a helper thread,
> and setting up an atexit() handler that coordinates completion with
> the helper thread.  But since atexit registrations survive fork() but
> helper threads do not, this means that qemu-nbd configured to use the
> simple trace will deadlock waiting for a thread that no longer exists
> when it has daemonized.
> 
> Better is to follow the example of vl.c: don't call any setup
> functions that might spawn helper threads until we are in the final
> process that will be doing the work worth tracing.
> 
> Tested by configuring with --enable-trace-backends=simple, then running
>    qemu-nbd --fork --trace=nbd_\*,file=qemu-nbd.trace -f raw -r README.rst
> followed by `nbdinfo nbd://localhost`, and observing that the trace
> file is now created without hanging.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   qemu-nbd.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 05b61da51ea..ed5895861bb 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -852,10 +852,6 @@ int main(int argc, char **argv)
>           export_name = "";
>       }
> 
> -    if (!trace_init_backends()) {
> -        exit(1);
> -    }
> -    trace_init_file();
>       qemu_set_log(LOG_TRACE, &error_fatal);
> 
>       socket_activation = check_socket_activation();
> @@ -1045,6 +1041,18 @@ int main(int argc, char **argv)
>   #endif /* WIN32 */
>       }
> 
> +    /*
> +     * trace_init must be done after daemonization.  Why? Because at
> +     * least the simple backend spins up a helper thread as well as an
> +     * atexit() handler that waits on that thread, but the helper
> +     * thread won't survive a fork, leading to deadlock in the child
> +     * if we initialized pre-fork.
> +     */
> +    if (!trace_init_backends()) {
> +        exit(1);
> +    }
> +    trace_init_file();

This also sounds like the best option to me!

Reviewed-by: Thomas Huth <thuth@redhat.com>

On Wed, Mar 05, 2025 at 07:55:48AM +0100, Thomas Huth wrote:
> On 27/02/2025 23.06, Eric Blake wrote:
> > At least the simple trace backend works by spawning a helper thread,
> > and setting up an atexit() handler that coordinates completion with
> > the helper thread.  But since atexit registrations survive fork() but
> > helper threads do not, this means that qemu-nbd configured to use the
> > simple trace will deadlock waiting for a thread that no longer exists
> > when it has daemonized.
> > 
> > Better is to follow the example of vl.c: don't call any setup
> > functions that might spawn helper threads until we are in the final
> > process that will be doing the work worth tracing.
> > 

...
> This also sounds like the best option to me!
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks; added to my NBD queue.

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