diff mbox series

[3/3] migration: Clear error at entry of migrate_fd_connect()

Message ID 20210708190653.252961-4-peterx@redhat.com
State New
Headers show
Series migration: Three more fixes for postcopy recovery | expand

Commit Message

Peter Xu July 8, 2021, 7:06 p.m. UTC
For each "migrate" command, remember to clear the s->error before going on.
For one reason, when there's a new error it'll be still remembered; see
migrate_set_error() who only sets the error if error==NULL.  Meanwhile if a
failed migration completes (e.g., postcopy recovered and finished), we
shouldn't dump an error when calling migrate_fd_cleanup() at last.

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

Comments

Dr. David Alan Gilbert July 12, 2021, 6:40 p.m. UTC | #1
* Peter Xu (peterx@redhat.com) wrote:
> For each "migrate" command, remember to clear the s->error before going on.
> For one reason, when there's a new error it'll be still remembered; see
> migrate_set_error() who only sets the error if error==NULL.  Meanwhile if a
> failed migration completes (e.g., postcopy recovered and finished), we
> shouldn't dump an error when calling migrate_fd_cleanup() at last.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

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

> ---
>  migration/migration.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index bb1edf862a..338df01e97 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1855,6 +1855,15 @@ void migrate_set_error(MigrationState *s, const Error *error)
>      }
>  }
>  
> +static void migrate_error_free(MigrationState *s)
> +{
> +    QEMU_LOCK_GUARD(&s->error_mutex);
> +    if (s->error) {
> +        error_free(s->error);
> +        s->error = NULL;
> +    }
> +}
> +
>  void migrate_fd_error(MigrationState *s, const Error *error)
>  {
>      trace_migrate_fd_error(error_get_pretty(error));
> @@ -3966,6 +3975,13 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>      int64_t rate_limit;
>      bool resume = s->state == MIGRATION_STATUS_POSTCOPY_PAUSED;
>  
> +    /*
> +     * 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 migrate_fd_cleanup().
> +     */
> +    migrate_error_free(s);
> +
>      s->expected_downtime = s->parameters.downtime_limit;
>      if (resume) {
>          assert(s->cleanup_bh);
> -- 
> 2.31.1
>
diff mbox series

Patch

diff --git a/migration/migration.c b/migration/migration.c
index bb1edf862a..338df01e97 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1855,6 +1855,15 @@  void migrate_set_error(MigrationState *s, const Error *error)
     }
 }
 
+static void migrate_error_free(MigrationState *s)
+{
+    QEMU_LOCK_GUARD(&s->error_mutex);
+    if (s->error) {
+        error_free(s->error);
+        s->error = NULL;
+    }
+}
+
 void migrate_fd_error(MigrationState *s, const Error *error)
 {
     trace_migrate_fd_error(error_get_pretty(error));
@@ -3966,6 +3975,13 @@  void migrate_fd_connect(MigrationState *s, Error *error_in)
     int64_t rate_limit;
     bool resume = s->state == MIGRATION_STATUS_POSTCOPY_PAUSED;
 
+    /*
+     * 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 migrate_fd_cleanup().
+     */
+    migrate_error_free(s);
+
     s->expected_downtime = s->parameters.downtime_limit;
     if (resume) {
         assert(s->cleanup_bh);