mbox series

[for-4.0,00/11] qemu_thread_create: propagate the error to callers to handle

Message ID 20190128141506.12731-1-lifei1214@126.com
Headers show
Series qemu_thread_create: propagate the error to callers to handle | expand

Message

fei Jan. 28, 2019, 2:14 p.m. UTC
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! 

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.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     | 14 +++++--
 include/qemu/thread.h           |  6 +--
 include/qom/cpu.h               |  2 +-
 io/task.c                       |  3 +-
 iothread.c                      | 18 ++++++---
 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/atomic_add-bench.c        |  3 +-
 tests/iothread.c                |  2 +-
 tests/qht-bench.c               |  3 +-
 tests/rcutorture.c              |  3 +-
 tests/test-aio.c                |  2 +-
 tests/test-rcu-list.c           |  3 +-
 ui/vnc-jobs.c                   | 17 +++++---
 ui/vnc-jobs.h                   |  2 +-
 ui/vnc.c                        |  4 +-
 util/compatfd.c                 | 13 ++++++-
 util/oslib-posix.c              | 25 +++++++-----
 util/qemu-thread-posix.c        | 30 ++++++++++----
 util/qemu-thread-win32.c        | 13 +++++--
 util/rcu.c                      |  3 +-
 util/thread-pool.c              |  4 +-
 51 files changed, 337 insertions(+), 136 deletions(-)

Comments

Markus Armbruster Jan. 31, 2019, 6:05 p.m. UTC | #1
I'm afraid you made a bit of a mess :)

"[PATCH for-4.0 00/11]" suggests this is v1 of eleven patches.

Fei Li <lifei1214@126.com> writes:

> 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! 
>
> 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.

It's actually v10.

[...]
> 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

It is eleven patches.  However, I got only 01-07/11 with this cover
letter.  A later batch (no cover letter) has 07-11/11 and 07-09/11
again.

I could guess which of the duplicates I should review, but that's work.
Just resend the whole thing as v11, please.

[...]
fei Feb. 1, 2019, 4:55 a.m. UTC | #2
在 2019/2/1 上午2:05, Markus Armbruster 写道:
> I'm afraid you made a bit of a mess :)
>
> "[PATCH for-4.0 00/11]" suggests this is v1 of eleven patches.
Awkward, I forgot the v10.. =-O
>
> Fei Li <lifei1214@126.com> writes:
>
>> 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!
>>
>> 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.
> It's actually v10.
Yes.
>
> [...]
>> 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
> It is eleven patches.  However, I got only 01-07/11 with this cover
> letter.  A later batch (no cover letter) has 07-11/11 and 07-09/11
> again.
>
> I could guess which of the duplicates I should review, but that's work.
> Just resend the whole thing as v11, please.

Ok, and sorry for the trouble.

Have a nice day, thanks
Fei
>
> [...]