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.1On 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
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
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
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
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
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
© 2016 - 2026 Red Hat, Inc.