[PATCH 0/6] Shorten the runtime of some gitlab-CI shared runner jobs

[PATCH 0/6] Shorten the runtime of some gitlab-CI shared runner jobs

[Thomas Huth]

We're currently facing the problem that the gitlab-CI jobs for the
shared runners take too much of the limited CI minutes on gitlab.com.
Here are now some patches that optimize some of the jobs a little bit
to take less runtime. We slightly lose some test coverage by some of
these changes (e.g. by dropping ppc-softmmu from a Clang-based test
and only continue testing ppc64-softmmu with Clang in another job),
but that should still be much better than running out of CI minutes
after 3/4 of a month.

Thomas Huth (6):
  gitlab-ci.d/buildtest: Remove ppc-softmmu from the clang-system job
  gitlab-ci.d/buildtest: Remove aarch64-softmmu from the
    build-system-ubuntu job
  tests/qtest/display-vga-test: Add proper checks if a device is
    available
  gitlab-ci.d/buildtest: Merge the --without-default-* jobs
  gitlab-ci.d/buildtest: Merge the two gprof-gcov jobs
  gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack
    job

 tests/qtest/display-vga-test.c | 65 +++++++++++++---------------------
 .gitlab-ci.d/buildtest.yml     | 51 ++++++--------------------
 .gitlab-ci.d/crossbuilds.yml   |  2 +-
 3 files changed, 37 insertions(+), 81 deletions(-)

-- 
2.31.1

[PATCH 1/6] gitlab-ci.d/buildtest: Remove ppc-softmmu from the clang-system job

[Thomas Huth]

We are also compile-testing ppc64-softmmu with clang in the "tsan-build"
job, and ppc64-softmmu covers pretty much the same code as ppc-softmmu,
so we should not lose much test coverage here by removing ppc-softmmu
from the "clang-system" job.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/buildtest.yml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index f09a898c3e..406608e5fc 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -316,8 +316,7 @@ clang-system:
     IMAGE: fedora
     CONFIGURE_ARGS: --cc=clang --cxx=clang++
       --extra-cflags=-fsanitize=undefined --extra-cflags=-fno-sanitize-recover=undefined
-    TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
-      ppc-softmmu s390x-softmmu
+    TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu s390x-softmmu
     MAKE_CHECK_ARGS: check-qtest check-tcg
 
 clang-user:
-- 
2.31.1

Re: [PATCH 1/6] gitlab-ci.d/buildtest: Remove ppc-softmmu from the clang-system job

[Philippe Mathieu-Daudé]

On 30/1/23 11:44, Thomas Huth wrote:
> We are also compile-testing ppc64-softmmu with clang in the "tsan-build"
> job, and ppc64-softmmu covers pretty much the same code as ppc-softmmu,
> so we should not lose much test coverage here by removing ppc-softmmu
> from the "clang-system" job.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   .gitlab-ci.d/buildtest.yml | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index f09a898c3e..406608e5fc 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -316,8 +316,7 @@ clang-system:
>       IMAGE: fedora
>       CONFIGURE_ARGS: --cc=clang --cxx=clang++
>         --extra-cflags=-fsanitize=undefined --extra-cflags=-fno-sanitize-recover=undefined
> -    TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
> -      ppc-softmmu s390x-softmmu
> +    TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu s390x-softmmu
>       MAKE_CHECK_ARGS: check-qtest check-tcg

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 2/6] gitlab-ci.d/buildtest: Remove aarch64-softmmu from the build-system-ubuntu job

[Thomas Huth]

aarch64-softmmu is also checked on the same version of Ubuntu in the
gprov-gcov job, so it is redundant to check it again in the normal
ubuntu job.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/buildtest.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 406608e5fc..7b55dfc434 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -42,7 +42,7 @@ build-system-ubuntu:
   variables:
     IMAGE: ubuntu2004
     CONFIGURE_ARGS: --enable-docs --enable-fdt=system --enable-capstone
-    TARGETS: aarch64-softmmu alpha-softmmu cris-softmmu hppa-softmmu
+    TARGETS: alpha-softmmu cris-softmmu hppa-softmmu
       microblazeel-softmmu mips64el-softmmu
     MAKE_CHECK_ARGS: check-build
   artifacts:
-- 
2.31.1

[PATCH 3/6] tests/qtest/display-vga-test: Add proper checks if a device is available

[Thomas Huth]

display-vga-test currently tries to guess the usable VGA devices
according to the target architecture that is used for the test.
This of course does not work if QEMU has been built with the
"--without-default-devices" configure switch. To fix this, use the
qtest_has_device() function for the decision instead. This way
we can also consolidate most of the test functions into one single
function (that takes a parameter with the device name now), except
for the multihead test that tries to instantiate two devices and
thus is a little bit different.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/display-vga-test.c | 65 +++++++++++++---------------------
 1 file changed, 25 insertions(+), 40 deletions(-)

diff --git a/tests/qtest/display-vga-test.c b/tests/qtest/display-vga-test.c
index ace3bb28e0..1a26a66061 100644
--- a/tests/qtest/display-vga-test.c
+++ b/tests/qtest/display-vga-test.c
@@ -8,61 +8,46 @@
  */
 
 #include "qemu/osdep.h"
-#include "libqtest-single.h"
-
-static void pci_cirrus(void)
-{
-    qtest_start("-vga none -device cirrus-vga");
-    qtest_end();
-}
-
-static void pci_stdvga(void)
-{
-    qtest_start("-vga none -device VGA");
-    qtest_end();
-}
-
-static void pci_secondary(void)
-{
-    qtest_start("-vga none -device secondary-vga");
-    qtest_end();
-}
+#include "libqtest.h"
 
 static void pci_multihead(void)
 {
-    qtest_start("-vga none -device VGA -device secondary-vga");
-    qtest_end();
-}
+    QTestState *qts;
 
-static void pci_virtio_gpu(void)
-{
-    qtest_start("-vga none -device virtio-gpu-pci");
-    qtest_end();
+    qts = qtest_init("-vga none -device VGA -device secondary-vga");
+    qtest_quit(qts);
 }
 
-static void pci_virtio_vga(void)
+static void test_vga(gconstpointer data)
 {
-    qtest_start("-vga none -device virtio-vga");
-    qtest_end();
+    QTestState *qts;
+
+    qts = qtest_initf("-vga none -device %s", (const char *)data);
+    qtest_quit(qts);
 }
 
 int main(int argc, char **argv)
 {
-    const char *arch = qtest_get_arch();
+    const char *devices[] = {
+        "cirrus-vga",
+        "VGA",
+        "secondary-vga",
+        "virtio-gpu-pci",
+        "virtio-vga"
+    };
 
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "alpha") == 0 || strcmp(arch, "i386") == 0 ||
-        strcmp(arch, "mips") == 0 || strcmp(arch, "x86_64") == 0) {
-        qtest_add_func("/display/pci/cirrus", pci_cirrus);
+    for (int i = 0; i < ARRAY_SIZE(devices); i++) {
+        if (qtest_has_device(devices[i])) {
+            char *testpath = g_strdup_printf("/display/pci/%s", devices[i]);
+            qtest_add_data_func(testpath, devices[i], test_vga);
+            g_free(testpath);
+        }
     }
-    qtest_add_func("/display/pci/stdvga", pci_stdvga);
-    qtest_add_func("/display/pci/secondary", pci_secondary);
-    qtest_add_func("/display/pci/multihead", pci_multihead);
-    qtest_add_func("/display/pci/virtio-gpu", pci_virtio_gpu);
-    if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64") ||
-        g_str_equal(arch, "hppa") || g_str_equal(arch, "ppc64")) {
-        qtest_add_func("/display/pci/virtio-vga", pci_virtio_vga);
+
+    if (qtest_has_device("secondary-vga")) {
+        qtest_add_func("/display/pci/multihead", pci_multihead);
     }
 
     return g_test_run();
-- 
2.31.1

Re: [PATCH 3/6] tests/qtest/display-vga-test: Add proper checks if a device is available

[Richard Henderson]

On 1/30/23 00:44, Thomas Huth wrote:
>   int main(int argc, char **argv)
>   {
> -    const char *arch = qtest_get_arch();
> +    const char *devices[] = {
> +        "cirrus-vga",

static const char * const devices[]

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 4/6] gitlab-ci.d/buildtest: Merge the --without-default-* jobs

[Thomas Huth]

Let's safe some CI minutes by merging these two jobs. We can now
also drop "--disable-capstone" since the capstone submodule has
been removed a while ago. We should rather tes --disable-fdt now
to test a compilation without the "dtc" submodule (for this we
have to drop i386-softmmu from the target list unfortunately).
Additionally, the qtests with s390x and sh4 are not read for
"--without-default-devices" yet, so we can only test mips64 and
avr here now.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/buildtest.yml | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 7b55dfc434..86f9c37a07 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -557,29 +557,22 @@ build-coroutine-sigaltstack:
     MAKE_CHECK_ARGS: check-unit
 
 # Check our reduced build configurations
-build-without-default-devices:
+build-without-defaults:
   extends: .native_build_job_template
   needs:
     job: amd64-centos8-container
   variables:
     IMAGE: centos8
-    CONFIGURE_ARGS: --without-default-devices --disable-user
-
-build-without-default-features:
-  extends: .native_build_job_template
-  needs:
-    job: amd64-fedora-container
-  variables:
-    IMAGE: fedora
     CONFIGURE_ARGS:
+      --without-default-devices
       --without-default-features
-      --disable-capstone
+      --disable-fdt
       --disable-pie
       --disable-qom-cast-debug
       --disable-strip
-    TARGETS: avr-softmmu i386-softmmu mips64-softmmu s390x-softmmu sh4-softmmu
+    TARGETS: avr-softmmu mips64-softmmu s390x-softmmu sh4-softmmu
       sparc64-softmmu hexagon-linux-user i386-linux-user s390x-linux-user
-    MAKE_CHECK_ARGS: check-unit check-qtest SPEED=slow
+    MAKE_CHECK_ARGS: check-unit check-qtest-avr check-qtest-mips64
 
 build-libvhost-user:
   extends: .base_job_template
-- 
2.31.1

Re: [PATCH 4/6] gitlab-ci.d/buildtest: Merge the --without-default-* jobs

[Fabiano Rosas]

Thomas Huth <thuth@redhat.com> writes:

> Let's safe some CI minutes by merging these two jobs. We can now
> also drop "--disable-capstone" since the capstone submodule has
> been removed a while ago. We should rather tes --disable-fdt now
> to test a compilation without the "dtc" submodule (for this we
> have to drop i386-softmmu from the target list unfortunately).
> Additionally, the qtests with s390x and sh4 are not read for
> "--without-default-devices" yet,

Fyi, I've been working on a series to make the qtests work with the
--without-default-devices build for i386 or aarch64.

There are some qtests explicitly adding devices without checking and
hardcoded defaults on vl.c and machine init code without a matching
KConfig dependency.

> so we can only test mips64 and
> avr here now.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

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

Re: [PATCH 4/6] gitlab-ci.d/buildtest: Merge the --without-default-* jobs

[Thomas Huth]

On 30/01/2023 14.28, Fabiano Rosas wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> Let's safe some CI minutes by merging these two jobs. We can now
>> also drop "--disable-capstone" since the capstone submodule has
>> been removed a while ago. We should rather tes --disable-fdt now
>> to test a compilation without the "dtc" submodule (for this we
>> have to drop i386-softmmu from the target list unfortunately).
>> Additionally, the qtests with s390x and sh4 are not read for
>> "--without-default-devices" yet,
> 
> Fyi, I've been working on a series to make the qtests work with the
> --without-default-devices build for i386 or aarch64.

Great, I'm looking forward to this!

> There are some qtests explicitly adding devices without checking and
> hardcoded defaults on vl.c and machine init code without a matching
> KConfig dependency.

Yes, I also noticed this while working on these patches ... many other 
binaries even don't start since they try to instantiate a default NIC that 
might not be available ... fixing the whole of QEMU will certainly take some 
time, but if you get x86 and aarch64 working, that's certainly a good start!

>> so we can only test mips64 and
>> avr here now.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>

Thanks!

  Thomas

Re: [PATCH 4/6] gitlab-ci.d/buildtest: Merge the --without-default-* jobs

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> Let's safe some CI minutes by merging these two jobs. We can now
> also drop "--disable-capstone" since the capstone submodule has
> been removed a while ago. We should rather tes --disable-fdt now
> to test a compilation without the "dtc" submodule (for this we
> have to drop i386-softmmu from the target list unfortunately).
> Additionally, the qtests with s390x and sh4 are not read for
> "--without-default-devices" yet, so we can only test mips64 and
> avr here now.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 4/6] gitlab-ci.d/buildtest: Merge the --without-default-* jobs

[Philippe Mathieu-Daudé]

On 30/1/23 11:44, Thomas Huth wrote:
> Let's safe some CI minutes by merging these two jobs. We can now
> also drop "--disable-capstone" since the capstone submodule has
> been removed a while ago. We should rather tes --disable-fdt now

"test"

> to test a compilation without the "dtc" submodule (for this we
> have to drop i386-softmmu from the target list unfortunately).
> Additionally, the qtests with s390x and sh4 are not read for

"ready"

> "--without-default-devices" yet, so we can only test mips64 and
> avr here now.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   .gitlab-ci.d/buildtest.yml | 17 +++++------------
>   1 file changed, 5 insertions(+), 12 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 5/6] gitlab-ci.d/buildtest: Merge the two gprof-gcov jobs

[Thomas Huth]

