qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle
@ 2019-07-17  2:32 Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 01/11] qemu_thread: make qemu_thread_create() take Error ** argument Fei Li
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:32 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Markus Armbruster, Peter Xu, Dr . David Alan Gilbert,
	Gerd Hoffmann, David Gibson

Hi,

This idea comes from BiteSizedTasks, and this patch series implement
the error checking of qemu_thread_create: make qemu_thread_create
return a flag to indicate if it succeeded rather than failing with
an error; make all callers check it.

The first patch modifies the qemu_thread_create() by passing
&error_abort and returing a value to indicate if it succeeds. The next
10 patches will improve on &error_abort for callers who could handle
more properly.

Please help to review, thanks a lot! 

v14:
- For patch 1/11, complement qemu_thread_create(xxx, &error_abort) for all 
  corresponding files, esp for files under tests/.

v13:
- Add one Acked-by for patch 2/11.
- Resend as the last version was divided into several parts. 

v12:
- For patch 6/11, make event_thread terminate by stopping vevent
  thread when failing to create handle_apdu_thread.
- Rectify the commit message for patch 7/11, 8/11.
- For patch 9/11, change two cleanup sentences' order when failing
  to create multifd_recv_thread.
- For patch 11/11, fix the SIGBUS conflict for touch_all_pages().

v11:
- Resend as I sent the last version in a mess..

v10:
- Make qemu_thread_create() return -errno instead of a Boolean.
- Add more cleanup for pci_edu_realize()/emulated_realize(). 
- Polish for iothread_complete()/compress_threads_save_cleanup()/
  vnc_start_worker_thread()/touch_all_pages.
- Change to return H_HARDWARE for h_resize_hpt_prepare().
- Remove five derivative patches as they have been merged.

v9:
- To ease the review and involve the appropriate maintainers, split
  the previous 6/7 patch into 10 patches: the 6/16 patch passes
  the &error_abort to qemu_thread_create() everywhere, and the next
  9 patches will improve on &error_abort for callers who need.
- Add a new patch 5/7 to unify error handling for 
  process_incoming_migration_co().
- Merge the previous 2/7 to current 7/16 to collaboratively handle
  for qemu_X_start_vcpu and for the qemu_init_vpcu in each arch.
- Add comment for multifd_recv_new_channel() in current patch 2/7.

v8:
- Remove previous two patches trying to fix the multifd issue on the
  source side, as we are still waiting for maintainer's opinions.
- Use atomic_read to get multifd_recv_state->count in patch 3/7.
- Get three more "Reviewed-by:".

v7:
- Split the previous multifd-migration into two patches: the src and
  the dst. For the dst, only dump the error instead of quitting.
- Safely do the cleanup for postcopy_ram_enable_notify().
- Split the previous migration-error-handling patch into two patches.

v6:
- Add a new migration-multifd related patch. BTW, delete the previous
  vnc related patch as it has been upstreamed.
- Use error_setg_errno() to set the errno when qemu_thread_create()
  fails for both Linux and Windows implementation.
- Optimize the first patch, less codes are needed

v5:
- Remove `errno = err` in qemu_thread_create() for Linux, and change
  `return errno` to `return -1` in qemu_signal_init() to indicate
  the error in case qemu_thread_create() fails.
- Delete the v4-added qemu_cond/mutex_destroy() in iothread_complete()
  as the destroy() will be done by its callers' object_unref().

v4:
- Separate the migration compression patch from this series
- Add one more error handling patch related with migration
- Add more cleaning up code for touched functions

v3:
- Add two migration related patches to fix the segmentaion fault
- Extract the segmentation fault fix from v2's last patch to be a
  separate patch

v2:
- Pass errp straightly instead of using a local_err & error_propagate
- Return a bool: false/true to indicate if one function succeeds
- Merge v1's last two patches into one to avoid the compile error
- Fix one omitted error in patch1 and update some error messages


Fei Li (11):
  qemu_thread: make qemu_thread_create() take Error ** argument
  qemu_thread: supplement error handling for qemu_X_start_vcpu
  qemu_thread: supplement error handling for qmp_dump_guest_memory
  qemu_thread: supplement error handling for pci_edu_realize
  qemu_thread: supplement error handling for h_resize_hpt_prepare
  qemu_thread: supplement error handling for emulated_realize
  qemu_thread: supplement error handling for iothread_complete
  qemu_thread: supplement error handling for qemu_signalfd_compat
  qemu_thread: supplement error handling for migration
  qemu_thread: supplement error handling for vnc_start_worker_thread
  qemu_thread: supplement error handling for touch_all_pages

 accel/tcg/user-exec-stub.c      |  3 +-
 cpus.c                          | 69 +++++++++++++++++++++++++----------------
 dump/dump.c                     |  2 +-
 hw/misc/edu.c                   | 11 +++++--
 hw/ppc/spapr_hcall.c            | 10 ++++--
 hw/rdma/rdma_backend.c          |  3 +-
 hw/usb/ccid-card-emulated.c     | 16 +++++++---
 include/qemu/thread.h           |  6 ++--
 include/qom/cpu.h               |  2 +-
 io/task.c                       |  3 +-
 iothread.c                      | 17 +++++++---
 migration/migration.c           | 30 ++++++++++++++----
 migration/postcopy-ram.c        | 14 +++++++--
 migration/ram.c                 | 66 ++++++++++++++++++++++++++-------------
 migration/savevm.c              | 11 +++++--
 target/alpha/cpu.c              |  4 ++-
 target/arm/cpu.c                |  4 ++-
 target/cris/cpu.c               |  4 ++-
 target/hppa/cpu.c               |  4 ++-
 target/i386/cpu.c               |  4 ++-
 target/lm32/cpu.c               |  4 ++-
 target/m68k/cpu.c               |  4 ++-
 target/microblaze/cpu.c         |  4 ++-
 target/mips/cpu.c               |  4 ++-
 target/moxie/cpu.c              |  4 ++-
 target/nios2/cpu.c              |  4 ++-
 target/openrisc/cpu.c           |  4 ++-
 target/ppc/translate_init.inc.c |  4 ++-
 target/riscv/cpu.c              |  4 ++-
 target/s390x/cpu.c              |  4 ++-
 target/sh4/cpu.c                |  4 ++-
 target/sparc/cpu.c              |  4 ++-
 target/tilegx/cpu.c             |  4 ++-
 target/tricore/cpu.c            |  4 ++-
 target/unicore32/cpu.c          |  4 ++-
 target/xtensa/cpu.c             |  4 ++-
 tests/atomic64-bench.c          |  3 +-
 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-char.c               |  6 ++--
 tests/test-rcu-list.c           |  3 +-
 tests/test-util-filemonitor.c   |  2 +-
 ui/cocoa.m                      |  2 +-
 ui/vnc-jobs.c                   | 17 +++++++---
 ui/vnc-jobs.h                   |  2 +-
 ui/vnc.c                        |  4 ++-
 util/compatfd.c                 | 13 ++++++--
 util/oslib-posix.c              | 34 +++++++++++++-------
 util/qemu-thread-posix.c        | 30 +++++++++++++-----
 util/qemu-thread-win32.c        | 13 ++++++--
 util/rcu.c                      |  3 +-
 util/thread-pool.c              |  4 ++-
 55 files changed, 351 insertions(+), 145 deletions(-)

