| Message ID | 20240430085646.2359711-5-vsementsov@yandex-team.ru |
|---|---|
| State | New |
| Headers | show |
| Series | migration: do not exit on incoming failure | expand |
On 30/4/24 10:56, Vladimir Sementsov-Ogievskiy wrote: > Unify error reporting in the function. This simplifies the following > commit, which will not-exit-on-error behavior variant to the function. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > migration/migration.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index b307a4bc59..a9599838e6 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -735,14 +735,16 @@ static void process_incoming_migration_bh(void *opaque) > static void coroutine_fn > process_incoming_migration_co(void *opaque) > { > + MigrationState *s = migrate_get_current(); (see below) > MigrationIncomingState *mis = migration_incoming_get_current(); > PostcopyState ps; > int ret; > + Error *local_err = NULL; > > assert(mis->from_src_file); > > if (compress_threads_load_setup(mis->from_src_file)) { > - error_report("Failed to setup decompress threads"); > + error_setg(&local_err, "Failed to setup decompress threads"); > goto fail; > } > > @@ -779,19 +781,12 @@ process_incoming_migration_co(void *opaque) > } > > if (ret < 0) { > - MigrationState *s = migrate_get_current(); > - > - if (migrate_has_error(s)) { > - WITH_QEMU_LOCK_GUARD(&s->error_mutex) { > - error_report_err(s->error); > - s->error = NULL; > - } > - } > - error_report("load of migration failed: %s", strerror(-ret)); > + error_setg(&local_err, "load of migration failed: %s", strerror(-ret)); > goto fail; > } > > if (colo_incoming_co() < 0) { > + error_setg(&local_err, "colo incoming failed"); > goto fail; > } > > @@ -800,8 +795,16 @@ process_incoming_migration_co(void *opaque) > fail: Maybe just assign @s in error path here? s = migrate_get_current(); > migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > MIGRATION_STATUS_FAILED); > + migrate_set_error(s, local_err); > + error_free(local_err); > + > migration_incoming_state_destroy(); > > + WITH_QEMU_LOCK_GUARD(&s->error_mutex) { > + error_report_err(s->error); > + s->error = NULL; > + } > + > exit(EXIT_FAILURE); > } > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 30.04.24 12:16, Philippe Mathieu-Daudé wrote: > On 30/4/24 10:56, Vladimir Sementsov-Ogievskiy wrote: >> Unify error reporting in the function. This simplifies the following >> commit, which will not-exit-on-error behavior variant to the function. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> --- >> migration/migration.c | 23 +++++++++++++---------- >> 1 file changed, 13 insertions(+), 10 deletions(-) >> >> diff --git a/migration/migration.c b/migration/migration.c >> index b307a4bc59..a9599838e6 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -735,14 +735,16 @@ static void process_incoming_migration_bh(void *opaque) >> static void coroutine_fn >> process_incoming_migration_co(void *opaque) >> { >> + MigrationState *s = migrate_get_current(); > > (see below) > >> MigrationIncomingState *mis = migration_incoming_get_current(); >> PostcopyState ps; >> int ret; >> + Error *local_err = NULL; >> assert(mis->from_src_file); >> if (compress_threads_load_setup(mis->from_src_file)) { >> - error_report("Failed to setup decompress threads"); >> + error_setg(&local_err, "Failed to setup decompress threads"); >> goto fail; >> } >> @@ -779,19 +781,12 @@ process_incoming_migration_co(void *opaque) >> } >> if (ret < 0) { >> - MigrationState *s = migrate_get_current(); >> - >> - if (migrate_has_error(s)) { >> - WITH_QEMU_LOCK_GUARD(&s->error_mutex) { >> - error_report_err(s->error); >> - s->error = NULL; >> - } >> - } >> - error_report("load of migration failed: %s", strerror(-ret)); >> + error_setg(&local_err, "load of migration failed: %s", strerror(-ret)); >> goto fail; >> } >> if (colo_incoming_co() < 0) { >> + error_setg(&local_err, "colo incoming failed"); >> goto fail; >> } >> @@ -800,8 +795,16 @@ process_incoming_migration_co(void *opaque) >> fail: > > Maybe just assign @s in error path here? > > s = migrate_get_current(); I'd keep as is. If continue improving the function, I'd better split the logic to seperate function with classic "Error **errp" argument. And keep reporting error in caller. > >> migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, >> MIGRATION_STATUS_FAILED); >> + migrate_set_error(s, local_err); >> + error_free(local_err); >> + >> migration_incoming_state_destroy(); >> + WITH_QEMU_LOCK_GUARD(&s->error_mutex) { >> + error_report_err(s->error); >> + s->error = NULL; >> + } >> + >> exit(EXIT_FAILURE); >> } > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> >
diff --git a/migration/migration.c b/migration/migration.c index b307a4bc59..a9599838e6 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -735,14 +735,16 @@ static void process_incoming_migration_bh(void *opaque) static void coroutine_fn process_incoming_migration_co(void *opaque) { + MigrationState *s = migrate_get_current(); MigrationIncomingState *mis = migration_incoming_get_current(); PostcopyState ps; int ret; + Error *local_err = NULL; assert(mis->from_src_file); if (compress_threads_load_setup(mis->from_src_file)) { - error_report("Failed to setup decompress threads"); + error_setg(&local_err, "Failed to setup decompress threads"); goto fail; } @@ -779,19 +781,12 @@ process_incoming_migration_co(void *opaque) } if (ret < 0) { - MigrationState *s = migrate_get_current(); - - if (migrate_has_error(s)) { - WITH_QEMU_LOCK_GUARD(&s->error_mutex) { - error_report_err(s->error); - s->error = NULL; - } - } - error_report("load of migration failed: %s", strerror(-ret)); + error_setg(&local_err, "load of migration failed: %s", strerror(-ret)); goto fail; } if (colo_incoming_co() < 0) { + error_setg(&local_err, "colo incoming failed"); goto fail; } @@ -800,8 +795,16 @@ process_incoming_migration_co(void *opaque) fail: migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_FAILED); + migrate_set_error(s, local_err); + error_free(local_err); + migration_incoming_state_destroy(); + WITH_QEMU_LOCK_GUARD(&s->error_mutex) { + error_report_err(s->error); + s->error = NULL; + } + exit(EXIT_FAILURE); }
Unify error reporting in the function. This simplifies the following commit, which will not-exit-on-error behavior variant to the function. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- migration/migration.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)