There is only one job depending on the build-gprof-gcov job, so
there is no real need for keeping this separate. It likely only
has been split since the complete runtime is more than 60 minutes,
but that can be better handled with a proper timeout setting instead.
By merging the two jobs, we safe some precious CI minutes for starting
a new container for the second job each time.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/buildtest.yml | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 86f9c37a07..91c7467a66 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -468,26 +468,15 @@ tsan-build:
     MAKE_CHECK_ARGS: bench V=1
 
 # gprof/gcov are GCC features
-build-gprof-gcov:
+gprof-gcov:
   extends: .native_build_job_template
   needs:
     job: amd64-ubuntu2004-container
+  timeout: 80m
   variables:
     IMAGE: ubuntu2004
     CONFIGURE_ARGS: --enable-gprof --enable-gcov
     TARGETS: aarch64-softmmu ppc64-softmmu s390x-softmmu x86_64-softmmu
-  artifacts:
-    expire_in: 1 days
-    paths:
-      - build
-
-check-gprof-gcov:
-  extends: .native_test_job_template
-  needs:
-    - job: build-gprof-gcov
-      artifacts: true
-  variables:
-    IMAGE: ubuntu2004
     MAKE_CHECK_ARGS: check
   after_script:
     - cd build
-- 
2.31.1

Re: [PATCH 5/6] gitlab-ci.d/buildtest: Merge the two gprof-gcov jobs

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> There is only one job depending on the build-gprof-gcov job, so
> there is no real need for keeping this separate. It likely only
> has been split since the complete runtime is more than 60 minutes,
> but that can be better handled with a proper timeout setting instead.
> By merging the two jobs, we safe some precious CI minutes for starting
> a new container for the second job each time.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  .gitlab-ci.d/buildtest.yml | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index 86f9c37a07..91c7467a66 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -468,26 +468,15 @@ tsan-build:
>      MAKE_CHECK_ARGS: bench V=1
>  
>  # gprof/gcov are GCC features
> -build-gprof-gcov:
> +gprof-gcov:
>    extends: .native_build_job_template
>    needs:
>      job: amd64-ubuntu2004-container
> +  timeout: 80m
>    variables:
>      IMAGE: ubuntu2004
>      CONFIGURE_ARGS: --enable-gprof --enable-gcov
>      TARGETS: aarch64-softmmu ppc64-softmmu s390x-softmmu x86_64-softmmu
> -  artifacts:
> -    expire_in: 1 days
> -    paths:
> -      - build
> -
> -check-gprof-gcov:
> -  extends: .native_test_job_template
> -  needs:
> -    - job: build-gprof-gcov
> -      artifacts: true
> -  variables:
> -    IMAGE: ubuntu2004
>      MAKE_CHECK_ARGS: check
>    after_script:
>      - cd build

I'm tempted to deprecate gprof altogether as I find it hard to believe
anyone uses it when perf tools give you a much clearer indication of
where the bottlenecks are in "real world" environments.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 5/6] gitlab-ci.d/buildtest: Merge the two gprof-gcov jobs

[Thomas Huth]

On 30/01/2023 18.42, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
>> There is only one job depending on the build-gprof-gcov job, so
>> there is no real need for keeping this separate. It likely only
>> has been split since the complete runtime is more than 60 minutes,
>> but that can be better handled with a proper timeout setting instead.
>> By merging the two jobs, we safe some precious CI minutes for starting
>> a new container for the second job each time.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   .gitlab-ci.d/buildtest.yml | 15 ++-------------
>>   1 file changed, 2 insertions(+), 13 deletions(-)
>>
>> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
>> index 86f9c37a07..91c7467a66 100644
>> --- a/.gitlab-ci.d/buildtest.yml
>> +++ b/.gitlab-ci.d/buildtest.yml
>> @@ -468,26 +468,15 @@ tsan-build:
>>       MAKE_CHECK_ARGS: bench V=1
>>   
>>   # gprof/gcov are GCC features
>> -build-gprof-gcov:
>> +gprof-gcov:
>>     extends: .native_build_job_template
>>     needs:
>>       job: amd64-ubuntu2004-container
>> +  timeout: 80m
>>     variables:
>>       IMAGE: ubuntu2004
>>       CONFIGURE_ARGS: --enable-gprof --enable-gcov
>>       TARGETS: aarch64-softmmu ppc64-softmmu s390x-softmmu x86_64-softmmu
>> -  artifacts:
>> -    expire_in: 1 days
>> -    paths:
>> -      - build
>> -
>> -check-gprof-gcov:
>> -  extends: .native_test_job_template
>> -  needs:
>> -    - job: build-gprof-gcov
>> -      artifacts: true
>> -  variables:
>> -    IMAGE: ubuntu2004
>>       MAKE_CHECK_ARGS: check
>>     after_script:
>>       - cd build
> 
> I'm tempted to deprecate gprof altogether as I find it hard to believe
> anyone uses it when perf tools give you a much clearer indication of
> where the bottlenecks are in "real world" environments.

Sounds fine to me, too. I'll drop this patch here from my queue. Could you 
send a patch for removing the jobs instead?

  Thomas

[PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Thomas Huth]

We can get rid of the build-coroutine-sigaltstack job by moving
the configure flags that should be tested here to other jobs:
Move --with-coroutine=sigaltstack to the build-without-defaults job
and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/buildtest.yml   | 14 ++------------
 .gitlab-ci.d/crossbuilds.yml |  2 +-
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 91c7467a66..1438797a1c 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -533,19 +533,8 @@ build-tci:
     - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
     - make check-tcg
 
-# Alternate coroutines implementations are only really of interest to KVM users
-# However we can't test against KVM on Gitlab-CI so we can only run unit tests
-build-coroutine-sigaltstack:
-  extends: .native_build_job_template
-  needs:
-    job: amd64-ubuntu2004-container
-  variables:
-    IMAGE: ubuntu2004
-    CONFIGURE_ARGS: --with-coroutine=sigaltstack --disable-tcg
-                    --enable-trace-backends=ftrace
-    MAKE_CHECK_ARGS: check-unit
-
 # Check our reduced build configurations
+# (and an alternative coroutine implementation)
 build-without-defaults:
   extends: .native_build_job_template
   needs:
@@ -559,6 +548,7 @@ build-without-defaults:
       --disable-pie
       --disable-qom-cast-debug
       --disable-strip
+      --with-coroutine=sigaltstack
     TARGETS: avr-softmmu mips64-softmmu s390x-softmmu sh4-softmmu
       sparc64-softmmu hexagon-linux-user i386-linux-user s390x-linux-user
     MAKE_CHECK_ARGS: check-unit check-qtest-avr check-qtest-mips64
diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 8dbbb8f881..027d2088da 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -159,7 +159,7 @@ cross-s390x-kvm-only:
     job: s390x-debian-cross-container
   variables:
     IMAGE: debian-s390x-cross
-    EXTRA_CONFIGURE_OPTS: --disable-tcg
+    EXTRA_CONFIGURE_OPTS: --disable-tcg --enable-trace-backends=ftrace
 
 cross-mips64el-kvm-only:
   extends: .cross_accel_build_job
