qemu_thread_create: propagate errors to callers to check

Currently, when qemu_signal_init() fails it only returns a non-zero value but without propagating any Error. But its callers need a non-null err when runs error_report_err(err), or else 0->msg occurs.

To avoid such segmentation fault, add a new Error parameter to make the call trace to propagate the err to the final caller.

This patch also adds the omitted error handling when creating signalfd pipe fails in qemu_signalfd_compat().

Signed-off-by Fei Li <[email protected]>

---
 include/qemu/osdep.h |  2 +-
 util/compatfd.c      |  9 ++++++---
 util/main-loop.c     | 10 +++++-----
 3 files changed, 12 insertions(+), 9 deletions(-)

include/qemu/osdep.h +1 -1

index a91068df0e..09ed85fcb8 100644
@@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
                              additional fields in the future) */
 };
 
-int qemu_signalfd(const sigset_t *mask);
+int qemu_signalfd(const sigset_t *mask, Error **errp);
 void sigaction_invoke(struct sigaction *action,
                       struct qemu_signalfd_siginfo *info);
 #endif

util/compatfd.c +4 -1

index 980bd33e52..d3ed890405 100644
@@ -16,6 +16,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qemu/thread.h"
+#include "qapi/error.h"
 
 #include <sys/syscall.h>
 

@@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
     }
 }
 
-static int qemu_signalfd_compat(const sigset_t *mask)
+static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
 {
     struct sigfd_compat_info *info;
     QemuThread thread;

@@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
 
     info = malloc(sizeof(*info));
     if (info == NULL) {
+        error_setg(errp, "Failed to allocate signalfd memory");
         errno = ENOMEM;
         return -1;
     }
 
     if (pipe(fds) == -1) {
+        error_setg(errp, "Failed to create signalfd pipe");
         free(info);
         return -1;
     }

util/compatfd.c +1 -1

@@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
     }
 #endif
 
-    return qemu_signalfd_compat(mask);
+    return qemu_signalfd_compat(mask, errp);
 }

util/main-loop.c +1 -1

index affe0403c5..22aa2b1007 100644
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
     }
 }
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
 {
     int sigfd;
     sigset_t set;

util/main-loop.c +3 -3

-        return -errno;
+        return -1;
     }
 
     fcntl_setfl(sigfd, O_NONBLOCK);

@@ -109,7 +109,7 @@ static int qemu_signal_init(void)
 
 #else /* _WIN32 */
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
 {
     return 0;
 }

@@ -148,7 +148,7 @@ int qemu_init_main_loop(Error **errp)
 
     init_clocks(qemu_timer_notify_cb);
 
-    ret = qemu_signal_init();
+    ret = qemu_signal_init(errp);
     if (ret) {
         return ret;
     }

Signature · 1 line

2.13.7

Quoted 66 lines · Fei Li

Currently, when qemu_signal_init() fails it only returns a non-zero value but without propagating any Error. But its callers need a non-null err when runs error_report_err(err), or else 0->msg occurs.

To avoid such segmentation fault, add a new Error parameter to make the call trace to propagate the err to the final caller.

This patch also adds the omitted error handling when creating signalfd pipe fails in qemu_signalfd_compat().

Signed-off-by Fei Li <[email protected]>

---
  include/qemu/osdep.h |  2 +-
  util/compatfd.c      |  9 ++++++---
  util/main-loop.c     | 10 +++++-----
  3 files changed, 12 insertions(+), 9 deletions(-)

include/qemu/osdep.h +1 -1

index a91068df0e..09ed85fcb8 100644
@@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
                               additional fields in the future) */
  };
 
-int qemu_signalfd(const sigset_t *mask);
+int qemu_signalfd(const sigset_t *mask, Error **errp);
  void sigaction_invoke(struct sigaction *action,
                        struct qemu_signalfd_siginfo *info);
  #endif

util/compatfd.c +4 -1

index 980bd33e52..d3ed890405 100644
@@ -16,6 +16,7 @@
  #include "qemu/osdep.h"
  #include "qemu-common.h"
  #include "qemu/thread.h"
+#include "qapi/error.h"
 
  #include <sys/syscall.h>
 

@@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
      }
  }
 
-static int qemu_signalfd_compat(const sigset_t *mask)
+static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
  {
      struct sigfd_compat_info *info;
      QemuThread thread;

@@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
 
      info = malloc(sizeof(*info));
      if (info == NULL) {
+        error_setg(errp, "Failed to allocate signalfd memory");
          errno = ENOMEM;
          return -1;
      }
 
      if (pipe(fds) == -1) {
+        error_setg(errp, "Failed to create signalfd pipe");
          free(info);
          return -1;
      }

util/compatfd.c static int qemu_signalfd_compat(const sigset_t *mask) · line 103

Quoted 8 lines

util/compatfd.c +1 -1

@@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
      return fds[0];
  }
 
-int qemu_signalfd(const sigset_t *mask)
+int qemu_signalfd(const sigset_t *mask, Error **errp)
  {
  #if defined(CONFIG_SIGNALFD)
      int ret;

Extend the context:

        ret = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
        if (ret != -1) {
            qemu_set_cloexec(ret);
            return ret;
        }

This error path need error_setg() as well.

Quoted 24 lines

util/compatfd.c +1 -1

@@ -106,5 +109,5 @@ int qemu_signalfd(const sigset_t *mask)
      }
  #endif
 
-    return qemu_signalfd_compat(mask);
+    return qemu_signalfd_compat(mask, errp);
  }

util/main-loop.c +1 -1

index affe0403c5..22aa2b1007 100644
@@ -71,7 +71,7 @@ static void sigfd_handler(void *opaque)
      }
  }
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
  {
      int sigfd;
      sigset_t set;

@@ -94,10 +94,10 @@ static int qemu_signal_init(void)
      pthread_sigmask(SIG_BLOCK, &set, NULL);
 

util/main-loop.c static int qemu_signal_init(void) · line 99

Quoted 5 lines

util/main-loop.c +1 -1

      sigdelset(&set, SIG_IPI);
-    sigfd = qemu_signalfd(&set);
+    sigfd = qemu_signalfd(&set, errp);
      if (sigfd == -1) {
          fprintf(stderr, "failed to create signalfd\n");

This fprintf is not needed any more.

Quoted 27 lines

util/main-loop.c +3 -3

-        return -errno;
+        return -1;
      }
 
      fcntl_setfl(sigfd, O_NONBLOCK);

@@ -109,7 +109,7 @@ static int qemu_signal_init(void)
 
  #else /* _WIN32 */
 
-static int qemu_signal_init(void)
+static int qemu_signal_init(Error **errp)
  {
      return 0;
  }

@@ -148,7 +148,7 @@ int qemu_init_main_loop(Error **errp)
 
      init_clocks(qemu_timer_notify_cb);
 
-    ret = qemu_signal_init();
+    ret = qemu_signal_init(errp);
      if (ret) {
          return ret;
      }

Signature · 1 line

2.13.7

Fam

Quoted 70 lines · Fam Zheng

Quoted 60 lines · Fei Li

Currently, when qemu_signal_init() fails it only returns a non-zero value but without propagating any Error. But its callers need a non-null err when runs error_report_err(err), or else 0->msg occurs.

To avoid such segmentation fault, add a new Error parameter to make the call trace to propagate the err to the final caller.

This patch also adds the omitted error handling when creating signalfd pipe fails in qemu_signalfd_compat().

Signed-off-by Fei Li <[email protected]>

---
  include/qemu/osdep.h |  2 +-
  util/compatfd.c      |  9 ++++++---
  util/main-loop.c     | 10 +++++-----
  3 files changed, 12 insertions(+), 9 deletions(-)

include/qemu/osdep.h +1 -1

index a91068df0e..09ed85fcb8 100644
@@ -421,7 +421,7 @@ struct qemu_signalfd_siginfo {
                               additional fields in the future) */
  };
-int qemu_signalfd(const sigset_t *mask);
+int qemu_signalfd(const sigset_t *mask, Error **errp);
  void sigaction_invoke(struct sigaction *action,
                        struct qemu_signalfd_siginfo *info);
  #endif

util/compatfd.c +4 -1

index 980bd33e52..d3ed890405 100644
@@ -16,6 +16,7 @@
  #include "qemu/osdep.h"
  #include "qemu-common.h"
  #include "qemu/thread.h"
+#include "qapi/error.h"
  #include <sys/syscall.h>

@@ -65,7 +66,7 @@ static void *sigwait_compat(void *opaque)
      }
  }
-static int qemu_signalfd_compat(const sigset_t *mask)
+static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
  {
      struct sigfd_compat_info *info;
      QemuThread thread;

@@ -73,11 +74,13 @@ static int qemu_signalfd_compat(const sigset_t *mask)
      info = malloc(sizeof(*info));
      if (info == NULL) {
+        error_setg(errp, "Failed to allocate signalfd memory");
          errno = ENOMEM;
          return -1;
      }
      if (pipe(fds) == -1) {
+        error_setg(errp, "Failed to create signalfd pipe");
          free(info);
          return -1;
      }

util/compatfd.c static int qemu_signalfd_compat(const sigset_t *mask) · line 102

Quoted 7 lines

util/compatfd.c +1 -1

@@ -94,7 +97,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
      return fds[0];
  }
-int qemu_signalfd(const sigset_t *mask)
+int qemu_signalfd(const sigset_t *mask, Error **errp)
  {
  #if defined(CONFIG_SIGNALFD)
      int ret;

Extend the context:

Quoted 7 lines

        ret = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
        if (ret != -1) {
            qemu_set_cloexec(ret);
            return ret;
        }

This error path need error_setg() as well.

Thanks for the comment. :)
Like adding as below?

     if (ret != -1) {
         qemu_set_cloexec(ret);
         return ret;
+    } else {
+        error_setg(errp, "syscall SYS_signalfd failed: %s", strerror(errno));
     }
 #endif

    return qemu_signalfd_compat(mask, errp);

If yes, I'd like to confirm the logic:

Add a new Error parameter for vnc_display_init() to handle errors in its caller: vnc_init_func(), just like vnc_display_open() does. And let the call trace propagate the Error.

Besides, make vnc_start_worker_thread() return a bool to indicate whether it succeeds instead of returning nothing.

Signed-off-by Fei Li <[email protected]>

---
 include/ui/console.h |  2 +-
 ui/vnc-jobs.c        |  9 ++++++---
 ui/vnc-jobs.h        |  2 +-
 ui/vnc.c             | 12 +++++++++---
 4 files changed, 17 insertions(+), 8 deletions(-)

include/ui/console.h +1 -1

index fb969caf70..c17803c530 100644
@@ -453,7 +453,7 @@ void qemu_display_early_init(DisplayOptions *opts);
 void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
 
 /* vnc.c */
-void vnc_display_init(const char *id);
+void vnc_display_init(const char *id, Error **errp);
 void vnc_display_open(const char *id, Error **errp);
 void vnc_display_add_client(const char *id, int csock, bool skipauth);
 int vnc_display_password(const char *id, const char *password);

ui/vnc-jobs.c +1 -3

index 929391f85d..8807d7217c 100644
@@ -331,15 +331,18 @@ static bool vnc_worker_thread_running(void)
     return queue; /* Check global queue */
 }
 
-void vnc_start_worker_thread(void)
+bool vnc_start_worker_thread(Error **errp)
 {
     VncJobQueue *q;
 
-    if (vnc_worker_thread_running())
-        return ;

ui/vnc-jobs.c +0 -0

 }

ui/vnc-jobs.h +1 -1

index 59f66bcc35..14640593db 100644
@@ -37,7 +37,7 @@ void vnc_job_push(VncJob *job);
 void vnc_jobs_join(VncState *vs);
 
 void vnc_jobs_consume_buffer(VncState *vs);
-void vnc_start_worker_thread(void);
+bool vnc_start_worker_thread(Error **errp);
 
 /* Locks */
 static inline int vnc_trylock_display(VncDisplay *vd)

ui/vnc.c +9 -3

index ccb1335d86..f632635ea4 100644
@@ -3206,7 +3206,7 @@ static const DisplayChangeListenerOps dcl_ops = {
     .dpy_cursor_define    = vnc_dpy_cursor_define,
 };
 
-void vnc_display_init(const char *id)
+void vnc_display_init(const char *id, Error **errp)
 {
     VncDisplay *vd;
 

@@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id)
     vd->connections_limit = 32;
 
     qemu_mutex_init(&vd->mutex);
-    vnc_start_worker_thread();
+    if (!vnc_start_worker_thread(errp)) {
+        return;
+    }
 
     vd->dcl.ops = &dcl_ops;
     register_displaychangelistener(&vd->dcl);

@@ -4079,7 +4081,11 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
     char *id = (char *)qemu_opts_id(opts);
 
     assert(id);
-    vnc_display_init(id);
+    vnc_display_init(id, &local_err);
+    if (local_err) {
+        error_reportf_err(local_err, "Failed to init VNC server: ");
+        exit(1);
+    }
     vnc_display_open(id, &local_err);
     if (local_err != NULL) {
         error_reportf_err(local_err, "Failed to start VNC server: ");

Signature · 1 line

2.13.7

Quoted 47 lines · Fei Li

Add a new Error parameter for vnc_display_init() to handle errors in its caller: vnc_init_func(), just like vnc_display_open() does. And let the call trace propagate the Error.

Besides, make vnc_start_worker_thread() return a bool to indicate whether it succeeds instead of returning nothing.

Signed-off-by Fei Li <[email protected]>

---
  include/ui/console.h |  2 +-
  ui/vnc-jobs.c        |  9 ++++++---
  ui/vnc-jobs.h        |  2 +-
  ui/vnc.c             | 12 +++++++++---
  4 files changed, 17 insertions(+), 8 deletions(-)

include/ui/console.h +1 -1

index fb969caf70..c17803c530 100644
@@ -453,7 +453,7 @@ void qemu_display_early_init(DisplayOptions *opts);
  void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
 
  /* vnc.c */
-void vnc_display_init(const char *id);
+void vnc_display_init(const char *id, Error **errp);
  void vnc_display_open(const char *id, Error **errp);
  void vnc_display_add_client(const char *id, int csock, bool skipauth);
  int vnc_display_password(const char *id, const char *password);

ui/vnc-jobs.c +4 -3

index 929391f85d..8807d7217c 100644
@@ -331,15 +331,18 @@ static bool vnc_worker_thread_running(void)
      return queue; /* Check global queue */
  }
 