-- 
2.11.0



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 01/11] qemu_thread: make qemu_thread_create() take Error ** argument
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  5:53   ` David Gibson
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 02/11] qemu_thread: supplement error handling for qemu_X_start_vcpu Fei Li
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Markus Armbruster, Peter Xu, Dr . David Alan Gilbert, Fei Li,
	Gerd Hoffmann, Paolo Bonzini, David Gibson

From: Fei Li <fli@suse.com>

qemu_thread_create() abort()s on error. Not nice. Give it a return
value and an Error ** argument, so it can return success/failure.

Considering qemu_thread_create() is quite widely used in qemu, split
this into two steps: this patch passes the &error_abort to
qemu_thread_create() everywhere, and the next 10 patches will improve
on &error_abort for callers who need.  To differentiate callers who
need the improvement, temporarily add the "TODO:" comment for them.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fei Li <fli@suse.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 cpus.c                        | 23 +++++++++++++++--------
 dump/dump.c                   |  3 ++-
 hw/misc/edu.c                 |  4 +++-
 hw/ppc/spapr_hcall.c          |  4 +++-
 hw/rdma/rdma_backend.c        |  3 ++-
 hw/usb/ccid-card-emulated.c   |  5 +++--
 include/qemu/thread.h         |  6 +++---
 io/task.c                     |  3 ++-
 iothread.c                    |  3 ++-
 migration/migration.c         | 11 ++++++++---
 migration/postcopy-ram.c      |  4 +++-
 migration/ram.c               | 12 ++++++++----
 migration/savevm.c            |  3 ++-
 tests/atomic64-bench.c        |  3 ++-
 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-char.c             |  6 +++---
 tests/test-rcu-list.c         |  3 ++-
 tests/test-util-filemonitor.c |  2 +-
 ui/cocoa.m                    |  2 +-
 ui/vnc-jobs.c                 |  6 ++++--
 util/compatfd.c               |  6 ++++--
 util/oslib-posix.c            |  3 ++-
 util/qemu-thread-posix.c      | 30 +++++++++++++++++++++++-------
 util/qemu-thread-win32.c      | 13 ++++++++++---
 util/rcu.c                    |  3 ++-
 util/thread-pool.c            |  4 +++-
 30 files changed, 121 insertions(+), 57 deletions(-)

diff --git a/cpus.c b/cpus.c
index 927a00aa90..28a241a936 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1971,15 +1971,17 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
             snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
                  cpu->cpu_index);
 
+            /* TODO: let the callers handle the error instead of abort() here */
             qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn,
-                               cpu, QEMU_THREAD_JOINABLE);
+                               cpu, QEMU_THREAD_JOINABLE, &error_abort);
 
         } else {
             /* share a single thread for all cpus with TCG */
             snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
+            /* TODO: let the callers handle the error instead of abort() here */
             qemu_thread_create(cpu->thread, thread_name,
                                qemu_tcg_rr_cpu_thread_fn,
-                               cpu, QEMU_THREAD_JOINABLE);
+                               cpu, QEMU_THREAD_JOINABLE, &error_abort);
 
             single_tcg_halt_cond = cpu->halt_cond;
             single_tcg_cpu_thread = cpu->thread;
@@ -2007,8 +2009,9 @@ static void qemu_hax_start_vcpu(CPUState *cpu)
 
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
              cpu->cpu_index);
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
+                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
 #ifdef _WIN32
     cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif
@@ -2023,8 +2026,9 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
     qemu_cond_init(cpu->halt_cond);
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
              cpu->cpu_index);
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
+                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
 }
 
 static void qemu_hvf_start_vcpu(CPUState *cpu)
@@ -2041,8 +2045,9 @@ static void qemu_hvf_start_vcpu(CPUState *cpu)
 
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
              cpu->cpu_index);
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
+                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
 }
 
 static void qemu_whpx_start_vcpu(CPUState *cpu)
@@ -2054,8 +2059,9 @@ static void qemu_whpx_start_vcpu(CPUState *cpu)
     qemu_cond_init(cpu->halt_cond);
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX",
              cpu->cpu_index);
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE);
+                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
 #ifdef _WIN32
     cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif
@@ -2070,8 +2076,9 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
     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);
+    /* TODO: let the further caller handle the error instead of abort() here */
+    qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn,
+                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
 }
 
 void qemu_init_vcpu(CPUState *cpu)
diff --git a/dump/dump.c b/dump/dump.c
index c7b2301652..1358e62fb7 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -2022,8 +2022,9 @@ void qmp_dump_guest_memory(bool paging, const char *file,
     if (detach_p) {
         /* detached dump */
         s->detached = true;
+        /* TODO: let the further caller handle the error instead of abort() */
         qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread,
-                           s, QEMU_THREAD_DETACHED);
+                           s, QEMU_THREAD_DETACHED, &error_abort);
     } else {
         /* sync dump */
         dump_process(s, errp);
diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 3b6dba2c72..1edc66955a 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -29,6 +29,7 @@
 #include "qemu/timer.h"
 #include "qemu/main-loop.h" /* iothread mutex */
 #include "qemu/module.h"
+#include "qapi/error.h"
 #include "qapi/visitor.h"
 
 #define TYPE_PCI_EDU_DEVICE "edu"
@@ -370,8 +371,9 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
 
     qemu_mutex_init(&edu->thr_mutex);
     qemu_cond_init(&edu->thr_cond);
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
-                       edu, QEMU_THREAD_JOINABLE);
+                       edu, QEMU_THREAD_JOINABLE, &error_abort);
 
     memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu,
                     "edu-mmio", 1 * MiB);
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6808d4cda8..d64b634526 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -508,8 +508,10 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
     pending->shift = shift;
     pending->ret = H_HARDWARE;
 
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
-                       hpt_prepare_thread, pending, QEMU_THREAD_DETACHED);
+                       hpt_prepare_thread, pending,
+                       QEMU_THREAD_DETACHED, &error_abort);
 
     spapr->pending_hpt = pending;
 
diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index c39051068d..29f52955d0 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -281,7 +281,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)(void *ctx,
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index e20f8ed293..4d95ee07aa 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -545,10 +545,11 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
         error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
         goto out2;
     }
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
-                       card, QEMU_THREAD_JOINABLE);
+                       card, QEMU_THREAD_JOINABLE, &error_abort);
     qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
-                       card, QEMU_THREAD_JOINABLE);
+                       card, QEMU_THREAD_JOINABLE, &error_abort);
 
     return;
 
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 55d83a907c..ec18d8b171 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -152,9 +152,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,
-                        void *(*start_routine)(void *),
-                        void *arg, int mode);
+int qemu_thread_create(QemuThread *thread, const char *name,
+                       void *(*start_routine)(void *),
+                       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);
diff --git a/io/task.c b/io/task.c
index 64c4c7126a..1499540cdf 100644
--- a/io/task.c
+++ b/io/task.c
@@ -170,7 +170,8 @@ void qio_task_run_in_thread(QIOTask *task,
                        "io-task-worker",
                        qio_task_thread_worker,
                        task,
-                       QEMU_THREAD_DETACHED);
+                       QEMU_THREAD_DETACHED,
+                       &error_abort);
 }
 
 
diff --git a/iothread.c b/iothread.c
index 7130be58e3..2d5a5bfe6c 100644
--- a/iothread.c
+++ b/iothread.c
@@ -198,8 +198,9 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
      */
     name = object_get_canonical_path_component(OBJECT(obj));
     thread_name = g_strdup_printf("IO %s", name);
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(&iothread->thread, thread_name, iothread_run,
-                       iothread, QEMU_THREAD_JOINABLE);
+                       iothread, QEMU_THREAD_JOINABLE, &error_abort);
     g_free(thread_name);
     g_free(name);
 
diff --git a/migration/migration.c b/migration/migration.c
index 8a607fe1e2..1786afb63d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -483,8 +483,10 @@ static void process_incoming_migration_co(void *opaque)
             goto fail;
         }
 
+        /* TODO: let the further caller handle the error instead of abort() */
         qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
-             colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
+                           colo_process_incoming_thread, mis,
+                           QEMU_THREAD_JOINABLE, &error_abort);
         mis->have_colo_incoming_thread = true;
         qemu_coroutine_yield();
 
@@ -2496,8 +2498,10 @@ static int open_return_path_on_source(MigrationState *ms,
         return 0;
     }
 
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(&ms->rp_state.rp_thread, "return path",
-                       source_return_path_thread, ms, QEMU_THREAD_JOINABLE);
+                       source_return_path_thread, ms,
+                       QEMU_THREAD_JOINABLE, &error_abort);
 
     trace_open_return_path_on_source_continue();
 
@@ -3342,8 +3346,9 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
         migrate_fd_cleanup(s);
         return;
     }
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
-                       QEMU_THREAD_JOINABLE);
+                       QEMU_THREAD_JOINABLE, &error_abort);
     s->migration_thread_running = true;
 }
 
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 9faacacc9e..135e9cd746 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1121,8 +1121,10 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
     }
 
     qemu_sem_init(&mis->fault_thread_sem, 0);
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(&mis->fault_thread, "postcopy/fault",
-                       postcopy_ram_fault_thread, mis, QEMU_THREAD_JOINABLE);
+                       postcopy_ram_fault_thread, mis,
+                       QEMU_THREAD_JOINABLE, &error_abort);
     qemu_sem_wait(&mis->fault_thread_sem);
     qemu_sem_destroy(&mis->fault_thread_sem);
     mis->have_fault_thread = true;
diff --git a/migration/ram.c b/migration/ram.c
index 2b0774c2bf..0f677f2d27 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -565,9 +565,10 @@ 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);
+        /* TODO: let the further caller handle the error instead of abort() */
         qemu_thread_create(compress_threads + i, "compress",
                            do_data_compress, comp_param + i,
-                           QEMU_THREAD_JOINABLE);
+                           QEMU_THREAD_JOINABLE, &error_abort);
     }
     return 0;
 
@@ -1167,8 +1168,9 @@ 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;
+        /* TODO: let the further caller handle the error instead of abort() */
         qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
-                           QEMU_THREAD_JOINABLE);
+                           QEMU_THREAD_JOINABLE, &error_abort);
     }
 }
 
@@ -1447,8 +1449,9 @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
     p->num_packets = 1;
 
     p->running = true;
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
-                       QEMU_THREAD_JOINABLE);
+                       QEMU_THREAD_JOINABLE, &error_abort);
     atomic_inc(&multifd_recv_state->count);
     return atomic_read(&multifd_recv_state->count) ==
            migrate_multifd_channels();
@@ -3891,9 +3894,10 @@ 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;
+        /* TODO: let the further caller handle the error instead of abort() */
         qemu_thread_create(decompress_threads + i, "decompress",
                            do_data_decompress, decomp_param + i,
-                           QEMU_THREAD_JOINABLE);
+                           QEMU_THREAD_JOINABLE, &error_abort);
     }
     return 0;
 exit:
diff --git a/migration/savevm.c b/migration/savevm.c
index 79ed44d475..c792dee8aa 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1826,9 +1826,10 @@ 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);
+    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(&mis->listen_thread, "postcopy/listen",
                        postcopy_ram_listen_thread, NULL,
-                       QEMU_THREAD_DETACHED);
+                       QEMU_THREAD_DETACHED, &error_abort);
     qemu_sem_wait(&mis->listen_thread_sem);
     qemu_sem_destroy(&mis->listen_thread_sem);
 
diff --git a/tests/atomic64-bench.c b/tests/atomic64-bench.c
index 121a8c14f4..08ec753582 100644
--- a/tests/atomic64-bench.c
+++ b/tests/atomic64-bench.c
@@ -8,6 +8,7 @@
 #include "qemu/thread.h"
 #include "qemu/host-utils.h"
 #include "qemu/processor.h"
+#include "qapi/error.h"
 
 struct thread_info {
     uint64_t r;
@@ -103,7 +104,7 @@ static void create_threads(void)
         info->r = (i + 1) ^ time(NULL);
         info->accesses = 0;
         qemu_thread_create(&threads[i], NULL, thread_func, info,
-                           QEMU_THREAD_JOINABLE);
+                           QEMU_THREAD_JOINABLE, &error_abort);
     }
 }
 
diff --git a/tests/atomic_add-bench.c b/tests/atomic_add-bench.c
index 5666f6bbff..52071585a9 100644
--- a/tests/atomic_add-bench.c
+++ b/tests/atomic_add-bench.c
@@ -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;
@@ -108,7 +109,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);
     }
 }
 
diff --git a/tests/iothread.c b/tests/iothread.c
index 777d9eea46..f4ad992e61 100644
--- a/tests/iothread.c
+++ b/tests/iothread.c
@@ -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);
diff --git a/tests/qht-bench.c b/tests/qht-bench.c
index e3b512f26f..728c2e06c7 100644
--- a/tests/qht-bench.c
+++ b/tests/qht-bench.c
@@ -10,6 +10,7 @@
 #include "qemu/qht.h"
 #include "qemu/rcu.h"
 #include "qemu/xxhash.h"
+#include "qapi/error.h"
 
 struct thread_stats {
     size_t rd;
@@ -248,7 +249,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);
     }
 }
 
diff --git a/tests/rcutorture.c b/tests/rcutorture.c
index 49311c82ea..0e799ff256 100644
--- a/tests/rcutorture.c
+++ b/tests/rcutorture.c
@@ -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++;
 }
 
diff --git a/tests/test-aio.c b/tests/test-aio.c
index 86fb73b3d5..b3ac261724 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -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);
diff --git a/tests/test-char.c b/tests/test-char.c
index f9440cdcfd..093e26e333 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -793,7 +793,7 @@ static void char_socket_server_test(gconstpointer opaque)
      */
     qemu_thread_create(&thread, "client",
                        char_socket_server_client_thread,
-                       addr, QEMU_THREAD_JOINABLE);
+                       addr, QEMU_THREAD_JOINABLE, &error_abort);
     g_assert(data.event == -1);
 
     if (config->wait_connected) {
@@ -901,7 +901,7 @@ static void char_socket_client_test(gconstpointer opaque)
      */
     qemu_thread_create(&thread, "client",
                        char_socket_client_server_thread,
-                       ioc, QEMU_THREAD_JOINABLE);
+                       ioc, QEMU_THREAD_JOINABLE, &error_abort);
 
     /*
      * Populate the chardev address based on what the server
@@ -994,7 +994,7 @@ static void char_socket_client_test(gconstpointer opaque)
         reconnected = true;
         qemu_thread_create(&thread, "client",
                            char_socket_client_server_thread,
-                           ioc, QEMU_THREAD_JOINABLE);
+                           ioc, QEMU_THREAD_JOINABLE, &error_abort);
         goto reconnect;
     }
 
diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
index 6f076473e0..399744f03c 100644
--- a/tests/test-rcu-list.c
+++ b/tests/test-rcu-list.c
@@ -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++;
 }
 
diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
index 46e781c022..a7a7c1eeeb 100644
--- a/tests/test-util-filemonitor.c
+++ b/tests/test-util-filemonitor.c
@@ -397,7 +397,7 @@ test_file_monitor_events(void)
     evstopping = 0;
     qemu_thread_create(&th, "event-loop",
                        qemu_file_monitor_test_event_loop, NULL,
-                       QEMU_THREAD_JOINABLE);
+                       QEMU_THREAD_JOINABLE, &error_abort);
 
     if (local_err) {
         g_printerr("File monitoring not available: %s",
diff --git a/ui/cocoa.m b/ui/cocoa.m
index c2984028c5..d8cf1484d9 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1766,7 +1766,7 @@ int main (int argc, const char * argv[]) {
     qemu_sem_init(&app_started_sem, 0);
 
     qemu_thread_create(&thread, "qemu_main", call_qemu_main,
-                       NULL, QEMU_THREAD_DETACHED);
+                       NULL, QEMU_THREAD_DETACHED, &error_abort);
 
     COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
     qemu_sem_wait(&display_init_sem);
diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 929391f85d..5712f1f501 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -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"
 
 /*
@@ -339,7 +340,8 @@ void vnc_start_worker_thread(void)
         return ;
 
     q = vnc_queue_init();
-    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
-                       QEMU_THREAD_DETACHED);
+    /* TODO: let the further caller handle the error instead of abort() here */
+    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread,
+                       q, QEMU_THREAD_DETACHED, &error_abort);
     queue = q; /* Set global queue */
 }
diff --git a/util/compatfd.c b/util/compatfd.c
index c296f55d14..caad10ae56 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -15,6 +15,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/thread.h"
+#include "qapi/error.h"
 
 #include <sys/syscall.h>
 
@@ -87,8 +88,9 @@ static int qemu_signalfd_compat(const sigset_t *mask)
     memcpy(&info->mask, mask, sizeof(*mask));
     info->fd = fds[1];
 
-    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat, info,
-                       QEMU_THREAD_DETACHED);
+    /* TODO: let the further caller handle the error instead of abort() here */
+    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat,
+                       info, QEMU_THREAD_DETACHED, &error_abort);
 
     return fds[0];
 }
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index fe0309ce7a..55c1b9c098 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -465,9 +465,10 @@ 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;
         memset_thread[i].hpagesize = hpagesize;
+        /* TODO: let the callers handle the error instead of abort() here */
         qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
                            do_touch_pages, &memset_thread[i],
-                           QEMU_THREAD_JOINABLE);
+                           QEMU_THREAD_JOINABLE, &error_abort);
         addr += size_per_thread;
         numpages -= numpages_per_thread;
     }
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 1bf5e65dea..06a9e78ba8 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -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,18 @@ static void *qemu_thread_start(void *args)
     return r;
 }
 
-void qemu_thread_create(QemuThread *thread, const char *name,
-                       void *(*start_routine)(void*),
-                       void *arg, int mode)
+/*
+ * Create a new thread with name @name
+ * The thread executes @start_routine() with argument @arg.
+ * The thread will be created in a detached state if @mode is
+ * QEMU_THREAD_DETACHED, and in a jounable state if it's
+ * QEMU_THREAD_JOINABLE.
+ * On success, return 0.
+ * On failure, store an error through @errp and return negative errno.
+ */
+int 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 +525,8 @@ void qemu_thread_create(QemuThread *thread, const char *name,
 
     err = pthread_attr_init(&attr);
     if (err) {
-        error_exit(err, __func__);
+        error_setg(errp, "pthread_attr_init failed");
+        return -err;
     }
 
     if (mode == QEMU_THREAD_DETACHED) {
@@ -538,13 +549,18 @@ 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) {
+        error_setg(errp, "pthread_create failed");
+        pthread_attr_destroy(&attr);
+        g_free(qemu_thread_args->name);
+        g_free(qemu_thread_args);
+        return -err;
+    }
 
     pthread_sigmask(SIG_SETMASK, &oldset, NULL);
 
     pthread_attr_destroy(&attr);
+    return 0;
 }
 
 void qemu_thread_get_self(QemuThread *thread)
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 572f88535d..f2deea5250 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -16,6 +16,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;
@@ -384,9 +385,9 @@ void *qemu_thread_join(QemuThread *thread)
     return ret;
 }
 
-void qemu_thread_create(QemuThread *thread, const char *name,
+int qemu_thread_create(QemuThread *thread, const char *name,
                        void *(*start_routine)(void *),
-                       void *arg, int mode)
+                       void *arg, int mode, Error **errp)
 {
     HANDLE hThread;
     struct QemuThreadData *data;
@@ -405,10 +406,16 @@ void qemu_thread_create(QemuThread *thread, const char *name,
     hThread = (HANDLE) _beginthreadex(NULL, 0, win32_start_routine,
                                       data, 0, &thread->tid);
     if (!hThread) {
-        error_exit(GetLastError(), __func__);
+        if (data->mode != QEMU_THREAD_DETACHED) {
+            DeleteCriticalSection(&data->cs);
+        }
+        error_setg(errp, "failed to create win32_start_routine");
+        g_free(data);
+        return -errno;
     }
     CloseHandle(hThread);
     thread->data = data;
+    return 0;
 }
 
 void qemu_thread_get_self(QemuThread *thread)
diff --git a/util/rcu.c b/util/rcu.c
index 177a675619..ff11837b47 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -31,6 +31,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
@@ -324,7 +325,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();
 }
diff --git a/util/thread-pool.c b/util/thread-pool.c
index 4ed9b89ab2..9a867776f9 100644
--- a/util/thread-pool.c
+++ b/util/thread-pool.c
@@ -21,6 +21,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);
 
@@ -131,7 +132,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)
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 02/11] qemu_thread: supplement error handling for qemu_X_start_vcpu
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 01/11] qemu_thread: make qemu_thread_create() take Error ** argument Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 03/11] qemu_thread: supplement error handling for qmp_dump_guest_memory Fei Li
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Markus Armbruster, Peter Xu, Dr . David Alan Gilbert, Fei Li,
	Gerd Hoffmann, Paolo Bonzini, David Gibson

From: Fei Li <fli@suse.com>

The callers of qemu_init_vcpu() already passed the **errp to handle
errors. In view of this, add a new Error parameter to qemu_init_vcpu()
and all qemu_X_start_vcpu() functions called by qemu_init_vcpu() to
propagate the error and let the further callers check it.

Besides, make qemu_init_vcpu() return a Boolean value to let its
callers know whether it succeeds.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fei Li <fli@suse.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
ppc parts:
Acked-by: David Gibson <david@gibson.dropbear.id.au>
---
 accel/tcg/user-exec-stub.c      |  3 +-
 cpus.c                          | 74 +++++++++++++++++++++++------------------
 include/qom/cpu.h               |  2 +-
 target/alpha/cpu.c              |  4 ++-
 target/arm/cpu.c                |  4 ++-
 target/cris/cpu.c               |  4 ++-
 target/hppa/cpu.c               |  4 ++-
 target/i386/cpu.c               |  4 ++-
 target/lm32/cpu.c               |  4 ++-
 target/m68k/cpu.c               |  4 ++-
 target/microblaze/cpu.c         |  4 ++-
 target/mips/cpu.c               |  4 ++-
 target/moxie/cpu.c              |  4 ++-
 target/nios2/cpu.c              |  4 ++-
 target/openrisc/cpu.c           |  4 ++-
 target/ppc/translate_init.inc.c |  4 ++-
 target/riscv/cpu.c              |  4 ++-
 target/s390x/cpu.c              |  4 ++-
 target/sh4/cpu.c                |  4 ++-
 target/sparc/cpu.c              |  4 ++-
 target/tilegx/cpu.c             |  4 ++-
 target/tricore/cpu.c            |  4 ++-
 target/unicore32/cpu.c          |  4 ++-
 target/xtensa/cpu.c             |  4 ++-
 24 files changed, 108 insertions(+), 55 deletions(-)

diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index 4bf00b8496..215bd9eead 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -9,8 +9,9 @@ void cpu_resume(CPUState *cpu)
 {
 }
 
-void qemu_init_vcpu(CPUState *cpu)
+bool qemu_init_vcpu(CPUState *cpu, Error **errp)
 {
+    return true;
 }
 
 /* User mode emulation does not support record/replay yet.  */
diff --git a/cpus.c b/cpus.c
index 28a241a936..c913bdc29e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1941,7 +1941,7 @@ void cpu_remove_sync(CPUState *cpu)
 /* For temporary buffers for forming a name */
 #define VCPU_THREAD_NAME_SIZE 16
 
-static void qemu_tcg_init_vcpu(CPUState *cpu)
+static void qemu_tcg_init_vcpu(CPUState *cpu, Error **errp)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
     static QemuCond *single_tcg_halt_cond;
@@ -1971,17 +1971,20 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
             snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
                  cpu->cpu_index);
 
-            /* TODO: let the callers handle the error instead of abort() here */
-            qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn,
-                               cpu, QEMU_THREAD_JOINABLE, &error_abort);
+            if (qemu_thread_create(cpu->thread, thread_name,
+                                   qemu_tcg_cpu_thread_fn, cpu,
+                                   QEMU_THREAD_JOINABLE, errp) < 0) {
+                return;
+            }
 
         } else {
             /* share a single thread for all cpus with TCG */
             snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
-            /* TODO: let the callers handle the error instead of abort() here */
-            qemu_thread_create(cpu->thread, thread_name,
-                               qemu_tcg_rr_cpu_thread_fn,
-                               cpu, QEMU_THREAD_JOINABLE, &error_abort);
+            if (qemu_thread_create(cpu->thread, thread_name,
+                                   qemu_tcg_rr_cpu_thread_fn, cpu,
+                                   QEMU_THREAD_JOINABLE, errp) < 0) {
+                return;
+            }
 
             single_tcg_halt_cond = cpu->halt_cond;
             single_tcg_cpu_thread = cpu->thread;
@@ -1999,7 +2002,7 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
     }
 }
 
-static void qemu_hax_start_vcpu(CPUState *cpu)
+static void qemu_hax_start_vcpu(CPUState *cpu, Error **errp)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
 
@@ -2009,15 +2012,16 @@ static void qemu_hax_start_vcpu(CPUState *cpu)
 
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
              cpu->cpu_index);
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
+    if (qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
+                           cpu, QEMU_THREAD_JOINABLE, errp) < 0) {
+        return;
+    }
 #ifdef _WIN32
     cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif
 }
 
-static void qemu_kvm_start_vcpu(CPUState *cpu)
+static void qemu_kvm_start_vcpu(CPUState *cpu, Error **errp)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
 
@@ -2026,12 +2030,11 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
     qemu_cond_init(cpu->halt_cond);
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
              cpu->cpu_index);
-    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
+                       cpu, QEMU_THREAD_JOINABLE, errp);
 }
 
-static void qemu_hvf_start_vcpu(CPUState *cpu)
+static void qemu_hvf_start_vcpu(CPUState *cpu, Error **errp)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
 
@@ -2045,12 +2048,11 @@ static void qemu_hvf_start_vcpu(CPUState *cpu)
 
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
              cpu->cpu_index);
-    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
+                       cpu, QEMU_THREAD_JOINABLE, errp);
 }
 
-static void qemu_whpx_start_vcpu(CPUState *cpu)
+static void qemu_whpx_start_vcpu(CPUState *cpu, Error **errp)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
 
@@ -2059,15 +2061,16 @@ static void qemu_whpx_start_vcpu(CPUState *cpu)
     qemu_cond_init(cpu->halt_cond);
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX",
              cpu->cpu_index);
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
+    if (qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn,
+                           cpu, QEMU_THREAD_JOINABLE, errp) < 0) {
+        return;
+    }
 #ifdef _WIN32
     cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif
 }
 
-static void qemu_dummy_start_vcpu(CPUState *cpu)
+static void qemu_dummy_start_vcpu(CPUState *cpu, Error **errp)
 {
     char thread_name[VCPU_THREAD_NAME_SIZE];
 
@@ -2076,12 +2079,11 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
     qemu_cond_init(cpu->halt_cond);
     snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY",
              cpu->cpu_index);
-    /* TODO: let the further caller handle the error instead of abort() here */
     qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn,
-                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
+                       cpu, QEMU_THREAD_JOINABLE, errp);
 }
 
-void qemu_init_vcpu(CPUState *cpu)
+bool qemu_init_vcpu(CPUState *cpu, Error **errp)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
 
@@ -2089,6 +2091,7 @@ void qemu_init_vcpu(CPUState *cpu)
     cpu->nr_threads =  ms->smp.threads;
     cpu->stopped = true;
     cpu->random_seed = qemu_guest_random_seed_thread_part1();
+    Error *local_err = NULL;
 
     if (!cpu->as) {
         /* If the target cpu hasn't set up any address spaces itself,
@@ -2099,22 +2102,29 @@ void qemu_init_vcpu(CPUState *cpu)
     }
 
     if (kvm_enabled()) {
-        qemu_kvm_start_vcpu(cpu);
+        qemu_kvm_start_vcpu(cpu, &local_err);
     } else if (hax_enabled()) {
-        qemu_hax_start_vcpu(cpu);
+        qemu_hax_start_vcpu(cpu, &local_err);
     } else if (hvf_enabled()) {
-        qemu_hvf_start_vcpu(cpu);
+        qemu_hvf_start_vcpu(cpu, &local_err);
     } else if (tcg_enabled()) {
-        qemu_tcg_init_vcpu(cpu);
+        qemu_tcg_init_vcpu(cpu, &local_err);
     } else if (whpx_enabled()) {
-        qemu_whpx_start_vcpu(cpu);
+        qemu_whpx_start_vcpu(cpu, &local_err);
     } else {
-        qemu_dummy_start_vcpu(cpu);
+        qemu_dummy_start_vcpu(cpu, &local_err);
+    }
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return false;
     }
 
     while (!cpu->created) {
         qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
     }
+
+    return true;
 }
 
 void cpu_stop_current(void)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 5ee0046b62..37f2d3dad5 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -1021,7 +1021,7 @@ void end_exclusive(void);
  *
  * Initializes a vCPU.
  */
-void qemu_init_vcpu(CPUState *cpu);
+bool qemu_init_vcpu(CPUState *cpu, Error **errp);
 
 #define SSTEP_ENABLE  0x1  /* Enable simulated HW single stepping */
 #define SSTEP_NOIRQ   0x2  /* Do not use IRQ while single stepping */
diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index b3fd6643e8..84c9b517c8 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -66,7 +66,9 @@ static void alpha_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     acc->parent_realize(dev, errp);
 }
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 1959467fdc..8b9c186780 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1610,7 +1610,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 #endif
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
     cpu_reset(cs);
 
     acc->parent_realize(dev, errp);
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index 7adfd6caf4..229928ae72 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -135,7 +135,9 @@ static void cris_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
     cpu_reset(cs);
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     ccc->parent_realize(dev, errp);
 }
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 71b6aca45d..11837dc4f9 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -99,7 +99,9 @@ static void hppa_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
     acc->parent_realize(dev, errp);
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 805ce95247..004910ab93 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5377,7 +5377,9 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 #endif
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     /*
      * Most Intel and certain AMD CPUs support hyperthreading. Even though QEMU
diff --git a/target/lm32/cpu.c b/target/lm32/cpu.c
index b35537de62..503b75b32b 100644
--- a/target/lm32/cpu.c
+++ b/target/lm32/cpu.c
@@ -134,7 +134,9 @@ static void lm32_cpu_realizefn(DeviceState *dev, Error **errp)
 
     cpu_reset(cs);
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     lcc->parent_realize(dev, errp);
 }
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index e6596de29c..28b28cfb5c 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -232,7 +232,9 @@ static void m68k_cpu_realizefn(DeviceState *dev, Error **errp)
     m68k_cpu_init_gdb(cpu);
 
     cpu_reset(cs);
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     mcc->parent_realize(dev, errp);
 }
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 0bec54b2f8..610bff082d 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -161,7 +161,9 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     env->pvr.regs[0] = PVR0_USE_EXC_MASK \
                        | PVR0_USE_ICACHE_MASK \
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 39eafafc5c..0b3aeba140 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -145,7 +145,9 @@ static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
     cpu_mips_realize_env(&cpu->env);
 
     cpu_reset(cs);
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     mcc->parent_realize(dev, errp);
 }
diff --git a/target/moxie/cpu.c b/target/moxie/cpu.c
index 48996d0554..bb2df19a08 100644
--- a/target/moxie/cpu.c
+++ b/target/moxie/cpu.c
@@ -65,7 +65,9 @@ static void moxie_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
     cpu_reset(cs);
 
     mcc->parent_realize(dev, errp);
diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index ca9c7a6df5..2b5091071c 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -92,7 +92,9 @@ static void nios2_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
     cpu_reset(cs);
 
     ncc->parent_realize(dev, errp);
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index f19e482a55..28d448396b 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -83,7 +83,9 @@ static void openrisc_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
     cpu_reset(cs);
 
     occ->parent_realize(dev, errp);
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 86fc8f2e31..63a75dc115 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -9843,7 +9843,9 @@ static void ppc_cpu_realize(DeviceState *dev, Error **errp)
     gdb_register_coprocessor(cs, gdb_get_spr_reg, gdb_set_spr_reg,
                              pcc->gdb_num_sprs, "power-spr.xml", 0);
 #endif
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        goto unrealize;
+    }
 
     pcc->parent_realize(dev, errp);
 
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f8d07bd20a..24fa555db7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -412,7 +412,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
 
     riscv_cpu_register_gdb_regs_for_features(cs);
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
     cpu_reset(cs);
 
     mcc->parent_realize(dev, errp);
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 736a7903e2..98ffd2767c 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -226,7 +226,9 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
 #endif
     s390_cpu_gdb_init(cs);
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     /*
      * KVM requires the initial CPU reset ioctl to be executed on the target
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 816d6d7f31..dc67833e3a 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -186,7 +186,9 @@ static void superh_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
     cpu_reset(cs);
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     scc->parent_realize(dev, errp);
 }
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index ee60a5536a..1a584b3404 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -768,7 +768,9 @@ static void sparc_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     scc->parent_realize(dev, errp);
 }
diff --git a/target/tilegx/cpu.c b/target/tilegx/cpu.c
index 69971247f2..750b6fd110 100644
--- a/target/tilegx/cpu.c
+++ b/target/tilegx/cpu.c
@@ -93,7 +93,9 @@ static void tilegx_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
     cpu_reset(cs);
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     tcc->parent_realize(dev, errp);
 }
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index df807c1d74..1d0853c3f4 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -95,7 +95,9 @@ static void tricore_cpu_realizefn(DeviceState *dev, Error **errp)
         set_feature(env, TRICORE_FEATURE_13);
     }
     cpu_reset(cs);
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     tcc->parent_realize(dev, errp);
 }
diff --git a/target/unicore32/cpu.c b/target/unicore32/cpu.c
index 802e2f1eba..4a3da5f900 100644
--- a/target/unicore32/cpu.c
+++ b/target/unicore32/cpu.c
@@ -95,7 +95,9 @@ static void uc32_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     ucc->parent_realize(dev, errp);
 }
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 76db1741a7..46a15f0df5 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -131,7 +131,9 @@ static void xtensa_cpu_realizefn(DeviceState *dev, Error **errp)
 
     cs->gdb_num_regs = xcc->config->gdb_regmap.num_regs;
 
-    qemu_init_vcpu(cs);
+    if (!qemu_init_vcpu(cs, errp)) {
+        return;
+    }
 
     xcc->parent_realize(dev, errp);
 }
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 03/11] qemu_thread: supplement error handling for qmp_dump_guest_memory
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 01/11] qemu_thread: make qemu_thread_create() take Error ** argument Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 02/11] qemu_thread: supplement error handling for qemu_X_start_vcpu Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 04/11] qemu_thread: supplement error handling for pci_edu_realize Fei Li
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Markus Armbruster, Peter Xu, Dr . David Alan Gilbert, Fei Li,
	Gerd Hoffmann, Marc-André Lureau, David Gibson

From: Fei Li <fli@suse.com>

Utilize the existed errp to propagate the error instead of the
temporary &error_abort.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Fei Li <fli@suse.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 dump/dump.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/dump/dump.c b/dump/dump.c
index 1358e62fb7..07a983cf4c 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -2022,9 +2022,8 @@ void qmp_dump_guest_memory(bool paging, const char *file,
     if (detach_p) {
         /* detached dump */
         s->detached = true;
-        /* TODO: let the further caller handle the error instead of abort() */
         qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread,
-                           s, QEMU_THREAD_DETACHED, &error_abort);
+                           s, QEMU_THREAD_DETACHED, errp);
     } else {
         /* sync dump */
         dump_process(s, errp);
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 04/11] qemu_thread: supplement error handling for pci_edu_realize
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
                   ` (2 preceding siblings ...)
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 03/11] qemu_thread: supplement error handling for qmp_dump_guest_memory Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 05/11] qemu_thread: supplement error handling for h_resize_hpt_prepare Fei Li
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Jiri Slaby, Markus Armbruster, Peter Xu, Dr . David Alan Gilbert,
	Fei Li, Gerd Hoffmann, David Gibson