-- 
2.31.1

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Daniel P. Berrangé]

On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
> We can get rid of the build-coroutine-sigaltstack job by moving
> the configure flags that should be tested here to other jobs:
> Move --with-coroutine=sigaltstack to the build-without-defaults job
> and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.

The biggest user of coroutines is the block layer. So we probably
ought to have coroutines aligned with a job that triggers the
'make check-block' for iotests.  IIUC,  the without-defaults
job won't do that. How about, arbitrarily, using either the
'check-system-debian' or 'check-system-ubuntu' job. Those distros
are closely related, so getting sigaltstack vs ucontext coverage
between them is a good win, and they both trigger the block jobs
IIUC.

Incidentally sigaltstack is also covered by our Cirrus CI job
for macOS.

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  .gitlab-ci.d/buildtest.yml   | 14 ++------------
>  .gitlab-ci.d/crossbuilds.yml |  2 +-
>  2 files changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index 91c7467a66..1438797a1c 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -533,19 +533,8 @@ build-tci:
>      - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
>      - make check-tcg
>  
> -# Alternate coroutines implementations are only really of interest to KVM users
> -# However we can't test against KVM on Gitlab-CI so we can only run unit tests
> -build-coroutine-sigaltstack:
> -  extends: .native_build_job_template
> -  needs:
> -    job: amd64-ubuntu2004-container
> -  variables:
> -    IMAGE: ubuntu2004
> -    CONFIGURE_ARGS: --with-coroutine=sigaltstack --disable-tcg
> -                    --enable-trace-backends=ftrace
> -    MAKE_CHECK_ARGS: check-unit
> -
>  # Check our reduced build configurations
> +# (and an alternative coroutine implementation)
>  build-without-defaults:
>    extends: .native_build_job_template
>    needs:
> @@ -559,6 +548,7 @@ build-without-defaults:
>        --disable-pie
>        --disable-qom-cast-debug
>        --disable-strip
> +      --with-coroutine=sigaltstack
>      TARGETS: avr-softmmu mips64-softmmu s390x-softmmu sh4-softmmu
>        sparc64-softmmu hexagon-linux-user i386-linux-user s390x-linux-user
>      MAKE_CHECK_ARGS: check-unit check-qtest-avr check-qtest-mips64
> diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
> index 8dbbb8f881..027d2088da 100644
> --- a/.gitlab-ci.d/crossbuilds.yml
> +++ b/.gitlab-ci.d/crossbuilds.yml
> @@ -159,7 +159,7 @@ cross-s390x-kvm-only:
>      job: s390x-debian-cross-container
>    variables:
>      IMAGE: debian-s390x-cross
> -    EXTRA_CONFIGURE_OPTS: --disable-tcg
> +    EXTRA_CONFIGURE_OPTS: --disable-tcg --enable-trace-backends=ftrace
>  
>  cross-mips64el-kvm-only:
>    extends: .cross_accel_build_job
> -- 
> 2.31.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Thomas Huth]

On 30/01/2023 11.58, Daniel P. Berrangé wrote:
> On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
>> We can get rid of the build-coroutine-sigaltstack job by moving
>> the configure flags that should be tested here to other jobs:
>> Move --with-coroutine=sigaltstack to the build-without-defaults job
>> and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.
> 
> The biggest user of coroutines is the block layer. So we probably
> ought to have coroutines aligned with a job that triggers the
> 'make check-block' for iotests.  IIUC,  the without-defaults
> job won't do that. How about, arbitrarily, using either the
> 'check-system-debian' or 'check-system-ubuntu' job. Those distros
> are closely related, so getting sigaltstack vs ucontext coverage
> between them is a good win, and they both trigger the block jobs
> IIUC.

Ok ... but let's put qemu-block@nongnu.org on CC: first, to see what the 
block layer folks think about this.

> Incidentally sigaltstack is also covered by our Cirrus CI job
> for macOS.

Oh, nice, so we already have some "check-block" test coverage there!

  Thomas


>> ---
>>   .gitlab-ci.d/buildtest.yml   | 14 ++------------
>>   .gitlab-ci.d/crossbuilds.yml |  2 +-
>>   2 files changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
>> index 91c7467a66..1438797a1c 100644
>> --- a/.gitlab-ci.d/buildtest.yml
>> +++ b/.gitlab-ci.d/buildtest.yml
>> @@ -533,19 +533,8 @@ build-tci:
>>       - QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
>>       - make check-tcg
>>   
>> -# Alternate coroutines implementations are only really of interest to KVM users
>> -# However we can't test against KVM on Gitlab-CI so we can only run unit tests
>> -build-coroutine-sigaltstack:
>> -  extends: .native_build_job_template
>> -  needs:
>> -    job: amd64-ubuntu2004-container
>> -  variables:
>> -    IMAGE: ubuntu2004
>> -    CONFIGURE_ARGS: --with-coroutine=sigaltstack --disable-tcg
>> -                    --enable-trace-backends=ftrace
>> -    MAKE_CHECK_ARGS: check-unit
>> -
>>   # Check our reduced build configurations
>> +# (and an alternative coroutine implementation)
>>   build-without-defaults:
>>     extends: .native_build_job_template
>>     needs:
>> @@ -559,6 +548,7 @@ build-without-defaults:
>>         --disable-pie
>>         --disable-qom-cast-debug
>>         --disable-strip
>> +      --with-coroutine=sigaltstack
>>       TARGETS: avr-softmmu mips64-softmmu s390x-softmmu sh4-softmmu
>>         sparc64-softmmu hexagon-linux-user i386-linux-user s390x-linux-user
>>       MAKE_CHECK_ARGS: check-unit check-qtest-avr check-qtest-mips64
>> diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
>> index 8dbbb8f881..027d2088da 100644
>> --- a/.gitlab-ci.d/crossbuilds.yml
>> +++ b/.gitlab-ci.d/crossbuilds.yml
>> @@ -159,7 +159,7 @@ cross-s390x-kvm-only:
>>       job: s390x-debian-cross-container
>>     variables:
>>       IMAGE: debian-s390x-cross
>> -    EXTRA_CONFIGURE_OPTS: --disable-tcg
>> +    EXTRA_CONFIGURE_OPTS: --disable-tcg --enable-trace-backends=ftrace
>>   
>>   cross-mips64el-kvm-only:
>>     extends: .cross_accel_build_job
>> -- 
>> 2.31.1
>>
>>
> 
> With regards,
> Daniel

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Thomas Huth]

On 30/01/2023 11.58, Daniel P. Berrangé wrote:
> On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
>> We can get rid of the build-coroutine-sigaltstack job by moving
>> the configure flags that should be tested here to other jobs:
>> Move --with-coroutine=sigaltstack to the build-without-defaults job
>> and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.
> 
> The biggest user of coroutines is the block layer. So we probably
> ought to have coroutines aligned with a job that triggers the
> 'make check-block' for iotests.  IIUC,  the without-defaults
> job won't do that. How about, arbitrarily, using either the
> 'check-system-debian' or 'check-system-ubuntu' job. Those distros
> are closely related, so getting sigaltstack vs ucontext coverage
> between them is a good win, and they both trigger the block jobs
> IIUC.

