[PATCH] tests/qtest/migration-test: Disable postcopy/preempt tests

Peter Maydell posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230314133310.1177954-1-peter.maydell@linaro.org
Maintainers: Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
tests/qtest/migration-test.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
[PATCH] tests/qtest/migration-test: Disable postcopy/preempt tests
Posted by Peter Maydell 1 year, 1 month ago
The postcopy/preempt tests seem to have a race which makes them hang
on the s390x CI runner.  Disable them for the moment, while we
investigate.  As with the other disabled subtest, you can opt back in
by setting QEMU_TEST_FLAKY_TESTS=1 in your environment.

Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/migration-test.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d4ab3934ed2..4643f7f49dc 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2464,6 +2464,11 @@ int main(int argc, char **argv)
     const char *arch = qtest_get_arch();
     g_autoptr(GError) err = NULL;
     int ret;
+    /*
+     * Race condition suspected in the postcopy/preempt tests: see
+     * https://lore.kernel.org/qemu-devel/CAFEAcA-q1UwPePdHTzXNSX4i6Urh3j6h51kymy6=7SzDAFU87w@mail.gmail.com/
+     */
+    bool skip_postcopy_preempt = getenv("QEMU_TEST_FLAKY_TESTS");
 
     g_test_init(&argc, &argv, NULL);
 
@@ -2500,9 +2505,11 @@ int main(int argc, char **argv)
         qtest_add_func("/migration/postcopy/plain", test_postcopy);
         qtest_add_func("/migration/postcopy/recovery/plain",
                        test_postcopy_recovery);
-        qtest_add_func("/migration/postcopy/preempt/plain", test_postcopy_preempt);
-        qtest_add_func("/migration/postcopy/preempt/recovery/plain",
-                       test_postcopy_preempt_recovery);
+        if (!skip_postcopy_preempt) {
+            qtest_add_func("/migration/postcopy/preempt/plain", test_postcopy_preempt);
+            qtest_add_func("/migration/postcopy/preempt/recovery/plain",
+                           test_postcopy_preempt_recovery);
+        }
     }
 
     qtest_add_func("/migration/bad_dest", test_baddest);
@@ -2521,10 +2528,12 @@ int main(int argc, char **argv)
         qtest_add_func("/migration/postcopy/tls/psk", test_postcopy_tls_psk);
         qtest_add_func("/migration/postcopy/recovery/tls/psk",
                        test_postcopy_recovery_tls_psk);