-void vnc_start_worker_thread(void)
+bool vnc_start_worker_thread(Error **errp)
  {
      VncJobQueue *q;
 
-    if (vnc_worker_thread_running())
-        return ;
+    if (vnc_worker_thread_running()) {
+        goto out;
+    }
 

ui/vnc-jobs.c static bool vnc_worker_thread_running(void) · line 347

Quoted 6 lines

ui/vnc-jobs.c +2 -0

      q = vnc_queue_init();
      qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
                         QEMU_THREAD_DETACHED);
      queue = q; /* Set global queue */
+out:
+    return true;

This function always returns true?

Quoted 55 lines

ui/vnc-jobs.c +0 -0

  }

ui/vnc-jobs.h +1 -1

index 59f66bcc35..14640593db 100644
@@ -37,7 +37,7 @@ void vnc_job_push(VncJob *job);
  void vnc_jobs_join(VncState *vs);
 
  void vnc_jobs_consume_buffer(VncState *vs);
-void vnc_start_worker_thread(void);
+bool vnc_start_worker_thread(Error **errp);
 
  /* Locks */
  static inline int vnc_trylock_display(VncDisplay *vd)

ui/vnc.c +9 -3

index ccb1335d86..f632635ea4 100644
@@ -3206,7 +3206,7 @@ static const DisplayChangeListenerOps dcl_ops = {
      .dpy_cursor_define    = vnc_dpy_cursor_define,
  };
 
-void vnc_display_init(const char *id)
+void vnc_display_init(const char *id, Error **errp)
  {
      VncDisplay *vd;
 

@@ -3236,7 +3236,9 @@ void vnc_display_init(const char *id)
      vd->connections_limit = 32;
 
      qemu_mutex_init(&vd->mutex);
-    vnc_start_worker_thread();
+    if (!vnc_start_worker_thread(errp)) {
+        return;
+    }
 
      vd->dcl.ops = &dcl_ops;
      register_displaychangelistener(&vd->dcl);

@@ -4079,7 +4081,11 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
      char *id = (char *)qemu_opts_id(opts);
 
      assert(id);
-    vnc_display_init(id);
+    vnc_display_init(id, &local_err);
+    if (local_err) {
+        error_reportf_err(local_err, "Failed to init VNC server: ");
+        exit(1);
+    }
      vnc_display_open(id, &local_err);
      if (local_err != NULL) {
          error_reportf_err(local_err, "Failed to start VNC server: ");

Signature · 1 line

2.13.7

Fam

Make qemu_thread_create() return a Boolean to indicate if it succeeds rather than failing with an error. And add an Error parameter to hold the error message and let the callers handle it.

Besides, directly return if thread->data is NULL to avoid the segmentation fault in qemu_thread_join in qemu-thread-win32.c.

Signed-off-by Fei Li <[email protected]>

---
 cpus.c                      | 45 +++++++++++++++++++++++++++--------------
 dump.c                      |  6 ++++--
 hw/misc/edu.c               |  6 ++++--
 hw/ppc/spapr_hcall.c        |  9 +++++++--
 hw/rdma/rdma_backend.c      |  3 ++-
 hw/usb/ccid-card-emulated.c | 13 ++++++++----
 include/qemu/thread.h       |  4 ++--
 io/task.c                   |  3 ++-
 iothread.c                  | 16 ++++++++++-----
 migration/migration.c       | 49 +++++++++++++++++++++++++++++++--------------
 migration/postcopy-ram.c    | 12 +++++++++--
 migration/ram.c             | 40 +++++++++++++++++++++++++++---------
 migration/savevm.c          | 11 +++++++---
 tests/atomic_add-bench.c    |  3 ++-
 tests/iothread.c            |  2 +-
 tests/qht-bench.c           |  3 ++-
 tests/rcutorture.c          |  3 ++-
 tests/test-aio.c            |  2 +-
 tests/test-rcu-list.c       |  3 ++-
 ui/vnc-jobs.c               |  8 ++++++--
 util/compatfd.c             |  7 +++++--
 util/oslib-posix.c          | 12 ++++++++---
 util/qemu-thread-posix.c    | 18 +++++++++++------
 util/qemu-thread-win32.c    | 15 +++++++++-----
 util/rcu.c                  |  3 ++-
 util/thread-pool.c          |  4 +++-
 26 files changed, 210 insertions(+), 90 deletions(-)

cpus.c +30 -15

index 1feb308123..40db5c378f 100644
@@ -1928,15 +1928,20 @@ static void qemu_tcg_init_vcpu(CPUState *cpu, Error **errp)
             snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
                  cpu->cpu_index);
 
-            qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn,
-                               cpu, QEMU_THREAD_JOINABLE);
+            if (!qemu_thread_create(cpu->thread, thread_name,
+                                    qemu_tcg_cpu_thread_fn, cpu,
+                                    QEMU_THREAD_JOINABLE, errp)) {
+                return;
+            }
 
         } else {
             /* share a single thread for all cpus with TCG */
             snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
-            qemu_thread_create(cpu->thread, thread_name,
-                               qemu_tcg_rr_cpu_thread_fn,
-                               cpu, QEMU_THREAD_JOINABLE);
+            if (!qemu_thread_create(cpu->thread, thread_name,
+                                    qemu_tcg_rr_cpu_thread_fn, cpu,
+                                    QEMU_THREAD_JOINABLE, errp)) {
+                return;
+            }
 
             single_tcg_halt_cond = cpu->halt_cond;
             single_tcg_cpu_thread = cpu->thread;

@@ -1964,8 +1969,10 @@ static void qemu_hax_start_vcpu(CPUState *cpu, Error **errp)
 
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
              cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
+                            cpu, QEMU_THREAD_JOINABLE, errp)) {
+        return;
+    }
 #ifdef _WIN32
     cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif

@@ -1980,8 +1987,10 @@ static void qemu_kvm_start_vcpu(CPUState *cpu, Error **errp)
     qemu_cond_init(cpu->halt_cond);
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
              cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
+                            cpu, QEMU_THREAD_JOINABLE, errp)) {
+        return;
+    }
 }
 
 static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp)

@@ -1998,8 +2007,10 @@ static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp)
 
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
              cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
+                            cpu, QEMU_THREAD_JOINABLE, errp)) {
+        return;
+    }
 }
 
 static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp)

@@ -2011,8 +2022,10 @@ static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp)
     qemu_cond_init(cpu->halt_cond);
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX",
              cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn,
