[PATCH v3 07/25] migration: Free the error earlier in the resume case

Fabiano Rosas posted 25 patches 1 month ago
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Mark Kanda <mark.kanda@oracle.com>, Ben Chaney <bchaney@akamai.com>, Li Zhijian <lizhijian@fujitsu.com>
There is a newer version of this series
[PATCH v3 07/25] migration: Free the error earlier in the resume case
Posted by Fabiano Rosas 1 month ago
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.

Also change migrate_init() to use migrate_error_free(), so it's easier
to see where are the places the error gets freed.

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/migration.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 388e0be5a2..9204029c88 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1811,8 +1811,8 @@ int migrate_init(MigrationState *s, Error **errp)
     s->setup_time = 0;
     s->start_postcopy = false;
     s->migration_thread_running = false;
-    error_free(s->error);
-    s->error = NULL;
+
+    migrate_error_free(s);
 
     if (should_send_vmdesc()) {
         s->vmdesc = json_writer_new(false);
@@ -2087,6 +2087,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;
     }
@@ -4015,13 +4022,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
Re: [PATCH v3 07/25] migration: Free the error earlier in the resume case
Posted by Prasad Pandit 3 weeks, 3 days ago
On Fri, 9 Jan 2026 at 18:11, Fabiano Rosas <farosas@suse.de> wrote:
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1811,8 +1811,8 @@ int migrate_init(MigrationState *s, Error **errp)
>      s->setup_time = 0;
>      s->start_postcopy = false;
>      s->migration_thread_running = false;
> -    error_free(s->error);
> -    s->error = NULL;
> +
> +    migrate_error_free(s);
>
>      if (should_send_vmdesc()) {
>          s->vmdesc = json_writer_new(false);
> @@ -2087,6 +2087,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;
>      }
> @@ -4015,13 +4022,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);

* Looks okay.
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>

Thank you.
---
  - Prasad