Re: [Qemu-devel] [PATCH for-4.0 v9 08/16] qemu_thread: supplement error handling for qmp_dump_guest_memory

[Markus Armbruster]

Fei Li <f...@suse.com> writes:

> Utilize the existed errp to propagate the error instead of the
> temporary &error_abort.
>
> Cc: Markus Armbruster <arm...@redhat.com>
> Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> Signed-off-by: Fei Li <f...@suse.com>
> ---
>  dump.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/dump.c b/dump.c
> index c35d6ddd22..ef5ea324fa 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -2020,9 +2020,10 @@ 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);
> +        if (!qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread,
> +                           s, QEMU_THREAD_DETACHED, errp)) {
> +            /* keep 'if' here in case there is further error handling logic 
> */
> +        }

I don't think keeping the conditional "just in case" is worthwhile.
Plain

           qemu_thread_create(&s->dump_thread, "dump_thread", dump_thread,
                              s, QEMU_THREAD_DETACHED, errp);

should do fine.

>      } else {
>          /* sync dump */
>          dump_process(s, errp);