+                            cpu, QEMU_THREAD_JOINABLE, errp)) {
+        return;
+    }
 #ifdef _WIN32
     cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif

@@ -2027,8 +2040,10 @@ static void qemu_dummy_start_vcpu(CPUState *cpu, Error **errp)
     qemu_cond_init(cpu->halt_cond);
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY",
              cpu->cpu_index);
-    qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn, cpu,
-                       QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn,
+                           cpu, QEMU_THREAD_JOINABLE, errp)) {
+        return;
+    }
 }
 
 bool qemu_init_vcpu(CPUState *cpu, Error **errp)

dump.c +4 -2

index 500b554523..4175b95d12 100644
@@ -2021,8 +2021,10 @@ void qmp_dump_guest_memory(bool paging, const char *file,
     if (detach_p) {
         /* detached dump */
         s->detached = true;
-        qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread,
-                           s, QEMU_THREAD_DETACHED);
+        if (!qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread,
+                                s, QEMU_THREAD_DETACHED, errp)) {
+            return;
+        }
     } else {
         /* sync dump */
         dump_process(s, errp);

hw/misc/edu.c +4 -2

index df26a4d046..2810192b1f 100644
@@ -354,8 +354,10 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
 
     qemu_mutex_init(&edu->thr_mutex);
     qemu_cond_init(&edu->thr_cond);
-    qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
-                       edu, QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
+                            edu, QEMU_THREAD_JOINABLE, errp)) {
+        return;
+    }
 
     memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu,
                     "edu-mmio", 1 * MiB);

hw/ppc/spapr_hcall.c +7 -2

index ae913d070f..86eb22ea55 100644
@@ -478,6 +478,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
     sPAPRPendingHPT *pending = spapr->pending_hpt;
     uint64_t current_ram_size;
     int rc;
+    Error *local_err = NULL;
 
     if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED) {
         return H_AUTHORITY;

@@ -538,8 +539,12 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
     pending->shift = shift;
     pending->ret = H_HARDWARE;
 
-    qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
-                       hpt_prepare_thread, pending, QEMU_THREAD_DETACHED);
+    if (!qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
+                            hpt_prepare_thread, pending,
+                            QEMU_THREAD_DETACHED, &local_err)) {
+        error_reportf_err(local_err, "Failed to create hpt_prepare_thread.");
+        return H_RESOURCE;
+    }
 
     spapr->pending_hpt = pending;
 

hw/rdma/rdma_backend.c +2 -1

index d7a4bbd91f..e7cbb0c368 100644
@@ -165,7 +165,8 @@ static void start_comp_thread(RdmaBackendDev *backend_dev)
              ibv_get_device_name(backend_dev->ib_dev));
     backend_dev->comp_thread.run = true;
     qemu_thread_create(&backend_dev->comp_thread.thread, thread_name,
-                       comp_handler_thread, backend_dev, QEMU_THREAD_DETACHED);
+                       comp_handler_thread, backend_dev,
+                       QEMU_THREAD_DETACHED, &error_abort);
 }
 
 void rdma_backend_register_comp_handler(void (*handler)(int status,

hw/usb/ccid-card-emulated.c +9 -4

index 5c8b3c9907..0d630c27db 100644
@@ -538,10 +538,15 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
         error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
         return;
     }
-    qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
-                       card, QEMU_THREAD_JOINABLE);
-    qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
-                       card, QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
+                            card, QEMU_THREAD_JOINABLE, errp)) {
+        return;
+    }
+    if (!qemu_thread_create(&card->apdu_thread_id, "ccid/apdu",
+                            handle_apdu_thread, card,
+                            QEMU_THREAD_JOINABLE, errp)) {
+        return;
+    }
 }
 
 static void emulated_unrealize(CCIDCardState *base, Error **errp)

include/qemu/thread.h +2 -2

index dacebcfff0..1fb84a07d2 100644
@@ -135,9 +135,9 @@ void qemu_event_reset(QemuEvent *ev);
 void qemu_event_wait(QemuEvent *ev);
 void qemu_event_destroy(QemuEvent *ev);
 
-void qemu_thread_create(QemuThread *thread, const char *name,
+bool qemu_thread_create(QemuThread *thread, const char *name,
                         void *(*start_routine)(void *),
-                        void *arg, int mode);
+                        void *arg, int mode, Error **errp);
 void *qemu_thread_join(QemuThread *thread);
 void qemu_thread_get_self(QemuThread *thread);
 bool qemu_thread_is_self(QemuThread *thread);

io/task.c +2 -1

index 2886a2c1bc..6d3a18ab80 100644
@@ -149,7 +149,8 @@ void qio_task_run_in_thread(QIOTask *task,
                        "io-task-worker",
                        qio_task_thread_worker,
                        data,
-                       QEMU_THREAD_DETACHED);
+                       QEMU_THREAD_DETACHED,
+                       &error_abort);
 }
 
 

iothread.c +11 -5

index aff1281257..5b2a1df36d 100644
@@ -161,9 +161,7 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
                                 &local_error);
     if (local_error) {
         error_propagate(errp, local_error);
-        aio_context_unref(iothread->ctx);
-        iothread->ctx = NULL;
-        return;
+        goto fail;
     }
 
     qemu_mutex_init(&iothread->init_done_lock);

@@ -175,8 +173,12 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
      */
     name = object_get_canonical_path_component(OBJECT(obj));
     thread_name = g_strdup_printf("IO %s", name);
-    qemu_thread_create(&iothread->thread, thread_name, iothread_run,
-                       iothread, QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(&iothread->thread, thread_name, iothread_run,
+                            iothread, QEMU_THREAD_JOINABLE, errp)) {
+        g_free(thread_name);
+        g_free(name);
+        goto fail;
+    }
     g_free(thread_name);
     g_free(name);
 

@@ -187,6 +189,10 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
                        &iothread->init_done_lock);
     }
     qemu_mutex_unlock(&iothread->init_done_lock);
+    return;
+fail:
+    aio_context_unref(iothread->ctx);
+    iothread->ctx = NULL;
 }
 
 typedef struct {

migration/migration.c +34 -15

index 4b316ec343..52e476a8f3 100644
@@ -388,6 +388,7 @@ static void process_incoming_migration_co(void *opaque)
     MigrationIncomingState *mis = migration_incoming_get_current();
     PostcopyState ps;
     int ret;
+    Error *local_err = NULL;
 
     assert(mis->from_src_file);
     mis->migration_incoming_co = qemu_coroutine_self();

@@ -420,8 +421,13 @@ static void process_incoming_migration_co(void *opaque)
 
     /* we get COLO info, and know if we are in COLO mode */
     if (!ret && migration_incoming_enable_colo()) {
-        qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
-             colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
+        if (!qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
+                                colo_process_incoming_thread, mis,
+                                QEMU_THREAD_JOINABLE, &local_err)) {
+            error_reportf_err(local_err, "Failed to create "
+                              "colo_process_incoming_thread.");
+            goto fail;
+        }
         mis->have_colo_incoming_thread = true;
         qemu_coroutine_yield();
 

@@ -430,20 +436,22 @@ static void process_incoming_migration_co(void *opaque)
     }
 
     if (ret < 0) {
-        Error *local_err = NULL;
-
-        migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
-                          MIGRATION_STATUS_FAILED);
         error_report("load of migration failed: %s", strerror(-ret));
