[PATCH] tests/qtest/migration-test: Run some basic tests on s390x and ppc64 with TCG, too

[PATCH] tests/qtest/migration-test: Run some basic tests on s390x and ppc64 with TCG, too

[Thomas Huth]

On s390x, we recently had a regression that broke migration / savevm
(see commit bebe9603fc ("hw/intc/s390_flic: Fix crash that occurs when
saving the machine state"). The problem was merged without being noticed
since we currently do not run any migration / savevm related tests on
x86 hosts.
While we currently cannot run all migration tests for the s390x target
on x86 hosts yet (due to some unresolved issues with TCG), we can at
least run some of the non-live tests to avoid such problems in the future.
Thus enable the "analyze-script" and the "bad_dest" tests before checking
for KVM on s390x or ppc64 (this also fixes the problem that the
"analyze-script" test was not run on s390x at all anymore since it got
disabled again by accident in a previous refactoring of the code).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 PS: Before anyone asks, yes, the quoted problem has been detected by the
 s390x runner in the gitlab-CI, but since that occasionally shows failure
 due to its slowness, it's considered as non-gating and nobody really looked
 at the failing jobs :-(

 tests/qtest/migration-test.c | 39 ++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index e8d3555f56..5b4eca2b20 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -3437,6 +3437,20 @@ int main(int argc, char **argv)
     arch = qtest_get_arch();
     is_x86 = !strcmp(arch, "i386") || !strcmp(arch, "x86_64");
 
+    tmpfs = g_dir_make_tmp("migration-test-XXXXXX", &err);
+    if (!tmpfs) {
+        g_test_message("Can't create temporary directory in %s: %s",
+                       g_get_tmp_dir(), err->message);
+    }
+    g_assert(tmpfs);
+
+    module_call_init(MODULE_INIT_QOM);
+
+    migration_test_add("/migration/bad_dest", test_baddest);
+#ifndef _WIN32
+    migration_test_add("/migration/analyze-script", test_analyze_script);
+#endif
+
     /*
      * On ppc64, the test only works with kvm-hv, but not with kvm-pr and TCG
      * is touchy due to race conditions on dirty bits (especially on PPC for
@@ -3444,8 +3458,8 @@ int main(int argc, char **argv)
      */
     if (g_str_equal(arch, "ppc64") &&
         (!has_kvm || access("/sys/module/kvm_hv", F_OK))) {
-        g_test_message("Skipping test: kvm_hv not available");
-        return g_test_run();
+        g_test_message("Skipping tests: kvm_hv not available");
+        goto test_add_done;
     }
 
     /*
@@ -3453,19 +3467,10 @@ int main(int argc, char **argv)
      * there until the problems are resolved
      */
     if (g_str_equal(arch, "s390x") && !has_kvm) {
-        g_test_message("Skipping test: s390x host with KVM is required");
-        return g_test_run();
+        g_test_message("Skipping tests: s390x host with KVM is required");
+        goto test_add_done;
     }
 
-    tmpfs = g_dir_make_tmp("migration-test-XXXXXX", &err);
-    if (!tmpfs) {
-        g_test_message("Can't create temporary directory in %s: %s",
-                       g_get_tmp_dir(), err->message);
-    }
-    g_assert(tmpfs);
-
-    module_call_init(MODULE_INIT_QOM);
-
     if (is_x86) {
         migration_test_add("/migration/precopy/unix/suspend/live",
                            test_precopy_unix_suspend_live);
@@ -3491,12 +3496,6 @@ int main(int argc, char **argv)
         }
     }
 
-    migration_test_add("/migration/bad_dest", test_baddest);
-#ifndef _WIN32
-    if (!g_str_equal(arch, "s390x")) {
-        migration_test_add("/migration/analyze-script", test_analyze_script);
-    }
-#endif
     migration_test_add("/migration/precopy/unix/plain",
                        test_precopy_unix_plain);
     migration_test_add("/migration/precopy/unix/xbzrle",
@@ -3653,6 +3652,8 @@ int main(int argc, char **argv)
                            test_vcpu_dirty_limit);
     }
 
+test_add_done:
+
     ret = g_test_run();
 
     g_assert_cmpint(ret, ==, 0);
-- 
2.45.1

Re: [PATCH] tests/qtest/migration-test: Run some basic tests on s390x and ppc64 with TCG, too

[Fabiano Rosas]

Thomas Huth <thuth@redhat.com> writes:

> On s390x, we recently had a regression that broke migration / savevm
> (see commit bebe9603fc ("hw/intc/s390_flic: Fix crash that occurs when
> saving the machine state"). The problem was merged without being noticed
> since we currently do not run any migration / savevm related tests on
> x86 hosts.
> While we currently cannot run all migration tests for the s390x target
> on x86 hosts yet (due to some unresolved issues with TCG), we can at
> least run some of the non-live tests to avoid such problems in the future.
> Thus enable the "analyze-script" and the "bad_dest" tests before checking
> for KVM on s390x or ppc64 (this also fixes the problem that the
> "analyze-script" test was not run on s390x at all anymore since it got
> disabled again by accident in a previous refactoring of the code).
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

And thanks for fixing my mistake.

Re: [PATCH] tests/qtest/migration-test: Run some basic tests on s390x and ppc64 with TCG, too

[Peter Xu]

On Wed, May 22, 2024 at 11:12:55AM +0200, Thomas Huth wrote:
> On s390x, we recently had a regression that broke migration / savevm
> (see commit bebe9603fc ("hw/intc/s390_flic: Fix crash that occurs when
> saving the machine state"). The problem was merged without being noticed
> since we currently do not run any migration / savevm related tests on
> x86 hosts.
> While we currently cannot run all migration tests for the s390x target
> on x86 hosts yet (due to some unresolved issues with TCG), we can at
> least run some of the non-live tests to avoid such problems in the future.
> Thus enable the "analyze-script" and the "bad_dest" tests before checking
> for KVM on s390x or ppc64 (this also fixes the problem that the
> "analyze-script" test was not run on s390x at all anymore since it got
> disabled again by accident in a previous refactoring of the code).
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

Irrelevant of this patch, when I was looking at cleaning of bootfile it
looks to me that we're leaking bootpath in the test loops.. maybe we need a
bootfile_delete() at the entry of bootfile_create()?

Or even better, prepare bootfile once only and use it in all tests.  The
only trick arch is x86 who needs to support suspend_me=true, but maybe we
can provide two bootfiles.

-- 
Peter Xu

Re: [PATCH] tests/qtest/migration-test: Run some basic tests on s390x and ppc64 with TCG, too

[Fabiano Rosas]

On Wed, 22 May 2024 11:12:55 +0200, Thomas Huth wrote:
> On s390x, we recently had a regression that broke migration / savevm
> (see commit bebe9603fc ("hw/intc/s390_flic: Fix crash that occurs when
> saving the machine state"). The problem was merged without being noticed
> since we currently do not run any migration / savevm related tests on
> x86 hosts.
> While we currently cannot run all migration tests for the s390x target
> on x86 hosts yet (due to some unresolved issues with TCG), we can at
> least run some of the non-live tests to avoid such problems in the future.
> Thus enable the "analyze-script" and the "bad_dest" tests before checking
> for KVM on s390x or ppc64 (this also fixes the problem that the
> "analyze-script" test was not run on s390x at all anymore since it got
> disabled again by accident in a previous refactoring of the code).
> 
> [...]

Queued, thanks!

Re: [PATCH] tests/qtest/migration-test: Run some basic tests on s390x and ppc64 with TCG, too

[Nicholas Piggin]

On Wed May 22, 2024 at 7:12 PM AEST, Thomas Huth wrote:
> On s390x, we recently had a regression that broke migration / savevm
> (see commit bebe9603fc ("hw/intc/s390_flic: Fix crash that occurs when
> saving the machine state"). The problem was merged without being noticed
> since we currently do not run any migration / savevm related tests on
> x86 hosts.
> While we currently cannot run all migration tests for the s390x target
> on x86 hosts yet (due to some unresolved issues with TCG), we can at
> least run some of the non-live tests to avoid such problems in the future.
> Thus enable the "analyze-script" and the "bad_dest" tests before checking
> for KVM on s390x or ppc64 (this also fixes the problem that the
> "analyze-script" test was not run on s390x at all anymore since it got
> disabled again by accident in a previous refactoring of the code).

ppc64 is working for me, can it be enabled fully, or is it still
breaking somewhere? FWIW I have a patch to change it from using
open-firmware commands to a boot file which speeds it up.

Would be nice to get to the bottom of the s390x problem too :(

Thanks,
Nick

Re: [PATCH] tests/qtest/migration-test: Run some basic tests on s390x and ppc64 with TCG, too

[Thomas Huth]

On 24/05/2024 02.05, Nicholas Piggin wrote:
> On Wed May 22, 2024 at 7:12 PM AEST, Thomas Huth wrote:
>> On s390x, we recently had a regression that broke migration / savevm
>> (see commit bebe9603fc ("hw/intc/s390_flic: Fix crash that occurs when
>> saving the machine state"). The problem was merged without being noticed
>> since we currently do not run any migration / savevm related tests on
>> x86 hosts.
>> While we currently cannot run all migration tests for the s390x target
>> on x86 hosts yet (due to some unresolved issues with TCG), we can at
>> least run some of the non-live tests to avoid such problems in the future.
>> Thus enable the "analyze-script" and the "bad_dest" tests before checking
>> for KVM on s390x or ppc64 (this also fixes the problem that the
>> "analyze-script" test was not run on s390x at all anymore since it got
>> disabled again by accident in a previous refactoring of the code).
> 
> ppc64 is working for me, can it be enabled fully, or is it still
> breaking somewhere? FWIW I have a patch to change it from using
> open-firmware commands to a boot file which speeds it up.

IIRC last time that I tried it was working fine for me, too, but getting a 
speedup here first would be very welcome since using the Forth code slows 
down the whole testing quite a bit.

  Thomas

Re: [PATCH] tests/qtest/migration-test: Run some basic tests on s390x and ppc64 with TCG, too

[Fabiano Rosas]

Thomas Huth <thuth@redhat.com> writes:

> On 24/05/2024 02.05, Nicholas Piggin wrote:
>> On Wed May 22, 2024 at 7:12 PM AEST, Thomas Huth wrote:
>>> On s390x, we recently had a regression that broke migration / savevm
>>> (see commit bebe9603fc ("hw/intc/s390_flic: Fix crash that occurs when
>>> saving the machine state"). The problem was merged without being noticed
>>> since we currently do not run any migration / savevm related tests on
>>> x86 hosts.
>>> While we currently cannot run all migration tests for the s390x target
>>> on x86 hosts yet (due to some unresolved issues with TCG), we can at
>>> least run some of the non-live tests to avoid such problems in the future.
>>> Thus enable the "analyze-script" and the "bad_dest" tests before checking
>>> for KVM on s390x or ppc64 (this also fixes the problem that the
>>> "analyze-script" test was not run on s390x at all anymore since it got
>>> disabled again by accident in a previous refactoring of the code).
>> 
>> ppc64 is working for me, can it be enabled fully, or is it still
>> breaking somewhere? FWIW I have a patch to change it from using
>> open-firmware commands to a boot file which speeds it up.
>
> IIRC last time that I tried it was working fine for me, too, but getting a 
> speedup here first would be very welcome since using the Forth code slows 
> down the whole testing quite a bit.

Yeah, we're all gonna get kicked from the project if we add 10m to make
check in CI. =)

@Nick, send us that patch and I'd be glad to reenable the tests.