| Message ID | 20220517195730.32312-12-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: Postcopy Preemption | expand |
* Peter Xu (peterx@redhat.com) wrote: > We just added TLS tests for precopy but not postcopy. Add the > corresponding test for vanilla postcopy. > > Rename the vanilla postcopy to "postcopy/plain" because all postcopy tests > will only use unix sockets as channel. > > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > tests/qtest/migration-test.c | 50 +++++++++++++++++++++++++++++++----- > 1 file changed, 43 insertions(+), 7 deletions(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index d33e8060f9..e8304aa454 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -481,6 +481,10 @@ typedef struct { > bool only_target; > /* Use dirty ring if true; dirty logging otherwise */ > bool use_dirty_ring; > + /* Whether use TLS channels for postcopy test? */ > + bool postcopy_tls; > + /* Used only if postcopy_tls==true, to cache the data object */ > + void *postcopy_tls_data; > const char *opts_source; > const char *opts_target; > } MigrateStart; > @@ -980,6 +984,10 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, > return -1; > } > > + if (args->postcopy_tls) { > + args->postcopy_tls_data = test_migrate_tls_psk_start_match(from, to); > + } > + > migrate_set_capability(from, "postcopy-ram", true); > migrate_set_capability(to, "postcopy-ram", true); > migrate_set_capability(to, "postcopy-blocktime", true); > @@ -1004,7 +1012,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, > return 0; > } > > -static void migrate_postcopy_complete(QTestState *from, QTestState *to) > +static void migrate_postcopy_complete(QTestState *from, QTestState *to, > + MigrateStart *args) > { > wait_for_migration_complete(from); > > @@ -1015,19 +1024,38 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to) > read_blocktime(to); > } > > + if (args->postcopy_tls) { > + assert(args->postcopy_tls_data); > + test_migrate_tls_psk_finish(from, to, args->postcopy_tls_data); > + args->postcopy_tls_data = NULL; > + } > + > test_migrate_end(from, to, true); > } > > -static void test_postcopy(void) > +static void test_postcopy_common(MigrateStart *args) > { > - MigrateStart args = {}; > QTestState *from, *to; > > - if (migrate_postcopy_prepare(&from, &to, &args)) { > + if (migrate_postcopy_prepare(&from, &to, args)) { > return; > } > migrate_postcopy_start(from, to); > - migrate_postcopy_complete(from, to); > + migrate_postcopy_complete(from, to, args); > +} > + > +static void test_postcopy(void) > +{ > + MigrateStart args = { }; > + > + test_postcopy_common(&args); > +} > + > +static void test_postcopy_tls_psk(void) > +{ > + MigrateStart args = { .postcopy_tls = true }; > + > + test_postcopy_common(&args); > } > > static void test_postcopy_recovery(void) > @@ -1089,7 +1117,7 @@ static void test_postcopy_recovery(void) > /* Restore the postcopy bandwidth to unlimited */ > migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0); > > - migrate_postcopy_complete(from, to); > + migrate_postcopy_complete(from, to, &args); > } > > static void test_baddest(void) > @@ -2133,7 +2161,15 @@ int main(int argc, char **argv) > > module_call_init(MODULE_INIT_QOM); > > - qtest_add_func("/migration/postcopy/unix", test_postcopy); > + qtest_add_func("/migration/postcopy/plain", test_postcopy); > +#ifdef CONFIG_GNUTLS > + /* > + * NOTE: psk test is enough for postcopy, as other types of TLS > + * channels are tested under precopy. Here what we want to test is the > + * general postcopy path that has TLS channel enabled. > + */ > + qtest_add_func("/migration/postcopy/tls/psk", test_postcopy_tls_psk); > +#endif /* CONFIG_GNUTLS */ > qtest_add_func("/migration/postcopy/recovery", test_postcopy_recovery); > qtest_add_func("/migration/bad_dest", test_baddest); > qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain); > -- > 2.32.0 >
On Tue, May 17, 2022 at 03:57:28PM -0400, Peter Xu wrote: > We just added TLS tests for precopy but not postcopy. Add the > corresponding test for vanilla postcopy. > > Rename the vanilla postcopy to "postcopy/plain" because all postcopy tests > will only use unix sockets as channel. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > tests/qtest/migration-test.c | 50 +++++++++++++++++++++++++++++++----- > 1 file changed, 43 insertions(+), 7 deletions(-) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index d33e8060f9..e8304aa454 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -481,6 +481,10 @@ typedef struct { > bool only_target; > /* Use dirty ring if true; dirty logging otherwise */ > bool use_dirty_ring; > + /* Whether use TLS channels for postcopy test? */ > + bool postcopy_tls; > + /* Used only if postcopy_tls==true, to cache the data object */ > + void *postcopy_tls_data; Rather than adding these fields, I think it would be preferrable to pass the hooks in the same way I did for the precopy tests. > const char *opts_source; > const char *opts_target; > } MigrateStart; > @@ -980,6 +984,10 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, > return -1; Add 'TestMigrateStartHook start_hook' to this method > } > > + if (args->postcopy_tls) { > + args->postcopy_tls_data = test_migrate_tls_psk_start_match(from, to); > + } call the 'start_hook' > + > migrate_set_capability(from, "postcopy-ram", true); > migrate_set_capability(to, "postcopy-ram", true); > migrate_set_capability(to, "postcopy-blocktime", true); > @@ -1004,7 +1012,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, > return 0; > } > > -static void migrate_postcopy_complete(QTestState *from, QTestState *to) > +static void migrate_postcopy_complete(QTestState *from, QTestState *to, > + MigrateStart *args) Here pass in the TestMigrateFinishHook , and the void* data from the migrate_postcoy_prepare method. > { > wait_for_migration_complete(from); > > @@ -1015,19 +1024,38 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to) > read_blocktime(to); > } > > + if (args->postcopy_tls) { > + assert(args->postcopy_tls_data); > + test_migrate_tls_psk_finish(from, to, args->postcopy_tls_data); > + args->postcopy_tls_data = NULL; > + } > + > test_migrate_end(from, to, true); > } > > -static void test_postcopy(void) > +static void test_postcopy_common(MigrateStart *args) Use 'MigrateCommon' here, which embeds a copy of 'MigrateStart', while also having fields to the TestMigrateStartHook and TestMigrateFinishHook. > { > - MigrateStart args = {}; > QTestState *from, *to; > > - if (migrate_postcopy_prepare(&from, &to, &args)) { > + if (migrate_postcopy_prepare(&from, &to, args)) { > return; > } > migrate_postcopy_start(from, to); > - migrate_postcopy_complete(from, to); > + migrate_postcopy_complete(from, to, args); eg this would become void *hook_data; if (migrate_postcopy_prepare(&from, &to, args.start_hook, &hook_data)) { return; } migrate_postcopy_start(from, to); migrate_postcopy_complete(from, to, args.finish_hook, hook_data); > +} > + > +static void test_postcopy(void) > +{ > + MigrateStart args = { }; MigrateCommon args = {} > + > + test_postcopy_common(&args); > +} > + > +static void test_postcopy_tls_psk(void) > +{ > + MigrateStart args = { .postcopy_tls = true }; MigrateCommon args = { .start_hook = test_migrate_tls_psk_start_match, .finish_hook = test_migrate_tls_psk_finish, } > + > + test_postcopy_common(&args); > } > > static void test_postcopy_recovery(void) With regards, Daniel
On Thu, May 19, 2022 at 11:11:34AM +0100, Daniel P. Berrangé wrote: > On Tue, May 17, 2022 at 03:57:28PM -0400, Peter Xu wrote: > > We just added TLS tests for precopy but not postcopy. Add the > > corresponding test for vanilla postcopy. > > > > Rename the vanilla postcopy to "postcopy/plain" because all postcopy tests > > will only use unix sockets as channel. > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > --- > > tests/qtest/migration-test.c | 50 +++++++++++++++++++++++++++++++----- > > 1 file changed, 43 insertions(+), 7 deletions(-) > > > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > > index d33e8060f9..e8304aa454 100644 > > --- a/tests/qtest/migration-test.c > > +++ b/tests/qtest/migration-test.c > > @@ -481,6 +481,10 @@ typedef struct { > > bool only_target; > > /* Use dirty ring if true; dirty logging otherwise */ > > bool use_dirty_ring; > > + /* Whether use TLS channels for postcopy test? */ > > + bool postcopy_tls; > > + /* Used only if postcopy_tls==true, to cache the data object */ > > + void *postcopy_tls_data; > > Rather than adding these fields, I think it would be preferrable to > pass the hooks in the same way I did for the precopy tests. I can give it a shot. Ideally I think we should rename MigrationCommon to MigrationPrecopy and keep all the precopy stuff there, meanwhile we could have MigrationPostcopy which will also include MigrationStart but keeps the postcopy bits around. Then I'd need to move start_hook and so into MigrationStart. But let me start from simple..
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index d33e8060f9..e8304aa454 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -481,6 +481,10 @@ typedef struct { bool only_target; /* Use dirty ring if true; dirty logging otherwise */ bool use_dirty_ring; + /* Whether use TLS channels for postcopy test? */ + bool postcopy_tls; + /* Used only if postcopy_tls==true, to cache the data object */ + void *postcopy_tls_data; const char *opts_source; const char *opts_target; } MigrateStart; @@ -980,6 +984,10 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, return -1; } + if (args->postcopy_tls) { + args->postcopy_tls_data = test_migrate_tls_psk_start_match(from, to); + } + migrate_set_capability(from, "postcopy-ram", true); migrate_set_capability(to, "postcopy-ram", true); migrate_set_capability(to, "postcopy-blocktime", true); @@ -1004,7 +1012,8 @@ static int migrate_postcopy_prepare(QTestState **from_ptr, return 0; } -static void migrate_postcopy_complete(QTestState *from, QTestState *to) +static void migrate_postcopy_complete(QTestState *from, QTestState *to, + MigrateStart *args) { wait_for_migration_complete(from); @@ -1015,19 +1024,38 @@ static void migrate_postcopy_complete(QTestState *from, QTestState *to) read_blocktime(to); } + if (args->postcopy_tls) { + assert(args->postcopy_tls_data); + test_migrate_tls_psk_finish(from, to, args->postcopy_tls_data); + args->postcopy_tls_data = NULL; + } + test_migrate_end(from, to, true); } -static void test_postcopy(void) +static void test_postcopy_common(MigrateStart *args) { - MigrateStart args = {}; QTestState *from, *to; - if (migrate_postcopy_prepare(&from, &to, &args)) { + if (migrate_postcopy_prepare(&from, &to, args)) { return; } migrate_postcopy_start(from, to); - migrate_postcopy_complete(from, to); + migrate_postcopy_complete(from, to, args); +} + +static void test_postcopy(void) +{ + MigrateStart args = { }; + + test_postcopy_common(&args); +} + +static void test_postcopy_tls_psk(void) +{ + MigrateStart args = { .postcopy_tls = true }; + + test_postcopy_common(&args); } static void test_postcopy_recovery(void) @@ -1089,7 +1117,7 @@ static void test_postcopy_recovery(void) /* Restore the postcopy bandwidth to unlimited */ migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0); - migrate_postcopy_complete(from, to); + migrate_postcopy_complete(from, to, &args); } static void test_baddest(void) @@ -2133,7 +2161,15 @@ int main(int argc, char **argv) module_call_init(MODULE_INIT_QOM); - qtest_add_func("/migration/postcopy/unix", test_postcopy); + qtest_add_func("/migration/postcopy/plain", test_postcopy); +#ifdef CONFIG_GNUTLS + /* + * NOTE: psk test is enough for postcopy, as other types of TLS + * channels are tested under precopy. Here what we want to test is the + * general postcopy path that has TLS channel enabled. + */ + qtest_add_func("/migration/postcopy/tls/psk", test_postcopy_tls_psk); +#endif /* CONFIG_GNUTLS */ qtest_add_func("/migration/postcopy/recovery", test_postcopy_recovery); qtest_add_func("/migration/bad_dest", test_baddest); qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
We just added TLS tests for precopy but not postcopy. Add the corresponding test for vanilla postcopy. Rename the vanilla postcopy to "postcopy/plain" because all postcopy tests will only use unix sockets as channel. Signed-off-by: Peter Xu <peterx@redhat.com> --- tests/qtest/migration-test.c | 50 +++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 7 deletions(-)