| Message ID | 20181225140449.15786-6-fli@suse.com |
|---|---|
| State | New |
| Headers | show |
| Series | [for-4.0,v9,01/16] Fix segmentation fault when qemu_signal_init fails | expand |
* 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> > -- > 2.13.7 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
在 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
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)
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(-)