[PATCH for 5.3-rc3 v1 0/6] testing fixes (avocado, gitlab)

[PATCH for 5.3-rc3 v1 0/6] testing fixes (avocado, gitlab)

[Alex Bennée]

Hi,

Here are a few more minor fixes for 5.2-rc3:

  - a tweak to the gitlab status script (from last series)
  - moving check-tcg to gitlab (also in last series)
  - fix some tempdir names and left overs
  - moving some more tests to gitlab

The following need review:

  - gitlab: move remaining x86 check-tcg targets to gitlab
  - tests/avocado: clean-up socket directory after run
  - tests: add prefixes to the bare mkdtemp calls
  - scripts/ci: clean up default args logic a little

Alex Bennée (4):
  scripts/ci: clean up default args logic a little
  tests: add prefixes to the bare mkdtemp calls
  tests/avocado: clean-up socket directory after run
  gitlab: move remaining x86 check-tcg targets to gitlab

Philippe Mathieu-Daudé (2):
  tests/docker: Install liblttng-ust-dev package in Ubuntu 20.04 image
  gitlab-ci: Move trace backend tests across to gitlab

 .gitlab-ci.yml                             | 35 +++++++++++++++++
 .travis.yml                                | 45 ----------------------
 python/qemu/machine.py                     |  3 +-
 scripts/ci/gitlab-pipeline-status          | 24 ++++++------
 tests/acceptance/avocado_qemu/__init__.py  |  4 +-
 tests/docker/dockerfiles/ubuntu2004.docker |  1 +
 6 files changed, 54 insertions(+), 58 deletions(-)

-- 
2.20.1

[PATCH v1 1/6] scripts/ci: clean up default args logic a little

[Alex Bennée]

This allows us to do:

  ./scripts/ci/gitlab-pipeline-status -w -b HEAD -p 2961854

to check out own pipeline status of a recently pushed branch.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 scripts/ci/gitlab-pipeline-status | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/scripts/ci/gitlab-pipeline-status b/scripts/ci/gitlab-pipeline-status
index bac8233079..78e72f6008 100755
--- a/scripts/ci/gitlab-pipeline-status
+++ b/scripts/ci/gitlab-pipeline-status
@@ -31,7 +31,7 @@ class NoPipelineFound(Exception):
     """Communication is successfull but pipeline is not found."""
 
 
-def get_local_branch_commit(branch='staging'):
+def get_local_branch_commit(branch):
     """
     Returns the commit sha1 for the *local* branch named "staging"
     """
@@ -126,18 +126,16 @@ def create_parser():
                         help=('The GitLab project ID. Defaults to the project '
                               'for https://gitlab.com/qemu-project/qemu, that '
                               'is, "%(default)s"'))
-    try:
-        default_commit = get_local_branch_commit()
-        commit_required = False
-    except ValueError:
-        default_commit = ''
-        commit_required = True
-    parser.add_argument('-c', '--commit', required=commit_required,
-                        default=default_commit,
+    parser.add_argument('-b', '--branch', type=str, default="staging",
+                        help=('Specify the branch to check. '
+                              'Use HEAD for your current branch. '
+                              'Otherwise looks at "%(default)s"'))
+    parser.add_argument('-c', '--commit',
+                        default=None,
                         help=('Look for a pipeline associated with the given '
                               'commit.  If one is not explicitly given, the '
-                              'commit associated with the local branch named '
-                              '"staging" is used.  Default: %(default)s'))
+                              'commit associated with the default branch '
+                              'is used.'))
     parser.add_argument('--verbose', action='store_true', default=False,
                         help=('A minimal verbosity level that prints the '
                               'overall result of the check/wait'))
@@ -149,6 +147,10 @@ def main():
     """
     parser = create_parser()
     args = parser.parse_args()
+
+    if not args.commit:
+        args.commit = get_local_branch_commit(args.branch)
+
     success = False
     try:
         if args.wait:
-- 
2.20.1

[PATCH v1 2/6] tests: add prefixes to the bare mkdtemp calls

[Alex Bennée]

The first step to debug a thing is to know what created the thing in
the first place. Add some prefixes so random tmpdir's have something
grep in the code.

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

---
v2
  - fix long lines
---
 python/qemu/machine.py                    | 3 ++-
 tests/acceptance/avocado_qemu/__init__.py | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6420f01bed..64d966aeeb 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -303,7 +303,8 @@ class QEMUMachine:
         return args
 
     def _pre_launch(self) -> None:
-        self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
+        self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
+                                          dir=self._test_dir)
         self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
         self._qemu_log_file = open(self._qemu_log_path, 'wb')
 
diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 4cda037187..3033b2cabe 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -171,7 +171,8 @@ class Test(avocado.Test):
             self.cancel("No QEMU binary defined or found in the build tree")
 
     def _new_vm(self, *args):
-        vm = QEMUMachine(self.qemu_bin, sock_dir=tempfile.mkdtemp())
+        sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_")
+        vm = QEMUMachine(self.qemu_bin, sock_dir=sd)
         if args:
             vm.add_args(*args)
         return vm
-- 
2.20.1

[PATCH v1 3/6] tests/avocado: clean-up socket directory after run

[Alex Bennée]

Previously we were leaving temporary directories behind. While the
QEMUMachine does make efforts to clean up after itself the directory
belongs to the calling function. We use TemporaryDirectory to wrap
this although we explicitly clear the reference in tearDown() as it
doesn't get cleaned up otherwise.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/acceptance/avocado_qemu/__init__.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 3033b2cabe..bf54e419da 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -171,8 +171,8 @@ class Test(avocado.Test):
             self.cancel("No QEMU binary defined or found in the build tree")
 
     def _new_vm(self, *args):
-        sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_")
-        vm = QEMUMachine(self.qemu_bin, sock_dir=sd)
+        self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")
+        vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name)
         if args:
             vm.add_args(*args)
         return vm
@@ -193,6 +193,7 @@ class Test(avocado.Test):
     def tearDown(self):
         for vm in self._vms.values():
             vm.shutdown()
+        self._sd = None
 
     def fetch_asset(self, name,
                     asset_hash=None, algorithm=None,
-- 
2.20.1

[PATCH v1 4/6] gitlab: move remaining x86 check-tcg targets to gitlab

[Alex Bennée]

The GCC check-tcg (user) test in particular was very prone to timing
out on Travis. We only actually need to move the some-softmmu builds
across as we already have coverage for linux-user.

As --enable-debug-tcg does increase the run time somewhat as more
debug is put in let's restrict that to just the plugins build. It's
unlikely that a plugins enabled build is going to hide a sanity
failure in core TCG code so let the plugin builds do the heavy lifting
on checking TCG sanity so the non-plugin builds can run swiftly.

Now the only remaining check-tcg builds on Travis are for the various
non-x86 arches.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20201110192316.26397-10-alex.bennee@linaro.org>
---
 .gitlab-ci.yml | 17 +++++++++++++++++
 .travis.yml    | 26 --------------------------
 2 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 9a8b375188..b406027a55 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -247,6 +247,15 @@ build-user:
     CONFIGURE_ARGS: --disable-tools --disable-system
     MAKE_CHECK_ARGS: check-tcg
 
+# Only build the softmmu targets we have check-tcg tests for
+build-some-softmmu:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: debian-all-test-cross
+    CONFIGURE_ARGS: --disable-tools --enable-debug-tcg
+    TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
+    MAKE_CHECK_ARGS: check-tcg
+
 # Run check-tcg against linux-user (with plugins)
 # we skip sparc64-linux-user until it has been fixed somewhat
 # we skip cris-linux-user as it doesn't use the common run loop
@@ -258,6 +267,14 @@ build-user-plugins:
     MAKE_CHECK_ARGS: check-tcg
   timeout: 1h 30m
 
+build-some-softmmu-plugins:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: debian-all-test-cross
+    CONFIGURE_ARGS: --disable-tools --disable-user --enable-plugins --enable-debug-tcg
+    TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
+    MAKE_CHECK_ARGS: check-tcg
+
 build-clang:
   <<: *native_build_job_definition
   variables:
diff --git a/.travis.yml b/.travis.yml
index a3d78171ca..bac085f800 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -301,32 +301,6 @@ jobs:
         - ${SRC_DIR}/configure ${CONFIG} --extra-cflags="-g3 -O0 -fsanitize=thread" || { cat config.log meson-logs/meson-log.txt && exit 1; }
 
 
-    # Run check-tcg against linux-user
-    - name: "GCC check-tcg (user)"
-      env:
-        - CONFIG="--disable-system --enable-debug-tcg"
-        - TEST_BUILD_CMD="make build-tcg"
-        - TEST_CMD="make check-tcg"
-        - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
-
-
-    # Run check-tcg against softmmu targets
-    - name: "GCC check-tcg (some-softmmu)"
-      env:
-        - CONFIG="--enable-debug-tcg --target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
-        - TEST_BUILD_CMD="make build-tcg"
-        - TEST_CMD="make check-tcg"
-        - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
-
-
-    # Run check-tcg against softmmu targets (with plugins)
-    - name: "GCC plugins check-tcg (some-softmmu)"
-      env:
-        - CONFIG="--enable-plugins --enable-debug-tcg --target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
-        - TEST_BUILD_CMD="make build-tcg"
-        - TEST_CMD="make check-tcg"
-        - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
-
     - name: "[aarch64] GCC check-tcg"
       arch: arm64
       dist: focal
-- 
2.20.1

[PATCH v1 5/6] tests/docker: Install liblttng-ust-dev package in Ubuntu 20.04 image

[Alex Bennée]

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

Install the liblttng-ust-dev package to be able to
build QEMU using the User-Space Tracer trace backend
(configure --enable-trace-backends=ust).

Suggested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201111121234.3246812-2-philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/dockerfiles/ubuntu2004.docker | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/dockerfiles/ubuntu2004.docker b/tests/docker/dockerfiles/ubuntu2004.docker
index 355bbb3c63..ae889d8482 100644
--- a/tests/docker/dockerfiles/ubuntu2004.docker
+++ b/tests/docker/dockerfiles/ubuntu2004.docker
@@ -23,6 +23,7 @@ ENV PACKAGES flex bison \
     libiscsi-dev \
     libjemalloc-dev \
     libjpeg-turbo8-dev \
+    liblttng-ust-dev \
     liblzo2-dev \
     libncurses5-dev \
     libncursesw5-dev \
-- 
2.20.1

[PATCH v1 6/6] gitlab-ci: Move trace backend tests across to gitlab

[Alex Bennée]

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

Similarly to commit 8cdb2cef3f1, move the trace backend
tests to GitLab.

Note the User-Space Tracer backend is still tested on
Ubuntu by the s390x jobs on Travis-CI.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20201111121234.3246812-3-philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .gitlab-ci.yml | 18 ++++++++++++++++++
 .travis.yml    | 19 -------------------
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index b406027a55..d0173e82b1 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -415,6 +415,24 @@ check-crypto-only-gnutls:
     IMAGE: centos7
     MAKE_CHECK_ARGS: check
 
+# We don't need to exercise every backend with every front-end
+build-trace-multi-user:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: ubuntu2004
+    CONFIGURE_ARGS: --enable-trace-backends=log,simple,syslog --disable-system
+
+build-trace-ftrace-system:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: ubuntu2004
+    CONFIGURE_ARGS: --enable-trace-backends=ftrace --target-list=x86_64-softmmu
+
+build-trace-ust-system:
+  <<: *native_build_job_definition
+  variables:
+    IMAGE: ubuntu2004
+    CONFIGURE_ARGS: --enable-trace-backends=ust --target-list=x86_64-softmmu
 
 check-patch:
   stage: build
diff --git a/.travis.yml b/.travis.yml
index bac085f800..1f80bdb624 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -232,25 +232,6 @@ jobs:
         - TEST_CMD=""
 
 
-    # We don't need to exercise every backend with every front-end
-    - name: "GCC trace log,simple,syslog (user)"
-      env:
-        - CONFIG="--enable-trace-backends=log,simple,syslog --disable-system"
-        - TEST_CMD=""
-
-
-    - name: "GCC trace ftrace (x86_64-softmmu)"
-      env:
-        - CONFIG="--enable-trace-backends=ftrace --target-list=x86_64-softmmu"
-        - TEST_CMD=""
-
-
-    - name: "GCC trace ust (x86_64-softmmu)"
-      env:
-        - CONFIG="--enable-trace-backends=ust --target-list=x86_64-softmmu"
-        - TEST_CMD=""
-
-
     # Using newer GCC with sanitizers
     - name: "GCC9 with sanitizers (softmmu)"
       dist: bionic
-- 
2.20.1

Re: [PATCH v1 2/6] tests: add prefixes to the bare mkdtemp calls

[Philippe Mathieu-Daudé]

On 11/17/20 6:36 PM, Alex Bennée wrote:
> The first step to debug a thing is to know what created the thing in
> the first place. Add some prefixes so random tmpdir's have something
> grep in the code.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>   - fix long lines
> ---
>  python/qemu/machine.py                    | 3 ++-
>  tests/acceptance/avocado_qemu/__init__.py | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)

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

Re: [PATCH v1 2/6] tests: add prefixes to the bare mkdtemp calls

[Thomas Huth]

On 17/11/2020 18.36, Alex Bennée wrote:
> The first step to debug a thing is to know what created the thing in
> the first place. Add some prefixes so random tmpdir's have something
> grep in the code.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>   - fix long lines
> ---
>  python/qemu/machine.py                    | 3 ++-
>  tests/acceptance/avocado_qemu/__init__.py | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index 6420f01bed..64d966aeeb 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -303,7 +303,8 @@ class QEMUMachine:
>          return args
>  
>      def _pre_launch(self) -> None:
> -        self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
> +        self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
> +                                          dir=self._test_dir)
>          self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
>          self._qemu_log_file = open(self._qemu_log_path, 'wb')
>  
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 4cda037187..3033b2cabe 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -171,7 +171,8 @@ class Test(avocado.Test):
>              self.cancel("No QEMU binary defined or found in the build tree")
>  
>      def _new_vm(self, *args):
> -        vm = QEMUMachine(self.qemu_bin, sock_dir=tempfile.mkdtemp())
> +        sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_")
> +        vm = QEMUMachine(self.qemu_bin, sock_dir=sd)
>          if args:
>              vm.add_args(*args)
>          return vm
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 4/6] gitlab: move remaining x86 check-tcg targets to gitlab

[Thomas Huth]

On 17/11/2020 18.36, Alex Bennée wrote:
> The GCC check-tcg (user) test in particular was very prone to timing
> out on Travis. We only actually need to move the some-softmmu builds
> across as we already have coverage for linux-user.
> 
> As --enable-debug-tcg does increase the run time somewhat as more
> debug is put in let's restrict that to just the plugins build. It's

"so" or "," -----^ ?

> unlikely that a plugins enabled build is going to hide a sanity
> failure in core TCG code so let the plugin builds do the heavy lifting
> on checking TCG sanity so the non-plugin builds can run swiftly.
> 
> Now the only remaining check-tcg builds on Travis are for the various
> non-x86 arches.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20201110192316.26397-10-alex.bennee@linaro.org>
> ---
>  .gitlab-ci.yml | 17 +++++++++++++++++
>  .travis.yml    | 26 --------------------------
>  2 files changed, 17 insertions(+), 26 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 6/6] gitlab-ci: Move trace backend tests across to gitlab

[Thomas Huth]

On 17/11/2020 18.36, Alex Bennée wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Similarly to commit 8cdb2cef3f1, move the trace backend
> tests to GitLab.
> 
> Note the User-Space Tracer backend is still tested on
> Ubuntu by the s390x jobs on Travis-CI.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Message-Id: <20201111121234.3246812-3-philmd@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  .gitlab-ci.yml | 18 ++++++++++++++++++
>  .travis.yml    | 19 -------------------
>  2 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index b406027a55..d0173e82b1 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -415,6 +415,24 @@ check-crypto-only-gnutls:
>      IMAGE: centos7
>      MAKE_CHECK_ARGS: check
>  
> +# We don't need to exercise every backend with every front-end
> +build-trace-multi-user:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: ubuntu2004
> +    CONFIGURE_ARGS: --enable-trace-backends=log,simple,syslog --disable-system
> +
> +build-trace-ftrace-system:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: ubuntu2004
> +    CONFIGURE_ARGS: --enable-trace-backends=ftrace --target-list=x86_64-softmmu
> +
> +build-trace-ust-system:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: ubuntu2004
> +    CONFIGURE_ARGS: --enable-trace-backends=ust --target-list=x86_64-softmmu

Hmmm, do we really need separate build jobs for this, or could we maybe
rather simply add the options to some existing jobs instead (to save some CI
cycles)?

 Thomas

Re: [PATCH for 5.2-rc3 v1 0/6] testing fixes (avocado, gitlab)

[Alex Bennée]

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

> Hi,
>

Gentle ping:<snip>

> The following need review:
>
<snip>
>   - tests/avocado: clean-up socket directory after run
<snip>
>   - scripts/ci: clean up default args logic a little
<snip>

-- 
Alex Bennée

Re: [PATCH v1 6/6] gitlab-ci: Move trace backend tests across to gitlab

[Thomas Huth]

On 18/11/2020 10.54, Thomas Huth wrote:
> On 17/11/2020 18.36, Alex Bennée wrote:
>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> Similarly to commit 8cdb2cef3f1, move the trace backend
>> tests to GitLab.
>>
>> Note the User-Space Tracer backend is still tested on
>> Ubuntu by the s390x jobs on Travis-CI.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Message-Id: <20201111121234.3246812-3-philmd@redhat.com>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  .gitlab-ci.yml | 18 ++++++++++++++++++
>>  .travis.yml    | 19 -------------------
>>  2 files changed, 18 insertions(+), 19 deletions(-)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index b406027a55..d0173e82b1 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -415,6 +415,24 @@ check-crypto-only-gnutls:
>>      IMAGE: centos7
>>      MAKE_CHECK_ARGS: check
>>  
>> +# We don't need to exercise every backend with every front-end
>> +build-trace-multi-user:
>> +  <<: *native_build_job_definition
>> +  variables:
>> +    IMAGE: ubuntu2004
>> +    CONFIGURE_ARGS: --enable-trace-backends=log,simple,syslog --disable-system
>> +
>> +build-trace-ftrace-system:
>> +  <<: *native_build_job_definition
>> +  variables:
>> +    IMAGE: ubuntu2004
>> +    CONFIGURE_ARGS: --enable-trace-backends=ftrace --target-list=x86_64-softmmu
>> +
>> +build-trace-ust-system:
>> +  <<: *native_build_job_definition
>> +  variables:
>> +    IMAGE: ubuntu2004
>> +    CONFIGURE_ARGS: --enable-trace-backends=ust --target-list=x86_64-softmmu
> 
> Hmmm, do we really need separate build jobs for this, or could we maybe
> rather simply add the options to some existing jobs instead (to save some CI
> cycles)?

I guess we can still consolidate later ... but since Travis is now limiting
the CI minutes, we definitely have to move this over, so:

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v1 4/6] gitlab: move remaining x86 check-tcg targets to gitlab

[Wainer dos Santos Moschetta]

On 11/17/20 2:36 PM, Alex Bennée wrote:
> The GCC check-tcg (user) test in particular was very prone to timing
> out on Travis. We only actually need to move the some-softmmu builds
> across as we already have coverage for linux-user.
>
> As --enable-debug-tcg does increase the run time somewhat as more
> debug is put in let's restrict that to just the plugins build. It's
> unlikely that a plugins enabled build is going to hide a sanity
> failure in core TCG code so let the plugin builds do the heavy lifting
> on checking TCG sanity so the non-plugin builds can run swiftly.
>
> Now the only remaining check-tcg builds on Travis are for the various
> non-x86 arches.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20201110192316.26397-10-alex.bennee@linaro.org>
> ---
>   .gitlab-ci.yml | 17 +++++++++++++++++
>   .travis.yml    | 26 --------------------------
>   2 files changed, 17 insertions(+), 26 deletions(-)


Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 9a8b375188..b406027a55 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -247,6 +247,15 @@ build-user:
>       CONFIGURE_ARGS: --disable-tools --disable-system
>       MAKE_CHECK_ARGS: check-tcg
>   
> +# Only build the softmmu targets we have check-tcg tests for
> +build-some-softmmu:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: debian-all-test-cross
> +    CONFIGURE_ARGS: --disable-tools --enable-debug-tcg
> +    TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
> +    MAKE_CHECK_ARGS: check-tcg
> +
>   # Run check-tcg against linux-user (with plugins)
>   # we skip sparc64-linux-user until it has been fixed somewhat
>   # we skip cris-linux-user as it doesn't use the common run loop
> @@ -258,6 +267,14 @@ build-user-plugins:
>       MAKE_CHECK_ARGS: check-tcg
>     timeout: 1h 30m
>   
> +build-some-softmmu-plugins:
> +  <<: *native_build_job_definition
> +  variables:
> +    IMAGE: debian-all-test-cross
> +    CONFIGURE_ARGS: --disable-tools --disable-user --enable-plugins --enable-debug-tcg
> +    TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
> +    MAKE_CHECK_ARGS: check-tcg
> +
>   build-clang:
>     <<: *native_build_job_definition
>     variables:
> diff --git a/.travis.yml b/.travis.yml
> index a3d78171ca..bac085f800 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -301,32 +301,6 @@ jobs:
>           - ${SRC_DIR}/configure ${CONFIG} --extra-cflags="-g3 -O0 -fsanitize=thread" || { cat config.log meson-logs/meson-log.txt && exit 1; }
>   
>   
> -    # Run check-tcg against linux-user
> -    - name: "GCC check-tcg (user)"
> -      env:
> -        - CONFIG="--disable-system --enable-debug-tcg"
> -        - TEST_BUILD_CMD="make build-tcg"
> -        - TEST_CMD="make check-tcg"
> -        - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
> -
> -
> -    # Run check-tcg against softmmu targets
> -    - name: "GCC check-tcg (some-softmmu)"
> -      env:
> -        - CONFIG="--enable-debug-tcg --target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
> -        - TEST_BUILD_CMD="make build-tcg"
> -        - TEST_CMD="make check-tcg"
> -        - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
> -
> -
> -    # Run check-tcg against softmmu targets (with plugins)
> -    - name: "GCC plugins check-tcg (some-softmmu)"
> -      env:
> -        - CONFIG="--enable-plugins --enable-debug-tcg --target-list=xtensa-softmmu,arm-softmmu,aarch64-softmmu,alpha-softmmu"
> -        - TEST_BUILD_CMD="make build-tcg"
> -        - TEST_CMD="make check-tcg"
> -        - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
> -
>       - name: "[aarch64] GCC check-tcg"
>         arch: arm64
>         dist: focal

Re: [PATCH v1 2/6] tests: add prefixes to the bare mkdtemp calls

[Wainer dos Santos Moschetta]

On 11/17/20 2:36 PM, Alex Bennée wrote:
> The first step to debug a thing is to know what created the thing in
> the first place. Add some prefixes so random tmpdir's have something
> grep in the code.


Yeah, it indeed helps.


>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2
>    - fix long lines
> ---
>   python/qemu/machine.py                    | 3 ++-
>   tests/acceptance/avocado_qemu/__init__.py | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)


Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index 6420f01bed..64d966aeeb 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -303,7 +303,8 @@ class QEMUMachine:
>           return args
>   
>       def _pre_launch(self) -> None:
> -        self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
> +        self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
> +                                          dir=self._test_dir)
>           self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
>           self._qemu_log_file = open(self._qemu_log_path, 'wb')
>   
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 4cda037187..3033b2cabe 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -171,7 +171,8 @@ class Test(avocado.Test):
>               self.cancel("No QEMU binary defined or found in the build tree")
>   
>       def _new_vm(self, *args):
> -        vm = QEMUMachine(self.qemu_bin, sock_dir=tempfile.mkdtemp())
> +        sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_")
> +        vm = QEMUMachine(self.qemu_bin, sock_dir=sd)
>           if args:
>               vm.add_args(*args)
>           return vm

Re: [PATCH v1 3/6] tests/avocado: clean-up socket directory after run

[Wainer dos Santos Moschetta]

On 11/17/20 2:36 PM, Alex Bennée wrote:
> Previously we were leaving temporary directories behind. While the
> QEMUMachine does make efforts to clean up after itself the directory
> belongs to the calling function. We use TemporaryDirectory to wrap
> this although we explicitly clear the reference in tearDown() as it
> doesn't get cleaned up otherwise.

This patch fixes the problem introduced on patch 02 of this series.

>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 3033b2cabe..bf54e419da 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -171,8 +171,8 @@ class Test(avocado.Test):
>               self.cancel("No QEMU binary defined or found in the build tree")
>   
>       def _new_vm(self, *args):
> -        sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_")
> -        vm = QEMUMachine(self.qemu_bin, sock_dir=sd)
> +        self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")

Double-checking that you really meant "avo" or if your fingers forgot to 
type the remaining letters. :)

- Wainer

> +        vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name)
>           if args:
>               vm.add_args(*args)
>           return vm
> @@ -193,6 +193,7 @@ class Test(avocado.Test):
>       def tearDown(self):
>           for vm in self._vms.values():
>               vm.shutdown()
> +        self._sd = None
>   
>       def fetch_asset(self, name,
>                       asset_hash=None, algorithm=None,

Re: [PATCH v1 1/6] scripts/ci: clean up default args logic a little

[Wainer dos Santos Moschetta]

On 11/17/20 2:36 PM, Alex Bennée wrote:
> This allows us to do:
>
>    ./scripts/ci/gitlab-pipeline-status -w -b HEAD -p 2961854
>
> to check out own pipeline status of a recently pushed branch.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   scripts/ci/gitlab-pipeline-status | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/ci/gitlab-pipeline-status b/scripts/ci/gitlab-pipeline-status
> index bac8233079..78e72f6008 100755
> --- a/scripts/ci/gitlab-pipeline-status
> +++ b/scripts/ci/gitlab-pipeline-status
> @@ -31,7 +31,7 @@ class NoPipelineFound(Exception):
>       """Communication is successfull but pipeline is not found."""
>   
>   
> -def get_local_branch_commit(branch='staging'):
> +def get_local_branch_commit(branch):
>       """
>       Returns the commit sha1 for the *local* branch named "staging"


Need to reword the method's docstring.

Otherwise,

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>       """
> @@ -126,18 +126,16 @@ def create_parser():
>                           help=('The GitLab project ID. Defaults to the project '
>                                 'for https://gitlab.com/qemu-project/qemu, that '
>                                 'is, "%(default)s"'))
> -    try:
> -        default_commit = get_local_branch_commit()
> -        commit_required = False
> -    except ValueError:
> -        default_commit = ''
> -        commit_required = True
> -    parser.add_argument('-c', '--commit', required=commit_required,
> -                        default=default_commit,
> +    parser.add_argument('-b', '--branch', type=str, default="staging",
> +                        help=('Specify the branch to check. '
> +                              'Use HEAD for your current branch. '
> +                              'Otherwise looks at "%(default)s"'))
> +    parser.add_argument('-c', '--commit',
> +                        default=None,
>                           help=('Look for a pipeline associated with the given '
>                                 'commit.  If one is not explicitly given, the '
> -                              'commit associated with the local branch named '
> -                              '"staging" is used.  Default: %(default)s'))
> +                              'commit associated with the default branch '
> +                              'is used.'))
>       parser.add_argument('--verbose', action='store_true', default=False,
>                           help=('A minimal verbosity level that prints the '
>                                 'overall result of the check/wait'))
> @@ -149,6 +147,10 @@ def main():
>       """
>       parser = create_parser()
>       args = parser.parse_args()
> +
> +    if not args.commit:
> +        args.commit = get_local_branch_commit(args.branch)
> +
>       success = False
>       try:
>           if args.wait:

Re: [PATCH v1 3/6] tests/avocado: clean-up socket directory after run

[Alex Bennée]

Wainer dos Santos Moschetta <wainersm@redhat.com> writes:

> On 11/17/20 2:36 PM, Alex Bennée wrote:
>> Previously we were leaving temporary directories behind. While the
>> QEMUMachine does make efforts to clean up after itself the directory
>> belongs to the calling function. We use TemporaryDirectory to wrap
>> this although we explicitly clear the reference in tearDown() as it
>> doesn't get cleaned up otherwise.
>
> This patch fixes the problem introduced on patch 02 of this series.

It didn't introduce the problem in patch 2, it just moved it. The
mkdtemp() was never cleaned up before.

>
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   tests/acceptance/avocado_qemu/__init__.py | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
>> index 3033b2cabe..bf54e419da 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -171,8 +171,8 @@ class Test(avocado.Test):
>>               self.cancel("No QEMU binary defined or found in the build tree")
>>   
>>       def _new_vm(self, *args):
>> -        sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_")
>> -        vm = QEMUMachine(self.qemu_bin, sock_dir=sd)
>> +        self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")
>
> Double-checking that you really meant "avo" or if your fingers forgot to 
> type the remaining letters. :)

Hmm yeah I should probably just be consistent with the name in both
patches.

