[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 4/4] tests/qtest/migration: add postcopy tests with multif
|
From: |
Fabiano Rosas |
|
Subject: |
Re: [PATCH v4 4/4] tests/qtest/migration: add postcopy tests with multifd |
|
Date: |
Mon, 27 Jan 2025 18:13:20 -0300 |
Prasad Pandit <ppandit@redhat.com> writes:
> From: Prasad Pandit <pjp@fedoraproject.org>
>
> Add new postcopy tests to run postcopy migration with
> multifd channels enabled. Add a boolean fields 'multifd'
> and 'postcopy_ram' to the MigrateCommon structure.
> It helps to enable multifd channels and postcopy-ram
> during migration tests.
>
> Signed-off-by: Prasad Pandit <pjp@fedoraproject.org>
> ---
> tests/qtest/migration/compression-tests.c | 13 ++++++++
> tests/qtest/migration/framework.c | 13 ++++++++
> tests/qtest/migration/framework.h | 4 +++
> tests/qtest/migration/postcopy-tests.c | 23 +++++++++++++
> tests/qtest/migration/precopy-tests.c | 19 +++++++++++
> tests/qtest/migration/tls-tests.c | 40 +++++++++++++++++++++++
> 6 files changed, 112 insertions(+)
>
> v3: Add more qtests to cover precopy with 'postcopy-ram' attribute
> and postcopy with multifd channels enabled.
> -
> 20250121131032.1611245-1-ppandit@redhat.com/T/#t">https://lore.kernel.org/qemu-devel/20250121131032.1611245-1-ppandit@redhat.com/T/#t
>
> diff --git a/tests/qtest/migration/compression-tests.c
> b/tests/qtest/migration/compression-tests.c
> index d78f1f11f1..3252ba2f73 100644
> --- a/tests/qtest/migration/compression-tests.c
> +++ b/tests/qtest/migration/compression-tests.c
> @@ -39,6 +39,17 @@ static void test_multifd_tcp_zstd(void)
> };
> test_precopy_common(&args);
> }
> +
> +static void test_multifd_postcopy_tcp_zstd(void)
> +{
> + MigrateCommon args = {
> + .postcopy_ram = true,
> + .listen_uri = "defer",
> + .start_hook = migrate_hook_start_precopy_tcp_multifd_zstd,
> + };
> +
> + test_precopy_common(&args);
> +}
> #endif /* CONFIG_ZSTD */
>
> #ifdef CONFIG_QATZIP
> @@ -158,6 +169,8 @@ void migration_test_add_compression(MigrationTestEnv *env)
> #ifdef CONFIG_ZSTD
> migration_test_add("/migration/multifd/tcp/plain/zstd",
> test_multifd_tcp_zstd);
> + migration_test_add("/migration/multifd+postcopy/tcp/plain/zstd",
> + test_multifd_postcopy_tcp_zstd);
> #endif
>
> #ifdef CONFIG_QATZIP
> diff --git a/tests/qtest/migration/framework.c
> b/tests/qtest/migration/framework.c
> index 4550cda129..00776f858c 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -427,6 +427,14 @@ static int migrate_postcopy_prepare(QTestState
> **from_ptr,
> migrate_set_capability(to, "postcopy-preempt", true);
> }
>
> + if (args->multifd) {
> + migrate_set_capability(from, "multifd", true);
> + migrate_set_capability(to, "multifd", true);
This is slightly backwards because currently that's what the hooks are
for. I don't see the need for separate flags for multifd and
postcopy. This also makes the code less maintainable because it creates
two different ways of doing the same thing (hooks vs. args).
I suggest to keep with the hooks. Alternatively, we could add a more
generic args->caps and have every test set the capabilities it wants and
the _common code to iterate over those and set them to true. Something
like this perhaps:
if (migrate_start(&from, &to, args->listen_uri, &args->start)) {
return;
}
for (int i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
if (args->caps[i]) {
migrate_set_capability(from,
MigrationCapability_str(args->caps[i]), true);
migrate_set_capability(to, MigrationCapability_str(args->caps[i]),
true);
}
}
if (args->start_hook) {
data_hook = args->start_hook(from, to);
}
We could also set the number of channels as a default value. The tests
could overwrite it from the hook if needed.
> +
> + migrate_set_parameter_int(from, "multifd-channels", 8);
> + migrate_set_parameter_int(to, "multifd-channels", 8);
> + }
> +
> migrate_ensure_non_converge(from);
>
> migrate_prepare_for_dirty_mem(from);
> @@ -691,6 +699,11 @@ void test_precopy_common(MigrateCommon *args)
> return;
> }
>
> + if (args->postcopy_ram) {
> + migrate_set_capability(from, "postcopy-ram", true);
> + migrate_set_capability(to, "postcopy-ram", true);
> + }
> +
> if (args->start_hook) {
> data_hook = args->start_hook(from, to);
> }
> diff --git a/tests/qtest/migration/framework.h
> b/tests/qtest/migration/framework.h
> index 7991ee56b6..214288ca42 100644
> --- a/tests/qtest/migration/framework.h
> +++ b/tests/qtest/migration/framework.h
> @@ -193,7 +193,11 @@ typedef struct {
> */
> bool live;
>
> + /* set multifd on */
> + bool multifd;
> +
> /* Postcopy specific fields */
> + bool postcopy_ram;
> void *postcopy_data;
> bool postcopy_preempt;
> PostcopyRecoveryFailStage postcopy_recovery_fail_stage;
> diff --git a/tests/qtest/migration/postcopy-tests.c
> b/tests/qtest/migration/postcopy-tests.c
> index daf7449f2c..212a5ea600 100644
> --- a/tests/qtest/migration/postcopy-tests.c
> +++ b/tests/qtest/migration/postcopy-tests.c
> @@ -79,6 +79,25 @@ static void test_postcopy_preempt_recovery(void)
> test_postcopy_recovery_common(&args);
> }
>
> +static void test_multifd_postcopy(void)
> +{
> + MigrateCommon args = {
> + .multifd = true,
> + };
> +
> + test_postcopy_common(&args);
> +}
> +
> +static void test_multifd_postcopy_preempt(void)
> +{
> + MigrateCommon args = {
> + .multifd = true,
> + .postcopy_preempt = true,
> + };
> +
> + test_postcopy_common(&args);
> +}
> +
> void migration_test_add_postcopy(MigrationTestEnv *env)
> {
> if (env->has_uffd) {
> @@ -98,6 +117,10 @@ void migration_test_add_postcopy(MigrationTestEnv *env)
> "/migration/postcopy/recovery/double-failures/reconnect",
> test_postcopy_recovery_fail_reconnect);
>
> + migration_test_add("/migration/multifd+postcopy/plain",
> + test_multifd_postcopy);
> + migration_test_add("/migration/multifd+postcopy/preempt/plain",
> + test_multifd_postcopy_preempt);
> if (env->is_x86) {
> migration_test_add("/migration/postcopy/suspend",
> test_postcopy_suspend);
> diff --git a/tests/qtest/migration/precopy-tests.c
> b/tests/qtest/migration/precopy-tests.c
> index 23599b29ee..b1a4e7bbb1 100644
> --- a/tests/qtest/migration/precopy-tests.c
> +++ b/tests/qtest/migration/precopy-tests.c
> @@ -33,6 +33,7 @@
> #define DIRTYLIMIT_TOLERANCE_RANGE 25 /* MB/s */
>
> static char *tmpfs;
> +static bool postcopy_ram = false;
>
> static void test_precopy_unix_plain(void)
> {
> @@ -472,6 +473,11 @@ static void test_multifd_tcp_cancel(void)
> migrate_ensure_non_converge(from);
> migrate_prepare_for_dirty_mem(from);
>
> + if (postcopy_ram) {
> + migrate_set_capability(from, "postcopy-ram", true);
> + migrate_set_capability(to, "postcopy-ram", true);
> + }
> +
> migrate_set_parameter_int(from, "multifd-channels", 16);
> migrate_set_parameter_int(to, "multifd-channels", 16);
>
> @@ -513,6 +519,10 @@ static void test_multifd_tcp_cancel(void)
> return;
> }
>
> + if (postcopy_ram) {
> + migrate_set_capability(to2, "postcopy-ram", true);
> + }
> +
> migrate_set_parameter_int(to2, "multifd-channels", 16);
>
> migrate_set_capability(to2, "multifd", true);
> @@ -536,6 +546,13 @@ static void test_multifd_tcp_cancel(void)
> migrate_end(from, to2, true);
> }
>
> +static void test_multifd_postcopy_tcp_cancel(void)
> +{
> + postcopy_ram = true;
> + test_multifd_tcp_cancel();
> + postcopy_ram = false;
> +}
> +
> static void calc_dirty_rate(QTestState *who, uint64_t calc_time)
> {
> qtest_qmp_assert_success(who,
> @@ -999,6 +1016,8 @@ void migration_test_add_precopy(MigrationTestEnv *env)
> test_multifd_tcp_no_zero_page);
> migration_test_add("/migration/multifd/tcp/plain/cancel",
> test_multifd_tcp_cancel);
> + migration_test_add("migration/multifd+postcopy/tcp/plain/cancel",
> + test_multifd_postcopy_tcp_cancel);
> if (g_str_equal(env->arch, "x86_64")
> && env->has_kvm && env->has_dirty_ring) {
>
> diff --git a/tests/qtest/migration/tls-tests.c
> b/tests/qtest/migration/tls-tests.c
> index 5704a1f992..094dc1d814 100644
> --- a/tests/qtest/migration/tls-tests.c
> +++ b/tests/qtest/migration/tls-tests.c
> @@ -393,6 +393,17 @@ static void test_postcopy_recovery_tls_psk(void)
> test_postcopy_recovery_common(&args);
> }
>
> +static void test_multifd_postcopy_recovery_tls_psk(void)
> +{
> + MigrateCommon args = {
> + .start_hook = migrate_hook_start_tls_psk_match,
> + .end_hook = migrate_hook_end_tls_psk,
> + .multifd = true,
> + };
> +
> + test_postcopy_recovery_common(&args);
> +}
> +
> /* This contains preempt+recovery+tls test altogether */
> static void test_postcopy_preempt_all(void)
> {
> @@ -405,6 +416,17 @@ static void test_postcopy_preempt_all(void)
> test_postcopy_recovery_common(&args);
> }
>
> +static void test_multifd_postcopy_preempt_recovery_tls_psk(void)
> +{
> + MigrateCommon args = {
> + .start_hook = migrate_hook_start_tls_psk_match,
> + .end_hook = migrate_hook_end_tls_psk,
> + .multifd = true,
> + };
> +
> + test_postcopy_recovery_common(&args);
> +}
> +
> static void test_precopy_unix_tls_psk(void)
> {
> g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> @@ -649,6 +671,18 @@ static void test_multifd_tcp_tls_psk_mismatch(void)
> test_precopy_common(&args);
> }
>
> +static void test_multifd_postcopy_tcp_tls_psk_match(void)
> +{
> + MigrateCommon args = {
> + .multifd = true,
> + .listen_uri = "defer",
> + .start_hook = migrate_hook_start_multifd_tcp_tls_psk_match,
> + .end_hook = migrate_hook_end_tls_psk,
> + };
> +
> + test_precopy_common(&args);
> +}
> +
> #ifdef CONFIG_TASN1
> static void test_multifd_tcp_tls_x509_default_host(void)
> {
> @@ -743,6 +777,10 @@ void migration_test_add_tls(MigrationTestEnv *env)
> test_postcopy_preempt_tls_psk);
> migration_test_add("/migration/postcopy/preempt/recovery/tls/psk",
> test_postcopy_preempt_all);
> + migration_test_add("/migration/multifd+postcopy/recovery/tls/psk",
> + test_multifd_postcopy_recovery_tls_psk);
> +
> migration_test_add("/migration/multifd+postcopy/preempt/recovery/tls/psk",
> + test_multifd_postcopy_preempt_recovery_tls_psk);
> }
> #ifdef CONFIG_TASN1
> migration_test_add("/migration/precopy/unix/tls/x509/default-host",
> @@ -776,6 +814,8 @@ void migration_test_add_tls(MigrationTestEnv *env)
> test_multifd_tcp_tls_psk_match);
> migration_test_add("/migration/multifd/tcp/tls/psk/mismatch",
> test_multifd_tcp_tls_psk_mismatch);
> + migration_test_add("/migration/multifd+postcopy/tcp/tls/psk/match",
> + test_multifd_postcopy_tcp_tls_psk_match);
> #ifdef CONFIG_TASN1
> migration_test_add("/migration/multifd/tcp/tls/x509/default-host",
> test_multifd_tcp_tls_x509_default_host);