[RFC PATCH 10/25] migration: Expand migration_connect_error_propagate to cover cancelling

Fabiano Rosas posted 25 patches 1 week, 4 days ago
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Li Zhijian <lizhijian@fujitsu.com>
There is a newer version of this series
[RFC PATCH 10/25] migration: Expand migration_connect_error_propagate to cover cancelling
Posted by Fabiano Rosas 1 week, 4 days ago
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 <farosas@suse.de>
---
 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);
+    migration_connect_error_propagate(s, local_err);
     migration_cleanup(s);
     if (s->error) {
         error_report_err(error_copy(s->error));
-- 
2.51.0
Re: [RFC PATCH 10/25] migration: Expand migration_connect_error_propagate to cover cancelling
Posted by Peter Xu 1 week, 1 day ago
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 <farosas@suse.de>
> ---
>  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 <peterx@redhat.com>

> +    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