From: Fei Li <fli@suse.com>

Utilize the existed errp to propagate the error and do the
corresponding cleanup to replace the temporary &error_abort.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Fei Li <fli@suse.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hw/misc/edu.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 1edc66955a..632b56f64c 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -371,9 +371,14 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
 
     qemu_mutex_init(&edu->thr_mutex);
     qemu_cond_init(&edu->thr_cond);
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
-                       edu, QEMU_THREAD_JOINABLE, &error_abort);
+    if (qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
+                           edu, QEMU_THREAD_JOINABLE, errp) < 0) {
+        qemu_cond_destroy(&edu->thr_cond);
+        qemu_mutex_destroy(&edu->thr_mutex);
+        timer_del(&edu->dma_timer);
+        msi_uninit(pdev);
+        return;
+    }
 
     memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu,
                     "edu-mmio", 1 * MiB);
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 05/11] qemu_thread: supplement error handling for h_resize_hpt_prepare
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
                   ` (3 preceding siblings ...)
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 04/11] qemu_thread: supplement error handling for pci_edu_realize Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 06/11] qemu_thread: supplement error handling for emulated_realize Fei Li
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Markus Armbruster, Peter Xu, Dr . David Alan Gilbert, Fei Li,
	Gerd Hoffmann, David Gibson

From: Fei Li <fli@suse.com>

Add a local_err to hold the error, and return the corresponding
error code to replace the temporary &error_abort.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Fei Li <fli@suse.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ppc/spapr_hcall.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index d64b634526..c9de3c0204 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -448,6 +448,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;
@@ -508,10 +509,13 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
     pending->shift = shift;
     pending->ret = H_HARDWARE;
 
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
-                       hpt_prepare_thread, pending,
-                       QEMU_THREAD_DETACHED, &error_abort);
+    if (qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
+                           hpt_prepare_thread, pending,
+                           QEMU_THREAD_DETACHED, &local_err) < 0) {
+        error_reportf_err(local_err, "failed to create hpt_prepare_thread: ");
+        g_free(pending);
+        return H_HARDWARE;
+    }
 
     spapr->pending_hpt = pending;
 
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 06/11] qemu_thread: supplement error handling for emulated_realize
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
                   ` (4 preceding siblings ...)
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 05/11] qemu_thread: supplement error handling for h_resize_hpt_prepare Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 07/11] qemu_thread: supplement error handling for iothread_complete Fei Li
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Christophe Fergeau, Markus Armbruster, Peter Xu,
	Dr . David Alan Gilbert, Fei Li, Gerd Hoffmann,
	Marc-André Lureau, David Gibson

