Re: [PATCH v6 4/5] migration: process_incoming_migration_co(): rework error reporting

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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>