Re: [Qemu-devel] [PATCH for-4.0 v9 05/16] migration: unify error handling for process_incoming_migration_co

[Fei Li <lifei1214@126.com>]

在 2019/1/3 下午7:25, Dr. David Alan Gilbert 写道:
> * Fei Li (fli@suse.com) wrote:
>> In the current code, if process_incoming_migration_co() fails we do
>> the same error handing: set the error state, close the source file,
>> do the cleanup for multifd, and then exit(EXIT_FAILURE). To make the
>> code clearer, add a "goto fail" to unify the error handling.
>>
>> 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>
>> ---
>>   migration/migration.c | 26 +++++++++++++-------------
>>   1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 5d322eb9d6..ded151b1bf 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -438,15 +438,13 @@ static void process_incoming_migration_co(void *opaque)
>>           /* Make sure all file formats flush their mutable metadata */
>>           bdrv_invalidate_cache_all(&local_err);
>>           if (local_err) {
>> -            migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>> -                    MIGRATION_STATUS_FAILED);
>>               error_report_err(local_err);
>> -            exit(EXIT_FAILURE);
>> +            goto fail;
>>           }
>>   
>>           if (colo_init_ram_cache() < 0) {
>>               error_report("Init ram cache failed");
>> -            exit(EXIT_FAILURE);
>> +            goto fail;
>>           }
>>   
>>           qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
>> @@ -461,20 +459,22 @@ static void process_incoming_migration_co(void *opaque)
>>       }
>>   
>>       if (ret < 0) {
>> -        Error *local_err = NULL;
>> -
>> -        migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>> -                          MIGRATION_STATUS_FAILED);
>>           error_report("load of migration failed: %s", strerror(-ret));
>> -        qemu_fclose(mis->from_src_file);
>> -        if (multifd_load_cleanup(&local_err) != 0) {
>> -            error_report_err(local_err);
>> -        }
>> -        exit(EXIT_FAILURE);
>> +        goto fail;
>>       }
>>       mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
>>       qemu_bh_schedule(mis->bh);
>>       mis->migration_incoming_co = NULL;
>> +    return;
>> +fail:
>> +    local_err = NULL;
>> +    migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>> +                      MIGRATION_STATUS_FAILED);
>> +    qemu_fclose(mis->from_src_file);
>> +    if (multifd_load_cleanup(&local_err) != 0) {
>> +        error_report_err(local_err);
>> +    }
>> +    exit(EXIT_FAILURE);
>>   }
>>   
>>   static void migration_incoming_setup(QEMUFile *f)
> OK, so this is really unifying the normal error case and the two
> colo-incoming error cases; so I think that's fine.
>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thanks for the review :)

Fei

>
>> -- 
>> 2.13.7
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK