[PATCH 0/2] tests/migration-test: Small cleanup series on postcopy tests

[PATCH 0/2] tests/migration-test: Small cleanup series on postcopy tests

[Peter Xu]

This series removes two unnecessary fields in MigrateCommon that was only
for postcopy tests.

Comments welcomed, thanks.

Peter Xu (2):
  tests/migration-test: Remove postcopy_data from MigrateCommon
  tests/migration-test: Remove postcopy_recovery_fail_stage from
    MigrateCommon

 tests/qtest/migration/framework.h      |  7 ++-----
 tests/qtest/migration/framework.c      | 25 ++++++++++++++-----------
 tests/qtest/migration/postcopy-tests.c | 12 ++++--------
 tests/qtest/migration/tls-tests.c      |  8 ++++----
 4 files changed, 24 insertions(+), 28 deletions(-)

-- 
2.50.1

[PATCH 1/2] tests/migration-test: Remove postcopy_data from MigrateCommon

[Peter Xu]

Now postcopy is not the only user of start_hook / end_hook that will pass
in a opaque pointer.  It doesn't need to be defined in MigrateCommon as
part of the framework, as all other hook users can pass hook_data around.
Do it too for postcopy.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qtest/migration/framework.h |  1 -
 tests/qtest/migration/framework.c | 18 ++++++++++--------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
index ed85ed502d..0d39bb0d3c 100644
--- a/tests/qtest/migration/framework.h
+++ b/tests/qtest/migration/framework.h
@@ -230,7 +230,6 @@ typedef struct {
     bool live;
 
     /* Postcopy specific fields */
-    void *postcopy_data;
     PostcopyRecoveryFailStage postcopy_recovery_fail_stage;
 } MigrateCommon;
 
diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index e35839c95f..4f46cf8629 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -541,6 +541,7 @@ void migrate_end(QTestState *from, QTestState *to, bool test_dest)
 
 static int migrate_postcopy_prepare(QTestState **from_ptr,
                                     QTestState **to_ptr,
+                                    void **hook_data,
                                     MigrateCommon *args)
 {
     QTestState *from, *to;
@@ -554,7 +555,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
     }
 
     if (args->start_hook) {
-        args->postcopy_data = args->start_hook(from, to);
+        *hook_data = args->start_hook(from, to);
     }
 
     migrate_ensure_non_converge(from);
@@ -582,7 +583,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
 }
 
 static void migrate_postcopy_complete(QTestState *from, QTestState *to,
-                                      MigrateCommon *args)
+                                      void *hook_data, MigrateCommon *args)
 {
     MigrationTestEnv *env = migration_get_env();
 
@@ -601,8 +602,7 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to,
     }
 
     if (args->end_hook) {
-        args->end_hook(from, to, args->postcopy_data);
-        args->postcopy_data = NULL;
+        args->end_hook(from, to, hook_data);
     }
 
     migrate_end(from, to, true);
