Below process could crash qemu with postcopy recovery:

  1. (hmp) migrate -d ..
  2. (hmp) migrate_start_postcopy
  3. [network down, postcopy paused]
  4. (hmp) migrate -r $WRONG_PORT
     when try the recover on an invalid $WRONG_PORT, cleanup_bh will be cleared
  5. (hmp) migrate -r $RIGHT_PORT
     [qemu crash on assert(cleanup_bh)]

The thing is we shouldn't cleanup if it's postcopy resume; the error is set
mostly because the channel is wrong, so we return directly waiting for the user
to retry.

migrate_fd_cleanup() should only be called when migration is cancelled or
completed.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 8786104c9a..bb1edf862a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3975,7 +3975,18 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
     }
     if (error_in) {
         migrate_fd_error(s, error_in);
-        migrate_fd_cleanup(s);
+        if (resume) {
+            /*
+             * Don't do cleanup for resume if channel is invalid, but only dump
+             * the error.  We wait for another channel connect from the user.
+             * The error_report still gives HMP user a hint on what failed.
+             * It's normally done in migrate_fd_cleanup(), but call it here
+             * explicitly.
+             */
+            error_report_err(error_copy(s->error));
+        } else {
+            migrate_fd_cleanup(s);
+        }
         return;
     }
 
-- 
2.31.1

* Peter Xu (peterx@redhat.com) wrote:
> Below process could crash qemu with postcopy recovery:
> 
>   1. (hmp) migrate -d ..
>   2. (hmp) migrate_start_postcopy
>   3. [network down, postcopy paused]
>   4. (hmp) migrate -r $WRONG_PORT
>      when try the recover on an invalid $WRONG_PORT, cleanup_bh will be cleared
>   5. (hmp) migrate -r $RIGHT_PORT
>      [qemu crash on assert(cleanup_bh)]
> 
> The thing is we shouldn't cleanup if it's postcopy resume; the error is set
> mostly because the channel is wrong, so we return directly waiting for the user
> to retry.
> 
> migrate_fd_cleanup() should only be called when migration is cancelled or
> completed.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/migration.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 8786104c9a..bb1edf862a 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3975,7 +3975,18 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>      }
>      if (error_in) {
>          migrate_fd_error(s, error_in);
> -        migrate_fd_cleanup(s);
> +        if (resume) {
> +            /*
> +             * Don't do cleanup for resume if channel is invalid, but only dump
> +             * the error.  We wait for another channel connect from the user.
> +             * The error_report still gives HMP user a hint on what failed.
> +             * It's normally done in migrate_fd_cleanup(), but call it here
> +             * explicitly.
> +             */
> +            error_report_err(error_copy(s->error));
> +        } else {
> +            migrate_fd_cleanup(s);
> +        }
>          return;
>      }
>  
> -- 
> 2.31.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK