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 <[email protected]>

Reviewed-by: Peter Xu <[email protected]>

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


Reply via email to