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

[fei <lifei1214@126.com>]

> 在 2019年1月9日，23:26，Markus Armbruster <armbru@redhat.com> 写道：
> 
> Fei Li <fli@suse.com> writes:
> 
>> 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.
>> 
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Signed-off-by: Fei Li <fli@suse.com>
> [...]
>> diff --git a/migration/ram.c b/migration/ram.c
>> index eed1daf302..1e24a78eaa 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
> [...]
>> @@ -3625,6 +3637,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;
>> @@ -3646,10 +3659,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)) {
>> +            error_reportf_err(local_err,
>> +                              "failed to create do_data_decompress: ");
>> +            goto exit;
> 
> Broken error handling, see my review of PATCH 16.
Yep, seems both the compress_threads_save_setup() and compress_threads_load_setup() have such problem.

> 
>> +        }
>>     }
>>     return 0;
>> exit:
> [...]