>
> - Wainer
>
>> +        vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name)
>>           if args:
>>               vm.add_args(*args)
>>           return vm
>> @@ -193,6 +193,7 @@ class Test(avocado.Test):
>>       def tearDown(self):
>>           for vm in self._vms.values():
>>               vm.shutdown()
>> +        self._sd = None
>>   
>>       def fetch_asset(self, name,
>>                       asset_hash=None, algorithm=None,


-- 
Alex Bennée

Re: [PATCH v1 3/6] tests/avocado: clean-up socket directory after run

[Wainer dos Santos Moschetta]

On 11/23/20 6:23 AM, Alex Bennée wrote:
> Wainer dos Santos Moschetta <wainersm@redhat.com> writes:
>
>> On 11/17/20 2:36 PM, Alex Bennée wrote:
>>> Previously we were leaving temporary directories behind. While the
>>> QEMUMachine does make efforts to clean up after itself the directory
>>> belongs to the calling function. We use TemporaryDirectory to wrap
>>> this although we explicitly clear the reference in tearDown() as it
>>> doesn't get cleaned up otherwise.
>> This patch fixes the problem introduced on patch 02 of this series.
> It didn't introduce the problem in patch 2, it just moved it. The
> mkdtemp() was never cleaned up before.


True. My bad.

- Wainer

>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>    tests/acceptance/avocado_qemu/__init__.py | 5 +++--
>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
>>> index 3033b2cabe..bf54e419da 100644
>>> --- a/tests/acceptance/avocado_qemu/__init__.py
>>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>>> @@ -171,8 +171,8 @@ class Test(avocado.Test):
>>>                self.cancel("No QEMU binary defined or found in the build tree")
>>>    
>>>        def _new_vm(self, *args):
>>> -        sd = tempfile.mkdtemp(prefix="avocado_qemu_sock_")
>>> -        vm = QEMUMachine(self.qemu_bin, sock_dir=sd)
>>> +        self._sd = tempfile.TemporaryDirectory(prefix="avo_qemu_sock_")
>> Double-checking that you really meant "avo" or if your fingers forgot to
>> type the remaining letters. :)
> Hmm yeah I should probably just be consistent with the name in both
> patches.


Anyway,

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>
>> - Wainer
>>
>>> +        vm = QEMUMachine(self.qemu_bin, sock_dir=self._sd.name)
>>>            if args:
>>>                vm.add_args(*args)
>>>            return vm
>>> @@ -193,6 +193,7 @@ class Test(avocado.Test):
>>>        def tearDown(self):
>>>            for vm in self._vms.values():
>>>                vm.shutdown()
>>> +        self._sd = None
>>>    
>>>        def fetch_asset(self, name,
>>>                        asset_hash=None, algorithm=None,
>