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

[Peter Xu <peterx@redhat.com>]

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