@@ -610,13 +610,14 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to,
 
 void test_postcopy_common(MigrateCommon *args)
 {
+    void *hook_data = NULL;
     QTestState *from, *to;
 
-    if (migrate_postcopy_prepare(&from, &to, args)) {
+    if (migrate_postcopy_prepare(&from, &to, &hook_data, args)) {
         return;
     }
     migrate_postcopy_start(from, to, &src_state);
-    migrate_postcopy_complete(from, to, args);
+    migrate_postcopy_complete(from, to, hook_data, args);
 }
 
 static void wait_for_postcopy_status(QTestState *one, const char *status)
@@ -742,6 +743,7 @@ void test_postcopy_recovery_common(MigrateCommon *args)
 {
     QTestState *from, *to;
     g_autofree char *uri = NULL;
+    void *hook_data = NULL;
 
     /*
      * Always enable OOB QMP capability for recovery tests, migrate-recover is
@@ -752,7 +754,7 @@ void test_postcopy_recovery_common(MigrateCommon *args)
     /* Always hide errors for postcopy recover tests since they're expected */
     args->start.hide_stderr = true;
 
-    if (migrate_postcopy_prepare(&from, &to, args)) {
+    if (migrate_postcopy_prepare(&from, &to, &hook_data, args)) {
         return;
     }
 
@@ -808,7 +810,7 @@ void test_postcopy_recovery_common(MigrateCommon *args)
     /* Restore the postcopy bandwidth to unlimited */
     migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
 
-    migrate_postcopy_complete(from, to, args);
+    migrate_postcopy_complete(from, to, hook_data, args);
 }
 
 int test_precopy_common(MigrateCommon *args)
-- 
2.50.1

[PATCH 2/2] tests/migration-test: Remove postcopy_recovery_fail_stage from MigrateCommon

[Peter Xu]

The parameter can be instead passed into the function.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qtest/migration/framework.h      |  6 ++----
 tests/qtest/migration/framework.c      |  7 ++++---
 tests/qtest/migration/postcopy-tests.c | 12 ++++--------
 tests/qtest/migration/tls-tests.c      |  8 ++++----
 4 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
index 0d39bb0d3c..bc6cf6040f 100644
--- a/tests/qtest/migration/framework.h
+++ b/tests/qtest/migration/framework.h
@@ -228,9 +228,6 @@ typedef struct {
      * refer to existing ones with live=true), or use live=off by default.
      */
     bool live;
-
-    /* Postcopy specific fields */
-    PostcopyRecoveryFailStage postcopy_recovery_fail_stage;
 } MigrateCommon;
 
 void wait_for_serial(const char *side);
@@ -243,7 +240,8 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
 void migrate_end(QTestState *from, QTestState *to, bool test_dest);
 
 void test_postcopy_common(MigrateCommon *args);
-void test_postcopy_recovery_common(MigrateCommon *args);
+void test_postcopy_recovery_common(MigrateCommon *args,
+                                   PostcopyRecoveryFailStage fail_stage);
 int test_precopy_common(MigrateCommon *args);
 void test_file_common(MigrateCommon *args, bool stop_src);
 void *migrate_hook_start_precopy_tcp_multifd_common(QTestState *from,
diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index 4f46cf8629..d7a5ae56f9 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -739,7 +739,8 @@ static void postcopy_recover_fail(QTestState *from, QTestState *to,
 #endif
 }
 
-void test_postcopy_recovery_common(MigrateCommon *args)
+void test_postcopy_recovery_common(MigrateCommon *args,
+                                   PostcopyRecoveryFailStage fail_stage)
 {
     QTestState *from, *to;
     g_autofree char *uri = NULL;
@@ -784,12 +785,12 @@ void test_postcopy_recovery_common(MigrateCommon *args)
     wait_for_postcopy_status(to, "postcopy-paused");
     wait_for_postcopy_status(from, "postcopy-paused");
 
-    if (args->postcopy_recovery_fail_stage) {
+    if (fail_stage) {
         /*
          * Test when a wrong socket specified for recover, and then the
          * ability to kick it out, and continue with a correct socket.
          */
-        postcopy_recover_fail(from, to, args->postcopy_recovery_fail_stage);
+        postcopy_recover_fail(from, to, fail_stage);
         /* continue with a good recovery */
     }
 
diff --git a/tests/qtest/migration/postcopy-tests.c b/tests/qtest/migration/postcopy-tests.c
index 7ae4d765d7..13a5759655 100644
--- a/tests/qtest/migration/postcopy-tests.c
+++ b/tests/qtest/migration/postcopy-tests.c
@@ -41,30 +41,26 @@ static void test_postcopy_preempt(char *name, MigrateCommon *args)
 
 static void test_postcopy_recovery(char *name, MigrateCommon *args)
 {
-    test_postcopy_recovery_common(args);
+    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
 }
 
 static void test_postcopy_recovery_fail_handshake(char *name,
                                                   MigrateCommon *args)
 {
-    args->postcopy_recovery_fail_stage = POSTCOPY_FAIL_RECOVERY;
-
-    test_postcopy_recovery_common(args);
+    test_postcopy_recovery_common(args, POSTCOPY_FAIL_RECOVERY);
 }
 
 static void test_postcopy_recovery_fail_reconnect(char *name,
                                                   MigrateCommon *args)
 {
-    args->postcopy_recovery_fail_stage = POSTCOPY_FAIL_CHANNEL_ESTABLISH;
-
-    test_postcopy_recovery_common(args);
+    test_postcopy_recovery_common(args, POSTCOPY_FAIL_CHANNEL_ESTABLISH);
 }
 
 static void test_postcopy_preempt_recovery(char *name, MigrateCommon *args)
 {
     args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
 
-    test_postcopy_recovery_common(args);
+    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
 }
 
 static void migration_test_add_postcopy_smoke(MigrationTestEnv *env)
diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c
index 6a20c65104..bf0bb06a29 100644
--- a/tests/qtest/migration/tls-tests.c
+++ b/tests/qtest/migration/tls-tests.c
@@ -385,7 +385,7 @@ static void test_postcopy_recovery_tls_psk(char *name, MigrateCommon *args)
     args->start_hook = migrate_hook_start_tls_psk_match;
     args->end_hook = migrate_hook_end_tls_psk;
 
-    test_postcopy_recovery_common(args);
+    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
 }
 
 static void test_multifd_postcopy_recovery_tls_psk(char *name,
@@ -396,7 +396,7 @@ static void test_multifd_postcopy_recovery_tls_psk(char *name,
 
     args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
 
-    test_postcopy_recovery_common(args);
+    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
 }
 
 /* This contains preempt+recovery+tls test altogether */
@@ -407,7 +407,7 @@ static void test_postcopy_preempt_all(char *name, MigrateCommon *args)
 
     args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
 
-    test_postcopy_recovery_common(args);
+    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
 }
 
 static void test_multifd_postcopy_preempt_recovery_tls_psk(char *name,
@@ -419,7 +419,7 @@ static void test_multifd_postcopy_preempt_recovery_tls_psk(char *name,
     args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
     args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
 
-    test_postcopy_recovery_common(args);
+    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
 }
 
 static void test_precopy_unix_tls_psk(char *name, MigrateCommon *args)
-- 
2.50.1

Re: [PATCH 1/2] tests/migration-test: Remove postcopy_data from MigrateCommon

[Prasad Pandit]

On Wed, 7 Jan 2026 at 02:04, Peter Xu <peterx@redhat.com> wrote:
> Now postcopy is not the only user of start_hook / end_hook that will pass
> in a opaque pointer.  It doesn't need to be defined in MigrateCommon as
> part of the framework, as all other hook users can pass hook_data around.
> Do it too for postcopy.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tests/qtest/migration/framework.h |  1 -
>  tests/qtest/migration/framework.c | 18 ++++++++++--------
>  2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
> index ed85ed502d..0d39bb0d3c 100644
> --- a/tests/qtest/migration/framework.h
> +++ b/tests/qtest/migration/framework.h
> @@ -230,7 +230,6 @@ typedef struct {
>      bool live;
>
>      /* Postcopy specific fields */
> -    void *postcopy_data;
>      PostcopyRecoveryFailStage postcopy_recovery_fail_stage;
>  } MigrateCommon;
>
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index e35839c95f..4f46cf8629 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -541,6 +541,7 @@ void migrate_end(QTestState *from, QTestState *to, bool test_dest)
>
>  static int migrate_postcopy_prepare(QTestState **from_ptr,
>                                      QTestState **to_ptr,
> +                                    void **hook_data,
>                                      MigrateCommon *args)
>  {
>      QTestState *from, *to;
> @@ -554,7 +555,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>      }
>
>      if (args->start_hook) {
> -        args->postcopy_data = args->start_hook(from, to);
> +        *hook_data = args->start_hook(from, to);
>      }
>      migrate_ensure_non_converge(from);
> @@ -582,7 +583,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
>  }
>
>  static void migrate_postcopy_complete(QTestState *from, QTestState *to,
> -                                      MigrateCommon *args)
> +                                      void *hook_data, MigrateCommon *args)
>  {
>      MigrationTestEnv *env = migration_get_env();
>
> @@ -601,8 +602,7 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to,
>      }
>
>      if (args->end_hook) {
> -        args->end_hook(from, to, args->postcopy_data);
> -        args->postcopy_data = NULL;
> +        args->end_hook(from, to, hook_data);
>      }
>
>      migrate_end(from, to, true);
> @@ -610,13 +610,14 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to,
>
>  void test_postcopy_common(MigrateCommon *args)
>  {
> +    void *hook_data = NULL;
>      QTestState *from, *to;
>
> -    if (migrate_postcopy_prepare(&from, &to, args)) {
> +    if (migrate_postcopy_prepare(&from, &to, &hook_data, args)) {
>          return;
>      }
>      migrate_postcopy_start(from, to, &src_state);
> -    migrate_postcopy_complete(from, to, args);
> +    migrate_postcopy_complete(from, to, hook_data, args);
>  }
>
>  static void wait_for_postcopy_status(QTestState *one, const char *status)
> @@ -742,6 +743,7 @@ void test_postcopy_recovery_common(MigrateCommon *args)
>  {
>      QTestState *from, *to;
>      g_autofree char *uri = NULL;
> +    void *hook_data = NULL;

* Should 'hook_data' pointer be g_autofree too? Where is it free'd otherwise?

>      /*
>       * Always enable OOB QMP capability for recovery tests, migrate-recover is
> @@ -752,7 +754,7 @@ void test_postcopy_recovery_common(MigrateCommon *args)
>      /* Always hide errors for postcopy recover tests since they're expected */
>      args->start.hide_stderr = true;
>
> -    if (migrate_postcopy_prepare(&from, &to, args)) {
> +    if (migrate_postcopy_prepare(&from, &to, &hook_data, args)) {
>          return;
>      }
>
> @@ -808,7 +810,7 @@ void test_postcopy_recovery_common(MigrateCommon *args)
>      /* Restore the postcopy bandwidth to unlimited */
>      migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
>
> -    migrate_postcopy_complete(from, to, args);
> +    migrate_postcopy_complete(from, to, hook_data, args);
>  }
>
>  int test_precopy_common(MigrateCommon *args)

* The changes look okay; But if tests define hook_data = NULL; Where
does it get populated?

Thank you.
---
  - Prasad

Re: [PATCH 2/2] tests/migration-test: Remove postcopy_recovery_fail_stage from MigrateCommon

[Prasad Pandit]

On Wed, 7 Jan 2026 at 02:04, Peter Xu <peterx@redhat.com> wrote:
> The parameter can be instead passed into the function.

* It'll help to include - why? pass the parameter instead.

> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  tests/qtest/migration/framework.h      |  6 ++----
>  tests/qtest/migration/framework.c      |  7 ++++---
>  tests/qtest/migration/postcopy-tests.c | 12 ++++--------
>  tests/qtest/migration/tls-tests.c      |  8 ++++----
>  4 files changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
> index 0d39bb0d3c..bc6cf6040f 100644
> --- a/tests/qtest/migration/framework.h
> +++ b/tests/qtest/migration/framework.h
> @@ -228,9 +228,6 @@ typedef struct {
>       * refer to existing ones with live=true), or use live=off by default.
>       */
>      bool live;
> -
> -    /* Postcopy specific fields */
> -    PostcopyRecoveryFailStage postcopy_recovery_fail_stage;
>  } MigrateCommon;
>
>  void wait_for_serial(const char *side);
> @@ -243,7 +240,8 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>  void migrate_end(QTestState *from, QTestState *to, bool test_dest);
>
>  void test_postcopy_common(MigrateCommon *args);
> -void test_postcopy_recovery_common(MigrateCommon *args);
> +void test_postcopy_recovery_common(MigrateCommon *args,
> +                                   PostcopyRecoveryFailStage fail_stage);
>  int test_precopy_common(MigrateCommon *args);
>  void test_file_common(MigrateCommon *args, bool stop_src);
>  void *migrate_hook_start_precopy_tcp_multifd_common(QTestState *from,
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index 4f46cf8629..d7a5ae56f9 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -739,7 +739,8 @@ static void postcopy_recover_fail(QTestState *from, QTestState *to,
>  #endif
>  }
>
> -void test_postcopy_recovery_common(MigrateCommon *args)
> +void test_postcopy_recovery_common(MigrateCommon *args,
> +                                   PostcopyRecoveryFailStage fail_stage)
===
    static void postcopy_recover_fail(QTestState *from, QTestState
*to,
                                       PostcopyRecoveryFailStage stage)
===

* To keep it consistent, maybe we can call the variable 'stage' as above?

>  {
>      QTestState *from, *to;
>      g_autofree char *uri = NULL;
> @@ -784,12 +785,12 @@ void test_postcopy_recovery_common(MigrateCommon *args)
>      wait_for_postcopy_status(to, "postcopy-paused");
>      wait_for_postcopy_status(from, "postcopy-paused");
>
> -    if (args->postcopy_recovery_fail_stage) {
> +    if (fail_stage) {
>          /*
>           * Test when a wrong socket specified for recover, and then the
>           * ability to kick it out, and continue with a correct socket.
>           */
> -        postcopy_recover_fail(from, to, args->postcopy_recovery_fail_stage);
> +        postcopy_recover_fail(from, to, fail_stage);
>          /* continue with a good recovery */
>      }
>
> diff --git a/tests/qtest/migration/postcopy-tests.c b/tests/qtest/migration/postcopy-tests.c
> index 7ae4d765d7..13a5759655 100644
> --- a/tests/qtest/migration/postcopy-tests.c
> +++ b/tests/qtest/migration/postcopy-tests.c
> @@ -41,30 +41,26 @@ static void test_postcopy_preempt(char *name, MigrateCommon *args)
>
>  static void test_postcopy_recovery(char *name, MigrateCommon *args)
>  {
> -    test_postcopy_recovery_common(args);
> +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
>  }
>
>  static void test_postcopy_recovery_fail_handshake(char *name,
>                                                    MigrateCommon *args)
>  {
> -    args->postcopy_recovery_fail_stage = POSTCOPY_FAIL_RECOVERY;
> -
> -    test_postcopy_recovery_common(args);
> +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_RECOVERY);
>  }
>
>  static void test_postcopy_recovery_fail_reconnect(char *name,
>                                                    MigrateCommon *args)
>  {
> -    args->postcopy_recovery_fail_stage = POSTCOPY_FAIL_CHANNEL_ESTABLISH;
> -
> -    test_postcopy_recovery_common(args);
> +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_CHANNEL_ESTABLISH);
>  }
>
>  static void test_postcopy_preempt_recovery(char *name, MigrateCommon *args)
>  {
>      args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
>
> -    test_postcopy_recovery_common(args);
> +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
>  }
>
>  static void migration_test_add_postcopy_smoke(MigrationTestEnv *env)
> diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c
> index 6a20c65104..bf0bb06a29 100644
> --- a/tests/qtest/migration/tls-tests.c
> +++ b/tests/qtest/migration/tls-tests.c
> @@ -385,7 +385,7 @@ static void test_postcopy_recovery_tls_psk(char *name, MigrateCommon *args)
>      args->start_hook = migrate_hook_start_tls_psk_match;
>      args->end_hook = migrate_hook_end_tls_psk;
>
> -    test_postcopy_recovery_common(args);
> +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
>  }
>
>  static void test_multifd_postcopy_recovery_tls_psk(char *name,
> @@ -396,7 +396,7 @@ static void test_multifd_postcopy_recovery_tls_psk(char *name,
>
>      args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
>
> -    test_postcopy_recovery_common(args);
> +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
>  }
>
>  /* This contains preempt+recovery+tls test altogether */
> @@ -407,7 +407,7 @@ static void test_postcopy_preempt_all(char *name, MigrateCommon *args)
>
>      args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
>
> -    test_postcopy_recovery_common(args);
> +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
>  }
>
>  static void test_multifd_postcopy_preempt_recovery_tls_psk(char *name,
> @@ -419,7 +419,7 @@ static void test_multifd_postcopy_preempt_recovery_tls_psk(char *name,
>      args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
>      args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
>
> -    test_postcopy_recovery_common(args);
> +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
>  }
>
>  static void test_precopy_unix_tls_psk(char *name, MigrateCommon *args)

* Change looks okay otherwise.

Thank you.
---
  - Prasad

Re: [PATCH 1/2] tests/migration-test: Remove postcopy_data from MigrateCommon

[Peter Xu]

On Wed, Jan 07, 2026 at 04:53:17PM +0530, Prasad Pandit wrote:
> On Wed, 7 Jan 2026 at 02:04, Peter Xu <peterx@redhat.com> wrote:
> > Now postcopy is not the only user of start_hook / end_hook that will pass
> > in a opaque pointer.  It doesn't need to be defined in MigrateCommon as
> > part of the framework, as all other hook users can pass hook_data around.
> > Do it too for postcopy.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  tests/qtest/migration/framework.h |  1 -
> >  tests/qtest/migration/framework.c | 18 ++++++++++--------
> >  2 files changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
> > index ed85ed502d..0d39bb0d3c 100644
> > --- a/tests/qtest/migration/framework.h
> > +++ b/tests/qtest/migration/framework.h
> > @@ -230,7 +230,6 @@ typedef struct {
> >      bool live;
> >
> >      /* Postcopy specific fields */
> > -    void *postcopy_data;
> >      PostcopyRecoveryFailStage postcopy_recovery_fail_stage;
> >  } MigrateCommon;
> >
> > diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> > index e35839c95f..4f46cf8629 100644
> > --- a/tests/qtest/migration/framework.c
> > +++ b/tests/qtest/migration/framework.c
> > @@ -541,6 +541,7 @@ void migrate_end(QTestState *from, QTestState *to, bool test_dest)
> >
> >  static int migrate_postcopy_prepare(QTestState **from_ptr,
> >                                      QTestState **to_ptr,
> > +                                    void **hook_data,
> >                                      MigrateCommon *args)
> >  {
> >      QTestState *from, *to;
> > @@ -554,7 +555,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
> >      }
> >
> >      if (args->start_hook) {
> > -        args->postcopy_data = args->start_hook(from, to);
> > +        *hook_data = args->start_hook(from, to);
> >      }
> >      migrate_ensure_non_converge(from);
> > @@ -582,7 +583,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
> >  }
> >
> >  static void migrate_postcopy_complete(QTestState *from, QTestState *to,
> > -                                      MigrateCommon *args)
> > +                                      void *hook_data, MigrateCommon *args)
> >  {
> >      MigrationTestEnv *env = migration_get_env();
> >
> > @@ -601,8 +602,7 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to,
> >      }
> >
> >      if (args->end_hook) {
> > -        args->end_hook(from, to, args->postcopy_data);
> > -        args->postcopy_data = NULL;
> > +        args->end_hook(from, to, hook_data);
> >      }
> >
> >      migrate_end(from, to, true);
> > @@ -610,13 +610,14 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to,
> >
> >  void test_postcopy_common(MigrateCommon *args)
> >  {
> > +    void *hook_data = NULL;
> >      QTestState *from, *to;
> >
> > -    if (migrate_postcopy_prepare(&from, &to, args)) {
> > +    if (migrate_postcopy_prepare(&from, &to, &hook_data, args)) {
> >          return;
> >      }
> >      migrate_postcopy_start(from, to, &src_state);
> > -    migrate_postcopy_complete(from, to, args);
> > +    migrate_postcopy_complete(from, to, hook_data, args);
> >  }
> >
> >  static void wait_for_postcopy_status(QTestState *one, const char *status)
> > @@ -742,6 +743,7 @@ void test_postcopy_recovery_common(MigrateCommon *args)
> >  {
> >      QTestState *from, *to;
> >      g_autofree char *uri = NULL;
> > +    void *hook_data = NULL;
> 
> * Should 'hook_data' pointer be g_autofree too? Where is it free'd otherwise?

hook_data is freed in end_hook().  This patch doesn't change that fact for
postcopy.  It's the smae to non-postcopy tests.

> 
> >      /*
> >       * Always enable OOB QMP capability for recovery tests, migrate-recover is
> > @@ -752,7 +754,7 @@ void test_postcopy_recovery_common(MigrateCommon *args)
> >      /* Always hide errors for postcopy recover tests since they're expected */
> >      args->start.hide_stderr = true;
> >
> > -    if (migrate_postcopy_prepare(&from, &to, args)) {
> > +    if (migrate_postcopy_prepare(&from, &to, &hook_data, args)) {
> >          return;
> >      }
> >
> > @@ -808,7 +810,7 @@ void test_postcopy_recovery_common(MigrateCommon *args)
> >      /* Restore the postcopy bandwidth to unlimited */
> >      migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
> >
> > -    migrate_postcopy_complete(from, to, args);
> > +    migrate_postcopy_complete(from, to, hook_data, args);
> >  }
> >
> >  int test_precopy_common(MigrateCommon *args)
> 
> * The changes look okay; But if tests define hook_data = NULL; Where
> does it get populated?

It's populated in start_hook() conditionally.  When populated, it is always
(and a must) to be released in end_hook().

Thanks,

-- 
Peter Xu

Re: [PATCH 2/2] tests/migration-test: Remove postcopy_recovery_fail_stage from MigrateCommon

[Peter Xu]

On Wed, Jan 07, 2026 at 05:07:40PM +0530, Prasad Pandit wrote:
> On Wed, 7 Jan 2026 at 02:04, Peter Xu <peterx@redhat.com> wrote:
> > The parameter can be instead passed into the function.
> 
> * It'll help to include - why? pass the parameter instead.

I want to remove special and unnecessary fields in MigrateCommon struct.

I'll add a sentence when repost.

> 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  tests/qtest/migration/framework.h      |  6 ++----
> >  tests/qtest/migration/framework.c      |  7 ++++---
> >  tests/qtest/migration/postcopy-tests.c | 12 ++++--------
> >  tests/qtest/migration/tls-tests.c      |  8 ++++----
> >  4 files changed, 14 insertions(+), 19 deletions(-)
> >
> > diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
> > index 0d39bb0d3c..bc6cf6040f 100644
> > --- a/tests/qtest/migration/framework.h
> > +++ b/tests/qtest/migration/framework.h
> > @@ -228,9 +228,6 @@ typedef struct {
> >       * refer to existing ones with live=true), or use live=off by default.
> >       */
> >      bool live;
> > -
> > -    /* Postcopy specific fields */
> > -    PostcopyRecoveryFailStage postcopy_recovery_fail_stage;
> >  } MigrateCommon;
> >
> >  void wait_for_serial(const char *side);
> > @@ -243,7 +240,8 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
> >  void migrate_end(QTestState *from, QTestState *to, bool test_dest);
> >
> >  void test_postcopy_common(MigrateCommon *args);
> > -void test_postcopy_recovery_common(MigrateCommon *args);
> > +void test_postcopy_recovery_common(MigrateCommon *args,
> > +                                   PostcopyRecoveryFailStage fail_stage);
> >  int test_precopy_common(MigrateCommon *args);
> >  void test_file_common(MigrateCommon *args, bool stop_src);
> >  void *migrate_hook_start_precopy_tcp_multifd_common(QTestState *from,
> > diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> > index 4f46cf8629..d7a5ae56f9 100644
> > --- a/tests/qtest/migration/framework.c
> > +++ b/tests/qtest/migration/framework.c
> > @@ -739,7 +739,8 @@ static void postcopy_recover_fail(QTestState *from, QTestState *to,
> >  #endif
> >  }
> >
> > -void test_postcopy_recovery_common(MigrateCommon *args)
> > +void test_postcopy_recovery_common(MigrateCommon *args,
> > +                                   PostcopyRecoveryFailStage fail_stage)
> ===
>     static void postcopy_recover_fail(QTestState *from, QTestState
> *to,
>                                        PostcopyRecoveryFailStage stage)
> ===
> 
> * To keep it consistent, maybe we can call the variable 'stage' as above?

Personally I prefer fail_stage, e.g. fail_stage=NONE means it never fails.
stage==NONE is less clear.

Thanks,

> 
> >  {
> >      QTestState *from, *to;
> >      g_autofree char *uri = NULL;
> > @@ -784,12 +785,12 @@ void test_postcopy_recovery_common(MigrateCommon *args)
> >      wait_for_postcopy_status(to, "postcopy-paused");
> >      wait_for_postcopy_status(from, "postcopy-paused");
> >
> > -    if (args->postcopy_recovery_fail_stage) {
> > +    if (fail_stage) {
> >          /*
> >           * Test when a wrong socket specified for recover, and then the
> >           * ability to kick it out, and continue with a correct socket.
> >           */
> > -        postcopy_recover_fail(from, to, args->postcopy_recovery_fail_stage);
> > +        postcopy_recover_fail(from, to, fail_stage);
> >          /* continue with a good recovery */
> >      }
> >
> > diff --git a/tests/qtest/migration/postcopy-tests.c b/tests/qtest/migration/postcopy-tests.c
> > index 7ae4d765d7..13a5759655 100644
> > --- a/tests/qtest/migration/postcopy-tests.c
> > +++ b/tests/qtest/migration/postcopy-tests.c
> > @@ -41,30 +41,26 @@ static void test_postcopy_preempt(char *name, MigrateCommon *args)
> >
> >  static void test_postcopy_recovery(char *name, MigrateCommon *args)
> >  {
> > -    test_postcopy_recovery_common(args);
> > +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
> >  }
> >
> >  static void test_postcopy_recovery_fail_handshake(char *name,
> >                                                    MigrateCommon *args)
> >  {
> > -    args->postcopy_recovery_fail_stage = POSTCOPY_FAIL_RECOVERY;
> > -
> > -    test_postcopy_recovery_common(args);
> > +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_RECOVERY);
> >  }
> >
> >  static void test_postcopy_recovery_fail_reconnect(char *name,
> >                                                    MigrateCommon *args)
> >  {
> > -    args->postcopy_recovery_fail_stage = POSTCOPY_FAIL_CHANNEL_ESTABLISH;
> > -
> > -    test_postcopy_recovery_common(args);
> > +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_CHANNEL_ESTABLISH);
> >  }
> >
> >  static void test_postcopy_preempt_recovery(char *name, MigrateCommon *args)
> >  {
> >      args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
> >
> > -    test_postcopy_recovery_common(args);
> > +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
> >  }
> >
> >  static void migration_test_add_postcopy_smoke(MigrationTestEnv *env)
> > diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c
> > index 6a20c65104..bf0bb06a29 100644
> > --- a/tests/qtest/migration/tls-tests.c
> > +++ b/tests/qtest/migration/tls-tests.c
> > @@ -385,7 +385,7 @@ static void test_postcopy_recovery_tls_psk(char *name, MigrateCommon *args)
> >      args->start_hook = migrate_hook_start_tls_psk_match;
> >      args->end_hook = migrate_hook_end_tls_psk;
> >
> > -    test_postcopy_recovery_common(args);
> > +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
> >  }
> >
> >  static void test_multifd_postcopy_recovery_tls_psk(char *name,
> > @@ -396,7 +396,7 @@ static void test_multifd_postcopy_recovery_tls_psk(char *name,
> >
> >      args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
> >
> > -    test_postcopy_recovery_common(args);
> > +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
> >  }
> >
> >  /* This contains preempt+recovery+tls test altogether */
> > @@ -407,7 +407,7 @@ static void test_postcopy_preempt_all(char *name, MigrateCommon *args)
> >
> >      args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
> >
> > -    test_postcopy_recovery_common(args);
> > +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
> >  }
> >
> >  static void test_multifd_postcopy_preempt_recovery_tls_psk(char *name,
> > @@ -419,7 +419,7 @@ static void test_multifd_postcopy_preempt_recovery_tls_psk(char *name,
> >      args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
> >      args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
> >
> > -    test_postcopy_recovery_common(args);
> > +    test_postcopy_recovery_common(args, POSTCOPY_FAIL_NONE);
> >  }
> >
> >  static void test_precopy_unix_tls_psk(char *name, MigrateCommon *args)
> 
> * Change looks okay otherwise.
> 
> Thank you.
> ---
>   - Prasad
> 

-- 
Peter Xu

Re: [PATCH 1/2] tests/migration-test: Remove postcopy_data from MigrateCommon

[Prasad Pandit]

On Wed, 7 Jan 2026 at 22:42, Peter Xu <peterx@redhat.com> wrote:
>> * Should 'hook_data' pointer be g_autofree too? Where is it free'd otherwise?
>
> hook_data is freed in end_hook().  This patch doesn't change that fact for
> postcopy.  It's the smae to non-postcopy tests.
>
>> * The changes look okay; But if tests define hook_data = NULL; Where
>> does it get populated?
>
> It's populated in start_hook() conditionally.  When populated, it is always
> (and a must) to be released in end_hook().

===
$ grep -Eri 'test_postcopy_common|test_postcopy_recovery_common'
tests/qtest/migration/ -l
tests/qtest/migration/framework.h
tests/qtest/migration/framework.c
tests/qtest/migration/tls-tests.c
tests/qtest/migration/postcopy-tests.c
===

* Only tls-tests above seem to define and use these hooks properly
along with the hook_data. Postcopy-tests and all other users of
start_hook/end_hook don't seem to use [postcopy|hook]_data at all. Do
we really need this hook_data parameter? Couldn't it be defined as a
tls-tests specific object. (just wondering)

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

Thank you.
---
  - Prasad

Re: [PATCH 2/2] tests/migration-test: Remove postcopy_recovery_fail_stage from MigrateCommon

[Prasad Pandit]

On Wed, 7 Jan 2026 at 22:45, Peter Xu <peterx@redhat.com> wrote:
> On Wed, Jan 07, 2026 at 05:07:40PM +0530, Prasad Pandit wrote:
> > On Wed, 7 Jan 2026 at 02:04, Peter Xu <peterx@redhat.com> wrote:
> > > The parameter can be instead passed into the function.
> >
> > * It'll help to include - why? pass the parameter instead.
>
> I want to remove special and unnecessary fields in MigrateCommon struct.
>
> I'll add a sentence when repost.
...
> > * To keep it consistent, maybe we can call the variable 'stage' as above?
>
> Personally I prefer fail_stage, e.g. fail_stage=NONE means it never fails.
> stage==NONE is less clear.

* Let's make it fail_stage in both places then?

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

Thank you.
---
  - Prasad

Re: [PATCH 1/2] tests/migration-test: Remove postcopy_data from MigrateCommon

[Peter Xu]

On Thu, Jan 08, 2026 at 03:08:04PM +0530, Prasad Pandit wrote:
> On Wed, 7 Jan 2026 at 22:42, Peter Xu <peterx@redhat.com> wrote:
> >> * Should 'hook_data' pointer be g_autofree too? Where is it free'd otherwise?
> >
> > hook_data is freed in end_hook().  This patch doesn't change that fact for
> > postcopy.  It's the smae to non-postcopy tests.
> >
> >> * The changes look okay; But if tests define hook_data = NULL; Where
> >> does it get populated?
> >
> > It's populated in start_hook() conditionally.  When populated, it is always
> > (and a must) to be released in end_hook().
> 
> ===
> $ grep -Eri 'test_postcopy_common|test_postcopy_recovery_common'
> tests/qtest/migration/ -l
> tests/qtest/migration/framework.h
> tests/qtest/migration/framework.c
> tests/qtest/migration/tls-tests.c
> tests/qtest/migration/postcopy-tests.c
> ===
> 
> * Only tls-tests above seem to define and use these hooks properly
> along with the hook_data. Postcopy-tests and all other users of
> start_hook/end_hook don't seem to use [postcopy|hook]_data at all. Do
> we really need this hook_data parameter? Couldn't it be defined as a
> tls-tests specific object. (just wondering)

Sorry I don't follow.  We need the hook_data for cleaning up tls objects
later in end_hook, for either postcopy or other tls tests.

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

Thanks,

> 
> Thank you.
> ---
>   - Prasad
> 

-- 
Peter Xu

Re: [PATCH 2/2] tests/migration-test: Remove postcopy_recovery_fail_stage from MigrateCommon

[Peter Xu]

On Thu, Jan 08, 2026 at 03:11:32PM +0530, Prasad Pandit wrote:
> On Wed, 7 Jan 2026 at 22:45, Peter Xu <peterx@redhat.com> wrote:
> > On Wed, Jan 07, 2026 at 05:07:40PM +0530, Prasad Pandit wrote:
> > > On Wed, 7 Jan 2026 at 02:04, Peter Xu <peterx@redhat.com> wrote:
> > > > The parameter can be instead passed into the function.
> > >
> > > * It'll help to include - why? pass the parameter instead.
> >
> > I want to remove special and unnecessary fields in MigrateCommon struct.
> >
> > I'll add a sentence when repost.
> ...
> > > * To keep it consistent, maybe we can call the variable 'stage' as above?
> >
> > Personally I prefer fail_stage, e.g. fail_stage=NONE means it never fails.
> > stage==NONE is less clear.
> 
> * Let's make it fail_stage in both places then?

Could you explain what's the 2nd place to use it besides the parameter in
test_postcopy_recovery_common()?

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

Thanks,

> 
> Thank you.
> ---
>   - Prasad
> 

-- 
Peter Xu

Re: [PATCH 1/2] tests/migration-test: Remove postcopy_data from MigrateCommon

[Prasad Pandit]

On Wed, 14 Jan 2026 at 21:04, Peter Xu <peterx@redhat.com> wrote:
> Sorry I don't follow.  We need the hook_data for cleaning up tls objects
> later in end_hook, for either postcopy or other tls tests.

* Yes, but only start_hook() in tls-tests.c seem to return *hook_data
object and end_hook() there free it. postcopy-tests.c and other tests
don't seem to use *hook_data object; Other start_hooks() functions
return NULL, so there's nothing to free in the end_hook() function.

Thank you.
---
  - Prasad

Re: [PATCH 2/2] tests/migration-test: Remove postcopy_recovery_fail_stage from MigrateCommon

[Prasad Pandit]

On Wed, 14 Jan 2026 at 21:06, Peter Xu <peterx@redhat.com> wrote:
> > * Let's make it fail_stage in both places then?
>
> Could you explain what's the 2nd place to use it besides the parameter in
> test_postcopy_recovery_common()?
===
tests/qtest/migration/framework.c:
    static void postcopy_recover_fail(QTestState *from, QTestState *to,
                                       PostcopyRecoveryFailStage stage)
===
This one. ^^

Thank you.
---
  - Prasad

Re: [PATCH 2/2] tests/migration-test: Remove postcopy_recovery_fail_stage from MigrateCommon

[Peter Xu]

On Thu, Jan 15, 2026 at 05:27:30PM +0530, Prasad Pandit wrote:
> On Wed, 14 Jan 2026 at 21:06, Peter Xu <peterx@redhat.com> wrote:
> > > * Let's make it fail_stage in both places then?
> >
> > Could you explain what's the 2nd place to use it besides the parameter in
> > test_postcopy_recovery_common()?
> ===
> tests/qtest/migration/framework.c:
>     static void postcopy_recover_fail(QTestState *from, QTestState *to,
>                                        PostcopyRecoveryFailStage stage)
> ===
> This one. ^^

I don't have a strong opinion, since that's unrelevant to the current
change, IMHO we can keep it as is.

Thanks,

-- 
Peter Xu