I gave it a try with the ubuntu job, but this apparently trips up the iotests:

  https://gitlab.com/thuth/qemu/-/jobs/3705965062#L212

Does anybody have a clue what could be going wrong here?

  Thomas

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Kevin Wolf]

Am 03.02.2023 um 12:23 hat Thomas Huth geschrieben:
> On 30/01/2023 11.58, Daniel P. Berrangé wrote:
> > On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
> > > We can get rid of the build-coroutine-sigaltstack job by moving
> > > the configure flags that should be tested here to other jobs:
> > > Move --with-coroutine=sigaltstack to the build-without-defaults job
> > > and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.
> > 
> > The biggest user of coroutines is the block layer. So we probably
> > ought to have coroutines aligned with a job that triggers the
> > 'make check-block' for iotests.  IIUC,  the without-defaults
> > job won't do that. How about, arbitrarily, using either the
> > 'check-system-debian' or 'check-system-ubuntu' job. Those distros
> > are closely related, so getting sigaltstack vs ucontext coverage
> > between them is a good win, and they both trigger the block jobs
> > IIUC.
> 
> I gave it a try with the ubuntu job, but this apparently trips up the iotests:
> 
>  https://gitlab.com/thuth/qemu/-/jobs/3705965062#L212
> 
> Does anybody have a clue what could be going wrong here?

I'm not sure how changing the coroutine backend could cause it, but
primarily this looks like an assertion failure in migration code.

Dave, Juan, any ideas what this assertion checks and why it could be
failing?

Kevin

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Thomas Huth]

On 03/02/2023 13.08, Kevin Wolf wrote:
> Am 03.02.2023 um 12:23 hat Thomas Huth geschrieben:
>> On 30/01/2023 11.58, Daniel P. Berrangé wrote:
>>> On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
>>>> We can get rid of the build-coroutine-sigaltstack job by moving
>>>> the configure flags that should be tested here to other jobs:
>>>> Move --with-coroutine=sigaltstack to the build-without-defaults job
>>>> and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.
>>>
>>> The biggest user of coroutines is the block layer. So we probably
>>> ought to have coroutines aligned with a job that triggers the
>>> 'make check-block' for iotests.  IIUC,  the without-defaults
>>> job won't do that. How about, arbitrarily, using either the
>>> 'check-system-debian' or 'check-system-ubuntu' job. Those distros
>>> are closely related, so getting sigaltstack vs ucontext coverage
>>> between them is a good win, and they both trigger the block jobs
>>> IIUC.
>>
>> I gave it a try with the ubuntu job, but this apparently trips up the iotests:
>>
>>   https://gitlab.com/thuth/qemu/-/jobs/3705965062#L212
>>
>> Does anybody have a clue what could be going wrong here?
> 
> I'm not sure how changing the coroutine backend could cause it, but
> primarily this looks like an assertion failure in migration code.
> 
> Dave, Juan, any ideas what this assertion checks and why it could be
> failing?

Ah, I think it's the bug that will be fixed by:

  https://lore.kernel.org/qemu-devel/20230202160640.2300-2-quintela@redhat.com/

The fix hasn't hit the master branch yet (I think), and I had another patch 
in my CI that disables the aarch64 binary in that runner, so the iotests 
suddenly have been executed with the alpha binary there --> migration fails.

So never mind, it will be fixed as soon as Juan's pull request gets included.

  Thomas

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Peter Maydell]

On Fri, 3 Feb 2023 at 15:44, Thomas Huth <thuth@redhat.com> wrote:
>
> On 03/02/2023 13.08, Kevin Wolf wrote:
> > Am 03.02.2023 um 12:23 hat Thomas Huth geschrieben:
> >> On 30/01/2023 11.58, Daniel P. Berrangé wrote:
> >>> On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
> >>>> We can get rid of the build-coroutine-sigaltstack job by moving
> >>>> the configure flags that should be tested here to other jobs:
> >>>> Move --with-coroutine=sigaltstack to the build-without-defaults job
> >>>> and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.
> >>>
> >>> The biggest user of coroutines is the block layer. So we probably
> >>> ought to have coroutines aligned with a job that triggers the
> >>> 'make check-block' for iotests.  IIUC,  the without-defaults
> >>> job won't do that. How about, arbitrarily, using either the
> >>> 'check-system-debian' or 'check-system-ubuntu' job. Those distros
> >>> are closely related, so getting sigaltstack vs ucontext coverage
> >>> between them is a good win, and they both trigger the block jobs
> >>> IIUC.
> >>
> >> I gave it a try with the ubuntu job, but this apparently trips up the iotests:
> >>
> >>   https://gitlab.com/thuth/qemu/-/jobs/3705965062#L212
> >>
> >> Does anybody have a clue what could be going wrong here?
> >
> > I'm not sure how changing the coroutine backend could cause it, but
> > primarily this looks like an assertion failure in migration code.
> >
> > Dave, Juan, any ideas what this assertion checks and why it could be
> > failing?
>
> Ah, I think it's the bug that will be fixed by:
>
>   https://lore.kernel.org/qemu-devel/20230202160640.2300-2-quintela@redhat.com/
>
> The fix hasn't hit the master branch yet (I think), and I had another patch
> in my CI that disables the aarch64 binary in that runner, so the iotests
> suddenly have been executed with the alpha binary there --> migration fails.
>
> So never mind, it will be fixed as soon as Juan's pull request gets included.

The migration tests have been flaky for a while now,
including setups where host and guest page sizes are the same.
(For instance, my x86 macos box pretty reliably sees failures
when the machine is under load.)

