Re: [kvm-unit-tests PATCH 5/7] arch-run: Add a "continuous" migration option for tests

Thomas Huth
2024-03-04T06:17:44Z

On 26/02/2024 10.38, Nicholas Piggin wrote:
> The cooperative migration protocol is very good to control precise
> pre and post conditions for a migration event. However in some cases
> its intrusiveness to the test program, can mask problems and make
> analysis more difficult.
>
> For example to stress test migration vs concurrent complicated
> memory access, including TLB refill, ram dirtying, etc., then the
> tight spin at getchar() and resumption of the workload after
> migration is unhelpful.
>
> This adds a continuous migration mode that directs the harness to
> perform migrations continually. This is added to the migration
> selftests, which also sees cooperative migration iterations reduced
> to avoid increasing test time too much.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> common/selftest-migration.c | 16 +++++++++--
> lib/migrate.c | 18 ++++++++++++
> lib/migrate.h | 3 ++
> scripts/arch-run.bash | 55 ++++++++++++++++++++++++++++++++-----
> 4 files changed, 82 insertions(+), 10 deletions(-)
>
> diff --git a/common/selftest-migration.c b/common/selftest-migration.c
> index 0afd8581c..9a9b61835 100644
> --- a/common/selftest-migration.c
> +++ b/common/selftest-migration.c
> @@ -9,12 +9,13 @@
> */
> #include <libcflat.h>
> #include <migrate.h>
> +#include <asm/time.h>
>
> -#define NR_MIGRATIONS 30
> +#define NR_MIGRATIONS 15
>
> int main(int argc, char **argv)
> {
> - report_prefix_push("migration");
> + report_prefix_push("migration harness");
>
> if (argc > 1 && !strcmp(argv[1], "skip")) {
> migrate_skip();
> @@ -24,7 +25,16 @@ int main(int argc, char **argv)
>
> for (i = 0; i < NR_MIGRATIONS; i++)
> migrate_quiet();
> - report(true, "simple harness stress");
> + report(true, "cooperative migration");
> +
> + migrate_begin_continuous();
> + mdelay(2000);
> + migrate_end_continuous();
> + mdelay(1000);
> + migrate_begin_continuous();
> + mdelay(2000);
> + migrate_end_continuous();
> + report(true, "continuous migration");
> }
>
> report_prefix_pop();
> diff --git a/lib/migrate.c b/lib/migrate.c
> index 1d22196b7..770f76d5c 100644
> --- a/lib/migrate.c
> +++ b/lib/migrate.c
> @@ -60,3 +60,21 @@ void migrate_skip(void)
> puts("Skipped VM migration (quiet)\n");
> (void)getchar();
> }
> +
> +void migrate_begin_continuous(void)
> +{
> + puts("Begin continuous migration\n");
> + (void)getchar();
> +}
> +
> +void migrate_end_continuous(void)
> +{
> + /*
> + * Migration can split this output between source and dest QEMU
> + * output files, print twice and match once to always cope with
> + * a split.
> + */
> + puts("End continuous migration\n");
> + puts("End continuous migration (quiet)\n");
> + (void)getchar();
> +}
> diff --git a/lib/migrate.h b/lib/migrate.h
> index db6e0c501..35b6703a2 100644
> --- a/lib/migrate.h
> +++ b/lib/migrate.h
> @@ -11,3 +11,6 @@ void migrate_quiet(void);
> void migrate_once(void);
>
> void migrate_skip(void);
> +
> +void migrate_begin_continuous(void);
> +void migrate_end_continuous(void);
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index d0f6f098f..5c7e72036 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -125,15 +125,17 @@ qmp_events ()
> filter_quiet_msgs ()
> {
> grep -v "Now migrate the VM (quiet)" |
> + grep -v "Begin continuous migration (quiet)" |
> + grep -v "End continuous migration (quiet)" |
> grep -v "Skipped VM migration (quiet)"
> }
>
> seen_migrate_msg ()
> {
> if [ $skip_migration -eq 1 ]; then
> - grep -q -e "Now migrate the VM" < $1
> + grep -q -e "Now migrate the VM" -e "Begin continuous migration" < $1
> else
> - grep -q -e "Now migrate the VM" -e "Skipped VM migration" < $1
> + grep -q -e "Now migrate the VM" -e "Begin continuous migration" -e "Skipped VM migration" < $1
> fi
> }
>
> @@ -161,6 +163,7 @@ run_migration ()
> src_qmpout=/dev/null
> dst_qmpout=/dev/null
> skip_migration=0
> + continuous_migration=0
>
> mkfifo ${src_outfifo}
> mkfifo ${dst_outfifo}
> @@ -186,9 +189,12 @@ run_migration ()
> do_migration || return $?
>
> while ps -p ${live_pid} > /dev/null ; do
> - # Wait for test exit or further migration messages.
> - if ! seen_migrate_msg ${src_out} ; then
> + if [[ ${continuous_migration} -eq 1 ]] ; then

Here you're using "[[" for testing ...

> + do_migration || return $?
> + elif ! seen_migrate_msg ${src_out} ; then
> sleep 0.1
> + elif grep -q "Begin continuous migration" < ${src_out} ; then
> + do_migration || return $?
> elif grep -q "Now migrate the VM" < ${src_out} ; then
> do_migration || return $?
> elif [ $skip_migration -eq 0 ] && grep -q "Skipped VM migration" < ${src_out} ; then

... while the other code seems to use "[" for testing values. Can we try to
stick to one style, please (unless it's really required to use "[[" somewhere)?

Thanks,
Thomas