| Message ID | 20230210063630.532185-1-leobras@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/4] migration/multifd: Change multifd_load_cleanup() signature and usage | expand |
Leonardo Bras <leobras@redhat.com> wrote: > Since it's introduction in commit f986c3d256 ("migration: Create multifd > migration threads"), multifd_load_cleanup() never returned any value > different than 0, neither set up any error on errp. > > Even though, on process_incoming_migration_bh() an if clause uses it's > return value to decide on setting autostart = false, which will never > happen. > > In order to simplify the codebase, change multifd_load_cleanup() signature > to 'void multifd_load_cleanup(void)', and for every usage remove error > handling or decision made based on return value != 0. > > Signed-off-by: Leonardo Bras <leobras@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>
Leo, Please still provide a cover letter as long as >1 patches will be posted as a set. Not only because it still always help to provide an overview for reviewers before reading each of them (e.g. I have a habit of prioritizing reviews based on cover letters first), but also when you're confident enough the reviewer(s) can ACK the patches in one reply. :-) On Fri, Feb 10, 2023 at 03:36:28AM -0300, Leonardo Bras wrote: > Since it's introduction in commit f986c3d256 ("migration: Create multifd > migration threads"), multifd_load_cleanup() never returned any value > different than 0, neither set up any error on errp. > > Even though, on process_incoming_migration_bh() an if clause uses it's > return value to decide on setting autostart = false, which will never > happen. > > In order to simplify the codebase, change multifd_load_cleanup() signature > to 'void multifd_load_cleanup(void)', and for every usage remove error > handling or decision made based on return value != 0. > > Signed-off-by: Leonardo Bras <leobras@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com>
diff --git a/migration/multifd.h b/migration/multifd.h index ff3aa2e2e9..9a7e1a8826 100644 --- a/migration/multifd.h +++ b/migration/multifd.h @@ -16,7 +16,7 @@ int multifd_save_setup(Error **errp); void multifd_save_cleanup(void); int multifd_load_setup(Error **errp); -int multifd_load_cleanup(Error **errp); +void multifd_load_cleanup(void); bool multifd_recv_all_channels_created(void); void multifd_recv_new_channel(QIOChannel *ioc, Error **errp); void multifd_recv_sync_main(void); diff --git a/migration/migration.c b/migration/migration.c index 7a14aa98d8..ce962ea577 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -543,13 +543,7 @@ static void process_incoming_migration_bh(void *opaque) */ qemu_announce_self(&mis->announce_timer, migrate_announce_params()); - if (multifd_load_cleanup(&local_err) != 0) { - error_report_err(local_err); - autostart = false; - } - /* If global state section was not received or we are in running - state, we need to obey autostart. Any other state is set with - runstate_set. */ + multifd_load_cleanup(); dirty_bitmap_mig_before_vm_start(); @@ -649,9 +643,9 @@ fail: 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); - } + + multifd_load_cleanup(); + exit(EXIT_FAILURE); } diff --git a/migration/multifd.c b/migration/multifd.c index b7ad7002e0..174726982c 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -1022,12 +1022,12 @@ static void multifd_recv_terminate_threads(Error *err) } } -int multifd_load_cleanup(Error **errp) +void multifd_load_cleanup(void) { int i; if (!migrate_use_multifd() || !migrate_multi_channels_is_allowed()) { - return 0; + return; } multifd_recv_terminate_threads(NULL); for (i = 0; i < migrate_multifd_channels(); i++) { @@ -1067,8 +1067,6 @@ int multifd_load_cleanup(Error **errp) multifd_recv_state->params = NULL; g_free(multifd_recv_state); multifd_recv_state = NULL; - - return 0; } void multifd_recv_sync_main(void)
Since it's introduction in commit f986c3d256 ("migration: Create multifd migration threads"), multifd_load_cleanup() never returned any value different than 0, neither set up any error on errp. Even though, on process_incoming_migration_bh() an if clause uses it's return value to decide on setting autostart = false, which will never happen. In order to simplify the codebase, change multifd_load_cleanup() signature to 'void multifd_load_cleanup(void)', and for every usage remove error handling or decision made based on return value != 0. Signed-off-by: Leonardo Bras <leobras@redhat.com> --- migration/multifd.h | 2 +- migration/migration.c | 14 ++++---------- migration/multifd.c | 6 ++---- 3 files changed, 7 insertions(+), 15 deletions(-)