-        qemu_fclose(mis->from_src_file);
-        if (multifd_load_cleanup(&local_err) != 0) {
-            error_report_err(local_err);
-        }
-        exit(EXIT_FAILURE);
+        goto fail;
     }
     mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
     qemu_bh_schedule(mis->bh);
     mis->migration_incoming_co = NULL;
+    return;
+fail:
+    local_err = NULL;
+    migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
+                      MIGRATION_STATUS_FAILED);
+    qemu_fclose(mis->from_src_file);
+    if (multifd_load_cleanup(&local_err) != 0) {
+        error_report_err(local_err);
+    }
+    exit(EXIT_FAILURE);
 }
 
 static void migration_incoming_setup(QEMUFile *f)

@@ -2288,6 +2296,7 @@ out:
 static int open_return_path_on_source(MigrationState *ms,
                                       bool create_thread)
 {
+    Error *local_err = NULL;
 
     ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
     if (!ms->rp_state.from_dst_file) {

@@ -2301,8 +2310,13 @@ static int open_return_path_on_source(MigrationState *ms,
         return 0;
     }
 
-    qemu_thread_create(&ms->rp_state.rp_thread, "return path",
-                       source_return_path_thread, ms, QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(&ms->rp_state.rp_thread, "return path",
+                            source_return_path_thread, ms,
+                            QEMU_THREAD_JOINABLE, &local_err)) {
+        error_reportf_err(local_err,
+                          "Failed to create source_return_path_thread.");
+        return -1;
+    }
 
     trace_open_return_path_on_source_continue();
 

@@ -3127,8 +3141,13 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
         migrate_fd_cleanup(s);
         return;
     }
-    qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
-                       QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(&s->thread, "live_migration", migration_thread,
+                            s, QEMU_THREAD_JOINABLE, &error_in)) {
+        error_reportf_err(error_in, "Failed to create migration_thread.");
+        migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
+        migrate_fd_cleanup(s);
+        return;
+    }
     s->migration_thread_running = true;
 }
 

migration/postcopy-ram.c +10 -2

index 853d8b32ca..ebc6213c5e 100644
@@ -1082,6 +1082,8 @@ retry:
 
 int postcopy_ram_enable_notify(MigrationIncomingState *mis)
 {
+    Error *local_err = NULL;
+
     /* Open the fd for the kernel to give us userfaults */
     mis->userfault_fd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
     if (mis->userfault_fd == -1) {

@@ -1108,8 +1110,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
     }
 
     qemu_sem_init(&mis->fault_thread_sem, 0);
-    qemu_thread_create(&mis->fault_thread, "postcopy/fault",
-                       postcopy_ram_fault_thread, mis, QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(&mis->fault_thread, "postcopy/fault",
+                            postcopy_ram_fault_thread, mis,
+                            QEMU_THREAD_JOINABLE, &local_err)) {
+        error_reportf_err(local_err,
+                          "Failed to create postcopy_ram_fault_thread.");
+        qemu_sem_destroy(&mis->fault_thread_sem);
+        return -1;
+    }
     qemu_sem_wait(&mis->fault_thread_sem);
     qemu_sem_destroy(&mis->fault_thread_sem);
     mis->have_fault_thread = true;

migration/ram.c +30 -10

index 79c89425a3..7bf7901d9c 100644
@@ -470,6 +470,7 @@ static void compress_threads_save_cleanup(void)
 static int compress_threads_save_setup(void)
 {
     int i, thread_count;
+    Error *local_err = NULL;
 
     if (!migrate_use_compression()) {
         return 0;

@@ -499,9 +500,12 @@ static int compress_threads_save_setup(void)
         comp_param[i].quit = false;
         qemu_mutex_init(&comp_param[i].mutex);
         qemu_cond_init(&comp_param[i].cond);
-        qemu_thread_create(compress_threads + i, "compress",
-                           do_data_compress, comp_param + i,
-                           QEMU_THREAD_JOINABLE);
+        if (!qemu_thread_create(compress_threads + i, "compress",
+                                do_data_compress, comp_param + i,
+                                QEMU_THREAD_JOINABLE, &local_err)) {
+            error_reportf_err(local_err, "Failed to create do_data_compress.");
+            goto exit;
+        }
     }
     return 0;
 

@@ -1075,8 +1079,15 @@ static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
         p->c = QIO_CHANNEL(sioc);
         qio_channel_set_delay(p->c, false);
         p->running = true;
-        qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
-                           QEMU_THREAD_JOINABLE);
+        if (!qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
+                                QEMU_THREAD_JOINABLE, &local_err)) {
+            error_reportf_err(local_err,
+                              "Failed to create multifd_send_thread.");
+            if (multifd_save_cleanup(&local_err) != 0) {
+                migrate_set_error(migrate_get_current(), local_err);
+            }
+            return;
+        }
 
         atomic_inc(&multifd_send_state->count);
     }

@@ -1345,8 +1356,12 @@ bool multifd_recv_new_channel(QIOChannel *ioc)
     p->num_packets = 1;
 
     p->running = true;
-    qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
-                       QEMU_THREAD_JOINABLE);
+    if (!qemu_thread_create(&p->thread, p->name, multifd_recv_thread,
+                            p, QEMU_THREAD_JOINABLE, &local_err)) {
+        error_reportf_err(local_err, "Failed to create multifd_recv_thread.");
+        multifd_recv_terminate_threads(local_err);
+        return false;
+    }
     atomic_inc(&multifd_recv_state->count);
     return multifd_recv_state->count == migrate_multifd_channels();
 }