-- PMM

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On Fri, 3 Feb 2023 at 15:44, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 03/02/2023 13.08, Kevin Wolf wrote:
>> > Am 03.02.2023 um 12:23 hat Thomas Huth geschrieben:
>> >> On 30/01/2023 11.58, Daniel P. Berrangé wrote:
>> >>> On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
>> >>>> We can get rid of the build-coroutine-sigaltstack job by moving
>> >>>> the configure flags that should be tested here to other jobs:
>> >>>> Move --with-coroutine=sigaltstack to the build-without-defaults job
>> >>>> and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.
>> >>>
>> >>> The biggest user of coroutines is the block layer. So we probably
>> >>> ought to have coroutines aligned with a job that triggers the
>> >>> 'make check-block' for iotests.  IIUC,  the without-defaults
>> >>> job won't do that. How about, arbitrarily, using either the
>> >>> 'check-system-debian' or 'check-system-ubuntu' job. Those distros
>> >>> are closely related, so getting sigaltstack vs ucontext coverage
>> >>> between them is a good win, and they both trigger the block jobs
>> >>> IIUC.
>> >>
>> >> I gave it a try with the ubuntu job, but this apparently trips up the iotests:
>> >>
>> >>   https://gitlab.com/thuth/qemu/-/jobs/3705965062#L212
>> >>
>> >> Does anybody have a clue what could be going wrong here?
>> >
>> > I'm not sure how changing the coroutine backend could cause it, but
>> > primarily this looks like an assertion failure in migration code.
>> >
>> > Dave, Juan, any ideas what this assertion checks and why it could be
>> > failing?
>>
>> Ah, I think it's the bug that will be fixed by:
>>
>>   https://lore.kernel.org/qemu-devel/20230202160640.2300-2-quintela@redhat.com/
>>
>> The fix hasn't hit the master branch yet (I think), and I had another patch
>> in my CI that disables the aarch64 binary in that runner, so the iotests
>> suddenly have been executed with the alpha binary there --> migration fails.
>>
>> So never mind, it will be fixed as soon as Juan's pull request gets included.
>
> The migration tests have been flaky for a while now,
> including setups where host and guest page sizes are the same.
> (For instance, my x86 macos box pretty reliably sees failures
> when the machine is under load.)

I *thought* that we had fixed all of those.

But it is difficult for me to know because:
- I only happens when one runs "make check"
- running ./migration-test have never failed to me
- When it fails (and it has been a while since it has failed to me)
  it is impossible to me to detect what is going on, and as said, I have
  never been able to reproduce running only migration-test.

I will try to run several at the same time and see if it happens.

And as Thomas said, I *think* that the fix that Peter Xu posted should
fix this issue.  Famous last words.

Later, Juan.

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Peter Maydell]

On Fri, 3 Feb 2023 at 21:14, Juan Quintela <quintela@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> wrote:
> > The migration tests have been flaky for a while now,
> > including setups where host and guest page sizes are the same.
> > (For instance, my x86 macos box pretty reliably sees failures
> > when the machine is under load.)
>
> I *thought* that we had fixed all of those.
>
> But it is difficult for me to know because:
> - I only happens when one runs "make check"
> - running ./migration-test have never failed to me
> - When it fails (and it has been a while since it has failed to me)
>   it is impossible to me to detect what is going on, and as said, I have
>   never been able to reproduce running only migration-test.

Yes. If we could improve the logging in the test so that when
an intermittent failure does happen the test prints better
clues about what happened, I think that would help a lot.

https://lore.kernel.org/qemu-devel/CAFEAcA8x_iM3hN2-P9F+huXnXFXy+D6FzE+Leq4erLdg7zkVGw@mail.gmail.com/
is the thread from late December about the macos failures.

-- PMM

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Juan Quintela]

Peter Maydell <peter.maydell@linaro.org> wrote:
> On Fri, 3 Feb 2023 at 21:14, Juan Quintela <quintela@redhat.com> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>> > The migration tests have been flaky for a while now,
>> > including setups where host and guest page sizes are the same.
>> > (For instance, my x86 macos box pretty reliably sees failures
>> > when the machine is under load.)
>>
>> I *thought* that we had fixed all of those.
>>
>> But it is difficult for me to know because:
>> - I only happens when one runs "make check"
>> - running ./migration-test have never failed to me
>> - When it fails (and it has been a while since it has failed to me)
>>   it is impossible to me to detect what is going on, and as said, I have
>>   never been able to reproduce running only migration-test.
>
> Yes. If we could improve the logging in the test so that when
> an intermittent failure does happen the test prints better
> clues about what happened, I think that would help a lot.
>
> https://lore.kernel.org/qemu-devel/CAFEAcA8x_iM3hN2-P9F+huXnXFXy+D6FzE+Leq4erLdg7zkVGw@mail.gmail.com/
> is the thread from late December about the macos failures.

We (red hat) found a similar problem with aarch64, but only when using
zero copy.  Will try to see if I can reproduce this other there.

https://bugzilla.redhat.com/show_bug.cgi?id=2160929

the similar thing to what you have is:
- they are trying to cancel
- they are on aarch64

but:

- they can only reproduce with zero copy enabled.

Later, Juan.

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Thomas Huth]

On 03/02/2023 22.14, Juan Quintela wrote:
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> On Fri, 3 Feb 2023 at 15:44, Thomas Huth <thuth@redhat.com> wrote:
>>>
>>> On 03/02/2023 13.08, Kevin Wolf wrote:
>>>> Am 03.02.2023 um 12:23 hat Thomas Huth geschrieben:
>>>>> On 30/01/2023 11.58, Daniel P. Berrangé wrote:
>>>>>> On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
>>>>>>> We can get rid of the build-coroutine-sigaltstack job by moving
>>>>>>> the configure flags that should be tested here to other jobs:
>>>>>>> Move --with-coroutine=sigaltstack to the build-without-defaults job
>>>>>>> and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.
>>>>>>
>>>>>> The biggest user of coroutines is the block layer. So we probably
>>>>>> ought to have coroutines aligned with a job that triggers the
>>>>>> 'make check-block' for iotests.  IIUC,  the without-defaults
>>>>>> job won't do that. How about, arbitrarily, using either the
>>>>>> 'check-system-debian' or 'check-system-ubuntu' job. Those distros
>>>>>> are closely related, so getting sigaltstack vs ucontext coverage
>>>>>> between them is a good win, and they both trigger the block jobs
>>>>>> IIUC.
>>>>>
>>>>> I gave it a try with the ubuntu job, but this apparently trips up the iotests:
>>>>>
>>>>>    https://gitlab.com/thuth/qemu/-/jobs/3705965062#L212
>>>>>
>>>>> Does anybody have a clue what could be going wrong here?
>>>>
>>>> I'm not sure how changing the coroutine backend could cause it, but
>>>> primarily this looks like an assertion failure in migration code.
>>>>
>>>> Dave, Juan, any ideas what this assertion checks and why it could be
>>>> failing?
>>>
>>> Ah, I think it's the bug that will be fixed by:
>>>
>>>    https://lore.kernel.org/qemu-devel/20230202160640.2300-2-quintela@redhat.com/
>>>
>>> The fix hasn't hit the master branch yet (I think), and I had another patch
>>> in my CI that disables the aarch64 binary in that runner, so the iotests
>>> suddenly have been executed with the alpha binary there --> migration fails.
>>>
>>> So never mind, it will be fixed as soon as Juan's pull request gets included.
>>
>> The migration tests have been flaky for a while now,
>> including setups where host and guest page sizes are the same.
>> (For instance, my x86 macos box pretty reliably sees failures
>> when the machine is under load.)
> 
> I *thought* that we had fixed all of those.
> 
> But it is difficult for me to know because:
> - I only happens when one runs "make check"
> - running ./migration-test have never failed to me
> - When it fails (and it has been a while since it has failed to me)
>    it is impossible to me to detect what is going on, and as said, I have
>    never been able to reproduce running only migration-test.
> 
> I will try to run several at the same time and see if it happens.
> 
> And as Thomas said, I *think* that the fix that Peter Xu posted should
> fix this issue.  Famous last words.

