diff mbox series

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

Message ID 20240525131241.378473-3-npiggin@gmail.com (mailing list archive)
State New, archived
Headers show
Series Fix s390x flic migration and add some more qtests | expand

Commit Message

Nicholas Piggin May 25, 2024, 1:12 p.m. UTC
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(-)

Comments

Prasad Pandit May 27, 2024, 5:46 a.m. UTC | #1
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
Nicholas Piggin May 27, 2024, 7:04 a.m. UTC | #2
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
Prasad Pandit May 27, 2024, 7:40 a.m. UTC | #3
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
diff mbox series

Patch

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);