From: Fei Li <fli@suse.com>

Utilize the existed errp to propagate the error and do the
corresponding cleanup to replace the temporary &error_abort.
If the second thread fails to be created, use a VEvent to
cancel the first thread before the join().

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Christophe Fergeau <cfergeau@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Fei Li <fli@suse.com>
---
 hw/usb/ccid-card-emulated.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index 4d95ee07aa..bdf1971e6b 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -545,11 +545,18 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
         error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
         goto out2;
     }
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
-                       card, QEMU_THREAD_JOINABLE, &error_abort);
-    qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
-                       card, QEMU_THREAD_JOINABLE, &error_abort);
+    if (qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
+                           card, QEMU_THREAD_JOINABLE, errp) < 0) {
+        goto out2;
+    }
+    if (qemu_thread_create(&card->apdu_thread_id, "ccid/apdu",
+                           handle_apdu_thread, card,
+                           QEMU_THREAD_JOINABLE, errp) < 0) {
+        VEvent *vevent = vevent_new(VEVENT_LAST, NULL, NULL);
+        vevent_queue_vevent(vevent); /* stop vevent thread */
+        qemu_thread_join(&card->event_thread_id);
+        goto out2;
+    }
 
     return;
 
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 07/11] qemu_thread: supplement error handling for iothread_complete
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
                   ` (5 preceding siblings ...)
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 06/11] qemu_thread: supplement error handling for emulated_realize Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 08/11] qemu_thread: supplement error handling for qemu_signalfd_compat Fei Li
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Markus Armbruster, Peter Xu, Dr . David Alan Gilbert, Fei Li,
	Gerd Hoffmann, Stefan Hajnoczi, David Gibson

From: Fei Li <fli@suse.com>

Utilize the existed errp to propagate the error and do the
corresponding cleanup to replace the temporary &error_abort.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Signed-off-by: Fei Li <fli@suse.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 iothread.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/iothread.c b/iothread.c
index 2d5a5bfe6c..1ebacaf089 100644
--- a/iothread.c
+++ b/iothread.c
@@ -166,6 +166,7 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
     Error *local_error = NULL;
     IOThread *iothread = IOTHREAD(obj);
     char *name, *thread_name;
+    int thread_ok;
 
     iothread->stopping = false;
     iothread->running = true;
@@ -188,9 +189,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;
     }
 
     /* This assumes we are called from a thread with useful CPU affinity for us
@@ -198,16 +197,23 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
      */
     name = object_get_canonical_path_component(OBJECT(obj));
     thread_name = g_strdup_printf("IO %s", name);
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&iothread->thread, thread_name, iothread_run,
-                       iothread, QEMU_THREAD_JOINABLE, &error_abort);
+    thread_ok = qemu_thread_create(&iothread->thread, thread_name, iothread_run,
+                                   iothread, QEMU_THREAD_JOINABLE, errp);
     g_free(thread_name);
     g_free(name);
+    if (thread_ok < 0) {
+        qemu_sem_destroy(&iothread->init_done_sem);
+        goto fail;
+    }
 
     /* Wait for initialization to complete */
     while (iothread->thread_id == -1) {
         qemu_sem_wait(&iothread->init_done_sem);
     }
+    return;
+fail:
+    aio_context_unref(iothread->ctx);
+    iothread->ctx = NULL;
 }
 
 typedef struct {
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 08/11] qemu_thread: supplement error handling for qemu_signalfd_compat
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
                   ` (6 preceding siblings ...)
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 07/11] qemu_thread: supplement error handling for iothread_complete Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 09/11] qemu_thread: supplement error handling for migration Fei Li
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Markus Armbruster, Peter Xu, Dr . David Alan Gilbert,
	Gerd Hoffmann, David Gibson

Set errno, do some cleanup, and return -1 to replace the temporary
&error_abort when failing to create sigwait_compat.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Signed-off-by: Fei Li <shirley17fei@gmail.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 util/compatfd.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/util/compatfd.c b/util/compatfd.c
index caad10ae56..bd15464586 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -70,6 +70,7 @@ static int qemu_signalfd_compat(const sigset_t *mask)
     struct sigfd_compat_info *info;
     QemuThread thread;
     int fds[2];
