[RFC PATCH 2/3] tests/qtest/migration-test: enable on s390x

Nicholas Piggin posted 3 patches 6 months ago
[RFC PATCH 2/3] tests/qtest/migration-test: enable on s390x
Posted by Nicholas Piggin 6 months ago
s390x is more stable now. Enable it.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 tests/qtest/migration-test.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 94d5057857..7987faaded 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -3428,16 +3428,6 @@ int main(int argc, char **argv)
     migration_test_add("/migration/analyze-script", test_analyze_script);
 #endif
 
-    /*
-     * On s390x, the test seems to be touchy with TCG, perhaps due to race
-     * conditions on dirty bits, so disable it there until the problems are
-     * resolved.
-     */
-    if (g_str_equal(arch, "s390x") && !has_kvm) {
-        g_test_message("Skipping tests: s390x host with KVM is required");
-        goto test_add_done;
-    }
-
     if (is_x86) {
         migration_test_add("/migration/precopy/unix/suspend/live",
                            test_precopy_unix_suspend_live);
@@ -3619,8 +3609,6 @@ int main(int argc, char **argv)
                            test_vcpu_dirty_limit);
     }
 
-test_add_done:
-
     ret = g_test_run();
 
     g_assert_cmpint(ret, ==, 0);
-- 
2.43.0
Re: [RFC PATCH 2/3] tests/qtest/migration-test: enable on s390x
Posted by Prasad Pandit 6 months ago
Hi,

On Sat, 25 May 2024 at 18:44, Nicholas Piggin <npiggin@gmail.com> wrote:
> s390x is more stable now. Enable it.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  tests/qtest/migration-test.c | 12 ------------
>  1 file changed, 12 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 94d5057857..7987faaded 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -3428,16 +3428,6 @@ int main(int argc, char **argv)
>      migration_test_add("/migration/analyze-script", test_analyze_script);
>  #endif
>
> -    /*
> -     * On s390x, the test seems to be touchy with TCG, perhaps due to race
> -     * conditions on dirty bits, so disable it there until the problems are
> -     * resolved.
> -     */

    -> https://lists.nongnu.org/archive/html/qemu-devel/2024-05/msg04774.html

* Above patch (not reviewed yet) adds comment about sporadic problems
on s390x, and this patch says s390x is stable now? It'll help to
mention in the commit log - what changed to make it stable in 1 day.

* IIUC, this and the ppc64 patch above enable 'migration-test' for
s390x and ppc64 platforms, when KVM is not available for them? ie.
When running s390x & ppc64 migration-tests with TCG on non s390x or
non-ppc64 machines, right? Maybe the commit message could say
something to the effect of - enable s390x and ppc64 'migration-test'
to run with TCG across platforms where KVM for s390x  OR  KVM for
ppc64 is not available.

> -    if (g_str_equal(arch, "s390x") && !has_kvm) {
> -        g_test_message("Skipping tests: s390x host with KVM is required");
> -        goto test_add_done;
> -    }
> -
>      if (is_x86) {
>          migration_test_add("/migration/precopy/unix/suspend/live",
>                             test_precopy_unix_suspend_live);
> @@ -3619,8 +3609,6 @@ int main(int argc, char **argv)
>                             test_vcpu_dirty_limit);
>      }
>
> -test_add_done:
> -
>      ret = g_test_run();
>
>      g_assert_cmpint(ret, ==, 0);
> --

* Otherwise, the change looks okay to enable 'migration-test' for
s390x with TCG IIUC.

Thank you.
---
  - Prasad
Re: [RFC PATCH 2/3] tests/qtest/migration-test: enable on s390x
Posted by Nicholas Piggin 6 months ago
On Mon May 27, 2024 at 3:46 PM AEST, Prasad Pandit wrote:
> Hi,
>
> On Sat, 25 May 2024 at 18:44, Nicholas Piggin <npiggin@gmail.com> wrote:
> > s390x is more stable now. Enable it.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  tests/qtest/migration-test.c | 12 ------------
> >  1 file changed, 12 deletions(-)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index 94d5057857..7987faaded 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -3428,16 +3428,6 @@ int main(int argc, char **argv)
> >      migration_test_add("/migration/analyze-script", test_analyze_script);
> >  #endif
> >
> > -    /*
> > -     * On s390x, the test seems to be touchy with TCG, perhaps due to race
> > -     * conditions on dirty bits, so disable it there until the problems are
> > -     * resolved.
> > -     */
>
>     -> https://lists.nongnu.org/archive/html/qemu-devel/2024-05/msg04774.html
>
> * Above patch (not reviewed yet) adds comment about sporadic problems
> on s390x, and this patch says s390x is stable now? It'll help to
> mention in the commit log - what changed to make it stable in 1 day.

Patch 1 of this series.

> * IIUC, this and the ppc64 patch above enable 'migration-test' for
> s390x and ppc64 platforms, when KVM is not available for them? ie.
> When running s390x & ppc64 migration-tests with TCG on non s390x or
> non-ppc64 machines, right? Maybe the commit message could say
> something to the effect of - enable s390x and ppc64 'migration-test'
> to run with TCG across platforms where KVM for s390x  OR  KVM for
> ppc64 is not available.

Yes they should be called "enable for TCG" indeed.

> > -    if (g_str_equal(arch, "s390x") && !has_kvm) {
> > -        g_test_message("Skipping tests: s390x host with KVM is required");
> > -        goto test_add_done;
> > -    }
> > -
> >      if (is_x86) {
> >          migration_test_add("/migration/precopy/unix/suspend/live",
> >                             test_precopy_unix_suspend_live);
> > @@ -3619,8 +3609,6 @@ int main(int argc, char **argv)
> >                             test_vcpu_dirty_limit);
> >      }
> >
> > -test_add_done:
> > -
> >      ret = g_test_run();
> >
> >      g_assert_cmpint(ret, ==, 0);
> > --
>
> * Otherwise, the change looks okay to enable 'migration-test' for
> s390x with TCG IIUC.

Thanks,
Nick
Re: [RFC PATCH 2/3] tests/qtest/migration-test: enable on s390x
Posted by Prasad Pandit 6 months ago
On Mon, 27 May 2024 at 12:34, Nicholas Piggin <npiggin@gmail.com> wrote:
> > * Above patch (not reviewed yet) adds comment about sporadic problems
> > on s390x, and this patch says s390x is stable now? It'll help to
> > mention in the commit log - what changed to make it stable in 1 day.
>
> Patch 1 of this series.

* Ah, that one got filtered to another folder. It'll help to add
something about the 'flic pending' case being specific to migration of
TCG device and so it's reasonable to remove 'has_kvm' check and enable
migration-test to run with TCG.

> Yes they should be called "enable for TCG" indeed.
>

Ack with commit message changes:
  Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>

Thank you.
---
  - Prasad