| Message ID | 20240523201922.28007-3-farosas@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,1/4] tests/qtest/libqtest: Introduce another qtest_init version with no handshake | expand |
On Thu, May 23, 2024 at 05:19:20PM -0300, Fabiano Rosas wrote: > We have the vmstate-static-checker script that takes the output of: > '$QEMU -M $machine -dump-vmstate' for two different QEMU versions and > compares them to check for compatibility breakages. This is just too > simple and useful for us to pass on it. Add a test that runs the > script. > > Since this needs to use two different QEMU versions, the test is > skipped if only one QEMU is provided. The infrastructure for passing > more than one binary is already in place: > > $ PYTHON=$(which python3.11) \ > QTEST_QEMU_BINARY_SRC=../build-previous/qemu-system-x86_64 \ > QTEST_QEMU_BINARY=./qemu-system-x86_64 \ > ./tests/qtest/migration-test -p /x86_64/migration/vmstate-checker-script > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > some code duplication for now, just so we can reason about this > without too much noise > --- > tests/qtest/migration-test.c | 82 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 82 insertions(+) > > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > index e8d3555f56..2253e0fc5b 100644 > --- a/tests/qtest/migration-test.c > +++ b/tests/qtest/migration-test.c > @@ -63,6 +63,7 @@ static QTestMigrationState dst_state; > #define DIRTYLIMIT_TOLERANCE_RANGE 25 /* MB/s */ > > #define ANALYZE_SCRIPT "scripts/analyze-migration.py" > +#define VMSTATE_CHECKER_SCRIPT "scripts/vmstate-static-checker.py" > > #define QEMU_VM_FILE_MAGIC 0x5145564d > #define FILE_TEST_FILENAME "migfile" > @@ -1611,6 +1612,85 @@ static void test_analyze_script(void) > test_migrate_end(from, to, false); > cleanup("migfile"); > } > + > +static void test_vmstate_checker_script(void) > +{ > + g_autofree gchar *cmd_src = NULL; > + g_autofree gchar *cmd_dst = NULL; > + g_autofree gchar *vmstate_src = NULL; > + g_autofree gchar *vmstate_dst = NULL; > + const char *machine_alias, *machine_opts = ""; > + g_autofree char *machine = NULL; > + const char *arch = qtest_get_arch(); > + int pid, wstatus; > + const char *python = g_getenv("PYTHON"); > + > + if (!getenv(QEMU_ENV_SRC) && !getenv(QEMU_ENV_DST)) { > + g_test_skip("Test needs two different QEMU versions"); > + return; > + } > + > + if (!python) { > + g_test_skip("PYTHON variable not set"); > + return; > + } > + > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > + if (g_str_equal(arch, "i386")) { > + machine_alias = "pc"; > + } else { > + machine_alias = "q35"; > + } > + } else if (g_str_equal(arch, "s390x")) { > + machine_alias = "s390-ccw-virtio"; > + } else if (strcmp(arch, "ppc64") == 0) { > + machine_alias = "pseries"; > + } else if (strcmp(arch, "aarch64") == 0) { > + machine_alias = "virt"; > + } else { > + g_assert_not_reached(); > + } > + > + if (!qtest_has_machine(machine_alias)) { > + g_autofree char *msg = g_strdup_printf("machine %s not supported", machine_alias); > + g_test_skip(msg); > + return; > + } > + > + machine = resolve_machine_version(machine_alias, QEMU_ENV_SRC, > + QEMU_ENV_DST); > + > + vmstate_src = g_strdup_printf("%s/vmstate-src", tmpfs); > + vmstate_dst = g_strdup_printf("%s/vmstate-dst", tmpfs); > + > + cmd_dst = g_strdup_printf("-machine %s,%s -dump-vmstate %s", > + machine, machine_opts, vmstate_dst); > + cmd_src = g_strdup_printf("-machine %s,%s -dump-vmstate %s", > + machine, machine_opts, vmstate_src); > + > + qtest_init_with_env_no_handshake(QEMU_ENV_SRC, cmd_src); > + qtest_init_with_env_no_handshake(QEMU_ENV_DST, cmd_dst); > + > + pid = fork(); > + if (!pid) { > + close(1); > + open("/dev/null", O_WRONLY); > + execl(python, python, VMSTATE_CHECKER_SCRIPT, > + "-s", vmstate_src, > + "-d", vmstate_dst, > + NULL); > + g_assert_not_reached(); > + } > + > + g_assert(waitpid(pid, &wstatus, 0) == pid); > + if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0) { > + g_test_message("Failed to run vmstate-static-checker.py"); > + g_test_fail(); > + } > + > + cleanup("vmstate-src"); > + cleanup("vmstate-dst"); > +} Did I ask before on whether this can be written without C? I think this and also the analyze-script are more suitable to be written in other ways, e.g., bash or python, no? > #endif > > static void test_precopy_common(MigrateCommon *args) > @@ -3495,6 +3575,8 @@ int main(int argc, char **argv) > #ifndef _WIN32 > if (!g_str_equal(arch, "s390x")) { > migration_test_add("/migration/analyze-script", test_analyze_script); > + migration_test_add("/migration/vmstate-checker-script", > + test_vmstate_checker_script); > } > #endif > migration_test_add("/migration/precopy/unix/plain", > -- > 2.35.3 >
Peter Xu <peterx@redhat.com> writes: > On Thu, May 23, 2024 at 05:19:20PM -0300, Fabiano Rosas wrote: >> We have the vmstate-static-checker script that takes the output of: >> '$QEMU -M $machine -dump-vmstate' for two different QEMU versions and >> compares them to check for compatibility breakages. This is just too >> simple and useful for us to pass on it. Add a test that runs the >> script. >> >> Since this needs to use two different QEMU versions, the test is >> skipped if only one QEMU is provided. The infrastructure for passing >> more than one binary is already in place: >> >> $ PYTHON=$(which python3.11) \ >> QTEST_QEMU_BINARY_SRC=../build-previous/qemu-system-x86_64 \ >> QTEST_QEMU_BINARY=./qemu-system-x86_64 \ >> ./tests/qtest/migration-test -p /x86_64/migration/vmstate-checker-script >> >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> some code duplication for now, just so we can reason about this >> without too much noise >> --- >> tests/qtest/migration-test.c | 82 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 82 insertions(+) >> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c >> index e8d3555f56..2253e0fc5b 100644 >> --- a/tests/qtest/migration-test.c >> +++ b/tests/qtest/migration-test.c >> @@ -63,6 +63,7 @@ static QTestMigrationState dst_state; >> #define DIRTYLIMIT_TOLERANCE_RANGE 25 /* MB/s */ >> >> #define ANALYZE_SCRIPT "scripts/analyze-migration.py" >> +#define VMSTATE_CHECKER_SCRIPT "scripts/vmstate-static-checker.py" >> >> #define QEMU_VM_FILE_MAGIC 0x5145564d >> #define FILE_TEST_FILENAME "migfile" >> @@ -1611,6 +1612,85 @@ static void test_analyze_script(void) >> test_migrate_end(from, to, false); >> cleanup("migfile"); >> } >> + >> +static void test_vmstate_checker_script(void) >> +{ >> + g_autofree gchar *cmd_src = NULL; >> + g_autofree gchar *cmd_dst = NULL; >> + g_autofree gchar *vmstate_src = NULL; >> + g_autofree gchar *vmstate_dst = NULL; >> + const char *machine_alias, *machine_opts = ""; >> + g_autofree char *machine = NULL; >> + const char *arch = qtest_get_arch(); >> + int pid, wstatus; >> + const char *python = g_getenv("PYTHON"); >> + >> + if (!getenv(QEMU_ENV_SRC) && !getenv(QEMU_ENV_DST)) { >> + g_test_skip("Test needs two different QEMU versions"); >> + return; >> + } >> + >> + if (!python) { >> + g_test_skip("PYTHON variable not set"); >> + return; >> + } >> + >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { >> + if (g_str_equal(arch, "i386")) { >> + machine_alias = "pc"; >> + } else { >> + machine_alias = "q35"; >> + } >> + } else if (g_str_equal(arch, "s390x")) { >> + machine_alias = "s390-ccw-virtio"; >> + } else if (strcmp(arch, "ppc64") == 0) { >> + machine_alias = "pseries"; >> + } else if (strcmp(arch, "aarch64") == 0) { >> + machine_alias = "virt"; >> + } else { >> + g_assert_not_reached(); >> + } >> + >> + if (!qtest_has_machine(machine_alias)) { >> + g_autofree char *msg = g_strdup_printf("machine %s not supported", machine_alias); >> + g_test_skip(msg); >> + return; >> + } >> + >> + machine = resolve_machine_version(machine_alias, QEMU_ENV_SRC, >> + QEMU_ENV_DST); >> + >> + vmstate_src = g_strdup_printf("%s/vmstate-src", tmpfs); >> + vmstate_dst = g_strdup_printf("%s/vmstate-dst", tmpfs); >> + >> + cmd_dst = g_strdup_printf("-machine %s,%s -dump-vmstate %s", >> + machine, machine_opts, vmstate_dst); >> + cmd_src = g_strdup_printf("-machine %s,%s -dump-vmstate %s", >> + machine, machine_opts, vmstate_src); >> + >> + qtest_init_with_env_no_handshake(QEMU_ENV_SRC, cmd_src); >> + qtest_init_with_env_no_handshake(QEMU_ENV_DST, cmd_dst); >> + >> + pid = fork(); >> + if (!pid) { >> + close(1); >> + open("/dev/null", O_WRONLY); >> + execl(python, python, VMSTATE_CHECKER_SCRIPT, >> + "-s", vmstate_src, >> + "-d", vmstate_dst, >> + NULL); >> + g_assert_not_reached(); >> + } >> + >> + g_assert(waitpid(pid, &wstatus, 0) == pid); >> + if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0) { >> + g_test_message("Failed to run vmstate-static-checker.py"); >> + g_test_fail(); >> + } >> + >> + cleanup("vmstate-src"); >> + cleanup("vmstate-dst"); >> +} > > Did I ask before on whether this can be written without C? If you did I forgot about it, sorry. > I think this and also the analyze-script are more suitable to be written in > other ways, e.g., bash or python, no? > I would prefer not to fragment the test framework. There's a bunch of infra already present in migration-test/libqtest that we would end up having to rewrite in the other languages. >> #endif >> >> static void test_precopy_common(MigrateCommon *args) >> @@ -3495,6 +3575,8 @@ int main(int argc, char **argv) >> #ifndef _WIN32 >> if (!g_str_equal(arch, "s390x")) { >> migration_test_add("/migration/analyze-script", test_analyze_script); >> + migration_test_add("/migration/vmstate-checker-script", >> + test_vmstate_checker_script); >> } >> #endif >> migration_test_add("/migration/precopy/unix/plain", >> -- >> 2.35.3 >>
On Mon, May 27, 2024 at 07:52:28PM -0300, Fabiano Rosas wrote: > Peter Xu <peterx@redhat.com> writes: > > > On Thu, May 23, 2024 at 05:19:20PM -0300, Fabiano Rosas wrote: > >> We have the vmstate-static-checker script that takes the output of: > >> '$QEMU -M $machine -dump-vmstate' for two different QEMU versions and > >> compares them to check for compatibility breakages. This is just too > >> simple and useful for us to pass on it. Add a test that runs the > >> script. > >> > >> Since this needs to use two different QEMU versions, the test is > >> skipped if only one QEMU is provided. The infrastructure for passing > >> more than one binary is already in place: > >> > >> $ PYTHON=$(which python3.11) \ > >> QTEST_QEMU_BINARY_SRC=../build-previous/qemu-system-x86_64 \ > >> QTEST_QEMU_BINARY=./qemu-system-x86_64 \ > >> ./tests/qtest/migration-test -p /x86_64/migration/vmstate-checker-script > >> > >> Signed-off-by: Fabiano Rosas <farosas@suse.de> > >> --- > >> some code duplication for now, just so we can reason about this > >> without too much noise > >> --- > >> tests/qtest/migration-test.c | 82 ++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 82 insertions(+) > >> > >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > >> index e8d3555f56..2253e0fc5b 100644 > >> --- a/tests/qtest/migration-test.c > >> +++ b/tests/qtest/migration-test.c > >> @@ -63,6 +63,7 @@ static QTestMigrationState dst_state; > >> #define DIRTYLIMIT_TOLERANCE_RANGE 25 /* MB/s */ > >> > >> #define ANALYZE_SCRIPT "scripts/analyze-migration.py" > >> +#define VMSTATE_CHECKER_SCRIPT "scripts/vmstate-static-checker.py" > >> > >> #define QEMU_VM_FILE_MAGIC 0x5145564d > >> #define FILE_TEST_FILENAME "migfile" > >> @@ -1611,6 +1612,85 @@ static void test_analyze_script(void) > >> test_migrate_end(from, to, false); > >> cleanup("migfile"); > >> } > >> + > >> +static void test_vmstate_checker_script(void) > >> +{ > >> + g_autofree gchar *cmd_src = NULL; > >> + g_autofree gchar *cmd_dst = NULL; > >> + g_autofree gchar *vmstate_src = NULL; > >> + g_autofree gchar *vmstate_dst = NULL; > >> + const char *machine_alias, *machine_opts = ""; > >> + g_autofree char *machine = NULL; > >> + const char *arch = qtest_get_arch(); > >> + int pid, wstatus; > >> + const char *python = g_getenv("PYTHON"); > >> + > >> + if (!getenv(QEMU_ENV_SRC) && !getenv(QEMU_ENV_DST)) { > >> + g_test_skip("Test needs two different QEMU versions"); > >> + return; > >> + } > >> + > >> + if (!python) { > >> + g_test_skip("PYTHON variable not set"); > >> + return; > >> + } > >> + > >> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { > >> + if (g_str_equal(arch, "i386")) { > >> + machine_alias = "pc"; > >> + } else { > >> + machine_alias = "q35"; > >> + } > >> + } else if (g_str_equal(arch, "s390x")) { > >> + machine_alias = "s390-ccw-virtio"; > >> + } else if (strcmp(arch, "ppc64") == 0) { > >> + machine_alias = "pseries"; > >> + } else if (strcmp(arch, "aarch64") == 0) { > >> + machine_alias = "virt"; > >> + } else { > >> + g_assert_not_reached(); > >> + } > >> + > >> + if (!qtest_has_machine(machine_alias)) { > >> + g_autofree char *msg = g_strdup_printf("machine %s not supported", machine_alias); > >> + g_test_skip(msg); > >> + return; > >> + } > >> + > >> + machine = resolve_machine_version(machine_alias, QEMU_ENV_SRC, > >> + QEMU_ENV_DST); > >> + > >> + vmstate_src = g_strdup_printf("%s/vmstate-src", tmpfs); > >> + vmstate_dst = g_strdup_printf("%s/vmstate-dst", tmpfs); > >> + > >> + cmd_dst = g_strdup_printf("-machine %s,%s -dump-vmstate %s", > >> + machine, machine_opts, vmstate_dst); > >> + cmd_src = g_strdup_printf("-machine %s,%s -dump-vmstate %s", > >> + machine, machine_opts, vmstate_src); > >> + > >> + qtest_init_with_env_no_handshake(QEMU_ENV_SRC, cmd_src); > >> + qtest_init_with_env_no_handshake(QEMU_ENV_DST, cmd_dst); > >> + > >> + pid = fork(); > >> + if (!pid) { > >> + close(1); > >> + open("/dev/null", O_WRONLY); > >> + execl(python, python, VMSTATE_CHECKER_SCRIPT, > >> + "-s", vmstate_src, > >> + "-d", vmstate_dst, > >> + NULL); > >> + g_assert_not_reached(); > >> + } > >> + > >> + g_assert(waitpid(pid, &wstatus, 0) == pid); > >> + if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0) { > >> + g_test_message("Failed to run vmstate-static-checker.py"); > >> + g_test_fail(); > >> + } > >> + > >> + cleanup("vmstate-src"); > >> + cleanup("vmstate-dst"); > >> +} > > > > Did I ask before on whether this can be written without C? > > If you did I forgot about it, sorry. Nah it's really about me forgetting things sometimes... nothing else. > > > I think this and also the analyze-script are more suitable to be written in > > other ways, e.g., bash or python, no? > > > > I would prefer not to fragment the test framework. There's a bunch of > infra already present in migration-test/libqtest that we would end up > having to rewrite in the other languages. It's ok; I don't feel strongly on this one, but I want us to figure out whether we should have such test enforced in CI. Let's discuss that, and I'm ok with this in C at least for now. Note that one worst case scenario (if you prefer running this in pulls for our submaintainer CI) is we can always introduce this test but only kick it with QEMU_CI_MIGRATION=1. As you said we lost the chance to fail exactly on the pull of another maintainer as they won't normally set this bit, but that goes back to the false negative issue, then we can cover this test more frequently than "softfreeze-only", and no need for manual triggers.
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index e8d3555f56..2253e0fc5b 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -63,6 +63,7 @@ static QTestMigrationState dst_state; #define DIRTYLIMIT_TOLERANCE_RANGE 25 /* MB/s */ #define ANALYZE_SCRIPT "scripts/analyze-migration.py" +#define VMSTATE_CHECKER_SCRIPT "scripts/vmstate-static-checker.py" #define QEMU_VM_FILE_MAGIC 0x5145564d #define FILE_TEST_FILENAME "migfile" @@ -1611,6 +1612,85 @@ static void test_analyze_script(void) test_migrate_end(from, to, false); cleanup("migfile"); } + +static void test_vmstate_checker_script(void) +{ + g_autofree gchar *cmd_src = NULL; + g_autofree gchar *cmd_dst = NULL; + g_autofree gchar *vmstate_src = NULL; + g_autofree gchar *vmstate_dst = NULL; + const char *machine_alias, *machine_opts = ""; + g_autofree char *machine = NULL; + const char *arch = qtest_get_arch(); + int pid, wstatus; + const char *python = g_getenv("PYTHON"); + + if (!getenv(QEMU_ENV_SRC) && !getenv(QEMU_ENV_DST)) { + g_test_skip("Test needs two different QEMU versions"); + return; + } + + if (!python) { + g_test_skip("PYTHON variable not set"); + return; + } + + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { + if (g_str_equal(arch, "i386")) { + machine_alias = "pc"; + } else { + machine_alias = "q35"; + } + } else if (g_str_equal(arch, "s390x")) { + machine_alias = "s390-ccw-virtio"; + } else if (strcmp(arch, "ppc64") == 0) { + machine_alias = "pseries"; + } else if (strcmp(arch, "aarch64") == 0) { + machine_alias = "virt"; + } else { + g_assert_not_reached(); + } + + if (!qtest_has_machine(machine_alias)) { + g_autofree char *msg = g_strdup_printf("machine %s not supported", machine_alias); + g_test_skip(msg); + return; + } + + machine = resolve_machine_version(machine_alias, QEMU_ENV_SRC, + QEMU_ENV_DST); + + vmstate_src = g_strdup_printf("%s/vmstate-src", tmpfs); + vmstate_dst = g_strdup_printf("%s/vmstate-dst", tmpfs); + + cmd_dst = g_strdup_printf("-machine %s,%s -dump-vmstate %s", + machine, machine_opts, vmstate_dst); + cmd_src = g_strdup_printf("-machine %s,%s -dump-vmstate %s", + machine, machine_opts, vmstate_src); + + qtest_init_with_env_no_handshake(QEMU_ENV_SRC, cmd_src); + qtest_init_with_env_no_handshake(QEMU_ENV_DST, cmd_dst); + + pid = fork(); + if (!pid) { + close(1); + open("/dev/null", O_WRONLY); + execl(python, python, VMSTATE_CHECKER_SCRIPT, + "-s", vmstate_src, + "-d", vmstate_dst, + NULL); + g_assert_not_reached(); + } + + g_assert(waitpid(pid, &wstatus, 0) == pid); + if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0) { + g_test_message("Failed to run vmstate-static-checker.py"); + g_test_fail(); + } + + cleanup("vmstate-src"); + cleanup("vmstate-dst"); +} #endif static void test_precopy_common(MigrateCommon *args) @@ -3495,6 +3575,8 @@ int main(int argc, char **argv) #ifndef _WIN32 if (!g_str_equal(arch, "s390x")) { migration_test_add("/migration/analyze-script", test_analyze_script); + migration_test_add("/migration/vmstate-checker-script", + test_vmstate_checker_script); } #endif migration_test_add("/migration/precopy/unix/plain",
We have the vmstate-static-checker script that takes the output of: '$QEMU -M $machine -dump-vmstate' for two different QEMU versions and compares them to check for compatibility breakages. This is just too simple and useful for us to pass on it. Add a test that runs the script. Since this needs to use two different QEMU versions, the test is skipped if only one QEMU is provided. The infrastructure for passing more than one binary is already in place: $ PYTHON=$(which python3.11) \ QTEST_QEMU_BINARY_SRC=../build-previous/qemu-system-x86_64 \ QTEST_QEMU_BINARY=./qemu-system-x86_64 \ ./tests/qtest/migration-test -p /x86_64/migration/vmstate-checker-script Signed-off-by: Fabiano Rosas <farosas@suse.de> --- some code duplication for now, just so we can reason about this without too much noise --- tests/qtest/migration-test.c | 82 ++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+)