On Fri, Dec 26, 2025 at 06:19:12PM -0300, Fabiano Rosas wrote:
> Cover the CANCELLING state in migration_connect_error_propagate() and
> use it to funnel errors from migrate_prepare() until the end of
> migration_connect().
> 
> (add some line breaks for legibility)
> 
> Signed-off-by: Fabiano Rosas <[email protected]>
> ---
>  migration/migration.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 83854d084a..e1c00867ab 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1575,18 +1575,25 @@ static void migrate_error_free(MigrationState *s)
>  static void migration_connect_error_propagate(MigrationState *s, Error 
> *error)
>  {
>      MigrationStatus current = s->state;
> -    MigrationStatus next;
> -
> -    assert(s->to_dst_file == NULL);
> +    MigrationStatus next = MIGRATION_STATUS_NONE;
>  
>      switch (current) {
>      case MIGRATION_STATUS_SETUP:
>          next = MIGRATION_STATUS_FAILED;
>          break;
> +
>      case MIGRATION_STATUS_POSTCOPY_RECOVER_SETUP:
>          /* Never fail a postcopy migration; switch back to PAUSED instead */
>          next = MIGRATION_STATUS_POSTCOPY_PAUSED;
>          break;
> +
> +    case MIGRATION_STATUS_CANCELLING:
> +        /*
> +         * Don't move out of CANCELLING, the only valid transition is to
> +         * CANCELLED, at migration_cleanup().
> +         */
> +        break;
> +
>      default:
>          /*
>           * This really shouldn't happen. Just be careful to not crash a VM
> @@ -1597,7 +1604,10 @@ static void 
> migration_connect_error_propagate(MigrationState *s, Error *error)
>          return;
>      }
>  
> -    migrate_set_state(&s->state, current, next);
> +    if (next) {
> +        migrate_set_state(&s->state, current, next);
> +    }
> +
>      migrate_error_propagate(s, error);
>  }
>  
> @@ -4107,11 +4117,7 @@ void migration_connect(MigrationState *s, Error 
> *error_in)
>      return;
>  
>  fail:
> -    migrate_error_propagate(s, error_copy(local_err));
> -    if (s->state != MIGRATION_STATUS_CANCELLING) {
> -        migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
> -    }
> -    error_report_err(local_err);

OK you removed this line here..  maybe it should belong to the previous
patch?

If you agree, feel free to take:

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

> +    migration_connect_error_propagate(s, local_err);
>      migration_cleanup(s);
>      if (s->error) {
>          error_report_err(error_copy(s->error));
> -- 
> 2.51.0
> 

-- 
Peter Xu


Reply via email to