-        qtest_add_func("/migration/postcopy/preempt/tls/psk",
-                       test_postcopy_preempt_tls_psk);
-        qtest_add_func("/migration/postcopy/preempt/recovery/tls/psk",
-                       test_postcopy_preempt_all);
+        if (!skip_postcopy_preempt) {
+            qtest_add_func("/migration/postcopy/preempt/tls/psk",
+                           test_postcopy_preempt_tls_psk);
+            qtest_add_func("/migration/postcopy/preempt/recovery/tls/psk",
+                           test_postcopy_preempt_all);
+        }
     }
 #ifdef CONFIG_TASN1
     qtest_add_func("/migration/precopy/unix/tls/x509/default-host",
-- 
2.34.1
Re: [PATCH] tests/qtest/migration-test: Disable postcopy/preempt tests
Posted by Thomas Huth 1 year, 1 month ago
On 14/03/2023 14.33, Peter Maydell wrote:
> The postcopy/preempt tests seem to have a race which makes them hang
> on the s390x CI runner.  Disable them for the moment, while we
> investigate.  As with the other disabled subtest, you can opt back in
> by setting QEMU_TEST_FLAKY_TESTS=1 in your environment.
> 
> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   tests/qtest/migration-test.c | 23 ++++++++++++++++-------
>   1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index d4ab3934ed2..4643f7f49dc 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2464,6 +2464,11 @@ int main(int argc, char **argv)
>       const char *arch = qtest_get_arch();
>       g_autoptr(GError) err = NULL;
>       int ret;
> +    /*
> +     * Race condition suspected in the postcopy/preempt tests: see
> +     * https://lore.kernel.org/qemu-devel/CAFEAcA-q1UwPePdHTzXNSX4i6Urh3j6h51kymy6=7SzDAFU87w@mail.gmail.com/
> +     */
> +    bool skip_postcopy_preempt = getenv("QEMU_TEST_FLAKY_TESTS");

Shouldn't that be "!getenv(...)" ?

  Thomas


>       g_test_init(&argc, &argv, NULL);
>   
> @@ -2500,9 +2505,11 @@ int main(int argc, char **argv)
>           qtest_add_func("/migration/postcopy/plain", test_postcopy);
>           qtest_add_func("/migration/postcopy/recovery/plain",
>                          test_postcopy_recovery);
> -        qtest_add_func("/migration/postcopy/preempt/plain", test_postcopy_preempt);
> -        qtest_add_func("/migration/postcopy/preempt/recovery/plain",
> -                       test_postcopy_preempt_recovery);
> +        if (!skip_postcopy_preempt) {
> +            qtest_add_func("/migration/postcopy/preempt/plain", test_postcopy_preempt);
> +            qtest_add_func("/migration/postcopy/preempt/recovery/plain",
> +                           test_postcopy_preempt_recovery);
> +        }
>       }
>   
>       qtest_add_func("/migration/bad_dest", test_baddest);
> @@ -2521,10 +2528,12 @@ int main(int argc, char **argv)
>           qtest_add_func("/migration/postcopy/tls/psk", test_postcopy_tls_psk);
>           qtest_add_func("/migration/postcopy/recovery/tls/psk",
>                          test_postcopy_recovery_tls_psk);
> -        qtest_add_func("/migration/postcopy/preempt/tls/psk",
> -                       test_postcopy_preempt_tls_psk);
> -        qtest_add_func("/migration/postcopy/preempt/recovery/tls/psk",
> -                       test_postcopy_preempt_all);
> +        if (!skip_postcopy_preempt) {
> +            qtest_add_func("/migration/postcopy/preempt/tls/psk",
> +                           test_postcopy_preempt_tls_psk);
> +            qtest_add_func("/migration/postcopy/preempt/recovery/tls/psk",
> +                           test_postcopy_preempt_all);
> +        }
>       }
>   #ifdef CONFIG_TASN1
>       qtest_add_func("/migration/precopy/unix/tls/x509/default-host",
Re: [PATCH] tests/qtest/migration-test: Disable postcopy/preempt tests
Posted by Peter Maydell 1 year, 1 month ago
On Tue, 14 Mar 2023 at 14:01, Thomas Huth <thuth@redhat.com> wrote:
>
> On 14/03/2023 14.33, Peter Maydell wrote:
> > The postcopy/preempt tests seem to have a race which makes them hang
> > on the s390x CI runner.  Disable them for the moment, while we
> > investigate.  As with the other disabled subtest, you can opt back in
> > by setting QEMU_TEST_FLAKY_TESTS=1 in your environment.
> >
> > Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   tests/qtest/migration-test.c | 23 ++++++++++++++++-------
> >   1 file changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index d4ab3934ed2..4643f7f49dc 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -2464,6 +2464,11 @@ int main(int argc, char **argv)
> >       const char *arch = qtest_get_arch();
> >       g_autoptr(GError) err = NULL;
> >       int ret;
> > +    /*
> > +     * Race condition suspected in the postcopy/preempt tests: see
> > +     * https://lore.kernel.org/qemu-devel/CAFEAcA-q1UwPePdHTzXNSX4i6Urh3j6h51kymy6=7SzDAFU87w@mail.gmail.com/
> > +     */
> > +    bool skip_postcopy_preempt = getenv("QEMU_TEST_FLAKY_TESTS");
>
> Shouldn't that be "!getenv(...)" ?

Doh!

thanks
-- PMM
Re: [PATCH] tests/qtest/migration-test: Disable postcopy/preempt tests
Posted by Thomas Huth 1 year, 1 month ago
On 14/03/2023 15.08, Peter Maydell wrote:
> On Tue, 14 Mar 2023 at 14:01, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 14/03/2023 14.33, Peter Maydell wrote:
>>> The postcopy/preempt tests seem to have a race which makes them hang
>>> on the s390x CI runner.  Disable them for the moment, while we
>>> investigate.  As with the other disabled subtest, you can opt back in
>>> by setting QEMU_TEST_FLAKY_TESTS=1 in your environment.
>>>
>>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>    tests/qtest/migration-test.c | 23 ++++++++++++++++-------
>>>    1 file changed, 16 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>>> index d4ab3934ed2..4643f7f49dc 100644
>>> --- a/tests/qtest/migration-test.c
>>> +++ b/tests/qtest/migration-test.c
>>> @@ -2464,6 +2464,11 @@ int main(int argc, char **argv)
>>>        const char *arch = qtest_get_arch();
>>>        g_autoptr(GError) err = NULL;
>>>        int ret;
>>> +    /*
>>> +     * Race condition suspected in the postcopy/preempt tests: see
>>> +     * https://lore.kernel.org/qemu-devel/CAFEAcA-q1UwPePdHTzXNSX4i6Urh3j6h51kymy6=7SzDAFU87w@mail.gmail.com/
>>> +     */
>>> +    bool skip_postcopy_preempt = getenv("QEMU_TEST_FLAKY_TESTS");

You could maybe also call the variale skip_flaky_tests and use it in the 
other spot where you recently added a getenv() already.

  Thomas
Re: [PATCH] tests/qtest/migration-test: Disable postcopy/preempt tests
Posted by Peter Maydell 1 year, 1 month ago
On Tue, 14 Mar 2023 at 14:14, Thomas Huth <thuth@redhat.com> wrote:
>
> On 14/03/2023 15.08, Peter Maydell wrote:
> > On Tue, 14 Mar 2023 at 14:01, Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> On 14/03/2023 14.33, Peter Maydell wrote:
> >>> The postcopy/preempt tests seem to have a race which makes them hang
> >>> on the s390x CI runner.  Disable them for the moment, while we
> >>> investigate.  As with the other disabled subtest, you can opt back in
> >>> by setting QEMU_TEST_FLAKY_TESTS=1 in your environment.
> >>>
> >>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >>> ---
> >>>    tests/qtest/migration-test.c | 23 ++++++++++++++++-------
> >>>    1 file changed, 16 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> >>> index d4ab3934ed2..4643f7f49dc 100644
> >>> --- a/tests/qtest/migration-test.c
> >>> +++ b/tests/qtest/migration-test.c
> >>> @@ -2464,6 +2464,11 @@ int main(int argc, char **argv)
> >>>        const char *arch = qtest_get_arch();
> >>>        g_autoptr(GError) err = NULL;
> >>>        int ret;
> >>> +    /*
> >>> +     * Race condition suspected in the postcopy/preempt tests: see
> >>> +     * https://lore.kernel.org/qemu-devel/CAFEAcA-q1UwPePdHTzXNSX4i6Urh3j6h51kymy6=7SzDAFU87w@mail.gmail.com/
> >>> +     */
> >>> +    bool skip_postcopy_preempt = getenv("QEMU_TEST_FLAKY_TESTS");
>
> You could maybe also call the variale skip_flaky_tests and use it in the
> other spot where you recently added a getenv() already.

That would make it a bit harder to do a simple revert of the
commits when we figure out the cause of the problem, though.

-- PMM
Re: [PATCH] tests/qtest/migration-test: Disable postcopy/preempt tests
Posted by Thomas Huth 1 year, 1 month ago
On 14/03/2023 15.17, Peter Maydell wrote:
> On Tue, 14 Mar 2023 at 14:14, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 14/03/2023 15.08, Peter Maydell wrote:
>>> On Tue, 14 Mar 2023 at 14:01, Thomas Huth <thuth@redhat.com> wrote:
>>>>
>>>> On 14/03/2023 14.33, Peter Maydell wrote:
>>>>> The postcopy/preempt tests seem to have a race which makes them hang
>>>>> on the s390x CI runner.  Disable them for the moment, while we
>>>>> investigate.  As with the other disabled subtest, you can opt back in
>>>>> by setting QEMU_TEST_FLAKY_TESTS=1 in your environment.
>>>>>
>>>>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>>> ---
>>>>>     tests/qtest/migration-test.c | 23 ++++++++++++++++-------
>>>>>     1 file changed, 16 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>>>>> index d4ab3934ed2..4643f7f49dc 100644
>>>>> --- a/tests/qtest/migration-test.c
>>>>> +++ b/tests/qtest/migration-test.c
>>>>> @@ -2464,6 +2464,11 @@ int main(int argc, char **argv)
>>>>>         const char *arch = qtest_get_arch();
>>>>>         g_autoptr(GError) err = NULL;
>>>>>         int ret;
>>>>> +    /*
>>>>> +     * Race condition suspected in the postcopy/preempt tests: see
>>>>> +     * https://lore.kernel.org/qemu-devel/CAFEAcA-q1UwPePdHTzXNSX4i6Urh3j6h51kymy6=7SzDAFU87w@mail.gmail.com/
>>>>> +     */
>>>>> +    bool skip_postcopy_preempt = getenv("QEMU_TEST_FLAKY_TESTS");
>>
>> You could maybe also call the variale skip_flaky_tests and use it in the
>> other spot where you recently added a getenv() already.
> 
> That would make it a bit harder to do a simple revert of the
> commits when we figure out the cause of the problem, though.

Ok, fair.

So with the "!" added before the getenv:
Reviewed-by: Thomas Huth <thuth@redhat.com>
Re: [PATCH] tests/qtest/migration-test: Disable postcopy/preempt tests
Posted by Dr. David Alan Gilbert 1 year, 1 month ago
* Peter Maydell (peter.maydell@linaro.org) wrote:
> The postcopy/preempt tests seem to have a race which makes them hang
> on the s390x CI runner.  Disable them for the moment, while we
> investigate.  As with the other disabled subtest, you can opt back in
> by setting QEMU_TEST_FLAKY_TESTS=1 in your environment.
> 
> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  tests/qtest/migration-test.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index d4ab3934ed2..4643f7f49dc 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2464,6 +2464,11 @@ int main(int argc, char **argv)
>      const char *arch = qtest_get_arch();
>      g_autoptr(GError) err = NULL;
>      int ret;
> +    /*
> +     * Race condition suspected in the postcopy/preempt tests: see
> +     * https://lore.kernel.org/qemu-devel/CAFEAcA-q1UwPePdHTzXNSX4i6Urh3j6h51kymy6=7SzDAFU87w@mail.gmail.com/
> +     */
> +    bool skip_postcopy_preempt = getenv("QEMU_TEST_FLAKY_TESTS");
>  
>      g_test_init(&argc, &argv, NULL);
>  
> @@ -2500,9 +2505,11 @@ int main(int argc, char **argv)
>          qtest_add_func("/migration/postcopy/plain", test_postcopy);
>          qtest_add_func("/migration/postcopy/recovery/plain",
>                         test_postcopy_recovery);
> -        qtest_add_func("/migration/postcopy/preempt/plain", test_postcopy_preempt);
> -        qtest_add_func("/migration/postcopy/preempt/recovery/plain",
> -                       test_postcopy_preempt_recovery);
> +        if (!skip_postcopy_preempt) {
> +            qtest_add_func("/migration/postcopy/preempt/plain", test_postcopy_preempt);
> +            qtest_add_func("/migration/postcopy/preempt/recovery/plain",
> +                           test_postcopy_preempt_recovery);
> +        }
>      }
>  
>      qtest_add_func("/migration/bad_dest", test_baddest);
> @@ -2521,10 +2528,12 @@ int main(int argc, char **argv)
>          qtest_add_func("/migration/postcopy/tls/psk", test_postcopy_tls_psk);
>          qtest_add_func("/migration/postcopy/recovery/tls/psk",
>                         test_postcopy_recovery_tls_psk);
> -        qtest_add_func("/migration/postcopy/preempt/tls/psk",
> -                       test_postcopy_preempt_tls_psk);
> -        qtest_add_func("/migration/postcopy/preempt/recovery/tls/psk",
> -                       test_postcopy_preempt_all);
> +        if (!skip_postcopy_preempt) {
> +            qtest_add_func("/migration/postcopy/preempt/tls/psk",
> +                           test_postcopy_preempt_tls_psk);
> +            qtest_add_func("/migration/postcopy/preempt/recovery/tls/psk",
> +                           test_postcopy_preempt_all);
> +        }
>      }
>  #ifdef CONFIG_TASN1
>      qtest_add_func("/migration/precopy/unix/tls/x509/default-host",
> -- 
> 2.34.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK