[PATCH 0/6] Enable Travis builds on arm64, ppc64le and s390x

[PATCH 0/6] Enable Travis builds on arm64, ppc64le and s390x

[Thomas Huth]

Travis recently added build hosts for arm64, ppc64le and s390x, so
this is a welcome addition to our Travis testing matrix.

Unfortunately, the builds are running in quite restricted LXD containers
there, for example it is not possible to create huge files there (even
if they are just sparse), and certain system calls are blocked. So we
have to change some tests first to stop them failing in such environments.

I also included Alex' patch for dropping some targets from the
MAIN_SOFTMMU_TARGETS config variable, since at least on arm64, the
build process seems to be rather slow and sometimes hits the 50 minutes
timeout otherwise.

For the curious, here's a test run (non-x86 at the end of the page):

 https://travis-ci.com/huth/qemu/jobs/258095792

 Thomas


Alex Bennée (1):
  travis.yml: drop 32 bit systems from MAIN_SOFTMMU_TARGETS

Thomas Huth (5):
  iotests: Skip test 060 if it is not possible to create large files
  iotests: Skip test 079 if it is not possible to create large files
  tests/hd-geo-test: Skip test when images can not be created
  tests/test-util-filemonitor: Skip test on non-x86 Travis containers
  travis.yml: Enable builds on arm64, ppc64le and s390x

 .travis.yml                   | 85 ++++++++++++++++++++++++++++++++++-
 tests/hd-geo-test.c           | 12 ++++-
 tests/qemu-iotests/060        |  6 +++
 tests/qemu-iotests/079        |  6 +++
 tests/test-util-filemonitor.c | 11 +++++
 5 files changed, 118 insertions(+), 2 deletions(-)

-- 
2.23.0

[PATCH 1/6] iotests: Skip test 060 if it is not possible to create large files

[Thomas Huth]

Test 060 fails in the arm64, s390x and ppc64le LXD containers, which
apparently do not allow large files to be created. The repair process
in test 060 creates a file of 64 GiB, so test first whether such large
files are possible and skip the test if that's not the case.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/060 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index b91d8321bb..e7ee865c55 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -49,6 +49,12 @@ _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
 
+# The repair process will create a large file - so check for availability first
+if ! truncate --size=64G "$TEST_IMG"; then
+    _notrun "file system on $TEST_DIR does not support large enough files"
+fi
+rm "$TEST_IMG"
+
 rt_offset=65536  # 0x10000 (XXX: just an assumption)
 rb_offset=131072 # 0x20000 (XXX: just an assumption)
 l1_offset=196608 # 0x30000 (XXX: just an assumption)
-- 
2.23.0

[PATCH 2/6] iotests: Skip test 079 if it is not possible to create large files

[Thomas Huth]

Test 079 fails in the arm64, s390x and ppc64le LXD containers, which
apparently do not allow large files to be created. Test 079 tries to
create a 4G sparse file, so check first whether we can really create
such files before executing the test.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/079 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
index 81f0c21f53..e9b81419b7 100755
--- a/tests/qemu-iotests/079
+++ b/tests/qemu-iotests/079
@@ -39,6 +39,12 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file nfs
 
+# Some containers (e.g. non-x86 on Travis) do not allow large files
+if ! truncate --size=4G "$TEST_IMG"; then
+    _notrun "file system on $TEST_DIR does not support large enough files"
+fi
+rm "$TEST_IMG"
+
 echo "=== Check option preallocation and cluster_size ==="
 echo
 cluster_sizes="16384 32768 65536 131072 262144 524288 1048576 2097152 4194304"
-- 
2.23.0

[PATCH 3/6] tests/hd-geo-test: Skip test when images can not be created

[Thomas Huth]

In certain environments like restricted containers, we can not create
huge test images. To be able to use "make check" in such container
environments, too, let's skip the hd-geo-test instead of failing when
the test images could not be created.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/hd-geo-test.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 7e86c5416c..a249800544 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -34,8 +34,13 @@ static char *create_test_img(int secs)
     fd = mkstemp(template);
     g_assert(fd >= 0);
     ret = ftruncate(fd, (off_t)secs * 512);
-    g_assert(ret == 0);
     close(fd);
+
+    if (ret) {
+        free(template);
+        template = NULL;
+    }
+
     return template;
 }
 
@@ -934,6 +939,10 @@ int main(int argc, char **argv)
     for (i = 0; i < backend_last; i++) {
         if (img_secs[i] >= 0) {
             img_file_name[i] = create_test_img(img_secs[i]);
+            if (!img_file_name[i]) {
+                g_test_message("Could not create test images.");
+                goto test_add_done;
+            }
         } else {
             img_file_name[i] = NULL;
         }
@@ -965,6 +974,7 @@ int main(int argc, char **argv)
                        "skipping hd-geo/override/* tests");
     }
 
+test_add_done:
     ret = g_test_run();
 
     for (i = 0; i < backend_last; i++) {
-- 
2.23.0

[PATCH 4/6] tests/test-util-filemonitor: Skip test on non-x86 Travis containers

[Thomas Huth]

test-util-filemonitor fails in restricted non-x86 Travis containers
since they apparently blacklisted some required system calls there.
Let's simply skip the test if we detect such an environment.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/test-util-filemonitor.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
index 301cd2db61..45009c69f4 100644
--- a/tests/test-util-filemonitor.c
+++ b/tests/test-util-filemonitor.c
@@ -406,10 +406,21 @@ test_file_monitor_events(void)
     char *pathdst = NULL;
     QFileMonitorTestData data;
     GHashTable *ids = g_hash_table_new(g_int64_hash, g_int64_equal);
+    char *travis_arch;
 
     qemu_mutex_init(&data.lock);
     data.records = NULL;
 
+    /*
+     * This test does not work on Travis LXD containers since some
+     * syscalls are blocked in that environment.
+     */
+    travis_arch = getenv("TRAVIS_ARCH");
+    if (travis_arch && !g_str_equal(travis_arch, "x86_64")) {
+        g_test_skip("Test does not work on non-x86 Travis containers.");
+        return;
+    }
+
     /*
      * The file monitor needs the main loop running in
      * order to receive events from inotify. We must
-- 
2.23.0

[PATCH 5/6] travis.yml: drop 32 bit systems from MAIN_SOFTMMU_TARGETS

[Thomas Huth]

From: Alex Bennée <alex.bennee@linaro.org>

The older clangs are still struggling to build and run everything
withing the 50 minute timeout so lets lighten the load a bit more. We
still have coverage for GCC and hopefully no obscure 32 bit guest only
breakages slip through the cracks.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index b9a026c8ee..c09b6a0014 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -79,7 +79,7 @@ env:
     - BASE_CONFIG="--disable-docs --disable-tools"
     - TEST_CMD="make check V=1"
     # This is broadly a list of "mainline" softmmu targets which have support across the major distros
-    - MAIN_SOFTMMU_TARGETS="aarch64-softmmu,arm-softmmu,i386-softmmu,mips-softmmu,mips64-softmmu,ppc64-softmmu,riscv64-softmmu,s390x-softmmu,x86_64-softmmu"
+    - MAIN_SOFTMMU_TARGETS="aarch64-softmmu,mips64-softmmu,ppc64-softmmu,riscv64-softmmu,s390x-softmmu,x86_64-softmmu"
     - CCACHE_SLOPPINESS="include_file_ctime,include_file_mtime"
     - CCACHE_MAXSIZE=1G
 
-- 
2.23.0

[PATCH 6/6] travis.yml: Enable builds on arm64, ppc64le and s390x

[Thomas Huth]

Travis recently added the possibility to test on these architectures,
too, so let's enable them in our travis.yml file to extend our test
coverage.

Unfortunately, the libssh in this Ubuntu version (bionic) is in a pretty
unusable Frankenstein state and libspice-server-dev is not available here,
so we can not use the global list of packages to install, but have to
provide individual package lists instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .travis.yml | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index c09b6a0014..cf48ee452c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -360,6 +360,89 @@ matrix:
         - TEST_CMD="make -j3 check-tcg V=1"
         - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
 
+    - arch: arm64
+      addons:
+        apt_packages:
+          - libaio-dev
+          - libattr1-dev
+          - libbrlapi-dev
+          - libcap-dev
+          - libcap-ng-dev
+          - libgcrypt20-dev
+          - libgnutls28-dev
+          - libgtk-3-dev
+          - libiscsi-dev
+          - liblttng-ust-dev
+          - libncurses5-dev
+          - libnfs-dev
+          - libnss3-dev
+          - libpixman-1-dev
+          - libpng-dev
+          - librados-dev
+          - libsdl2-dev
+          - libseccomp-dev
+          - liburcu-dev
+          - libusb-1.0-0-dev
+          - libvdeplug-dev
+          - libvte-2.91-dev
+      env:
+        - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS},x86_64-linux-user"
+
+    - arch: ppc64le
+      addons:
+        apt_packages:
+          - libaio-dev
+          - libattr1-dev
+          - libbrlapi-dev
+          - libcap-dev
+          - libcap-ng-dev
+          - libgcrypt20-dev
+          - libgnutls28-dev
+          - libgtk-3-dev
+          - libiscsi-dev
+          - liblttng-ust-dev
+          - libncurses5-dev
+          - libnfs-dev
+          - libnss3-dev
+          - libpixman-1-dev
+          - libpng-dev
+          - librados-dev
+          - libsdl2-dev
+          - libseccomp-dev
+          - liburcu-dev
+          - libusb-1.0-0-dev
+          - libvdeplug-dev
+          - libvte-2.91-dev
+      env:
+        - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS},x86_64-linux-user"
+
+    - arch: s390x
+      addons:
+        apt_packages:
+          - libaio-dev
+          - libattr1-dev
+          - libbrlapi-dev
+          - libcap-dev
+          - libcap-ng-dev
+          - libgcrypt20-dev
+          - libgnutls28-dev
+          - libgtk-3-dev
+          - libiscsi-dev
+          - liblttng-ust-dev
+          - libncurses5-dev
+          - libnfs-dev
+          - libnss3-dev
+          - libpixman-1-dev
+          - libpng-dev
+          - librados-dev
+          - libsdl2-dev
+          - libseccomp-dev
+          - liburcu-dev
+          - libusb-1.0-0-dev
+          - libvdeplug-dev
+          - libvte-2.91-dev
+      env:
+        - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS},x86_64-linux-user"
 
     # Release builds
     # The make-release script expect a QEMU version, so our tag must start with a 'v'.
-- 
2.23.0

Re: [PATCH 3/6] tests/hd-geo-test: Skip test when images can not be created

[Philippe Mathieu-Daudé]

On 11/19/19 6:08 PM, Thomas Huth wrote:
> In certain environments like restricted containers, we can not create
> huge test images. To be able to use "make check" in such container
> environments, too, let's skip the hd-geo-test instead of failing when
> the test images could not be created.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/hd-geo-test.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
> index 7e86c5416c..a249800544 100644
> --- a/tests/hd-geo-test.c
> +++ b/tests/hd-geo-test.c
> @@ -34,8 +34,13 @@ static char *create_test_img(int secs)
>       fd = mkstemp(template);
>       g_assert(fd >= 0);
>       ret = ftruncate(fd, (off_t)secs * 512);
> -    g_assert(ret == 0);
>       close(fd);
> +
> +    if (ret) {
> +        free(template);
> +        template = NULL;
> +    }
> +
>       return template;
>   }
>   
> @@ -934,6 +939,10 @@ int main(int argc, char **argv)
>       for (i = 0; i < backend_last; i++) {
>           if (img_secs[i] >= 0) {
>               img_file_name[i] = create_test_img(img_secs[i]);
> +            if (!img_file_name[i]) {
> +                g_test_message("Could not create test images.");
> +                goto test_add_done;
> +            }
>           } else {
>               img_file_name[i] = NULL;
>           }
> @@ -965,6 +974,7 @@ int main(int argc, char **argv)
>                          "skipping hd-geo/override/* tests");
>       }
>   
> +test_add_done:
>       ret = g_test_run();
>   
>       for (i = 0; i < backend_last; i++) {
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 2/6] iotests: Skip test 079 if it is not possible to create large files

[Philippe Mathieu-Daudé]

On 11/19/19 6:08 PM, Thomas Huth wrote:
> Test 079 fails in the arm64, s390x and ppc64le LXD containers, which
> apparently do not allow large files to be created. Test 079 tries to
> create a 4G sparse file, so check first whether we can really create
> such files before executing the test.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/qemu-iotests/079 | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
> index 81f0c21f53..e9b81419b7 100755
> --- a/tests/qemu-iotests/079
> +++ b/tests/qemu-iotests/079
> @@ -39,6 +39,12 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>   _supported_fmt qcow2
>   _supported_proto file nfs
>   
> +# Some containers (e.g. non-x86 on Travis) do not allow large files
> +if ! truncate --size=4G "$TEST_IMG"; then

Shouldn't we restrict that to Travis-CI by checking some environment var?

> +    _notrun "file system on $TEST_DIR does not support large enough files"
> +fi
> +rm "$TEST_IMG"
> +
>   echo "=== Check option preallocation and cluster_size ==="
>   echo
>   cluster_sizes="16384 32768 65536 131072 262144 524288 1048576 2097152 4194304"
> 

Re: [PATCH 4/6] tests/test-util-filemonitor: Skip test on non-x86 Travis containers

[Philippe Mathieu-Daudé]

On 11/19/19 6:08 PM, Thomas Huth wrote:
> test-util-filemonitor fails in restricted non-x86 Travis containers
> since they apparently blacklisted some required system calls there.
> Let's simply skip the test if we detect such an environment.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/test-util-filemonitor.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
> index 301cd2db61..45009c69f4 100644
> --- a/tests/test-util-filemonitor.c
> +++ b/tests/test-util-filemonitor.c
> @@ -406,10 +406,21 @@ test_file_monitor_events(void)
>       char *pathdst = NULL;
>       QFileMonitorTestData data;
>       GHashTable *ids = g_hash_table_new(g_int64_hash, g_int64_equal);
> +    char *travis_arch;
>   
>       qemu_mutex_init(&data.lock);
>       data.records = NULL;
>   
> +    /*
> +     * This test does not work on Travis LXD containers since some
> +     * syscalls are blocked in that environment.
> +     */
> +    travis_arch = getenv("TRAVIS_ARCH");
> +    if (travis_arch && !g_str_equal(travis_arch, "x86_64")) {
> +        g_test_skip("Test does not work on non-x86 Travis containers.");

We might need to refactor this hunk to a more generic place some day.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +        return;
> +    }
> +
>       /*
>        * The file monitor needs the main loop running in
>        * order to receive events from inotify. We must
> 

Re: [PATCH 5/6] travis.yml: drop 32 bit systems from MAIN_SOFTMMU_TARGETS

[Philippe Mathieu-Daudé]

On 11/19/19 6:08 PM, Thomas Huth wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> The older clangs are still struggling to build and run everything
> withing the 50 minute timeout so lets lighten the load a bit more. We
> still have coverage for GCC and hopefully no obscure 32 bit guest only
> breakages slip through the cracks.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   .travis.yml | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index b9a026c8ee..c09b6a0014 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -79,7 +79,7 @@ env:
>       - BASE_CONFIG="--disable-docs --disable-tools"
>       - TEST_CMD="make check V=1"
>       # This is broadly a list of "mainline" softmmu targets which have support across the major distros
> -    - MAIN_SOFTMMU_TARGETS="aarch64-softmmu,arm-softmmu,i386-softmmu,mips-softmmu,mips64-softmmu,ppc64-softmmu,riscv64-softmmu,s390x-softmmu,x86_64-softmmu"
> +    - MAIN_SOFTMMU_TARGETS="aarch64-softmmu,mips64-softmmu,ppc64-softmmu,riscv64-softmmu,s390x-softmmu,x86_64-softmmu"
>       - CCACHE_SLOPPINESS="include_file_ctime,include_file_mtime"
>       - CCACHE_MAXSIZE=1G
>   
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 2/6] iotests: Skip test 079 if it is not possible to create large files

[Thomas Huth]

On 19/11/2019 18.29, Philippe Mathieu-Daudé wrote:
> On 11/19/19 6:08 PM, Thomas Huth wrote:
>> Test 079 fails in the arm64, s390x and ppc64le LXD containers, which
>> apparently do not allow large files to be created. Test 079 tries to
>> create a 4G sparse file, so check first whether we can really create
>> such files before executing the test.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/qemu-iotests/079 | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
>> index 81f0c21f53..e9b81419b7 100755
>> --- a/tests/qemu-iotests/079
>> +++ b/tests/qemu-iotests/079
>> @@ -39,6 +39,12 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>>   _supported_fmt qcow2
>>   _supported_proto file nfs
>>   +# Some containers (e.g. non-x86 on Travis) do not allow large files
>> +if ! truncate --size=4G "$TEST_IMG"; then
> 
> Shouldn't we restrict that to Travis-CI by checking some environment var?

I'd rather like to keep it independent from Travis environment
variables, since somebody might want to run "make check" in other
non-Travis containers or on weird filesystems, and then the test should
ideally not fail, but simply skip, too.

 Thomas

Re: [PATCH 2/6] iotests: Skip test 079 if it is not possible to create large files

[Philippe Mathieu-Daudé]

On 11/19/19 6:34 PM, Thomas Huth wrote:
> On 19/11/2019 18.29, Philippe Mathieu-Daudé wrote:
>> On 11/19/19 6:08 PM, Thomas Huth wrote:
>>> Test 079 fails in the arm64, s390x and ppc64le LXD containers, which
>>> apparently do not allow large files to be created. Test 079 tries to
>>> create a 4G sparse file, so check first whether we can really create
>>> such files before executing the test.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>    tests/qemu-iotests/079 | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
>>> index 81f0c21f53..e9b81419b7 100755
>>> --- a/tests/qemu-iotests/079
>>> +++ b/tests/qemu-iotests/079
>>> @@ -39,6 +39,12 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>>>    _supported_fmt qcow2
>>>    _supported_proto file nfs
>>>    +# Some containers (e.g. non-x86 on Travis) do not allow large files
>>> +if ! truncate --size=4G "$TEST_IMG"; then
>>
>> Shouldn't we restrict that to Travis-CI by checking some environment var?
> 
> I'd rather like to keep it independent from Travis environment
> variables, since somebody might want to run "make check" in other
> non-Travis containers or on weird filesystems, and then the test should
> ideally not fail, but simply skip, too.

I see. But it would be bad if we stop catching normal bugs with this 
test because we restricted it to contained environments.

Maybe we can add a generic is_contained() routine that only checks for 
TRAVIS env var, and people using other containers can expand it.

Re: [PATCH 2/6] iotests: Skip test 079 if it is not possible to create large files

[Daniel P. Berrangé]

On Tue, Nov 19, 2019 at 06:38:20PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/19/19 6:34 PM, Thomas Huth wrote:
> > On 19/11/2019 18.29, Philippe Mathieu-Daudé wrote:
> > > On 11/19/19 6:08 PM, Thomas Huth wrote:
> > > > Test 079 fails in the arm64, s390x and ppc64le LXD containers, which
> > > > apparently do not allow large files to be created. Test 079 tries to
> > > > create a 4G sparse file, so check first whether we can really create
> > > > such files before executing the test.
> > > > 
> > > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > > ---
> > > >    tests/qemu-iotests/079 | 6 ++++++
> > > >    1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
> > > > index 81f0c21f53..e9b81419b7 100755
> > > > --- a/tests/qemu-iotests/079
> > > > +++ b/tests/qemu-iotests/079
> > > > @@ -39,6 +39,12 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> > > >    _supported_fmt qcow2
> > > >    _supported_proto file nfs
> > > >    +# Some containers (e.g. non-x86 on Travis) do not allow large files
> > > > +if ! truncate --size=4G "$TEST_IMG"; then
> > > 
> > > Shouldn't we restrict that to Travis-CI by checking some environment var?
> > 
> > I'd rather like to keep it independent from Travis environment
> > variables, since somebody might want to run "make check" in other
> > non-Travis containers or on weird filesystems, and then the test should
> > ideally not fail, but simply skip, too.
> 
> I see. But it would be bad if we stop catching normal bugs with this test
> because we restricted it to contained environments.
> 
> Maybe we can add a generic is_contained() routine that only checks for
> TRAVIS env var, and people using other containers can expand it.

"is_contained" is still expressing the environment.

What we need is a way to express features, and be able to switch beteen
autodetecting features & mandatory enablement.

eg

   if has_feature "large_file"
   then
       ...stuff using large files...
   fi

The "has_feature" helper would by default call out to
"has_feature_large_file" todo automatic probing so that things
"just work" according to whatever env the tests are run inside.

There should, however, be a flag to "./check" which force enables
the feature eg  "./check --require-feature large_file" will force
execution and not attempt to probe for it.

We could have "--require-feature :all" to force enable all optional
bits.

Any formal CI systems should use --require-feature to explicitly
force testing of features that are expected to always work.

so Travis x86 would use "--require-feature large_large", but
the arch64 version would not pass this flag and so do probing
which will auto-skip.

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 2/6] iotests: Skip test 079 if it is not possible to create large files

[Thomas Huth]

On 19/11/2019 18.50, Daniel P. Berrangé wrote:
> On Tue, Nov 19, 2019 at 06:38:20PM +0100, Philippe Mathieu-Daudé wrote:
>> On 11/19/19 6:34 PM, Thomas Huth wrote:
>>> On 19/11/2019 18.29, Philippe Mathieu-Daudé wrote:
>>>> On 11/19/19 6:08 PM, Thomas Huth wrote:
>>>>> Test 079 fails in the arm64, s390x and ppc64le LXD containers, which
>>>>> apparently do not allow large files to be created. Test 079 tries to
>>>>> create a 4G sparse file, so check first whether we can really create
>>>>> such files before executing the test.
>>>>>
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>    tests/qemu-iotests/079 | 6 ++++++
>>>>>    1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
>>>>> index 81f0c21f53..e9b81419b7 100755
>>>>> --- a/tests/qemu-iotests/079
>>>>> +++ b/tests/qemu-iotests/079
>>>>> @@ -39,6 +39,12 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>>>>>    _supported_fmt qcow2
>>>>>    _supported_proto file nfs
>>>>>    +# Some containers (e.g. non-x86 on Travis) do not allow large files
>>>>> +if ! truncate --size=4G "$TEST_IMG"; then
>>>>
>>>> Shouldn't we restrict that to Travis-CI by checking some environment var?
>>>
>>> I'd rather like to keep it independent from Travis environment
>>> variables, since somebody might want to run "make check" in other
>>> non-Travis containers or on weird filesystems, and then the test should
>>> ideally not fail, but simply skip, too.
>>
>> I see. But it would be bad if we stop catching normal bugs with this test
>> because we restricted it to contained environments.
>>
>> Maybe we can add a generic is_contained() routine that only checks for
>> TRAVIS env var, and people using other containers can expand it.
> 
> "is_contained" is still expressing the environment.
> 
> What we need is a way to express features, and be able to switch beteen
> autodetecting features & mandatory enablement.
> 
> eg
> 
>    if has_feature "large_file"
>    then
>        ...stuff using large files...
>    fi
> 
> The "has_feature" helper would by default call out to
> "has_feature_large_file" todo automatic probing so that things
> "just work" according to whatever env the tests are run inside.
> 
> There should, however, be a flag to "./check" which force enables
> the feature eg  "./check --require-feature large_file" will force
> execution and not attempt to probe for it.
> 
> We could have "--require-feature :all" to force enable all optional
> bits.
> 
> Any formal CI systems should use --require-feature to explicitly
> force testing of features that are expected to always work.
> 
> so Travis x86 would use "--require-feature large_large", but
> the arch64 version would not pass this flag and so do probing
> which will auto-skip.

Maybe a nice idea, but I think this is out of scope for this patch
series. We're using the "if ! truncate --size=4G" check in some other
tests already, so I'm not adding some new mechanism here.
If you want to see some more fine-grained control for the iotests,
please send some patches to rework these other tests first.

 Thomas

Re: [PATCH 1/6] iotests: Skip test 060 if it is not possible to create large files

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> Test 060 fails in the arm64, s390x and ppc64le LXD containers, which
> apparently do not allow large files to be created. The repair process
> in test 060 creates a file of 64 GiB, so test first whether such large
> files are possible and skip the test if that's not the case.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

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

> ---
>  tests/qemu-iotests/060 | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
> index b91d8321bb..e7ee865c55 100755
> --- a/tests/qemu-iotests/060
> +++ b/tests/qemu-iotests/060
> @@ -49,6 +49,12 @@ _supported_fmt qcow2
>  _supported_proto file
>  _supported_os Linux
>  
> +# The repair process will create a large file - so check for availability first
> +if ! truncate --size=64G "$TEST_IMG"; then
> +    _notrun "file system on $TEST_DIR does not support large enough files"
> +fi
> +rm "$TEST_IMG"
> +
>  rt_offset=65536  # 0x10000 (XXX: just an assumption)
>  rb_offset=131072 # 0x20000 (XXX: just an assumption)
>  l1_offset=196608 # 0x30000 (XXX: just an assumption)


-- 
Alex Bennée

Re: [PATCH 2/6] iotests: Skip test 079 if it is not possible to create large files

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> Test 079 fails in the arm64, s390x and ppc64le LXD containers, which
> apparently do not allow large files to be created. Test 079 tries to
> create a 4G sparse file, so check first whether we can really create
> such files before executing the test.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/qemu-iotests/079 | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
> index 81f0c21f53..e9b81419b7 100755
> --- a/tests/qemu-iotests/079
> +++ b/tests/qemu-iotests/079
> @@ -39,6 +39,12 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>  _supported_fmt qcow2
>  _supported_proto file nfs
>  
> +# Some containers (e.g. non-x86 on Travis) do not allow large files
> +if ! truncate --size=4G "$TEST_IMG"; then
> +    _notrun "file system on $TEST_DIR does not support large enough files"
> +fi
> +rm "$TEST_IMG"
> +

Hmm we are repeating ourselves here. Maybe the test should be wrapped up
as __supported_filesize and the test can just do:

  __supported_filesize 4G

along with the other tests above.

-- 
Alex Bennée

Re: [PATCH 3/6] tests/hd-geo-test: Skip test when images can not be created

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> In certain environments like restricted containers, we can not create
> huge test images. To be able to use "make check" in such container
> environments, too, let's skip the hd-geo-test instead of failing when
> the test images could not be created.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/hd-geo-test.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
> index 7e86c5416c..a249800544 100644
> --- a/tests/hd-geo-test.c
> +++ b/tests/hd-geo-test.c
> @@ -34,8 +34,13 @@ static char *create_test_img(int secs)
>      fd = mkstemp(template);
>      g_assert(fd >= 0);
>      ret = ftruncate(fd, (off_t)secs * 512);
> -    g_assert(ret == 0);
>      close(fd);
> +
> +    if (ret) {
> +        free(template);
> +        template = NULL;
> +    }
> +
>      return template;
>  }
>  
> @@ -934,6 +939,10 @@ int main(int argc, char **argv)
>      for (i = 0; i < backend_last; i++) {
>          if (img_secs[i] >= 0) {
>              img_file_name[i] = create_test_img(img_secs[i]);
> +            if (!img_file_name[i]) {
> +                g_test_message("Could not create test images.");
> +                goto test_add_done;
> +            }
>          } else {
>              img_file_name[i] = NULL;
>          }
> @@ -965,6 +974,7 @@ int main(int argc, char **argv)
>                         "skipping hd-geo/override/* tests");
>      }
>  
> +test_add_done:
>      ret = g_test_run();

It does seem a bit odd to call g_test_run if we have explicitly not set
any up. Personally I'd hoist all the test creation into a new function
so you could do:

  if (setup_images()) {
     setup_tests();
     ret = run_tests();
  } else {
     ret = 0; /* pass if we have no images */
  }

  cleanup_images();