@@ -3542,6 +3557,7 @@ static void compress_threads_load_cleanup(void)
 static int compress_threads_load_setup(QEMUFile *f)
 {
     int i, thread_count;
+    Error *local_err = NULL;
 
     if (!migrate_use_compression()) {
         return 0;

@@ -3563,9 +3579,13 @@ static int compress_threads_load_setup(QEMUFile *f)
         qemu_cond_init(&decomp_param[i].cond);
         decomp_param[i].done = true;
         decomp_param[i].quit = false;
-        qemu_thread_create(decompress_threads + i, "decompress",
-                           do_data_decompress, decomp_param + i,
-                           QEMU_THREAD_JOINABLE);
+        if (!qemu_thread_create(decompress_threads + i, "decompress",
+                                do_data_decompress, decomp_param + i,
+                                QEMU_THREAD_JOINABLE, &local_err)) {
+            error_reportf_err(local_err,
+                              "Failed to create do_data_decompress.");
+            goto exit;
+        }
     }
     return 0;
 exit:

migration/savevm.c +8 -3

index 13e51f0e34..f35a1c5f01 100644
@@ -1727,9 +1727,14 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
     mis->have_listen_thread = true;
     /* Start up the listening thread and wait for it to signal ready */
     qemu_sem_init(&mis->listen_thread_sem, 0);
-    qemu_thread_create(&mis->listen_thread, "postcopy/listen",
-                       postcopy_ram_listen_thread, NULL,
-                       QEMU_THREAD_DETACHED);
+    if (!qemu_thread_create(&mis->listen_thread, "postcopy/listen",
+                            postcopy_ram_listen_thread, NULL,
+                            QEMU_THREAD_DETACHED, &local_err)) {
+        error_reportf_err(local_err,
+                          "Failed to create postcopy_ram_listen_thread.");
+        qemu_sem_destroy(&mis->listen_thread_sem);
+        return -1;
+    }
     qemu_sem_wait(&mis->listen_thread_sem);
     qemu_sem_destroy(&mis->listen_thread_sem);
 

tests/atomic_add-bench.c +2 -1

index 2f6c72f63a..338b9563e3 100644
@@ -2,6 +2,7 @@
 #include "qemu/thread.h"
 #include "qemu/host-utils.h"
 #include "qemu/processor.h"
+#include "qapi/error.h"
 
 struct thread_info {
     uint64_t r;

@@ -110,7 +111,7 @@ static void create_threads(void)
 
         info->r = (i + 1) ^ time(NULL);
         qemu_thread_create(&threads[i], NULL, thread_func, info,
-                           QEMU_THREAD_JOINABLE);
+                           QEMU_THREAD_JOINABLE, &error_abort);
     }
 }
 

tests/iothread.c +1 -1

index 777d9eea46..f4ad992e61 100644
@@ -73,7 +73,7 @@ IOThread *iothread_new(void)
     qemu_mutex_init(&iothread->init_done_lock);
     qemu_cond_init(&iothread->init_done_cond);
     qemu_thread_create(&iothread->thread, NULL, iothread_run,
-                       iothread, QEMU_THREAD_JOINABLE);
+                       iothread, QEMU_THREAD_JOINABLE, &error_abort);
 
     /* Wait for initialization to complete */
     qemu_mutex_lock(&iothread->init_done_lock);

tests/qht-bench.c +2 -1

index f492b3a20a..20a4101a17 100644
@@ -9,6 +9,7 @@
 #include "qemu/atomic.h"
 #include "qemu/qht.h"
 #include "qemu/rcu.h"
+#include "qapi/error.h"
 #include "exec/tb-hash-xx.h"
 
 struct thread_stats {

@@ -239,7 +240,7 @@ th_create_n(QemuThread **threads, struct thread_info **infos, const char *name,
         prepare_thread_info(&info[i], offset + i);
         info[i].func = func;
         qemu_thread_create(&th[i], name, thread_func, &info[i],
-                           QEMU_THREAD_JOINABLE);
+                           QEMU_THREAD_JOINABLE, &error_abort);
     }
 }
 

tests/rcutorture.c +2 -1

index 49311c82ea..0e799ff256 100644
@@ -64,6 +64,7 @@
 #include "qemu/atomic.h"
 #include "qemu/rcu.h"
 #include "qemu/thread.h"
+#include "qapi/error.h"
 
 long long n_reads = 0LL;
 long n_updates = 0L;

@@ -90,7 +91,7 @@ static void create_thread(void *(*func)(void *))
         exit(-1);
     }
     qemu_thread_create(&threads[n_threads], "test", func, &data[n_threads],
-                       QEMU_THREAD_JOINABLE);
+                       QEMU_THREAD_JOINABLE, &error_abort);
     n_threads++;
 }
 

tests/test-aio.c +1 -1

index 86fb73b3d5..b3ac261724 100644
@@ -154,7 +154,7 @@ static void test_acquire(void)
 
     qemu_thread_create(&thread, "test_acquire_thread",
                        test_acquire_thread,
-                       &data, QEMU_THREAD_JOINABLE);
+                       &data, QEMU_THREAD_JOINABLE, &error_abort);
 
     /* Block in aio_poll(), let other thread kick us and acquire context */
     aio_context_acquire(ctx);

tests/test-rcu-list.c +2 -1

index 192bfbf02e..9ea35a3dad 100644
@@ -25,6 +25,7 @@
 #include "qemu/rcu.h"
 #include "qemu/thread.h"
 #include "qemu/rcu_queue.h"
+#include "qapi/error.h"
 
 /*
  * Test variables.

@@ -68,7 +69,7 @@ static void create_thread(void *(*func)(void *))
         exit(-1);
     }
     qemu_thread_create(&threads[n_threads], "test", func, &data[n_threads],
-                       QEMU_THREAD_JOINABLE);
+                       QEMU_THREAD_JOINABLE, &error_abort);
     n_threads++;
 }
 

ui/vnc-jobs.c +6 -2

index 8807d7217c..35a652d1fd 100644
@@ -31,6 +31,7 @@
 #include "vnc-jobs.h"
 #include "qemu/sockets.h"
 #include "qemu/main-loop.h"
+#include "qapi/error.h"
 #include "block/aio.h"
 
 /*

@@ -340,8 +341,11 @@ bool vnc_start_worker_thread(Error **errp)
     }
 
     q = vnc_queue_init();
-    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
-                       QEMU_THREAD_DETACHED);
+    if (!qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread,
+                            q, QEMU_THREAD_DETACHED, errp)) {
+        vnc_queue_clear(q);
+        return false;
+    }
     queue = q; /* Set global queue */
 out:
     return true;

util/compatfd.c +0 -0

index d3ed890405..2532fa4d4d 100644

util/compatfd.c +2 -0

+        return -1;
+    }
 
     return fds[0];
 }

util/oslib-posix.c +1 -0

index 13b6f8d776..f34ba7ac9a 100644
@@ -364,6 +364,7 @@ static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
     size_t size_per_thread;
     char *addr = area;
     int i = 0;
+    Error *local_err = NULL;
 
     memset_thread_failed = false;
     memset_num_threads = get_memset_num_threads(smp_cpus);

@@ -375,15 +376,20 @@ static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
         memset_thread[i].numpages = (i == (memset_num_threads - 1)) ?
                                     numpages : numpages_per_thread;

util/oslib-posix.c +0 -0

     g_free(memset_thread);
     memset_thread = NULL;
 

util/qemu-thread-posix.c +5 -6

index dfa66ff2fb..8e493bd653 100644
@@ -15,6 +15,7 @@
 #include "qemu/atomic.h"
 #include "qemu/notify.h"
 #include "qemu-thread-common.h"
+#include "qapi/error.h"
 
 static bool name_threads;
 

@@ -504,9 +505,9 @@ static void *qemu_thread_start(void *args)
     return start_routine(arg);
 }
 