+    int ret;
 
     info = malloc(sizeof(*info));
     if (info == NULL) {
@@ -88,9 +89,15 @@ static int qemu_signalfd_compat(const sigset_t *mask)
     memcpy(&info->mask, mask, sizeof(*mask));
     info->fd = fds[1];
 
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat,
-                       info, QEMU_THREAD_DETACHED, &error_abort);
+    ret = qemu_thread_create(&thread, "signalfd_compat", sigwait_compat,
+                             info, QEMU_THREAD_DETACHED, NULL);
+    if (ret < 0) {
+        close(fds[0]);
+        close(fds[1]);
+        free(info);
+        errno = -ret;
+        return -1;
+    }
 
     return fds[0];
 }
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 09/11] qemu_thread: supplement error handling for migration
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
                   ` (7 preceding siblings ...)
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 08/11] qemu_thread: supplement error handling for qemu_signalfd_compat Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 10/11] qemu_thread: supplement error handling for vnc_start_worker_thread Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 11/11] qemu_thread: supplement error handling for touch_all_pages Fei Li
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Juan Quintela, Markus Armbruster, Peter Xu,
	Dr . David Alan Gilbert, Fei Li, Gerd Hoffmann, David Gibson

From: Fei Li <fli@suse.com>

Update qemu_thread_create()'s callers by
- setting an error on qemu_thread_create() failure for callers that
  set an error on failure;
- reporting the error and returning failure for callers that return
  an error code on failure;
- reporting the error and setting some state for callers that just
  report errors and choose not to continue on.

Besides, make compress_threads_save_cleanup() cope with partially
initialized comp_param[i] to adapt to the new qemu_thread_create()
failure case.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Cc: "Peter Xu <peterx@redhat.com>"
Signed-off-by: Fei Li <fli@suse.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c    | 35 ++++++++++++++++--------
 migration/postcopy-ram.c | 16 ++++++++---
 migration/ram.c          | 70 +++++++++++++++++++++++++++++++-----------------
 migration/savevm.c       | 12 ++++++---
 4 files changed, 89 insertions(+), 44 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 1786afb63d..ab46ee65be 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -483,10 +483,13 @@ static void process_incoming_migration_co(void *opaque)
             goto fail;
         }
 
-        /* TODO: let the further caller handle the error instead of abort() */
-        qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
-                           colo_process_incoming_thread, mis,
-                           QEMU_THREAD_JOINABLE, &error_abort);
+        if (qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
+                               colo_process_incoming_thread, mis,
+                               QEMU_THREAD_JOINABLE, &local_err) < 0) {
+            error_reportf_err(local_err, "failed to create "
+                              "colo_process_incoming_thread: ");
+            goto fail;
+        }
         mis->have_colo_incoming_thread = true;
         qemu_coroutine_yield();
 
@@ -2485,6 +2488,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) {
@@ -2498,10 +2502,15 @@ static int open_return_path_on_source(MigrationState *ms,
         return 0;
     }
 
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&ms->rp_state.rp_thread, "return path",
-                       source_return_path_thread, ms,
-                       QEMU_THREAD_JOINABLE, &error_abort);
+    if (qemu_thread_create(&ms->rp_state.rp_thread, "return path",
+                           source_return_path_thread, ms,
+                           QEMU_THREAD_JOINABLE, &local_err) < 0) {
+        error_reportf_err(local_err,
+                          "failed to create source_return_path_thread: ");
+        qemu_fclose(ms->rp_state.from_dst_file);
+        ms->rp_state.from_dst_file = NULL;
+        return -1;
+     }
 
     trace_open_return_path_on_source_continue();
 
@@ -3346,9 +3355,13 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
         migrate_fd_cleanup(s);
         return;
     }
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
-                       QEMU_THREAD_JOINABLE, &error_abort);
+    if (qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
+                           QEMU_THREAD_JOINABLE, &error_in) < 0) {
+        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;
 }
 
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 135e9cd746..d82136962d 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1095,6 +1095,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) {
@@ -1121,10 +1123,16 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
     }
 
     qemu_sem_init(&mis->fault_thread_sem, 0);
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&mis->fault_thread, "postcopy/fault",
-                       postcopy_ram_fault_thread, mis,
-                       QEMU_THREAD_JOINABLE, &error_abort);
+    if (qemu_thread_create(&mis->fault_thread, "postcopy/fault",
+                           postcopy_ram_fault_thread, mis,
+                           QEMU_THREAD_JOINABLE, &local_err) < 0) {
+        error_reportf_err(local_err,
+                          "failed to create postcopy_ram_fault_thread: ");
+        close(mis->userfault_event_fd);
+        close(mis->userfault_fd);
+        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;
diff --git a/migration/ram.c b/migration/ram.c
index 0f677f2d27..16f59ad057 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -504,6 +504,14 @@ static void compress_threads_save_cleanup(void)
 
     thread_count = migrate_compress_threads();
     for (i = 0; i < thread_count; i++) {
+        qemu_mutex_lock(&comp_param[i].mutex);
+        comp_param[i].quit = true;
+        qemu_cond_signal(&comp_param[i].cond);
+        qemu_mutex_unlock(&comp_param[i].mutex);
+
+        qemu_mutex_destroy(&comp_param[i].mutex);
+        qemu_cond_destroy(&comp_param[i].cond);
+
         /*
          * we use it as a indicator which shows if the thread is
          * properly init'd or not
@@ -511,15 +519,7 @@ static void compress_threads_save_cleanup(void)
         if (!comp_param[i].file) {
             break;
         }
-
-        qemu_mutex_lock(&comp_param[i].mutex);
-        comp_param[i].quit = true;
-        qemu_cond_signal(&comp_param[i].cond);
-        qemu_mutex_unlock(&comp_param[i].mutex);
-
         qemu_thread_join(compress_threads + i);
-        qemu_mutex_destroy(&comp_param[i].mutex);
-        qemu_cond_destroy(&comp_param[i].cond);
         deflateEnd(&comp_param[i].stream);
         g_free(comp_param[i].originbuf);
         qemu_fclose(comp_param[i].file);
@@ -536,6 +536,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;
@@ -546,6 +547,9 @@ static int compress_threads_save_setup(void)
     qemu_cond_init(&comp_done_cond);
     qemu_mutex_init(&comp_done_lock);
     for (i = 0; i < thread_count; i++) {
+        qemu_mutex_init(&comp_param[i].mutex);
+        qemu_cond_init(&comp_param[i].cond);
+        comp_param[i].quit = false;
         comp_param[i].originbuf = g_try_malloc(TARGET_PAGE_SIZE);
         if (!comp_param[i].originbuf) {
             goto exit;
@@ -562,13 +566,16 @@ static int compress_threads_save_setup(void)
          */
         comp_param[i].file = qemu_fopen_ops(NULL, &empty_ops);
         comp_param[i].done = true;
-        comp_param[i].quit = false;
-        qemu_mutex_init(&comp_param[i].mutex);
-        qemu_cond_init(&comp_param[i].cond);
-        /* TODO: let the further caller handle the error instead of abort() */
-        qemu_thread_create(compress_threads + i, "compress",
-                           do_data_compress, comp_param + i,
-                           QEMU_THREAD_JOINABLE, &error_abort);
+        if (qemu_thread_create(compress_threads + i, "compress",
+                               do_data_compress, comp_param + i,
+                               QEMU_THREAD_JOINABLE, &local_err) < 0) {
+            error_reportf_err(local_err, "failed to create do_data_compress: ");
+            deflateEnd(&comp_param[i].stream);
+            g_free(comp_param[i].originbuf);
+            qemu_fclose(comp_param[i].file);
+            comp_param[i].file = NULL;
+            goto exit;
+        }
     }
     return 0;
 
@@ -1168,9 +1175,14 @@ 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;
-        /* TODO: let the further caller handle the error instead of abort() */
-        qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
-                           QEMU_THREAD_JOINABLE, &error_abort);
+        if (qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
+                               QEMU_THREAD_JOINABLE, &local_err) < 0) {
+            migrate_set_error(migrate_get_current(), local_err);
+            error_reportf_err(local_err,
+                              "failed to create multifd_send_thread: ");
+            multifd_save_cleanup();
+            return;
+        }
     }
 }
 
@@ -1449,9 +1461,13 @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
     p->num_packets = 1;
 
     p->running = true;
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
-                       QEMU_THREAD_JOINABLE, &error_abort);
+    if (qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
+                           QEMU_THREAD_JOINABLE, &local_err) < 0) {
+        multifd_recv_terminate_threads(local_err);
+        error_propagate_prepend(errp, local_err,
+                                "failed to create multifd_recv_thread: ");
+        return false;
+    }
     atomic_inc(&multifd_recv_state->count);
     return atomic_read(&multifd_recv_state->count) ==
            migrate_multifd_channels();
@@ -3873,6 +3889,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;
@@ -3894,10 +3911,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;
-        /* TODO: let the further caller handle the error instead of abort() */
-        qemu_thread_create(decompress_threads + i, "decompress",
-                           do_data_decompress, decomp_param + i,
-                           QEMU_THREAD_JOINABLE, &error_abort);
+        if (qemu_thread_create(decompress_threads + i, "decompress",
+                               do_data_decompress, decomp_param + i,
+                               QEMU_THREAD_JOINABLE, &local_err) < 0) {
+            error_reportf_err(local_err,
+                              "failed to create do_data_decompress: ");
+            goto exit;
+        }
     }
     return 0;
 exit:
diff --git a/migration/savevm.c b/migration/savevm.c
index c792dee8aa..d75d078946 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1826,10 +1826,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);
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&mis->listen_thread, "postcopy/listen",
-                       postcopy_ram_listen_thread, NULL,
-                       QEMU_THREAD_DETACHED, &error_abort);
+    if (qemu_thread_create(&mis->listen_thread, "postcopy/listen",
+                           postcopy_ram_listen_thread, NULL,
+                           QEMU_THREAD_DETACHED, &local_err) < 0) {
+        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);
 
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 10/11] qemu_thread: supplement error handling for vnc_start_worker_thread
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
                   ` (8 preceding siblings ...)
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 09/11] qemu_thread: supplement error handling for migration Fei Li
@ 2019-07-17  2:33 ` Fei Li
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 11/11] qemu_thread: supplement error handling for touch_all_pages Fei Li
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Markus Armbruster, Peter Xu, Dr . David Alan Gilbert, Fei Li,
	Gerd Hoffmann, David Gibson

From: Fei Li <fli@suse.com>

Supplement the error handling for vnc_thread_worker_thread: add
an Error parameter for it to propagate the error to its caller to
handle in case it fails, and make it return a Boolean to indicate
whether it succeeds.

Cc: Markus Armbruster <armbru@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Fei Li <fli@suse.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 ui/vnc-jobs.c | 17 +++++++++++------
 ui/vnc-jobs.h |  2 +-
 ui/vnc.c      |  4 +++-
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 5712f1f501..1371895513 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -332,16 +332,21 @@ 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()) {
+        return true;
+    }
 
     q = vnc_queue_init();
-    /* TODO: let the further caller handle the error instead of abort() here */
-    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread,
-                       q, QEMU_THREAD_DETACHED, &error_abort);
+    if (qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread,
+                           q, QEMU_THREAD_DETACHED, errp) < 0) {
+        vnc_queue_clear(q);
+        return false;
+    }
     queue = q; /* Set global queue */
+
+    return true;
 }
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index 59f66bcc35..14640593db 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -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)
diff --git a/ui/vnc.c b/ui/vnc.c
index 38f92bfca3..561fcb9a29 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3196,7 +3196,9 @@ void vnc_display_init(const char *id, Error **errp)
     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);
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v14 11/11] qemu_thread: supplement error handling for touch_all_pages
  2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
                   ` (9 preceding siblings ...)
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 10/11] qemu_thread: supplement error handling for vnc_start_worker_thread Fei Li
@ 2019-07-17  2:33 ` Fei Li
  10 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17  2:33 UTC (permalink / raw)
  To: qemu-devel, shirley17fei
  Cc: Markus Armbruster, Peter Xu, Dr . David Alan Gilbert, Fei Li,
	Gerd Hoffmann, David Gibson

From: Fei Li <fli@suse.com>

Supplement the error handling for touch_all_pages: add an Error
parameter for it to propagate the error to its caller to do the
handling in case it fails.

Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Fei Li <fli@suse.com>
---
 util/oslib-posix.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 55c1b9c098..eff5af3ccf 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -448,12 +448,12 @@ static inline int get_memset_num_threads(int smp_cpus)
 }
 
 static bool touch_all_pages(char *area, size_t hpagesize, size_t numpages,
-                            int smp_cpus)
+                            int smp_cpus, Error **errp)
 {
     size_t numpages_per_thread;
     size_t size_per_thread;
     char *addr = area;
-    int i = 0;
+    int i = 0, j = 0;
 
     memset_thread_failed = false;
     memset_num_threads = get_memset_num_threads(smp_cpus);
@@ -465,20 +465,32 @@ 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;
         memset_thread[i].hpagesize = hpagesize;
-        /* TODO: let the callers handle the error instead of abort() here */
-        qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
-                           do_touch_pages, &memset_thread[i],
-                           QEMU_THREAD_JOINABLE, &error_abort);
+        if (qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
+                               do_touch_pages, &memset_thread[i],
+                               QEMU_THREAD_JOINABLE, errp) < 0) {
+            break;
+        }
         addr += size_per_thread;
         numpages -= numpages_per_thread;
     }
-    for (i = 0; i < memset_num_threads; i++) {
-        qemu_thread_join(&memset_thread[i].pgthread);
+
+    for (j = 0; j < i; j++) {
+        qemu_thread_join(&memset_thread[j].pgthread);
     }
     g_free(memset_thread);
     memset_thread = NULL;
 
-    return memset_thread_failed;
+    if (i < memset_num_threads) {
+        /* qemu_thread_create() has set @errp */
+        return false;
+    }
+
+    if (memset_thread_failed) {
+        error_setg(errp, "os_mem_prealloc: Insufficient free host "
+                         "memory pages available to allocate guest RAM");
+        return false;
+    }
+    return true;
 }
 
 void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus,
@@ -501,10 +513,7 @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus,
     }
 
     /* touch pages simultaneously */
-    if (touch_all_pages(area, hpagesize, numpages, smp_cpus)) {
-        error_setg(errp, "os_mem_prealloc: Insufficient free host memory "
-            "pages available to allocate guest RAM");
-    }
+    touch_all_pages(area, hpagesize, numpages, smp_cpus, errp);
 
     ret = sigaction(SIGBUS, &oldact, NULL);
     if (ret) {
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v14 01/11] qemu_thread: make qemu_thread_create() take Error ** argument
  2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 01/11] qemu_thread: make qemu_thread_create() take Error ** argument Fei Li
@ 2019-07-17  5:53   ` David Gibson
  2019-07-17 12:27     ` Fei Li
  0 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2019-07-17  5:53 UTC (permalink / raw)
  To: Fei Li
  Cc: Markus Armbruster, qemu-devel, Peter Xu, Dr . David Alan Gilbert,
	Fei Li, Gerd Hoffmann, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 33192 bytes --]

On Wed, Jul 17, 2019 at 10:33:00AM +0800, Fei Li wrote:
> From: Fei Li <fli@suse.com>
> 
> qemu_thread_create() abort()s on error. Not nice. Give it a return
> value and an Error ** argument, so it can return success/failure.
> 
> Considering qemu_thread_create() is quite widely used in qemu, split
> this into two steps: this patch passes the &error_abort to
> qemu_thread_create() everywhere, and the next 10 patches will improve
> on &error_abort for callers who need.  To differentiate callers who
> need the improvement, temporarily add the "TODO:" comment for them.
> 
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Fei Li <fli@suse.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  cpus.c                        | 23 +++++++++++++++--------
>  dump/dump.c                   |  3 ++-
>  hw/misc/edu.c                 |  4 +++-
>  hw/ppc/spapr_hcall.c          |  4 +++-
>  hw/rdma/rdma_backend.c        |  3 ++-
>  hw/usb/ccid-card-emulated.c   |  5 +++--
>  include/qemu/thread.h         |  6 +++---
>  io/task.c                     |  3 ++-
>  iothread.c                    |  3 ++-
>  migration/migration.c         | 11 ++++++++---
>  migration/postcopy-ram.c      |  4 +++-
>  migration/ram.c               | 12 ++++++++----
>  migration/savevm.c            |  3 ++-
>  tests/atomic64-bench.c        |  3 ++-
>  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-char.c             |  6 +++---
>  tests/test-rcu-list.c         |  3 ++-
>  tests/test-util-filemonitor.c |  2 +-
>  ui/cocoa.m                    |  2 +-
>  ui/vnc-jobs.c                 |  6 ++++--
>  util/compatfd.c               |  6 ++++--
>  util/oslib-posix.c            |  3 ++-
>  util/qemu-thread-posix.c      | 30 +++++++++++++++++++++++-------
>  util/qemu-thread-win32.c      | 13 ++++++++++---
>  util/rcu.c                    |  3 ++-
>  util/thread-pool.c            |  4 +++-
>  30 files changed, 121 insertions(+), 57 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 927a00aa90..28a241a936 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1971,15 +1971,17 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>              snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
>                   cpu->cpu_index);
>  
> +            /* TODO: let the callers handle the error instead of abort() here */
>              qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn,
> -                               cpu, QEMU_THREAD_JOINABLE);
> +                               cpu, QEMU_THREAD_JOINABLE, &error_abort);
>  
>          } else {
>              /* share a single thread for all cpus with TCG */
>              snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
> +            /* TODO: let the callers handle the error instead of abort() here */
>              qemu_thread_create(cpu->thread, thread_name,
>                                 qemu_tcg_rr_cpu_thread_fn,
> -                               cpu, QEMU_THREAD_JOINABLE);
> +                               cpu, QEMU_THREAD_JOINABLE, &error_abort);
>  
>              single_tcg_halt_cond = cpu->halt_cond;
>              single_tcg_cpu_thread = cpu->thread;
> @@ -2007,8 +2009,9 @@ static void qemu_hax_start_vcpu(CPUState *cpu)
>  
>      snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
>               cpu->cpu_index);
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
> -                       cpu, QEMU_THREAD_JOINABLE);
> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>  #ifdef _WIN32
>      cpu->hThread = qemu_thread_get_handle(cpu->thread);
>  #endif
> @@ -2023,8 +2026,9 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
>      qemu_cond_init(cpu->halt_cond);
>      snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
>               cpu->cpu_index);
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
> -                       cpu, QEMU_THREAD_JOINABLE);
> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>  }
>  
>  static void qemu_hvf_start_vcpu(CPUState *cpu)
> @@ -2041,8 +2045,9 @@ static void qemu_hvf_start_vcpu(CPUState *cpu)
>  
>      snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
>               cpu->cpu_index);
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
> -                       cpu, QEMU_THREAD_JOINABLE);
> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>  }
>  
>  static void qemu_whpx_start_vcpu(CPUState *cpu)
> @@ -2054,8 +2059,9 @@ static void qemu_whpx_start_vcpu(CPUState *cpu)
>      qemu_cond_init(cpu->halt_cond);
>      snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX",
>               cpu->cpu_index);
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn,
> -                       cpu, QEMU_THREAD_JOINABLE);
> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>  #ifdef _WIN32
>      cpu->hThread = qemu_thread_get_handle(cpu->thread);
>  #endif
> @@ -2070,8 +2076,9 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
>      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);
> +    /* TODO: let the further caller handle the error instead of abort() here */
> +    qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn,
> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>  }
>  
>  void qemu_init_vcpu(CPUState *cpu)
> diff --git a/dump/dump.c b/dump/dump.c
> index c7b2301652..1358e62fb7 100644
> --- a/dump/dump.c
> +++ b/dump/dump.c
> @@ -2022,8 +2022,9 @@ void qmp_dump_guest_memory(bool paging, const char *file,
>      if (detach_p) {
>          /* detached dump */
>          s->detached = true;
> +        /* TODO: let the further caller handle the error instead of abort() */
>          qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread,
> -                           s, QEMU_THREAD_DETACHED);
> +                           s, QEMU_THREAD_DETACHED, &error_abort);
>      } else {
>          /* sync dump */
>          dump_process(s, errp);
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index 3b6dba2c72..1edc66955a 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -29,6 +29,7 @@
>  #include "qemu/timer.h"
>  #include "qemu/main-loop.h" /* iothread mutex */
>  #include "qemu/module.h"
> +#include "qapi/error.h"
>  #include "qapi/visitor.h"
>  
>  #define TYPE_PCI_EDU_DEVICE "edu"
> @@ -370,8 +371,9 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
>  
>      qemu_mutex_init(&edu->thr_mutex);
>      qemu_cond_init(&edu->thr_cond);
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
> -                       edu, QEMU_THREAD_JOINABLE);
> +                       edu, QEMU_THREAD_JOINABLE, &error_abort);
>  
>      memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu,
>                      "edu-mmio", 1 * MiB);
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 6808d4cda8..d64b634526 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -508,8 +508,10 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
>      pending->shift = shift;
>      pending->ret = H_HARDWARE;
>  
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
> -                       hpt_prepare_thread, pending, QEMU_THREAD_DETACHED);
> +                       hpt_prepare_thread, pending,
> +                       QEMU_THREAD_DETACHED, &error_abort);
>  
>      spapr->pending_hpt = pending;
>  
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index c39051068d..29f52955d0 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c
> @@ -281,7 +281,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)(void *ctx,
> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
> index e20f8ed293..4d95ee07aa 100644
> --- a/hw/usb/ccid-card-emulated.c
> +++ b/hw/usb/ccid-card-emulated.c
> @@ -545,10 +545,11 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
>          error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
>          goto out2;
>      }
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
> -                       card, QEMU_THREAD_JOINABLE);
> +                       card, QEMU_THREAD_JOINABLE, &error_abort);
>      qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
> -                       card, QEMU_THREAD_JOINABLE);
> +                       card, QEMU_THREAD_JOINABLE, &error_abort);
>  
>      return;
>  
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index 55d83a907c..ec18d8b171 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -152,9 +152,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,
> -                        void *(*start_routine)(void *),
> -                        void *arg, int mode);
> +int qemu_thread_create(QemuThread *thread, const char *name,
> +                       void *(*start_routine)(void *),
> +                       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);
> diff --git a/io/task.c b/io/task.c
> index 64c4c7126a..1499540cdf 100644
> --- a/io/task.c
> +++ b/io/task.c
> @@ -170,7 +170,8 @@ void qio_task_run_in_thread(QIOTask *task,
>                         "io-task-worker",
>                         qio_task_thread_worker,
>                         task,
> -                       QEMU_THREAD_DETACHED);
> +                       QEMU_THREAD_DETACHED,
> +                       &error_abort);
>  }
>  
>  
> diff --git a/iothread.c b/iothread.c
> index 7130be58e3..2d5a5bfe6c 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -198,8 +198,9 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
>       */
>      name = object_get_canonical_path_component(OBJECT(obj));
>      thread_name = g_strdup_printf("IO %s", name);
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(&iothread->thread, thread_name, iothread_run,
> -                       iothread, QEMU_THREAD_JOINABLE);
> +                       iothread, QEMU_THREAD_JOINABLE, &error_abort);
>      g_free(thread_name);
>      g_free(name);
>  
> diff --git a/migration/migration.c b/migration/migration.c
> index 8a607fe1e2..1786afb63d 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -483,8 +483,10 @@ static void process_incoming_migration_co(void *opaque)
>              goto fail;
>          }
>  
> +        /* TODO: let the further caller handle the error instead of abort() */
>          qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
> -             colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
> +                           colo_process_incoming_thread, mis,
> +                           QEMU_THREAD_JOINABLE, &error_abort);
>          mis->have_colo_incoming_thread = true;
>          qemu_coroutine_yield();
>  
> @@ -2496,8 +2498,10 @@ static int open_return_path_on_source(MigrationState *ms,
>          return 0;
>      }
>  
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(&ms->rp_state.rp_thread, "return path",
> -                       source_return_path_thread, ms, QEMU_THREAD_JOINABLE);
> +                       source_return_path_thread, ms,
> +                       QEMU_THREAD_JOINABLE, &error_abort);
>  
>      trace_open_return_path_on_source_continue();
>  
> @@ -3342,8 +3346,9 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>          migrate_fd_cleanup(s);
>          return;
>      }
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
> -                       QEMU_THREAD_JOINABLE);
> +                       QEMU_THREAD_JOINABLE, &error_abort);
>      s->migration_thread_running = true;
>  }
>  
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 9faacacc9e..135e9cd746 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1121,8 +1121,10 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>      }
>  
>      qemu_sem_init(&mis->fault_thread_sem, 0);
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(&mis->fault_thread, "postcopy/fault",
> -                       postcopy_ram_fault_thread, mis, QEMU_THREAD_JOINABLE);
> +                       postcopy_ram_fault_thread, mis,
> +                       QEMU_THREAD_JOINABLE, &error_abort);
>      qemu_sem_wait(&mis->fault_thread_sem);
>      qemu_sem_destroy(&mis->fault_thread_sem);
>      mis->have_fault_thread = true;
> diff --git a/migration/ram.c b/migration/ram.c
> index 2b0774c2bf..0f677f2d27 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -565,9 +565,10 @@ 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);
> +        /* TODO: let the further caller handle the error instead of abort() */
>          qemu_thread_create(compress_threads + i, "compress",
>                             do_data_compress, comp_param + i,
> -                           QEMU_THREAD_JOINABLE);
> +                           QEMU_THREAD_JOINABLE, &error_abort);
>      }
>      return 0;
>  
> @@ -1167,8 +1168,9 @@ 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;
> +        /* TODO: let the further caller handle the error instead of abort() */
>          qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
> -                           QEMU_THREAD_JOINABLE);
> +                           QEMU_THREAD_JOINABLE, &error_abort);
>      }
>  }
>  
> @@ -1447,8 +1449,9 @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
>      p->num_packets = 1;
>  
>      p->running = true;
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
> -                       QEMU_THREAD_JOINABLE);
> +                       QEMU_THREAD_JOINABLE, &error_abort);
>      atomic_inc(&multifd_recv_state->count);
>      return atomic_read(&multifd_recv_state->count) ==
>             migrate_multifd_channels();
> @@ -3891,9 +3894,10 @@ 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;
> +        /* TODO: let the further caller handle the error instead of abort() */
>          qemu_thread_create(decompress_threads + i, "decompress",
>                             do_data_decompress, decomp_param + i,
> -                           QEMU_THREAD_JOINABLE);
> +                           QEMU_THREAD_JOINABLE, &error_abort);
>      }
>      return 0;
>  exit:
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 79ed44d475..c792dee8aa 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1826,9 +1826,10 @@ 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);
> +    /* TODO: let the further caller handle the error instead of abort() here */
>      qemu_thread_create(&mis->listen_thread, "postcopy/listen",
>                         postcopy_ram_listen_thread, NULL,
> -                       QEMU_THREAD_DETACHED);
> +                       QEMU_THREAD_DETACHED, &error_abort);
>      qemu_sem_wait(&mis->listen_thread_sem);
>      qemu_sem_destroy(&mis->listen_thread_sem);
>  
> diff --git a/tests/atomic64-bench.c b/tests/atomic64-bench.c
> index 121a8c14f4..08ec753582 100644
> --- a/tests/atomic64-bench.c
> +++ b/tests/atomic64-bench.c
> @@ -8,6 +8,7 @@
>  #include "qemu/thread.h"
>  #include "qemu/host-utils.h"
>  #include "qemu/processor.h"
> +#include "qapi/error.h"
>  
>  struct thread_info {
>      uint64_t r;
> @@ -103,7 +104,7 @@ static void create_threads(void)
>          info->r = (i + 1) ^ time(NULL);
>          info->accesses = 0;
>          qemu_thread_create(&threads[i], NULL, thread_func, info,
> -                           QEMU_THREAD_JOINABLE);
> +                           QEMU_THREAD_JOINABLE, &error_abort);
>      }
>  }
>  
> diff --git a/tests/atomic_add-bench.c b/tests/atomic_add-bench.c
> index 5666f6bbff..52071585a9 100644
> --- a/tests/atomic_add-bench.c
> +++ b/tests/atomic_add-bench.c
> @@ -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;
> @@ -108,7 +109,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);
>      }
>  }
>  
> diff --git a/tests/iothread.c b/tests/iothread.c
> index 777d9eea46..f4ad992e61 100644
> --- a/tests/iothread.c
> +++ b/tests/iothread.c
> @@ -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);
> diff --git a/tests/qht-bench.c b/tests/qht-bench.c
> index e3b512f26f..728c2e06c7 100644
> --- a/tests/qht-bench.c
> +++ b/tests/qht-bench.c
> @@ -10,6 +10,7 @@
>  #include "qemu/qht.h"
>  #include "qemu/rcu.h"
>  #include "qemu/xxhash.h"
> +#include "qapi/error.h"
>  
>  struct thread_stats {
>      size_t rd;
> @@ -248,7 +249,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);
>      }
>  }
>  
> diff --git a/tests/rcutorture.c b/tests/rcutorture.c
> index 49311c82ea..0e799ff256 100644
> --- a/tests/rcutorture.c
> +++ b/tests/rcutorture.c
> @@ -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++;
>  }
>  
> diff --git a/tests/test-aio.c b/tests/test-aio.c
> index 86fb73b3d5..b3ac261724 100644
> --- a/tests/test-aio.c
> +++ b/tests/test-aio.c
> @@ -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);
> diff --git a/tests/test-char.c b/tests/test-char.c
> index f9440cdcfd..093e26e333 100644
> --- a/tests/test-char.c
> +++ b/tests/test-char.c
> @@ -793,7 +793,7 @@ static void char_socket_server_test(gconstpointer opaque)
>       */
>      qemu_thread_create(&thread, "client",
>                         char_socket_server_client_thread,
> -                       addr, QEMU_THREAD_JOINABLE);
> +                       addr, QEMU_THREAD_JOINABLE, &error_abort);
>      g_assert(data.event == -1);
>  
>      if (config->wait_connected) {
> @@ -901,7 +901,7 @@ static void char_socket_client_test(gconstpointer opaque)
>       */
>      qemu_thread_create(&thread, "client",
>                         char_socket_client_server_thread,
> -                       ioc, QEMU_THREAD_JOINABLE);
> +                       ioc, QEMU_THREAD_JOINABLE, &error_abort);
>  
>      /*
>       * Populate the chardev address based on what the server
> @@ -994,7 +994,7 @@ static void char_socket_client_test(gconstpointer opaque)
>          reconnected = true;
>          qemu_thread_create(&thread, "client",
>                             char_socket_client_server_thread,
> -                           ioc, QEMU_THREAD_JOINABLE);
> +                           ioc, QEMU_THREAD_JOINABLE, &error_abort);
>          goto reconnect;
>      }
>  
> diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
> index 6f076473e0..399744f03c 100644
> --- a/tests/test-rcu-list.c
> +++ b/tests/test-rcu-list.c
> @@ -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++;
>  }
>  
> diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
> index 46e781c022..a7a7c1eeeb 100644
> --- a/tests/test-util-filemonitor.c
> +++ b/tests/test-util-filemonitor.c
> @@ -397,7 +397,7 @@ test_file_monitor_events(void)
>      evstopping = 0;
>      qemu_thread_create(&th, "event-loop",
>                         qemu_file_monitor_test_event_loop, NULL,
> -                       QEMU_THREAD_JOINABLE);
> +                       QEMU_THREAD_JOINABLE, &error_abort);
>  
>      if (local_err) {
>          g_printerr("File monitoring not available: %s",
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index c2984028c5..d8cf1484d9 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1766,7 +1766,7 @@ int main (int argc, const char * argv[]) {
>      qemu_sem_init(&app_started_sem, 0);
>  
>      qemu_thread_create(&thread, "qemu_main", call_qemu_main,
> -                       NULL, QEMU_THREAD_DETACHED);
> +                       NULL, QEMU_THREAD_DETACHED, &error_abort);
>  
>      COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
>      qemu_sem_wait(&display_init_sem);
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index 929391f85d..5712f1f501 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -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"
>  
>  /*
> @@ -339,7 +340,8 @@ void vnc_start_worker_thread(void)
>          return ;
>  
>      q = vnc_queue_init();
> -    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
> -                       QEMU_THREAD_DETACHED);
> +    /* TODO: let the further caller handle the error instead of abort() here */
> +    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread,
> +                       q, QEMU_THREAD_DETACHED, &error_abort);
>      queue = q; /* Set global queue */
>  }
> diff --git a/util/compatfd.c b/util/compatfd.c
> index c296f55d14..caad10ae56 100644
> --- a/util/compatfd.c
> +++ b/util/compatfd.c
> @@ -15,6 +15,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/thread.h"
> +#include "qapi/error.h"
>  
>  #include <sys/syscall.h>
>  
> @@ -87,8 +88,9 @@ static int qemu_signalfd_compat(const sigset_t *mask)
>      memcpy(&info->mask, mask, sizeof(*mask));
>      info->fd = fds[1];
>  
> -    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat, info,
> -                       QEMU_THREAD_DETACHED);
> +    /* TODO: let the further caller handle the error instead of abort() here */
> +    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat,
> +                       info, QEMU_THREAD_DETACHED, &error_abort);
>  
>      return fds[0];
>  }
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index fe0309ce7a..55c1b9c098 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -465,9 +465,10 @@ 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;
>          memset_thread[i].hpagesize = hpagesize;
> +        /* TODO: let the callers handle the error instead of abort() here */
>          qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
>                             do_touch_pages, &memset_thread[i],
> -                           QEMU_THREAD_JOINABLE);
> +                           QEMU_THREAD_JOINABLE, &error_abort);
>          addr += size_per_thread;
>          numpages -= numpages_per_thread;
>      }
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 1bf5e65dea..06a9e78ba8 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -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,18 @@ static void *qemu_thread_start(void *args)
>      return r;
>  }
>  
> -void qemu_thread_create(QemuThread *thread, const char *name,
> -                       void *(*start_routine)(void*),
> -                       void *arg, int mode)
> +/*
> + * Create a new thread with name @name
> + * The thread executes @start_routine() with argument @arg.
> + * The thread will be created in a detached state if @mode is
> + * QEMU_THREAD_DETACHED, and in a jounable state if it's
> + * QEMU_THREAD_JOINABLE.
> + * On success, return 0.
> + * On failure, store an error through @errp and return negative errno.
> + */
> +int 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 +525,8 @@ void qemu_thread_create(QemuThread *thread, const char *name,
>  
>      err = pthread_attr_init(&attr);
>      if (err) {
> -        error_exit(err, __func__);
> +        error_setg(errp, "pthread_attr_init failed");
> +        return -err;
>      }
>  
>      if (mode == QEMU_THREAD_DETACHED) {
> @@ -538,13 +549,18 @@ 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) {
> +        error_setg(errp, "pthread_create failed");
> +        pthread_attr_destroy(&attr);
> +        g_free(qemu_thread_args->name);
> +        g_free(qemu_thread_args);
> +        return -err;
> +    }
>  
>      pthread_sigmask(SIG_SETMASK, &oldset, NULL);
>  
>      pthread_attr_destroy(&attr);
> +    return 0;
>  }
>  
>  void qemu_thread_get_self(QemuThread *thread)
> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
> index 572f88535d..f2deea5250 100644
> --- a/util/qemu-thread-win32.c
> +++ b/util/qemu-thread-win32.c
> @@ -16,6 +16,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;
> @@ -384,9 +385,9 @@ void *qemu_thread_join(QemuThread *thread)
>      return ret;
>  }
>  
> -void qemu_thread_create(QemuThread *thread, const char *name,
> +int qemu_thread_create(QemuThread *thread, const char *name,
>                         void *(*start_routine)(void *),
> -                       void *arg, int mode)
> +                       void *arg, int mode, Error **errp)
>  {
>      HANDLE hThread;
>      struct QemuThreadData *data;
> @@ -405,10 +406,16 @@ void qemu_thread_create(QemuThread *thread, const char *name,
>      hThread = (HANDLE) _beginthreadex(NULL, 0, win32_start_routine,
>                                        data, 0, &thread->tid);
>      if (!hThread) {
> -        error_exit(GetLastError(), __func__);
> +        if (data->mode != QEMU_THREAD_DETACHED) {
> +            DeleteCriticalSection(&data->cs);
> +        }
> +        error_setg(errp, "failed to create win32_start_routine");
> +        g_free(data);
> +        return -errno;
>      }
>      CloseHandle(hThread);
>      thread->data = data;
> +    return 0;
>  }
>  
>  void qemu_thread_get_self(QemuThread *thread)
> diff --git a/util/rcu.c b/util/rcu.c
> index 177a675619..ff11837b47 100644
> --- a/util/rcu.c
> +++ b/util/rcu.c
> @@ -31,6 +31,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
> @@ -324,7 +325,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();
>  }
> diff --git a/util/thread-pool.c b/util/thread-pool.c
> index 4ed9b89ab2..9a867776f9 100644
> --- a/util/thread-pool.c
> +++ b/util/thread-pool.c
> @@ -21,6 +21,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);
>  
> @@ -131,7 +132,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)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v14 01/11] qemu_thread: make qemu_thread_create() take Error ** argument
  2019-07-17  5:53   ` David Gibson
