On Fri, Dec 26, 2025 at 06:19:10PM -0300, Fabiano Rosas wrote:
> Freeing the error at migration_connect() is redundant in the normal
> migration case. The freeing already happened at migrate_init():
>
> qmp_migrate()
> -> migrate_prepare()
> -> migrate_init()
> -> qmp_migrate_finish()
> -> *_start_outgoing_migration()
> -> migration_channel_connect()
> -> migration_connect()
>
> For the resume case, migrate_prepare() returns early and doesn't reach
> migrate_init(). Move the extra migrate_error_free() call to
> migrate_prepare() along with the resume check.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
We could also use migrate_error_free() in migrate_init(), to be clear on
when the error can be erased.
> ---
> migration/migration.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 4b1afcab24..a56f8fb05e 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2088,6 +2088,13 @@ static bool migrate_prepare(MigrationState *s, bool resume, Error **errp)
> migrate_set_state(&s->state, MIGRATION_STATUS_POSTCOPY_PAUSED,
> MIGRATION_STATUS_POSTCOPY_RECOVER_SETUP);
>
> + /*
> + * If there's a previous error, free it and prepare for
> + * another one. For the non-resume case, this happens at
> + * migrate_init() below.
> + */
> + migrate_error_free(s);
> +
> /* This is a resume, skip init status */
> return true;
> }
> @@ -4016,13 +4023,6 @@ void migration_connect(MigrationState *s, Error *error_in)
> bool resume = (s->state == MIGRATION_STATUS_POSTCOPY_RECOVER_SETUP);
> int ret;
>
> - /*
> - * If there's a previous error, free it and prepare for another one.
> - * Meanwhile if migration completes successfully, there won't have an error
> - * dumped when calling migration_cleanup().
> - */
> - migrate_error_free(s);
> -
> s->expected_downtime = migrate_downtime_limit();
> if (error_in) {
> migration_connect_error_propagate(s, error_in);
> --
> 2.51.0
>
--
Peter Xu