-void qemu_thread_create(QemuThread *thread, const char *name,
-                       void *(*start_routine)(void*),
-                       void *arg, int mode)
+bool qemu_thread_create(QemuThread *thread, const char *name,
+                        void *(*start_routine)(void *),
+                        void *arg, int mode, Error **errp)
 {
     sigset_t set, oldset;
     int err;

@@ -515,7 +516,7 @@ void qemu_thread_create(QemuThread *thread, const char *name,
 
     err = pthread_attr_init(&attr);
     if (err) {
-        error_exit(err, __func__);
+        goto fail;
     }
 
     if (mode == QEMU_THREAD_DETACHED) {

@@ -534,12 +535,17 @@ void qemu_thread_create(QemuThread *thread, const char *name,
     err = pthread_create(&thread->thread, &attr,
                          qemu_thread_start, qemu_thread_args);
 
-    if (err)
-        error_exit(err, __func__);

util/qemu-thread-posix.c +1 -0

+    return false;
 }
 
 void qemu_thread_get_self(QemuThread *thread)

util/qemu-thread-win32.c +1 -0

index 4a363ca675..178a69ed40 100644
@@ -20,6 +20,7 @@
 #include "qemu/thread.h"
 #include "qemu/notify.h"
 #include "qemu-thread-common.h"
+#include "qapi/error.h"
 #include <process.h>
 
 static bool name_threads;

util/qemu-thread-win32.c +3 -3

@@ -388,9 +389,9 @@ void *qemu_thread_join(QemuThread *thread)
     return ret;
 }
 
-void qemu_thread_create(QemuThread *thread, const char *name,
-                       void *(*start_routine)(void *),
-                       void *arg, int mode)
+bool qemu_thread_create(QemuThread *thread, const char *name,
+                        void *(*start_routine)(void *),
+                        void *arg, int mode, Error **errp)
 {
     HANDLE hThread;
     struct QemuThreadData *data;

util/qemu-thread-win32.c +2 -0

+        return false;
     }
     CloseHandle(hThread);
     thread->data = data;
+    return true;
 }
 
 void qemu_thread_get_self(QemuThread *thread)

util/rcu.c +2 -1

index 5676c22bd1..145dcdb0c6 100644
@@ -32,6 +32,7 @@
 #include "qemu/atomic.h"
 #include "qemu/thread.h"
 #include "qemu/main-loop.h"
+#include "qapi/error.h"
 #if defined(CONFIG_MALLOC_TRIM)
 #include <malloc.h>
 #endif

@@ -325,7 +326,7 @@ static void rcu_init_complete(void)
      * must have been quiescent even after forking, just recreate it.
      */
     qemu_thread_create(&thread, "call_rcu", call_rcu_thread,
-                       NULL, QEMU_THREAD_DETACHED);
+                       NULL, QEMU_THREAD_DETACHED, &error_abort);
 
     rcu_register_thread();
 }

util/thread-pool.c +3 -1

index 610646d131..ad0f980783 100644
@@ -22,6 +22,7 @@
 #include "trace.h"
 #include "block/thread-pool.h"
 #include "qemu/main-loop.h"
+#include "qapi/error.h"
 
 static void do_spawn_thread(ThreadPool *pool);
 

@@ -132,7 +133,8 @@ static void do_spawn_thread(ThreadPool *pool)
     pool->new_threads--;
     pool->pending_threads++;
 
-    qemu_thread_create(&t, "worker", worker_thread, pool, QEMU_THREAD_DETACHED);
+    qemu_thread_create(&t, "worker", worker_thread, pool,
+                       QEMU_THREAD_DETACHED, &error_abort);
 }
 
 static void spawn_thread_bh_fn(void *opaque)

Signature · 1 line

2.13.7

Quoted 6 lines · Fei Li

Make qemu_thread_create() return a Boolean to indicate if it succeeds rather than failing with an error. And add an Error parameter to hold the error message and let the callers handle it.

Besides, directly return if thread->data is NULL to avoid the segmentation fault in qemu_thread_join in qemu-thread-win32.c.

Please pay special attention to cleaning up code of the touched functions since you are adding new error paths to them. I haven't reviewed the full patch after pointing out a couple two places. Please audit the resource cleaning up more carefully in v3.

Quoted 31 lines

Signed-off-by Fei Li <[email protected]>

---
  cpus.c                      | 45 +++++++++++++++++++++++++++--------------
  dump.c                      |  6 ++++--
  hw/misc/edu.c               |  6 ++++--
  hw/ppc/spapr_hcall.c        |  9 +++++++--
  hw/rdma/rdma_backend.c      |  3 ++-
  hw/usb/ccid-card-emulated.c | 13 ++++++++----
  include/qemu/thread.h       |  4 ++--
  io/task.c                   |  3 ++-
  iothread.c                  | 16 ++++++++++-----
  migration/migration.c       | 49 +++++++++++++++++++++++++++++++--------------
  migration/postcopy-ram.c    | 12 +++++++++--
  migration/ram.c             | 40 +++++++++++++++++++++++++++---------
  migration/savevm.c          | 11 +++++++---
  tests/atomic_add-bench.c    |  3 ++-
  tests/iothread.c            |  2 +-
  tests/qht-bench.c           |  3 ++-
  tests/rcutorture.c          |  3 ++-
  tests/test-aio.c            |  2 +-
  tests/test-rcu-list.c       |  3 ++-
  ui/vnc-jobs.c               |  8 ++++++--
  util/compatfd.c             |  7 +++++--
  util/oslib-posix.c          | 12 ++++++++---
  util/qemu-thread-posix.c    | 18 +++++++++++------
  util/qemu-thread-win32.c    | 15 +++++++++-----
  util/rcu.c                  |  3 ++-
  util/thread-pool.c          |  4 +++-
  26 files changed, 210 insertions(+), 90 deletions(-)

Quoted 9 lines

      return true;

util/compatfd.c +0 -0

index d3ed890405..2532fa4d4d 100644
@@ -91,8 +91,11 @@ static int qemu_signalfd_compat(const sigset_t *mask, Error **errp)
      memcpy(&info->mask, mask, sizeof(*mask));
      info->fd = fds[1];
 

util/compatfd.c static int qemu_signalfd_compat(const sigset_t *mask, Error **errp) · line 96

Quoted 5 lines

util/compatfd.c +3 -2

-    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat, info,
-                       QEMU_THREAD_DETACHED);
+    if (!qemu_thread_create(&thread, "signalfd_compat", sigwait_compat,
+                            info, QEMU_THREAD_DETACHED, errp)) {
+        free(info);

Clean up fds?

Quoted 21 lines

util/compatfd.c +2 -0

+        return -1;
+    }
 
      return fds[0];
  }

util/oslib-posix.c +1 -0

index 13b6f8d776..f34ba7ac9a 100644
@@ -364,6 +364,7 @@ static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
      size_t size_per_thread;
      char *addr = area;
      int i = 0;
+    Error *local_err = NULL;
 
      memset_thread_failed = false;
      memset_num_threads = get_memset_num_threads(smp_cpus);

@@ -375,15 +376,20 @@ static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
          memset_thread[i].numpages = (i == (memset_num_threads - 1)) ?
                                      numpages : numpages_per_thread;

util/oslib-posix.c static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages, · line 384

Quoted 10 lines

util/oslib-posix.c +6 -3

          memset_thread[i].hpagesize = hpagesize;