@ 2019-07-17 12:27     ` Fei Li
  0 siblings, 0 replies; 14+ messages in thread
From: Fei Li @ 2019-07-17 12:27 UTC (permalink / raw)
  To: David Gibson
  Cc: Markus Armbruster, qemu-devel, Peter Xu, Dr . David Alan Gilbert,
	Fei Li, Gerd Hoffmann, Paolo Bonzini


在 2019/7/17 下午1:53, David Gibson 写道:
> On Wed, Jul 17, 2019 at 10:33:00AM +0800, Fei Li wrote:
>> From: Fei Li <fli@suse.com>
>>
>> qemu_thread_create() abort()s on error. Not nice. Give it a return
>> value and an Error ** argument, so it can return success/failure.
>>
>> Considering qemu_thread_create() is quite widely used in qemu, split
>> this into two steps: this patch passes the &error_abort to
>> qemu_thread_create() everywhere, and the next 10 patches will improve
>> on &error_abort for callers who need.  To differentiate callers who
>> need the improvement, temporarily add the "TODO:" comment for them.
>>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Fei Li <fli@suse.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Thanks for the review. :)

Have a nice day

Fei

>
>> ---
>>   cpus.c                        | 23 +++++++++++++++--------
>>   dump/dump.c                   |  3 ++-
>>   hw/misc/edu.c                 |  4 +++-
>>   hw/ppc/spapr_hcall.c          |  4 +++-
>>   hw/rdma/rdma_backend.c        |  3 ++-
>>   hw/usb/ccid-card-emulated.c   |  5 +++--
>>   include/qemu/thread.h         |  6 +++---
>>   io/task.c                     |  3 ++-
>>   iothread.c                    |  3 ++-
>>   migration/migration.c         | 11 ++++++++---
>>   migration/postcopy-ram.c      |  4 +++-
>>   migration/ram.c               | 12 ++++++++----
>>   migration/savevm.c            |  3 ++-
>>   tests/atomic64-bench.c        |  3 ++-
>>   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-char.c             |  6 +++---
>>   tests/test-rcu-list.c         |  3 ++-
>>   tests/test-util-filemonitor.c |  2 +-
>>   ui/cocoa.m                    |  2 +-
>>   ui/vnc-jobs.c                 |  6 ++++--
>>   util/compatfd.c               |  6 ++++--
>>   util/oslib-posix.c            |  3 ++-
>>   util/qemu-thread-posix.c      | 30 +++++++++++++++++++++++-------
>>   util/qemu-thread-win32.c      | 13 ++++++++++---
>>   util/rcu.c                    |  3 ++-
>>   util/thread-pool.c            |  4 +++-
>>   30 files changed, 121 insertions(+), 57 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 927a00aa90..28a241a936 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -1971,15 +1971,17 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
>>               snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/TCG",
>>                    cpu->cpu_index);
>>   
>> +            /* TODO: let the callers handle the error instead of abort() here */
>>               qemu_thread_create(cpu->thread, thread_name, qemu_tcg_cpu_thread_fn,
>> -                               cpu, QEMU_THREAD_JOINABLE);
>> +                               cpu, QEMU_THREAD_JOINABLE, &error_abort);
>>   
>>           } else {
>>               /* share a single thread for all cpus with TCG */
>>               snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "ALL CPUs/TCG");
>> +            /* TODO: let the callers handle the error instead of abort() here */
>>               qemu_thread_create(cpu->thread, thread_name,
>>                                  qemu_tcg_rr_cpu_thread_fn,
>> -                               cpu, QEMU_THREAD_JOINABLE);
>> +                               cpu, QEMU_THREAD_JOINABLE, &error_abort);
>>   
>>               single_tcg_halt_cond = cpu->halt_cond;
>>               single_tcg_cpu_thread = cpu->thread;
>> @@ -2007,8 +2009,9 @@ static void qemu_hax_start_vcpu(CPUState *cpu)
>>   
>>       snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HAX",
>>                cpu->cpu_index);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(cpu->thread, thread_name, qemu_hax_cpu_thread_fn,
>> -                       cpu, QEMU_THREAD_JOINABLE);
>> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>>   #ifdef _WIN32
>>       cpu->hThread = qemu_thread_get_handle(cpu->thread);
>>   #endif
>> @@ -2023,8 +2026,9 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
>>       qemu_cond_init(cpu->halt_cond);
>>       snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/KVM",
>>                cpu->cpu_index);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
>> -                       cpu, QEMU_THREAD_JOINABLE);
>> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>>   }
>>   
>>   static void qemu_hvf_start_vcpu(CPUState *cpu)
>> @@ -2041,8 +2045,9 @@ static void qemu_hvf_start_vcpu(CPUState *cpu)
>>   
>>       snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
>>                cpu->cpu_index);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
>> -                       cpu, QEMU_THREAD_JOINABLE);
>> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>>   }
>>   
>>   static void qemu_whpx_start_vcpu(CPUState *cpu)
>> @@ -2054,8 +2059,9 @@ static void qemu_whpx_start_vcpu(CPUState *cpu)
>>       qemu_cond_init(cpu->halt_cond);
>>       snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/WHPX",
>>                cpu->cpu_index);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(cpu->thread, thread_name, qemu_whpx_cpu_thread_fn,
>> -                       cpu, QEMU_THREAD_JOINABLE);
>> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>>   #ifdef _WIN32
>>       cpu->hThread = qemu_thread_get_handle(cpu->thread);
>>   #endif
>> @@ -2070,8 +2076,9 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
>>       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);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>> +    qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn,
>> +                       cpu, QEMU_THREAD_JOINABLE, &error_abort);
>>   }
>>   
>>   void qemu_init_vcpu(CPUState *cpu)
>> diff --git a/dump/dump.c b/dump/dump.c
>> index c7b2301652..1358e62fb7 100644
>> --- a/dump/dump.c
>> +++ b/dump/dump.c
>> @@ -2022,8 +2022,9 @@ void qmp_dump_guest_memory(bool paging, const char *file,
>>       if (detach_p) {
>>           /* detached dump */
>>           s->detached = true;
>> +        /* TODO: let the further caller handle the error instead of abort() */
>>           qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread,
>> -                           s, QEMU_THREAD_DETACHED);
>> +                           s, QEMU_THREAD_DETACHED, &error_abort);
>>       } else {
>>           /* sync dump */
>>           dump_process(s, errp);
>> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
>> index 3b6dba2c72..1edc66955a 100644
>> --- a/hw/misc/edu.c
>> +++ b/hw/misc/edu.c
>> @@ -29,6 +29,7 @@
>>   #include "qemu/timer.h"
>>   #include "qemu/main-loop.h" /* iothread mutex */
>>   #include "qemu/module.h"
>> +#include "qapi/error.h"
>>   #include "qapi/visitor.h"
>>   
>>   #define TYPE_PCI_EDU_DEVICE "edu"
>> @@ -370,8 +371,9 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp)
>>   
>>       qemu_mutex_init(&edu->thr_mutex);
>>       qemu_cond_init(&edu->thr_cond);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(&edu->thread, "edu", edu_fact_thread,
>> -                       edu, QEMU_THREAD_JOINABLE);
>> +                       edu, QEMU_THREAD_JOINABLE, &error_abort);
>>   
>>       memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu,
>>                       "edu-mmio", 1 * MiB);
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 6808d4cda8..d64b634526 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -508,8 +508,10 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
>>       pending->shift = shift;
>>       pending->ret = H_HARDWARE;
>>   
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(&pending->thread, "sPAPR HPT prepare",
>> -                       hpt_prepare_thread, pending, QEMU_THREAD_DETACHED);
>> +                       hpt_prepare_thread, pending,
>> +                       QEMU_THREAD_DETACHED, &error_abort);
>>   
>>       spapr->pending_hpt = pending;
>>   
>> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
>> index c39051068d..29f52955d0 100644
>> --- a/hw/rdma/rdma_backend.c
>> +++ b/hw/rdma/rdma_backend.c
>> @@ -281,7 +281,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)(void *ctx,
>> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
>> index e20f8ed293..4d95ee07aa 100644
>> --- a/hw/usb/ccid-card-emulated.c
>> +++ b/hw/usb/ccid-card-emulated.c
>> @@ -545,10 +545,11 @@ static void emulated_realize(CCIDCardState *base, Error **errp)
>>           error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
>>           goto out2;
>>       }
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
>> -                       card, QEMU_THREAD_JOINABLE);
>> +                       card, QEMU_THREAD_JOINABLE, &error_abort);
>>       qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
>> -                       card, QEMU_THREAD_JOINABLE);
>> +                       card, QEMU_THREAD_JOINABLE, &error_abort);
>>   
>>       return;
>>   
>> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
>> index 55d83a907c..ec18d8b171 100644
>> --- a/include/qemu/thread.h
>> +++ b/include/qemu/thread.h
>> @@ -152,9 +152,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,
>> -                        void *(*start_routine)(void *),
>> -                        void *arg, int mode);
>> +int qemu_thread_create(QemuThread *thread, const char *name,
>> +                       void *(*start_routine)(void *),
>> +                       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);
>> diff --git a/io/task.c b/io/task.c
>> index 64c4c7126a..1499540cdf 100644
>> --- a/io/task.c
>> +++ b/io/task.c
>> @@ -170,7 +170,8 @@ void qio_task_run_in_thread(QIOTask *task,
>>                          "io-task-worker",
>>                          qio_task_thread_worker,
>>                          task,
>> -                       QEMU_THREAD_DETACHED);
>> +                       QEMU_THREAD_DETACHED,
>> +                       &error_abort);
>>   }
>>   
>>   
>> diff --git a/iothread.c b/iothread.c
>> index 7130be58e3..2d5a5bfe6c 100644
>> --- a/iothread.c
>> +++ b/iothread.c
>> @@ -198,8 +198,9 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
>>        */
>>       name = object_get_canonical_path_component(OBJECT(obj));
>>       thread_name = g_strdup_printf("IO %s", name);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(&iothread->thread, thread_name, iothread_run,
>> -                       iothread, QEMU_THREAD_JOINABLE);
>> +                       iothread, QEMU_THREAD_JOINABLE, &error_abort);
>>       g_free(thread_name);
>>       g_free(name);
>>   
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 8a607fe1e2..1786afb63d 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -483,8 +483,10 @@ static void process_incoming_migration_co(void *opaque)
>>               goto fail;
>>           }
>>   
>> +        /* TODO: let the further caller handle the error instead of abort() */
>>           qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
>> -             colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
>> +                           colo_process_incoming_thread, mis,
>> +                           QEMU_THREAD_JOINABLE, &error_abort);
>>           mis->have_colo_incoming_thread = true;
>>           qemu_coroutine_yield();
>>   
>> @@ -2496,8 +2498,10 @@ static int open_return_path_on_source(MigrationState *ms,
>>           return 0;
>>       }
>>   
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(&ms->rp_state.rp_thread, "return path",
>> -                       source_return_path_thread, ms, QEMU_THREAD_JOINABLE);
>> +                       source_return_path_thread, ms,
>> +                       QEMU_THREAD_JOINABLE, &error_abort);
>>   
>>       trace_open_return_path_on_source_continue();
>>   
>> @@ -3342,8 +3346,9 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>>           migrate_fd_cleanup(s);
>>           return;
>>       }
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
>> -                       QEMU_THREAD_JOINABLE);
>> +                       QEMU_THREAD_JOINABLE, &error_abort);
>>       s->migration_thread_running = true;
>>   }
>>   
>> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
>> index 9faacacc9e..135e9cd746 100644
>> --- a/migration/postcopy-ram.c
>> +++ b/migration/postcopy-ram.c
>> @@ -1121,8 +1121,10 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
>>       }
>>   
>>       qemu_sem_init(&mis->fault_thread_sem, 0);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(&mis->fault_thread, "postcopy/fault",
>> -                       postcopy_ram_fault_thread, mis, QEMU_THREAD_JOINABLE);
>> +                       postcopy_ram_fault_thread, mis,
>> +                       QEMU_THREAD_JOINABLE, &error_abort);
>>       qemu_sem_wait(&mis->fault_thread_sem);
>>       qemu_sem_destroy(&mis->fault_thread_sem);
>>       mis->have_fault_thread = true;
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 2b0774c2bf..0f677f2d27 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -565,9 +565,10 @@ 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);
>> +        /* TODO: let the further caller handle the error instead of abort() */
>>           qemu_thread_create(compress_threads + i, "compress",
>>                              do_data_compress, comp_param + i,
>> -                           QEMU_THREAD_JOINABLE);
>> +                           QEMU_THREAD_JOINABLE, &error_abort);
>>       }
>>       return 0;
>>   
>> @@ -1167,8 +1168,9 @@ 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;
>> +        /* TODO: let the further caller handle the error instead of abort() */
>>           qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
>> -                           QEMU_THREAD_JOINABLE);
>> +                           QEMU_THREAD_JOINABLE, &error_abort);
>>       }
>>   }
>>   
>> @@ -1447,8 +1449,9 @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
>>       p->num_packets = 1;
>>   
>>       p->running = true;
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
>> -                       QEMU_THREAD_JOINABLE);
>> +                       QEMU_THREAD_JOINABLE, &error_abort);
>>       atomic_inc(&multifd_recv_state->count);
>>       return atomic_read(&multifd_recv_state->count) ==
>>              migrate_multifd_channels();
>> @@ -3891,9 +3894,10 @@ 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;
>> +        /* TODO: let the further caller handle the error instead of abort() */
>>           qemu_thread_create(decompress_threads + i, "decompress",
>>                              do_data_decompress, decomp_param + i,
>> -                           QEMU_THREAD_JOINABLE);
>> +                           QEMU_THREAD_JOINABLE, &error_abort);
>>       }
>>       return 0;
>>   exit:
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 79ed44d475..c792dee8aa 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1826,9 +1826,10 @@ 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);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>>       qemu_thread_create(&mis->listen_thread, "postcopy/listen",
>>                          postcopy_ram_listen_thread, NULL,
>> -                       QEMU_THREAD_DETACHED);
>> +                       QEMU_THREAD_DETACHED, &error_abort);
>>       qemu_sem_wait(&mis->listen_thread_sem);
>>       qemu_sem_destroy(&mis->listen_thread_sem);
>>   
>> diff --git a/tests/atomic64-bench.c b/tests/atomic64-bench.c
>> index 121a8c14f4..08ec753582 100644
>> --- a/tests/atomic64-bench.c
>> +++ b/tests/atomic64-bench.c
>> @@ -8,6 +8,7 @@
>>   #include "qemu/thread.h"
>>   #include "qemu/host-utils.h"
>>   #include "qemu/processor.h"
>> +#include "qapi/error.h"
>>   
>>   struct thread_info {
>>       uint64_t r;
>> @@ -103,7 +104,7 @@ static void create_threads(void)
>>           info->r = (i + 1) ^ time(NULL);
>>           info->accesses = 0;
>>           qemu_thread_create(&threads[i], NULL, thread_func, info,
>> -                           QEMU_THREAD_JOINABLE);
>> +                           QEMU_THREAD_JOINABLE, &error_abort);
>>       }
>>   }
>>   
>> diff --git a/tests/atomic_add-bench.c b/tests/atomic_add-bench.c
>> index 5666f6bbff..52071585a9 100644
>> --- a/tests/atomic_add-bench.c
>> +++ b/tests/atomic_add-bench.c
>> @@ -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;
>> @@ -108,7 +109,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);
>>       }
>>   }
>>   
>> diff --git a/tests/iothread.c b/tests/iothread.c
>> index 777d9eea46..f4ad992e61 100644
>> --- a/tests/iothread.c
>> +++ b/tests/iothread.c
>> @@ -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);
>> diff --git a/tests/qht-bench.c b/tests/qht-bench.c
>> index e3b512f26f..728c2e06c7 100644
>> --- a/tests/qht-bench.c
>> +++ b/tests/qht-bench.c
>> @@ -10,6 +10,7 @@
>>   #include "qemu/qht.h"
>>   #include "qemu/rcu.h"
>>   #include "qemu/xxhash.h"
>> +#include "qapi/error.h"
>>   
>>   struct thread_stats {
>>       size_t rd;
>> @@ -248,7 +249,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);
>>       }
>>   }
>>   
>> diff --git a/tests/rcutorture.c b/tests/rcutorture.c
>> index 49311c82ea..0e799ff256 100644
>> --- a/tests/rcutorture.c
>> +++ b/tests/rcutorture.c
>> @@ -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++;
>>   }
>>   
>> diff --git a/tests/test-aio.c b/tests/test-aio.c
>> index 86fb73b3d5..b3ac261724 100644
>> --- a/tests/test-aio.c
>> +++ b/tests/test-aio.c
>> @@ -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);
>> diff --git a/tests/test-char.c b/tests/test-char.c
>> index f9440cdcfd..093e26e333 100644
>> --- a/tests/test-char.c
>> +++ b/tests/test-char.c
>> @@ -793,7 +793,7 @@ static void char_socket_server_test(gconstpointer opaque)
>>        */
>>       qemu_thread_create(&thread, "client",
>>                          char_socket_server_client_thread,
>> -                       addr, QEMU_THREAD_JOINABLE);
>> +                       addr, QEMU_THREAD_JOINABLE, &error_abort);
>>       g_assert(data.event == -1);
>>   
>>       if (config->wait_connected) {
>> @@ -901,7 +901,7 @@ static void char_socket_client_test(gconstpointer opaque)
>>        */
>>       qemu_thread_create(&thread, "client",
>>                          char_socket_client_server_thread,
>> -                       ioc, QEMU_THREAD_JOINABLE);
>> +                       ioc, QEMU_THREAD_JOINABLE, &error_abort);
>>   
>>       /*
>>        * Populate the chardev address based on what the server
>> @@ -994,7 +994,7 @@ static void char_socket_client_test(gconstpointer opaque)
>>           reconnected = true;
>>           qemu_thread_create(&thread, "client",
>>                              char_socket_client_server_thread,
>> -                           ioc, QEMU_THREAD_JOINABLE);
>> +                           ioc, QEMU_THREAD_JOINABLE, &error_abort);
>>           goto reconnect;
>>       }
>>   
>> diff --git a/tests/test-rcu-list.c b/tests/test-rcu-list.c
>> index 6f076473e0..399744f03c 100644
>> --- a/tests/test-rcu-list.c
>> +++ b/tests/test-rcu-list.c
>> @@ -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++;
>>   }
>>   
>> diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
>> index 46e781c022..a7a7c1eeeb 100644
>> --- a/tests/test-util-filemonitor.c
>> +++ b/tests/test-util-filemonitor.c
>> @@ -397,7 +397,7 @@ test_file_monitor_events(void)
>>       evstopping = 0;
>>       qemu_thread_create(&th, "event-loop",
>>                          qemu_file_monitor_test_event_loop, NULL,
>> -                       QEMU_THREAD_JOINABLE);
>> +                       QEMU_THREAD_JOINABLE, &error_abort);
>>   
>>       if (local_err) {
>>           g_printerr("File monitoring not available: %s",
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index c2984028c5..d8cf1484d9 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -1766,7 +1766,7 @@ int main (int argc, const char * argv[]) {
>>       qemu_sem_init(&app_started_sem, 0);
>>   
>>       qemu_thread_create(&thread, "qemu_main", call_qemu_main,
>> -                       NULL, QEMU_THREAD_DETACHED);
>> +                       NULL, QEMU_THREAD_DETACHED, &error_abort);
>>   
>>       COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
>>       qemu_sem_wait(&display_init_sem);
>> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
>> index 929391f85d..5712f1f501 100644
>> --- a/ui/vnc-jobs.c
>> +++ b/ui/vnc-jobs.c
>> @@ -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"
>>   
>>   /*
>> @@ -339,7 +340,8 @@ void vnc_start_worker_thread(void)
>>           return ;
>>   
>>       q = vnc_queue_init();
>> -    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread, q,
>> -                       QEMU_THREAD_DETACHED);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>> +    qemu_thread_create(&q->thread, "vnc_worker", vnc_worker_thread,
>> +                       q, QEMU_THREAD_DETACHED, &error_abort);
>>       queue = q; /* Set global queue */
>>   }
>> diff --git a/util/compatfd.c b/util/compatfd.c
>> index c296f55d14..caad10ae56 100644
>> --- a/util/compatfd.c
>> +++ b/util/compatfd.c
>> @@ -15,6 +15,7 @@
>>   
>>   #include "qemu/osdep.h"
>>   #include "qemu/thread.h"
>> +#include "qapi/error.h"
>>   
>>   #include <sys/syscall.h>
>>   
>> @@ -87,8 +88,9 @@ static int qemu_signalfd_compat(const sigset_t *mask)
>>       memcpy(&info->mask, mask, sizeof(*mask));
>>       info->fd = fds[1];
>>   
>> -    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat, info,
>> -                       QEMU_THREAD_DETACHED);
>> +    /* TODO: let the further caller handle the error instead of abort() here */
>> +    qemu_thread_create(&thread, "signalfd_compat", sigwait_compat,
>> +                       info, QEMU_THREAD_DETACHED, &error_abort);
>>   
>>       return fds[0];
>>   }
>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> index fe0309ce7a..55c1b9c098 100644
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -465,9 +465,10 @@ 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;
>>           memset_thread[i].hpagesize = hpagesize;
>> +        /* TODO: let the callers handle the error instead of abort() here */
>>           qemu_thread_create(&memset_thread[i].pgthread, "touch_pages",
>>                              do_touch_pages, &memset_thread[i],
>> -                           QEMU_THREAD_JOINABLE);
>> +                           QEMU_THREAD_JOINABLE, &error_abort);
>>           addr += size_per_thread;
>>           numpages -= numpages_per_thread;
>>       }
>> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
>> index 1bf5e65dea..06a9e78ba8 100644
>> --- a/util/qemu-thread-posix.c
>> +++ b/util/qemu-thread-posix.c
>> @@ -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,18 @@ static void *qemu_thread_start(void *args)
>>       return r;
>>   }
>>   
>> -void qemu_thread_create(QemuThread *thread, const char *name,
>> -                       void *(*start_routine)(void*),
>> -                       void *arg, int mode)
>> +/*
>> + * Create a new thread with name @name
>> + * The thread executes @start_routine() with argument @arg.
>> + * The thread will be created in a detached state if @mode is
>> + * QEMU_THREAD_DETACHED, and in a jounable state if it's
>> + * QEMU_THREAD_JOINABLE.
>> + * On success, return 0.
>> + * On failure, store an error through @errp and return negative errno.
>> + */
>> +int 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 +525,8 @@ void qemu_thread_create(QemuThread *thread, const char *name,
>>   
>>       err = pthread_attr_init(&attr);
>>       if (err) {
>> -        error_exit(err, __func__);
>> +        error_setg(errp, "pthread_attr_init failed");
>> +        return -err;
>>       }
>>   
>>       if (mode == QEMU_THREAD_DETACHED) {
>> @@ -538,13 +549,18 @@ 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) {
>> +        error_setg(errp, "pthread_create failed");
>> +        pthread_attr_destroy(&attr);
>> +        g_free(qemu_thread_args->name);
>> +        g_free(qemu_thread_args);
>> +        return -err;
>> +    }
>>   
>>       pthread_sigmask(SIG_SETMASK, &oldset, NULL);
>>   
>>       pthread_attr_destroy(&attr);
>> +    return 0;
>>   }
>>   
>>   void qemu_thread_get_self(QemuThread *thread)
>> diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
>> index 572f88535d..f2deea5250 100644
>> --- a/util/qemu-thread-win32.c
>> +++ b/util/qemu-thread-win32.c
>> @@ -16,6 +16,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;
>> @@ -384,9 +385,9 @@ void *qemu_thread_join(QemuThread *thread)
>>       return ret;
>>   }
>>   
>> -void qemu_thread_create(QemuThread *thread, const char *name,
>> +int qemu_thread_create(QemuThread *thread, const char *name,
>>                          void *(*start_routine)(void *),
>> -                       void *arg, int mode)
>> +                       void *arg, int mode, Error **errp)
>>   {
>>       HANDLE hThread;
>>       struct QemuThreadData *data;
>> @@ -405,10 +406,16 @@ void qemu_thread_create(QemuThread *thread, const char *name,
>>       hThread = (HANDLE) _beginthreadex(NULL, 0, win32_start_routine,
>>                                         data, 0, &thread->tid);
>>       if (!hThread) {
>> -        error_exit(GetLastError(), __func__);
>> +        if (data->mode != QEMU_THREAD_DETACHED) {
>> +            DeleteCriticalSection(&data->cs);
>> +        }
>> +        error_setg(errp, "failed to create win32_start_routine");
>> +        g_free(data);
>> +        return -errno;
>>       }
>>       CloseHandle(hThread);
>>       thread->data = data;
>> +    return 0;
>>   }
>>   
>>   void qemu_thread_get_self(QemuThread *thread)
>> diff --git a/util/rcu.c b/util/rcu.c
>> index 177a675619..ff11837b47 100644
>> --- a/util/rcu.c
>> +++ b/util/rcu.c
>> @@ -31,6 +31,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
>> @@ -324,7 +325,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();
>>   }
>> diff --git a/util/thread-pool.c b/util/thread-pool.c
>> index 4ed9b89ab2..9a867776f9 100644
>> --- a/util/thread-pool.c
>> +++ b/util/thread-pool.c
>> @@ -21,6 +21,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);
>>   
>> @@ -131,7 +132,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)


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-07-17 12:30 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-17  2:32 [Qemu-devel] [PATCH v14 00/11] qemu_thread_create: propagate the error to callers to handle Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 01/11] qemu_thread: make qemu_thread_create() take Error ** argument Fei Li
2019-07-17  5:53   ` David Gibson
2019-07-17 12:27     ` Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 02/11] qemu_thread: supplement error handling for qemu_X_start_vcpu Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 03/11] qemu_thread: supplement error handling for qmp_dump_guest_memory Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 04/11] qemu_thread: supplement error handling for pci_edu_realize Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 05/11] qemu_thread: supplement error handling for h_resize_hpt_prepare Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 06/11] qemu_thread: supplement error handling for emulated_realize Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 07/11] qemu_thread: supplement error handling for iothread_complete Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 08/11] qemu_thread: supplement error handling for qemu_signalfd_compat Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 09/11] qemu_thread: supplement error handling for migration Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 10/11] qemu_thread: supplement error handling for vnc_start_worker_thread Fei Li
2019-07-17  2:33 ` [Qemu-devel] [PATCH v14 11/11] qemu_thread: supplement error handling for touch_all_pages Fei Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).