The patch from Peter should fix my problems that I triggered via the iotests 
- but the migration-qtest is still unstable independent from that issue, I 
think. See for example the latest staging pipeline:

  https://gitlab.com/qemu-project/qemu/-/pipelines/767961842

The migration qtest failed in both, the x86-freebsd-build and the 
ubuntu-20.04-s390x-all pipelin.

  Thomas

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Juan Quintela]

Thomas Huth <thuth@redhat.com> wrote:
> On 03/02/2023 22.14, Juan Quintela wrote:
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On Fri, 3 Feb 2023 at 15:44, Thomas Huth <thuth@redhat.com> wrote:
>>>>
>>>> On 03/02/2023 13.08, Kevin Wolf wrote:
>>>>> Am 03.02.2023 um 12:23 hat Thomas Huth geschrieben:
>>>>>> On 30/01/2023 11.58, Daniel P. Berrangé wrote:
>>>>>>> On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
>>>>>>>> We can get rid of the build-coroutine-sigaltstack job by moving
>>>>>>>> the configure flags that should be tested here to other jobs:
>>>>>>>> Move --with-coroutine=sigaltstack to the build-without-defaults job
>>>>>>>> and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.
>>>>>>>
>>>>>>> The biggest user of coroutines is the block layer. So we probably
>>>>>>> ought to have coroutines aligned with a job that triggers the
>>>>>>> 'make check-block' for iotests.  IIUC,  the without-defaults
>>>>>>> job won't do that. How about, arbitrarily, using either the
>>>>>>> 'check-system-debian' or 'check-system-ubuntu' job. Those distros
>>>>>>> are closely related, so getting sigaltstack vs ucontext coverage
>>>>>>> between them is a good win, and they both trigger the block jobs
>>>>>>> IIUC.
>>>>>>
>>>>>> I gave it a try with the ubuntu job, but this apparently trips up the iotests:
>>>>>>
>>>>>>    https://gitlab.com/thuth/qemu/-/jobs/3705965062#L212
>>>>>>
>>>>>> Does anybody have a clue what could be going wrong here?
>>>>>
>>>>> I'm not sure how changing the coroutine backend could cause it, but
>>>>> primarily this looks like an assertion failure in migration code.
>>>>>
>>>>> Dave, Juan, any ideas what this assertion checks and why it could be
>>>>> failing?
>>>>
>>>> Ah, I think it's the bug that will be fixed by:
>>>>
>>>>    https://lore.kernel.org/qemu-devel/20230202160640.2300-2-quintela@redhat.com/
>>>>
>>>> The fix hasn't hit the master branch yet (I think), and I had another patch
>>>> in my CI that disables the aarch64 binary in that runner, so the iotests
>>>> suddenly have been executed with the alpha binary there --> migration fails.
>>>>
>>>> So never mind, it will be fixed as soon as Juan's pull request gets included.
>>>
>>> The migration tests have been flaky for a while now,
>>> including setups where host and guest page sizes are the same.
>>> (For instance, my x86 macos box pretty reliably sees failures
>>> when the machine is under load.)
>> I *thought* that we had fixed all of those.
>> But it is difficult for me to know because:
>> - I only happens when one runs "make check"
>> - running ./migration-test have never failed to me
>> - When it fails (and it has been a while since it has failed to me)
>>    it is impossible to me to detect what is going on, and as said, I have
>>    never been able to reproduce running only migration-test.
>> I will try to run several at the same time and see if it happens.
>> And as Thomas said, I *think* that the fix that Peter Xu posted
>> should
>> fix this issue.  Famous last words.
>
> The patch from Peter should fix my problems that I triggered via the
> iotests - but the migration-qtest is still unstable independent from
> that issue, I think. See for example the latest staging pipeline:
>
>  https://gitlab.com/qemu-project/qemu/-/pipelines/767961842
>
> The migration qtest failed in both, the x86-freebsd-build and the
> ubuntu-20.04-s390x-all pipelin.
>
>  Thomas

 31/659 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test                   ERROR          48.23s   killed by signal 6 SIGABRT
>>> G_TEST_DBUS_DAEMON=/home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/tests/dbus-vmstate-daemon.sh QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-aarch64 MALLOC_PERTURB_=124 QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/build/tests/qtest/migration-test --tap -k
――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――
stderr:
Broken pipe
../tests/qtest/libqtest.c:190: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped)
TAP parsing error: Too few tests run (expected 41, got 12)
(test program exited with status code -6)
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

I don't know hat to do with this:
- this is aarch64 tcg
- this *works* on f37, or at least I can't reproduce any error with make
  check on my box, and I *think* my configuration is quite extensive (as
  far as I know everything that can be compiled in fedora with packages
  in the distro):

configure file: /mnt/code/qemu/full/configure
--enable-trace-backends=log
	--prefix=/usr
	--sysconfdir=/etc/sysconfig/
	--audio-drv-list=pa,alsa
	--with-coroutine=ucontext
	--with-git-submodules=validate
	--enable-alsa
	--enable-attr
	--enable-auth-pam
	--enable-avx2
	--enable-avx512f
	--enable-bochs
	--enable-bpf
	--enable-brlapi
	--disable-bsd-user
	--enable-bzip2
	--enable-cap-ng
	--enable-capstone
	--disable-cfi
	--disable-cfi-debug
	--enable-cloop
	--disable-cocoa
	--enable-containers
	--disable-coreaudio
	--enable-coroutine-pool
	--enable-crypto-afalg
	--enable-curl
	--enable-curses
	--enable-dbus-display
	--enable-debug-info
	--disable-debug-mutex
	--disable-debug-stack-usage
	--disable-debug-tcg
	--enable-dmg
	--enable-docs
	--disable-dsound
	--enable-fdt
	--enable-fuse
	--enable-fuse-lseek
	--disable-fuzzing
	--disable-gcov
	--disable-gcrypt
	--enable-gettext
	--enable-gio
	--enable-glusterfs
	--enable-gnutls
	--disable-gprof
	--enable-gtk
	--enable-guest-agent
	--disable-guest-agent-msi
	--disable-hax
	--disable-hvf
	--enable-iconv
	--enable-install-blobs
	--enable-jack
	--enable-keyring
	--enable-kvm
	--enable-l2tpv3
	--enable-libdaxctl
	--enable-libiscsi
	--enable-libnfs
	--enable-libpmem
	--enable-libssh
	--enable-libudev
	--enable-libusb
	--enable-linux-aio
	--enable-linux-io-uring
	--enable-linux-user
	--enable-live-block-migration
	--disable-lto
	--disable-lzfse
	--enable-lzo
	--disable-malloc-trim
	--enable-membarrier
	--enable-module-upgrades
	--enable-modules
	--enable-mpath
	--enable-multiprocess
	--disable-netmap
	--enable-nettle
	--enable-numa
	--disable-nvmm
	--enable-opengl
	--enable-oss
	--enable-pa
	--enable-parallels
	--enable-pie
	--enable-plugins
	--enable-png
	--disable-profiler
	--enable-pvrdma
	--enable-qcow1
	--enable-qed
	--disable-qom-cast-debug
	--enable-rbd
	--enable-rdma
	--enable-replication
	--enable-rng-none
	--disable-safe-stack
	--disable-sanitizers
	--enable-stack-protector
	--enable-sdl
	--enable-sdl-image
	--enable-seccomp
	--enable-selinux
	--enable-slirp
	--enable-slirp-smbd
	--enable-smartcard
	--enable-snappy
	--enable-sparse
	--enable-spice
	--enable-spice-protocol
	--enable-system
	--enable-tcg
	--disable-tcg-interpreter
	--enable-tools
	--enable-tpm
	--disable-tsan
	--disable-u2f
	--enable-usb-redir
	--enable-user
	--disable-vde
	--enable-vdi
	--enable-vhost-crypto
	--enable-vhost-kernel
	--enable-vhost-net
	--enable-vhost-user
	--enable-vhost-user-blk-server
	--enable-vhost-vdpa
	--enable-virglrenderer
	--enable-virtfs
	--enable-virtiofsd
	--enable-vnc
	--enable-vnc-jpeg
	--enable-vnc-sasl
	--enable-vte
	--enable-vvfat
	--enable-werror
	--disable-whpx
	--enable-xen
	--enable-xen-pci-passthrough
	--enable-xkbcommon
	--enable-zstd

