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

[Peter Xu <peterx@redhat.com>]

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