-        qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
-                           do_touch_pages, &memset_thread[i],
-                           QEMU_THREAD_JOINABLE);
+        if (!qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
+                                do_touch_pages, &memset_thread[i],
+                                QEMU_THREAD_JOINABLE, &local_err)) {
+            error_reportf_err(local_err, "Failed to create do_touch_pages.");
+            memset_thread_failed = true;
+            goto out;

Before freeing memset_thread, you need to join all spawned threads. Which means you have to remember the current value of i (as 'started_thread') here and ...

util/oslib-posix.c static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages, · line 388

Quoted 4 lines

util/oslib-posix.c +1 -0

+        }
          addr += size_per_thread;
          numpages -= numpages_per_thread;
      }

the 'out' label should be here, and the loop condition should be changed to 'i < started_thread'.

util/oslib-posix.c static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages, · line 392

Quoted 4 lines

util/oslib-posix.c +1 -0

      for (i = 0; i < memset_num_threads; i++) {
          qemu_thread_join(&memset_thread[i].pgthread);
      }
+out:

Otherwise the threads can cause use-after-free.

Quoted 48 lines

util/oslib-posix.c +0 -0

      g_free(memset_thread);
      memset_thread = NULL;
 

util/qemu-thread-posix.c +8 -6

index dfa66ff2fb..8e493bd653 100644
@@ -15,6 +15,7 @@
  #include "qemu/atomic.h"
  #include "qemu/notify.h"
  #include "qemu-thread-common.h"
+#include "qapi/error.h"
 
  static bool name_threads;
 

@@ -504,9 +505,9 @@ static void *qemu_thread_start(void *args)
      return start_routine(arg);
  }
 
-void qemu_thread_create(QemuThread *thread, const char *name,
-                       void *(*start_routine)(void*),
-                       void *arg, int mode)
+bool qemu_thread_create(QemuThread *thread, const char *name,
+                        void *(*start_routine)(void *),
+                        void *arg, int mode, Error **errp)
  {
      sigset_t set, oldset;
      int err;

@@ -515,7 +516,7 @@ void qemu_thread_create(QemuThread *thread, const char *name,
 
      err = pthread_attr_init(&attr);
      if (err) {
-        error_exit(err, __func__);
+        goto fail;
      }
 
      if (mode == QEMU_THREAD_DETACHED) {

@@ -534,12 +535,17 @@ void qemu_thread_create(QemuThread *thread, const char *name,
      err = pthread_create(&thread->thread, &attr,
                           qemu_thread_start, qemu_thread_args);
 
-    if (err)
-        error_exit(err, __func__);
+    if (err) {
+        goto fail;
+    }
 

util/qemu-thread-posix.c void qemu_thread_create(QemuThread *thread, const char *name, · line 547

Quoted 6 lines

util/qemu-thread-posix.c +3 -0

      pthread_sigmask(SIG_SETMASK, &oldset, NULL);
 
      pthread_attr_destroy(&attr);
+    return true;
+fail:
+    error_setg(errp, "qemu_thread_create failed: %s", strerror(err));

It's better to do error_setg before 'goto fail', so that different error conditions have different error descriptions.

Quoted 20 lines

util/qemu-thread-posix.c +1 -0

+    return false;
  }
 
  void qemu_thread_get_self(QemuThread *thread)

util/qemu-thread-win32.c +1 -0

index 4a363ca675..178a69ed40 100644
@@ -20,6 +20,7 @@
  #include "qemu/thread.h"
  #include "qemu/notify.h"
  #include "qemu-thread-common.h"
+#include "qapi/error.h"
  #include <process.h>
 
  static bool name_threads;

@@ -366,7 +367,7 @@ void *qemu_thread_join(QemuThread *thread)
      HANDLE handle;
 

util/qemu-thread-win32.c void *qemu_thread_join(QemuThread *thread) · line 373

Quoted 6 lines

util/qemu-thread-win32.c +1 -1

      data = thread->data;
-    if (data->mode == QEMU_THREAD_DETACHED) {
+    if (data == NULL || data->mode == QEMU_THREAD_DETACHED) {
          return NULL;
      }
 

I think this could be a separate patch.

Quoted 15 lines

util/qemu-thread-win32.c +3 -3

@@ -388,9 +389,9 @@ void *qemu_thread_join(QemuThread *thread)
      return ret;
  }
 
-void qemu_thread_create(QemuThread *thread, const char *name,
-                       void *(*start_routine)(void *),
-                       void *arg, int mode)
+bool qemu_thread_create(QemuThread *thread, const char *name,
+                        void *(*start_routine)(void *),
+                        void *arg, int mode, Error **errp)
  {
      HANDLE hThread;
      struct QemuThreadData *data;

@@ -409,10 +410,14 @@ void qemu_thread_create(QemuThread *thread, const char *name,

util/qemu-thread-win32.c void qemu_thread_create(QemuThread *thread, const char *name, · line 415

Quoted 7 lines

util/qemu-thread-win32.c +3 -1

      hThread = (HANDLE) _beginthreadex(NULL, 0, win32_start_routine,
                                        data, 0, &thread->tid);
      if (!hThread) {
-        error_exit(GetLastError(), __func__);
+        g_free(data);
+        error_setg_win32(errp, GetLastError(),
+                         "failed to creat win32_start_routine");

"create".

Quoted 55 lines

util/qemu-thread-win32.c +2 -0

+        return false;
      }
      CloseHandle(hThread);
      thread->data = data;
+    return true;
  }
 
  void qemu_thread_get_self(QemuThread *thread)

util/rcu.c +2 -1

index 5676c22bd1..145dcdb0c6 100644
@@ -32,6 +32,7 @@
  #include "qemu/atomic.h"
  #include "qemu/thread.h"
  #include "qemu/main-loop.h"
+#include "qapi/error.h"
  #if defined(CONFIG_MALLOC_TRIM)
  #include <malloc.h>
  #endif

@@ -325,7 +326,7 @@ static void rcu_init_complete(void)
       * must have been quiescent even after forking, just recreate it.
       */
      qemu_thread_create(&thread, "call_rcu", call_rcu_thread,
-                       NULL, QEMU_THREAD_DETACHED);
+                       NULL, QEMU_THREAD_DETACHED, &error_abort);
 
      rcu_register_thread();
  }

util/thread-pool.c +3 -1

index 610646d131..ad0f980783 100644
@@ -22,6 +22,7 @@
  #include "trace.h"
  #include "block/thread-pool.h"
  #include "qemu/main-loop.h"
+#include "qapi/error.h"
 
  static void do_spawn_thread(ThreadPool *pool);
 

@@ -132,7 +133,8 @@ static void do_spawn_thread(ThreadPool *pool)
      pool->new_threads--;
      pool->pending_threads++;
 
-    qemu_thread_create(&t, "worker", worker_thread, pool, QEMU_THREAD_DETACHED);
+    qemu_thread_create(&t, "worker", worker_thread, pool,
+                       QEMU_THREAD_DETACHED, &error_abort);
  }
 
  static void spawn_thread_bh_fn(void *opaque)

Signature · 1 line

2.13.7