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