- It gives a segmentation fault.  Nothing else.

Can we get at least a backtrace to work from there?

Thanks, Juan.

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Peter Maydell]

On Mon, 6 Feb 2023 at 08:46, Juan Quintela <quintela@redhat.com> wrote:
>  31/659 qemu:qtest+qtest-aarch64 / qtest-aarch64/migration-test                   ERROR          48.23s   killed by signal 6 SIGABRT
> >>> G_TEST_DBUS_DAEMON=/home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/tests/dbus-vmstate-daemon.sh QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-aarch64 MALLOC_PERTURB_=124 QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/build/tests/qtest/migration-test --tap -k
> ――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――
> stderr:
> Broken pipe
> ../tests/qtest/libqtest.c:190: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped)
> TAP parsing error: Too few tests run (expected 41, got 12)
> (test program exited with status code -6)
> ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
>
> I don't know hat to do with this:
> - this is aarch64 tcg
> - this *works* on f37, or at least I can't reproduce any error with make
>   check on my box, and I *think* my configuration is quite extensive (as
>   far as I know everything that can be compiled in fedora with packages
>   in the distro):

> - It gives a segmentation fault.  Nothing else.
>
> Can we get at least a backtrace to work from there?

Improving the test program / test harness to provide better
information would help. Ultimately if we're going to be
doing CI in gitlab this is a necessity -- all we are ever
going to have is whatever the test program and harness can
print to the logs...

thanks
-- PMM

Re: [PATCH 6/6] gitlab-ci.d/buildtest: Disintegrate the build-coroutine-sigaltstack job

[Juan Quintela]

Kevin Wolf <kwolf@redhat.com> wrote:
> Am 03.02.2023 um 12:23 hat Thomas Huth geschrieben:
>> On 30/01/2023 11.58, Daniel P. Berrangé wrote:
>> > On Mon, Jan 30, 2023 at 11:44:46AM +0100, Thomas Huth wrote:
>> > > We can get rid of the build-coroutine-sigaltstack job by moving
>> > > the configure flags that should be tested here to other jobs:
>> > > Move --with-coroutine=sigaltstack to the build-without-defaults job
>> > > and --enable-trace-backends=ftrace to the cross-s390x-kvm-only job.
>> > 
>> > The biggest user of coroutines is the block layer. So we probably
>> > ought to have coroutines aligned with a job that triggers the
>> > 'make check-block' for iotests.  IIUC,  the without-defaults
>> > job won't do that. How about, arbitrarily, using either the
>> > 'check-system-debian' or 'check-system-ubuntu' job. Those distros
>> > are closely related, so getting sigaltstack vs ucontext coverage
>> > between them is a good win, and they both trigger the block jobs
>> > IIUC.
>> 
>> I gave it a try with the ubuntu job, but this apparently trips up the iotests:
>> 
>>  https://gitlab.com/thuth/qemu/-/jobs/3705965062#L212
>> 
>> Does anybody have a clue what could be going wrong here?
>
> I'm not sure how changing the coroutine backend could cause it, but
> primarily this looks like an assertion failure in migration code.

Adding Peter here, as he is the last one touching that code O:-)

> Dave, Juan, any ideas what this assertion checks and why it could be
> failing?

Really no.

+QEMU_PROG: ../migration/ram.c:874: pss_find_next_dirty: Assertion `pss->host_page_end' failed.
+./common.rc: line 195: 78727 Aborted                 (core dumped) ( if [ -n "${QEMU_NEED_PID}" ]; then
+    echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid";
+fi; GDB=""; if [ -n "${GDB_OPTIONS}" ]; then
+    GDB="gdbserver ${GDB_OPTIONS}";
+fi; VALGRIND_QEMU="${VALGRIND_QEMU_VM}" _qemu_proc_exec "${VALGRIND_LOGFILE}" $GDB "$QEMU_PROG" $QEMU_OPTIONS "$@" )

pss_find_next_dirty() is only called from three places:

find . -type f -exec grep --color=auto -nH --null -e pss_find_next_dirty \{\} +
./ram.c.847: * pss_find_next_dirty: find the next dirty page of current ramblock
./ram.c.857:static void pss_find_next_dirty(PageSearchStatus *pss)
./ram.c.1562:    pss_find_next_dirty(pss);
./ram.c.2391:        pss_find_next_dirty(pss);
./ram.c.2476:        pss_find_next_dirty(pss);

I can't see how this can be affected by coroutines changes.

What is the test that is failing, and what is the change that I have to
do to try to reproduce it?

Later, Juan.

Re: [PATCH 0/6] Shorten the runtime of some gitlab-CI shared runner jobs

[Philippe Mathieu-Daudé]

On 30/1/23 11:44, Thomas Huth wrote:
> We're currently facing the problem that the gitlab-CI jobs for the
> shared runners take too much of the limited CI minutes on gitlab.com.
> Here are now some patches that optimize some of the jobs a little bit
> to take less runtime. We slightly lose some test coverage by some of
> these changes (e.g. by dropping ppc-softmmu from a Clang-based test
> and only continue testing ppc64-softmmu with Clang in another job),
> but that should still be much better than running out of CI minutes
> after 3/4 of a month.

FWIW the last time I wanted to add some tests Alex suggested me to
run a before/after gcov report and justify the new path tested.

Maybe we should enforce something similar, either cover new paths
or fix a bug.