but that's just me going above and beyond to avoid goto's ;-)

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

>  
>      for (i = 0; i < backend_last; i++) {


-- 
Alex Bennée

Re: [PATCH 4/6] tests/test-util-filemonitor: Skip test on non-x86 Travis containers

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> test-util-filemonitor fails in restricted non-x86 Travis containers
> since they apparently blacklisted some required system calls there.
> Let's simply skip the test if we detect such an environment.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

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

> ---
>  tests/test-util-filemonitor.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
> index 301cd2db61..45009c69f4 100644
> --- a/tests/test-util-filemonitor.c
> +++ b/tests/test-util-filemonitor.c
> @@ -406,10 +406,21 @@ test_file_monitor_events(void)
>      char *pathdst = NULL;
>      QFileMonitorTestData data;
>      GHashTable *ids = g_hash_table_new(g_int64_hash, g_int64_equal);
> +    char *travis_arch;
>  
>      qemu_mutex_init(&data.lock);
>      data.records = NULL;
>  
> +    /*
> +     * This test does not work on Travis LXD containers since some
> +     * syscalls are blocked in that environment.
> +     */
> +    travis_arch = getenv("TRAVIS_ARCH");
> +    if (travis_arch && !g_str_equal(travis_arch, "x86_64")) {
> +        g_test_skip("Test does not work on non-x86 Travis containers.");
> +        return;
> +    }
> +
>      /*
>       * The file monitor needs the main loop running in
>       * order to receive events from inotify. We must


-- 
Alex Bennée

Re: [PATCH 6/6] travis.yml: Enable builds on arm64, ppc64le and s390x

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> Travis recently added the possibility to test on these architectures,
> too, so let's enable them in our travis.yml file to extend our test
> coverage.

This is good as far as it goes but it would be nice to exercise the
respective TCG backends. If added two commits to:

  https://github.com/stsquad/qemu/commits/review/multiarch-testing

which allow for that. I'll know if they worked properly in a hour or two
once the testing has finished.

>
> Unfortunately, the libssh in this Ubuntu version (bionic) is in a pretty
> unusable Frankenstein state and libspice-server-dev is not available here,
> so we can not use the global list of packages to install, but have to
> provide individual package lists instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  .travis.yml | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
>
> diff --git a/.travis.yml b/.travis.yml
> index c09b6a0014..cf48ee452c 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -360,6 +360,89 @@ matrix:
>          - TEST_CMD="make -j3 check-tcg V=1"
>          - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
>  
> +    - arch: arm64
> +      addons:
> +        apt_packages:
> +          - libaio-dev
> +          - libattr1-dev
> +          - libbrlapi-dev
> +          - libcap-dev
> +          - libcap-ng-dev
> +          - libgcrypt20-dev
> +          - libgnutls28-dev
> +          - libgtk-3-dev
> +          - libiscsi-dev
> +          - liblttng-ust-dev
> +          - libncurses5-dev
> +          - libnfs-dev
> +          - libnss3-dev
> +          - libpixman-1-dev
> +          - libpng-dev
> +          - librados-dev
> +          - libsdl2-dev
> +          - libseccomp-dev
> +          - liburcu-dev
> +          - libusb-1.0-0-dev
> +          - libvdeplug-dev
> +          - libvte-2.91-dev
> +      env:
> +        - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS},x86_64-linux-user"
> +
> +    - arch: ppc64le
> +      addons:
> +        apt_packages:
> +          - libaio-dev
> +          - libattr1-dev
> +          - libbrlapi-dev
> +          - libcap-dev
> +          - libcap-ng-dev
> +          - libgcrypt20-dev
> +          - libgnutls28-dev
> +          - libgtk-3-dev
> +          - libiscsi-dev
> +          - liblttng-ust-dev
> +          - libncurses5-dev
> +          - libnfs-dev
> +          - libnss3-dev
> +          - libpixman-1-dev
> +          - libpng-dev
> +          - librados-dev
> +          - libsdl2-dev
> +          - libseccomp-dev
> +          - liburcu-dev
> +          - libusb-1.0-0-dev
> +          - libvdeplug-dev
> +          - libvte-2.91-dev
> +      env:
> +        - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS},x86_64-linux-user"
> +
> +    - arch: s390x
> +      addons:
> +        apt_packages:
> +          - libaio-dev
> +          - libattr1-dev
> +          - libbrlapi-dev
> +          - libcap-dev
> +          - libcap-ng-dev
> +          - libgcrypt20-dev
> +          - libgnutls28-dev
> +          - libgtk-3-dev
> +          - libiscsi-dev
> +          - liblttng-ust-dev
> +          - libncurses5-dev
> +          - libnfs-dev
> +          - libnss3-dev
> +          - libpixman-1-dev
> +          - libpng-dev
> +          - librados-dev
> +          - libsdl2-dev
> +          - libseccomp-dev
> +          - liburcu-dev
> +          - libusb-1.0-0-dev
> +          - libvdeplug-dev
> +          - libvte-2.91-dev
> +      env:
> +        - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS},x86_64-linux-user"
>  
>      # Release builds
>      # The make-release script expect a QEMU version, so our tag must start with a 'v'.


-- 
Alex Bennée

Re: [PATCH 0/6] Enable Travis builds on arm64, ppc64le and s390x

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> Travis recently added build hosts for arm64, ppc64le and s390x, so
> this is a welcome addition to our Travis testing matrix.
>
> Unfortunately, the builds are running in quite restricted LXD containers
> there, for example it is not possible to create huge files there (even
> if they are just sparse), and certain system calls are blocked. So we
> have to change some tests first to stop them failing in such environments.
<snip>
>   iotests: Skip test 060 if it is not possible to create large files
>   iotests: Skip test 079 if it is not possible to create large files

It seems like 161 is also failing:

  https://travis-ci.org/stsquad/qemu/jobs/615672478


>   tests/hd-geo-test: Skip test when images can not be created
>   tests/test-util-filemonitor: Skip test on non-x86 Travis containers
>   travis.yml: Enable builds on arm64, ppc64le and s390x
>
>  .travis.yml                   | 85 ++++++++++++++++++++++++++++++++++-
>  tests/hd-geo-test.c           | 12 ++++-
>  tests/qemu-iotests/060        |  6 +++
>  tests/qemu-iotests/079        |  6 +++
>  tests/test-util-filemonitor.c | 11 +++++
>  5 files changed, 118 insertions(+), 2 deletions(-)


-- 
Alex Bennée

Re: [PATCH 0/6] Enable Travis builds on arm64, ppc64le and s390x

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> Thomas Huth <thuth@redhat.com> writes:
>
>> Travis recently added build hosts for arm64, ppc64le and s390x, so
>> this is a welcome addition to our Travis testing matrix.
>>
>> Unfortunately, the builds are running in quite restricted LXD containers
>> there, for example it is not possible to create huge files there (even
>> if they are just sparse), and certain system calls are blocked. So we
>> have to change some tests first to stop them failing in such environments.
> <snip>
>>   iotests: Skip test 060 if it is not possible to create large files
>>   iotests: Skip test 079 if it is not possible to create large files
>
> It seems like 161 is also failing:
>
>   https://travis-ci.org/stsquad/qemu/jobs/615672478

And sometimes 249

>
>
>>   tests/hd-geo-test: Skip test when images can not be created
>>   tests/test-util-filemonitor: Skip test on non-x86 Travis containers
>>   travis.yml: Enable builds on arm64, ppc64le and s390x
>>
>>  .travis.yml                   | 85 ++++++++++++++++++++++++++++++++++-
>>  tests/hd-geo-test.c           | 12 ++++-
>>  tests/qemu-iotests/060        |  6 +++
>>  tests/qemu-iotests/079        |  6 +++
>>  tests/test-util-filemonitor.c | 11 +++++
>>  5 files changed, 118 insertions(+), 2 deletions(-)


-- 
Alex Bennée

Re: [PATCH 0/6] Enable Travis builds on arm64, ppc64le and s390x

[Thomas Huth]

On 25/11/2019 11.28, Alex Bennée wrote:
> 
> Alex Bennée <alex.bennee@linaro.org> writes:
> 
>> Thomas Huth <thuth@redhat.com> writes:
>>
>>> Travis recently added build hosts for arm64, ppc64le and s390x, so
>>> this is a welcome addition to our Travis testing matrix.
>>>
>>> Unfortunately, the builds are running in quite restricted LXD containers
>>> there, for example it is not possible to create huge files there (even
>>> if they are just sparse), and certain system calls are blocked. So we
>>> have to change some tests first to stop them failing in such environments.
>> <snip>
>>>    iotests: Skip test 060 if it is not possible to create large files
>>>    iotests: Skip test 079 if it is not possible to create large files
>>
>> It seems like 161 is also failing:
>>
>>    https://travis-ci.org/stsquad/qemu/jobs/615672478
> 
> And sometimes 249

These must be intermittent problems ... I've seen 161 failing once at 
the very beginning of my tests, but then never again, so I assumed that 
it was a quirk with the test system that got fixed later. Seems like 
that was a wrong assumption. I've never seen 249 failing so far... I'll 
try to do some more tests when I've got some spare time...

  Thomas

Re: [PATCH 0/6] Enable Travis builds on arm64, ppc64le and s390x

[Thomas Huth]

On 27/11/2019 09.50, Thomas Huth wrote:
> On 25/11/2019 11.28, Alex Bennée wrote:
>>
>> Alex Bennée <alex.bennee@linaro.org> writes:
>>
>>> Thomas Huth <thuth@redhat.com> writes:
>>>
>>>> Travis recently added build hosts for arm64, ppc64le and s390x, so
>>>> this is a welcome addition to our Travis testing matrix.
>>>>
>>>> Unfortunately, the builds are running in quite restricted LXD
>>>> containers
>>>> there, for example it is not possible to create huge files there (even
>>>> if they are just sparse), and certain system calls are blocked. So we
>>>> have to change some tests first to stop them failing in such
>>>> environments.
>>> <snip>
>>>>    iotests: Skip test 060 if it is not possible to create large files
>>>>    iotests: Skip test 079 if it is not possible to create large files
>>>
>>> It seems like 161 is also failing:
>>>
>>>    https://travis-ci.org/stsquad/qemu/jobs/615672478
>>
>> And sometimes 249
> 
> These must be intermittent problems ... I've seen 161 failing once at
> the very beginning of my tests, but then never again, so I assumed that
> it was a quirk with the test system that got fixed later. Seems like
> that was a wrong assumption. I've never seen 249 failing so far... I'll
> try to do some more tests when I've got some spare time...

It's not intermittent, it's a problem with "dist: bionic". It seems to
work fine with "dist: xenial", as far as I can tell. So I think we
should simply stick with "dist: xenial" for the ppc64le builder unless
someone has a ppc64le bionic system at hand for debugging.

 Thomas