qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/10] Various testing and s390x patches
@ 2025-10-16 16:25 Thomas Huth
  2025-10-16 16:25 ` [PULL 01/10] python/qemu: Replace some remaining "avocados" with "functional tests" Thomas Huth
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson

 Hi Richard!

The following changes since commit 8109ebdb95c45d9062c7e6e7beae0ee571fca4f8:

  Merge tag 'pull-loongarch-20251015' of https://github.com/bibo-mao/qemu into staging (2025-10-15 14:49:51 -0700)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2025-10-16

for you to fetch changes up to d6f7f9254e333c56226bb7051e74ea57daea2fff:

  target/s390x/mmu_helper: Do not ignore address_space_rw() errors (2025-10-16 18:19:23 +0200)

----------------------------------------------------------------
* Improve cache handling for the msys2 CI and the functional asset cache
* Clean ups for some minor issues in functional tests
* Don't ignore errors of address_space_rw in s390x MMU code

----------------------------------------------------------------
Daniel P. Berrangé (3):
      gitlab: purge msys pacman cache
      tests/functional: remove use of getLogger in reverse debuging
      tests/functional: ensure GDB client is stopped on error

Philippe Mathieu-Daudé (2):
      target/s390x/mmu_helper: Simplify s390_cpu_virt_mem_rw() logic
      target/s390x/mmu_helper: Do not ignore address_space_rw() errors

Thomas Huth (5):
      python/qemu: Replace some remaining "avocados" with "functional tests"
      tests/functional/aarch64: Drop some sbsaref_alpine tests
      tests/functional: Set current time stamp of assets when using them
      tests: Evict stale files in the functional download cache after a while
      tests/functional/alpha: Remove superfluous fetch() line from the clipper test

 MAINTAINERS                                     |  1 +
 python/qemu/machine/README.rst                  |  2 +-
 python/qemu/utils/README.rst                    |  2 +-
 target/s390x/mmu_helper.c                       | 17 +++++--
 .gitlab-ci.d/windows.yml                        |  1 +
 scripts/clean_functional_cache.py               | 45 +++++++++++++++++
 tests/Makefile.include                          |  1 +
 tests/functional/aarch64/test_sbsaref_alpine.py |  6 ---
 tests/functional/alpha/test_clipper.py          |  1 -
 tests/functional/qemu_test/asset.py             | 13 +++++
 tests/functional/reverse_debugging.py           | 65 ++++++++++++-------------
 11 files changed, 105 insertions(+), 49 deletions(-)
 create mode 100755 scripts/clean_functional_cache.py



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PULL 01/10] python/qemu: Replace some remaining "avocados" with "functional tests"
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
@ 2025-10-16 16:25 ` Thomas Huth
  2025-10-16 16:25 ` [PULL 02/10] tests/functional/aarch64: Drop some sbsaref_alpine tests Thomas Huth
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Daniel P. Berrangé, Michael Tokarev,
	Alex Bennée

From: Thomas Huth <thuth@redhat.com>

The avocado tests have been replaced by the new functional tests,
so also update this in the README.rst files in the python directory
accordingly.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20251008131936.71160-1-thuth@redhat.com>
---
 python/qemu/machine/README.rst | 2 +-
 python/qemu/utils/README.rst   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/qemu/machine/README.rst b/python/qemu/machine/README.rst
index 8de2c3d7722..6554c693201 100644
--- a/python/qemu/machine/README.rst
+++ b/python/qemu/machine/README.rst
@@ -2,7 +2,7 @@ qemu.machine package
 ====================
 
 This package provides core utilities used for testing and debugging
-QEMU. It is used by the iotests, vm tests, avocado tests, and several
+QEMU. It is used by the iotests, vm tests, functional tests, and several
 other utilities in the ./scripts directory. It is not a fully-fledged
 SDK and it is subject to change at any time.
 
diff --git a/python/qemu/utils/README.rst b/python/qemu/utils/README.rst
index d5f2da14540..5027f0b5f11 100644
--- a/python/qemu/utils/README.rst
+++ b/python/qemu/utils/README.rst
@@ -2,6 +2,6 @@ qemu.utils package
 ==================
 
 This package provides miscellaneous utilities used for testing and
-debugging QEMU. It is used primarily by the vm and avocado tests.
+debugging QEMU. It is used primarily by the vm and functional tests.
 
 See the documentation in ``__init__.py`` for more information.
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 02/10] tests/functional/aarch64: Drop some sbsaref_alpine tests
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
  2025-10-16 16:25 ` [PULL 01/10] python/qemu: Replace some remaining "avocados" with "functional tests" Thomas Huth
@ 2025-10-16 16:25 ` Thomas Huth
  2025-10-16 16:25 ` [PULL 03/10] gitlab: purge msys pacman cache Thomas Huth
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Alex Bennée, Philippe Mathieu-Daudé

From: Thomas Huth <thuth@redhat.com>

test_sbsaref_alpine is one of the longest running test in our testsuite,
because it does a full Linux boot a couple of times, for various different
CPU configurations. That's quite a lot of testing each time, for a rather
small additional test coverage. Thus let's drop some of the tests that don't
provide much in addition to the other ones.

Suggested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20251006161850.181998-1-thuth@redhat.com>
---
 tests/functional/aarch64/test_sbsaref_alpine.py | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/tests/functional/aarch64/test_sbsaref_alpine.py b/tests/functional/aarch64/test_sbsaref_alpine.py
index abb8f5114bd..be84b7adb0c 100755
--- a/tests/functional/aarch64/test_sbsaref_alpine.py
+++ b/tests/functional/aarch64/test_sbsaref_alpine.py
@@ -41,15 +41,9 @@ def boot_alpine_linux(self, cpu=None):
         self.vm.launch()
         wait_for_console_pattern(self, "Welcome to Alpine Linux 3.17")
 
-    def test_sbsaref_alpine_linux_cortex_a57(self):
-        self.boot_alpine_linux("cortex-a57")
-
     def test_sbsaref_alpine_linux_default_cpu(self):
         self.boot_alpine_linux()
 
-    def test_sbsaref_alpine_linux_max_pauth_off(self):
-        self.boot_alpine_linux("max,pauth=off")
-
     def test_sbsaref_alpine_linux_max_pauth_impdef(self):
         self.boot_alpine_linux("max,pauth-impdef=on")
 
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 03/10] gitlab: purge msys pacman cache
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
  2025-10-16 16:25 ` [PULL 01/10] python/qemu: Replace some remaining "avocados" with "functional tests" Thomas Huth
  2025-10-16 16:25 ` [PULL 02/10] tests/functional/aarch64: Drop some sbsaref_alpine tests Thomas Huth
@ 2025-10-16 16:25 ` Thomas Huth
  2025-10-16 16:25 ` [PULL 04/10] tests/functional: Set current time stamp of assets when using them Thomas Huth
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Richard Henderson, Daniel P. Berrangé, Yonggang Luo,
	Peter Maydell

From: Daniel P. Berrangé <berrange@redhat.com>

For the Windows msys2 CI job we install many packages using pacman
and use the GitLab cache to preserve the pacman cache across CI
runs. While metadata still needs downloading, this avoids pacman
re-downloading packages from msys2 if they have not changed.

The problem is that pacman never automatically purges anything
from its package cache. Thus the GitLab cache is growing without
bound and packing/unpacking the cache is consuming an increasing
amount of time in the CI job.

If we run 'pacman -Sc' /after/ installing our desired package set,
it will purge any cached downloaded packages that are not matching
any installed package.

This will (currently) cap the pacman download cache at approx
256 MB.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20251010160545.144760-1-berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/windows.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml
index 1e6a01bd9ac..6e1135d8b86 100644
--- a/.gitlab-ci.d/windows.yml
+++ b/.gitlab-ci.d/windows.yml
@@ -87,6 +87,7 @@ msys2-64bit:
       mingw-w64-x86_64-pkgconf
       mingw-w64-x86_64-python
       mingw-w64-x86_64-zstd"
+  - .\msys64\usr\bin\bash -lc "pacman -Sc --noconfirm"
   - Write-Output "Running build at $(Get-Date -Format u)"
   - $env:JOBS = $(.\msys64\usr\bin\bash -lc nproc)
   - $env:CHERE_INVOKING = 'yes'  # Preserve the current working directory
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 04/10] tests/functional: Set current time stamp of assets when using them
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
                   ` (2 preceding siblings ...)
  2025-10-16 16:25 ` [PULL 03/10] gitlab: purge msys pacman cache Thomas Huth
@ 2025-10-16 16:25 ` Thomas Huth
  2025-10-16 16:25 ` [PULL 05/10] tests: Evict stale files in the functional download cache after a while Thomas Huth
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson, Daniel P. Berrangé

From: Thomas Huth <thuth@redhat.com>

We are going to remove obsolete assets from the cache, so keep
the time stamps of the assets that we use up-to-date to have a way
to detect stale assets later.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20251014083424.103202-2-thuth@redhat.com>
---
 tests/functional/qemu_test/asset.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
index f666125bfaf..ab3a7bb591d 100644
--- a/tests/functional/qemu_test/asset.py
+++ b/tests/functional/qemu_test/asset.py
@@ -10,6 +10,7 @@
 import os
 import stat
 import sys
+import time
 import unittest
 import urllib.request
 from time import sleep
@@ -113,6 +114,16 @@ def _wait_for_other_download(self, tmp_cache_file):
         self.log.debug("Time out while waiting for %s!", tmp_cache_file)
         raise
 
+    def _save_time_stamp(self):
+        '''
+        Update the time stamp of the asset in the cache. Unfortunately, we
+        cannot use the modification or access time of the asset file itself,
+        since e.g. the functional jobs in the gitlab CI reload the files
+        from the gitlab cache and thus always have recent file time stamps,
+        so we have to save our asset time stamp to a separate file instead.
+        '''
+        self.cache_file.with_suffix(".stamp").write_text(f"{int(time.time())}")
+
     def fetch(self):
         if not self.cache_dir.exists():
             self.cache_dir.mkdir(parents=True, exist_ok=True)
@@ -120,6 +131,7 @@ def fetch(self):
         if self.valid():
             self.log.debug("Using cached asset %s for %s",
                            self.cache_file, self.url)
+            self._save_time_stamp()
             return str(self.cache_file)
 
         if not self.fetchable():
@@ -208,6 +220,7 @@ def fetch(self):
             tmp_cache_file.unlink()
             raise AssetError(self, "Hash does not match %s" % self.hash)
         tmp_cache_file.replace(self.cache_file)
+        self._save_time_stamp()
         # Remove write perms to stop tests accidentally modifying them
         os.chmod(self.cache_file, stat.S_IRUSR | stat.S_IRGRP)
 
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 05/10] tests: Evict stale files in the functional download cache after a while
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
                   ` (3 preceding siblings ...)
  2025-10-16 16:25 ` [PULL 04/10] tests/functional: Set current time stamp of assets when using them Thomas Huth
@ 2025-10-16 16:25 ` Thomas Huth
  2025-10-16 16:25 ` [PULL 06/10] tests/functional/alpha: Remove superfluous fetch() line from the clipper test Thomas Huth
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson, Daniel P. Berrangé

From: Thomas Huth <thuth@redhat.com>

The download cache of the functional tests is currently only growing.
But sometimes tests get removed or changed to use different assets,
thus we should clean up the stale old assets after a while when they
are not in use anymore. So add a script that looks at the time stamps
of the assets and removes them if they haven't been touched for more
than half of a year. Since there might also be some assets around that
have been added to the cache before we added the time stamp files,
assume a default time stamp that is close to the creation date of this
patch, so that we don't delete these files too early (so we still have
all assets around in case we have to bisect an issue in the recent past
of QEMU).

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20251014083424.103202-3-thuth@redhat.com>
---
 MAINTAINERS                       |  1 +
 scripts/clean_functional_cache.py | 45 +++++++++++++++++++++++++++++++
 tests/Makefile.include            |  1 +
 3 files changed, 47 insertions(+)
 create mode 100755 scripts/clean_functional_cache.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 0c766961f39..667acd933c7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4397,6 +4397,7 @@ M: Thomas Huth <thuth@redhat.com>
 R: Philippe Mathieu-Daudé <philmd@linaro.org>
 R: Daniel P. Berrange <berrange@redhat.com>
 F: docs/devel/testing/functional.rst
+F: scripts/clean_functional_cache.py
 F: tests/functional/qemu_test/
 
 Windows Hosted Continuous Integration
diff --git a/scripts/clean_functional_cache.py b/scripts/clean_functional_cache.py
new file mode 100755
index 00000000000..c3370ffbb87
--- /dev/null
+++ b/scripts/clean_functional_cache.py
@@ -0,0 +1,45 @@
+#!/usr/bin/env python3
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+"""Delete stale assets from the download cache of the functional tests"""
+
+import os
+import stat
+import sys
+import time
+from pathlib import Path
+
+
+cache_dir_env = os.getenv('QEMU_TEST_CACHE_DIR')
+if cache_dir_env:
+    cache_dir = Path(cache_dir_env, "download")
+else:
+    cache_dir = Path(Path("~").expanduser(), ".cache", "qemu", "download")
+
+if not cache_dir.exists():
+    print(f"Cache dir {cache_dir} does not exist!", file=sys.stderr)
+    sys.exit(1)
+
+os.chdir(cache_dir)
+
+for file in cache_dir.iterdir():
+    # Only consider the files that use a sha256 as filename:
+    if len(file.name) != 64:
+        continue
+
+    try:
+        timestamp = int(file.with_suffix(".stamp").read_text())
+    except FileNotFoundError:
+        # Assume it's an old file that was already in the cache before we
+        # added the code for evicting stale assets. Use the release date
+        # of QEMU v10.1 as a default timestamp.
+        timestamp = time.mktime((2025, 8, 26, 0, 0, 0, 0, 0, 0))
+
+    age = time.time() - timestamp
+
+    # Delete files older than half of a year (183 days * 24h * 60m * 60s)
+    if age > 15811200:
+        print(f"Removing {cache_dir}/{file.name}.")
+        file.chmod(stat.S_IWRITE)
+        file.unlink()
diff --git a/tests/Makefile.include b/tests/Makefile.include
index e47ef4d45c9..d4dfbf3716d 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -111,6 +111,7 @@ $(FUNCTIONAL_TARGETS): check-venv
 .PHONY: check-functional
 check-functional: check-venv
 	@$(NINJA) precache-functional
+	@$(PYTHON) $(SRC_PATH)/scripts/clean_functional_cache.py
 	@QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func check-func-quick
 
 .PHONY: check-func check-func-quick
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 06/10] tests/functional/alpha: Remove superfluous fetch() line from the clipper test
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
                   ` (4 preceding siblings ...)
  2025-10-16 16:25 ` [PULL 05/10] tests: Evict stale files in the functional download cache after a while Thomas Huth
@ 2025-10-16 16:25 ` Thomas Huth
  2025-10-16 16:25 ` [PULL 07/10] tests/functional: remove use of getLogger in reverse debuging Thomas Huth
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson

From: Thomas Huth <thuth@redhat.com>

The kernel asset is retrieved automatically via the uncompress()
line below the fetch(), so the fetch() is simply not necessary here.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20251010144525.842462-1-thuth@redhat.com>
---
 tests/functional/alpha/test_clipper.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/functional/alpha/test_clipper.py b/tests/functional/alpha/test_clipper.py
index c5d71819531..d2a4c2a4ed9 100755
--- a/tests/functional/alpha/test_clipper.py
+++ b/tests/functional/alpha/test_clipper.py
@@ -17,7 +17,6 @@ class AlphaClipperTest(LinuxKernelTest):
 
     def test_alpha_clipper(self):
         self.set_machine('clipper')
-        kernel_path = self.ASSET_KERNEL.fetch()
 
         uncompressed_kernel = self.uncompress(self.ASSET_KERNEL, format="gz")
 
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 07/10] tests/functional: remove use of getLogger in reverse debuging
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
                   ` (5 preceding siblings ...)
  2025-10-16 16:25 ` [PULL 06/10] tests/functional/alpha: Remove superfluous fetch() line from the clipper test Thomas Huth
@ 2025-10-16 16:25 ` Thomas Huth
  2025-10-16 16:25 ` [PULL 08/10] tests/functional: ensure GDB client is stopped on error Thomas Huth
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson, Daniel P. Berrangé

From: Daniel P. Berrangé <berrange@redhat.com>

This fixes the gap left by

  commit 8a44d8c2ac0921c8064fbfd00ef28e3a2588918e
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Fri Sep 12 19:22:00 2025 +0100

    tests/functional: use self.log for all logging

ensuring that log message from the reverse debugging test actually
make it into the logfile on disk.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20251014140047.385347-2-berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/reverse_debugging.py | 49 ++++++++++++---------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
index 68cfcb39856..2c37a62cd06 100644
--- a/tests/functional/reverse_debugging.py
+++ b/tests/functional/reverse_debugging.py
@@ -36,14 +36,13 @@ class ReverseDebugging(LinuxKernelTest):
     STEPS = 10
 
     def run_vm(self, record, shift, args, replay_path, image_path, port):
-        logger = logging.getLogger('replay')
         vm = self.get_vm(name='record' if record else 'replay')
         vm.set_console()
         if record:
-            logger.info('recording the execution...')
+            self.log.info('recording the execution...')
             mode = 'record'
         else:
-            logger.info('replaying the execution...')
+            self.log.info('replaying the execution...')
             mode = 'replay'
             vm.add_args('-gdb', 'tcp::%d' % port, '-S')
         vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s,rrsnapshot=init' %
@@ -68,10 +67,8 @@ def vm_get_icount(vm):
     def reverse_debugging(self, gdb_arch, shift=7, args=None):
         from qemu_test import GDB
 
-        logger = logging.getLogger('replay')
-
         # create qcow2 for snapshots
-        logger.info('creating qcow2 image for VM snapshots')
+        self.log.info('creating qcow2 image for VM snapshots')
         image_path = os.path.join(self.workdir, 'disk.qcow2')
         qemu_img = get_qemu_img(self)
         if qemu_img is None:
@@ -79,7 +76,7 @@ def reverse_debugging(self, gdb_arch, shift=7, args=None):
                           'create the temporary qcow2 image')
         out = check_output([qemu_img, 'create', '-f', 'qcow2', image_path, '128M'],
                            encoding='utf8')
-        logger.info("qemu-img: %s" % out)
+        self.log.info("qemu-img: %s" % out)
 
         replay_path = os.path.join(self.workdir, 'replay.bin')
 
@@ -90,7 +87,7 @@ def reverse_debugging(self, gdb_arch, shift=7, args=None):
         last_icount = self.vm_get_icount(vm)
         vm.shutdown()
 
-        logger.info("recorded log with %s+ steps" % last_icount)
+        self.log.info("recorded log with %s+ steps" % last_icount)
 
         # replay and run debug commands
         with Ports() as ports:
@@ -98,9 +95,9 @@ def reverse_debugging(self, gdb_arch, shift=7, args=None):
             vm = self.run_vm(False, shift, args, replay_path, image_path, port)
 
         try:
-            logger.info('Connecting to gdbstub...')
+            self.log.info('Connecting to gdbstub...')
             self.reverse_debugging_run(vm, port, gdb_arch, last_icount)
-            logger.info('Test passed.')
+            self.log.info('Test passed.')
         except GDB.TimeoutError:
             # Convert a GDB timeout exception into a unittest failure exception.
             raise self.failureException("Timeout while connecting to or "
@@ -111,8 +108,6 @@ def reverse_debugging(self, gdb_arch, shift=7, args=None):
             raise
 
     def reverse_debugging_run(self, vm, port, gdb_arch, last_icount):
-        logger = logging.getLogger('replay')
-
         gdb_cmd = os.getenv('QEMU_TEST_GDB')
         gdb = GDB(gdb_cmd)
 
@@ -135,43 +130,43 @@ def reverse_debugging_run(self, vm, port, gdb_arch, last_icount):
 
         gdb.cli("set debug remote 0")
 
-        logger.info('stepping forward')
+        self.log.info('stepping forward')
         steps = []
         # record first instruction addresses
         for _ in range(self.STEPS):
             pc = self.get_pc(gdb)
-            logger.info('saving position %x' % pc)
+            self.log.info('saving position %x' % pc)
             steps.append(pc)
             gdb.cli("stepi")
 
         # visit the recorded instruction in reverse order
-        logger.info('stepping backward')
+        self.log.info('stepping backward')
         for addr in steps[::-1]:
-            logger.info('found position %x' % addr)
+            self.log.info('found position %x' % addr)
             gdb.cli("reverse-stepi")
             pc = self.get_pc(gdb)
             if pc != addr:
-                logger.info('Invalid PC (read %x instead of %x)' % (pc, addr))
+                self.log.info('Invalid PC (read %x instead of %x)' % (pc, addr))
                 self.fail('Reverse stepping failed!')
 
         # visit the recorded instruction in forward order
-        logger.info('stepping forward')
+        self.log.info('stepping forward')
         for addr in steps:
-            logger.info('found position %x' % addr)
+            self.log.info('found position %x' % addr)
             pc = self.get_pc(gdb)
             if pc != addr:
-                logger.info('Invalid PC (read %x instead of %x)' % (pc, addr))
+                self.log.info('Invalid PC (read %x instead of %x)' % (pc, addr))
                 self.fail('Forward stepping failed!')
             gdb.cli("stepi")
 
         # set breakpoints for the instructions just stepped over
-        logger.info('setting breakpoints')
+        self.log.info('setting breakpoints')
         for addr in steps:
             gdb.cli(f"break *{hex(addr)}")
 
         # this may hit a breakpoint if first instructions are executed
         # again
-        logger.info('continuing execution')
+        self.log.info('continuing execution')
         vm.qmp('replay-break', icount=last_icount - 1)
         # continue - will return after pausing
         # This can stop at the end of the replay-break and gdb gets a SIGINT,
@@ -180,12 +175,12 @@ def reverse_debugging_run(self, vm, port, gdb_arch, last_icount):
         gdb.cli("continue")
 
         if self.vm_get_icount(vm) == last_icount - 1:
-            logger.info('reached the end (icount %s)' % (last_icount - 1))
+            self.log.info('reached the end (icount %s)' % (last_icount - 1))
         else:
-            logger.info('hit a breakpoint again at %x (icount %s)' %
+            self.log.info('hit a breakpoint again at %x (icount %s)' %
                         (self.get_pc(gdb), self.vm_get_icount(vm)))
 
-        logger.info('running reverse continue to reach %x' % steps[-1])
+        self.log.info('running reverse continue to reach %x' % steps[-1])
         # reverse continue - will return after stopping at the breakpoint
         gdb.cli("reverse-continue")
 
@@ -195,8 +190,8 @@ def reverse_debugging_run(self, vm, port, gdb_arch, last_icount):
         if pc != steps[-1]:
             self.fail("'reverse-continue' did not hit the first PC in reverse order!")
 
-        logger.info('successfully reached %x' % steps[-1])
+        self.log.info('successfully reached %x' % steps[-1])
 
-        logger.info('exiting gdb and qemu')
+        self.log.info('exiting gdb and qemu')
         gdb.exit()
         vm.shutdown()
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 08/10] tests/functional: ensure GDB client is stopped on error
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
                   ` (6 preceding siblings ...)
  2025-10-16 16:25 ` [PULL 07/10] tests/functional: remove use of getLogger in reverse debuging Thomas Huth
@ 2025-10-16 16:25 ` Thomas Huth
  2025-10-16 16:26 ` [PULL 09/10] target/s390x/mmu_helper: Simplify s390_cpu_virt_mem_rw() logic Thomas Huth
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson, Daniel P. Berrangé

From: Daniel P. Berrangé <berrange@redhat.com>

If the reverse_debugging_run method fails, the GDB client will not
be closed resulting in python complaining about resource leaks.
Hoisting the GDB client creation into the caller allows this to
be cleaned up easily. While doing this, also move the VM shutdown
call to match.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20251014140047.385347-3-berrange@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/reverse_debugging.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/functional/reverse_debugging.py b/tests/functional/reverse_debugging.py
index 2c37a62cd06..86fca8d81f1 100644
--- a/tests/functional/reverse_debugging.py
+++ b/tests/functional/reverse_debugging.py
@@ -96,7 +96,14 @@ def reverse_debugging(self, gdb_arch, shift=7, args=None):
 
         try:
             self.log.info('Connecting to gdbstub...')
-            self.reverse_debugging_run(vm, port, gdb_arch, last_icount)
+            gdb_cmd = os.getenv('QEMU_TEST_GDB')
+            gdb = GDB(gdb_cmd)
+            try:
+                self.reverse_debugging_run(gdb, vm, port, gdb_arch, last_icount)
+            finally:
+                self.log.info('exiting gdb and qemu')
+                gdb.exit()
+                vm.shutdown()
             self.log.info('Test passed.')
         except GDB.TimeoutError:
             # Convert a GDB timeout exception into a unittest failure exception.
@@ -107,10 +114,7 @@ def reverse_debugging(self, gdb_arch, shift=7, args=None):
             # skipTest(), etc.
             raise
 
-    def reverse_debugging_run(self, vm, port, gdb_arch, last_icount):
-        gdb_cmd = os.getenv('QEMU_TEST_GDB')
-        gdb = GDB(gdb_cmd)
-
+    def reverse_debugging_run(self, gdb, vm, port, gdb_arch, last_icount):
         r = gdb.cli("set architecture").get_log()
         if gdb_arch not in r:
             self.skipTest(f"GDB does not support arch '{gdb_arch}'")
@@ -191,7 +195,3 @@ def reverse_debugging_run(self, vm, port, gdb_arch, last_icount):
             self.fail("'reverse-continue' did not hit the first PC in reverse order!")
 
         self.log.info('successfully reached %x' % steps[-1])
-
-        self.log.info('exiting gdb and qemu')
-        gdb.exit()
-        vm.shutdown()
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 09/10] target/s390x/mmu_helper: Simplify s390_cpu_virt_mem_rw() logic
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
                   ` (7 preceding siblings ...)
  2025-10-16 16:25 ` [PULL 08/10] tests/functional: ensure GDB client is stopped on error Thomas Huth
@ 2025-10-16 16:26 ` Thomas Huth
  2025-10-16 16:26 ` [PULL 10/10] target/s390x/mmu_helper: Do not ignore address_space_rw() errors Thomas Huth
  2025-10-17 15:12 ` [PULL 00/10] Various testing and s390x patches Richard Henderson
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson, Philippe Mathieu-Daudé

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

In order to simplify the next commit, move the
trigger_access_exception() call after the address_space_rw()
calls. No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20251008141410.99865-2-philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/s390x/mmu_helper.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 487c41bf933..22d3d4a97df 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -541,9 +541,7 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
     pages = g_malloc(nr_pages * sizeof(*pages));
 
     ret = translate_pages(cpu, laddr, nr_pages, pages, is_write, &tec);
-    if (ret) {
-        trigger_access_exception(&cpu->env, ret, tec);
-    } else if (hostbuf != NULL) {
+    if (ret == 0 && hostbuf != NULL) {
         AddressSpace *as = CPU(cpu)->as;
 
         /* Copy data by stepping through the area page by page */
@@ -556,6 +554,9 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
             len -= currlen;
         }
     }
+    if (ret) {
+        trigger_access_exception(&cpu->env, ret, tec);
+    }
 
     g_free(pages);
     return ret;
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PULL 10/10] target/s390x/mmu_helper: Do not ignore address_space_rw() errors
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
                   ` (8 preceding siblings ...)
  2025-10-16 16:26 ` [PULL 09/10] target/s390x/mmu_helper: Simplify s390_cpu_virt_mem_rw() logic Thomas Huth
@ 2025-10-16 16:26 ` Thomas Huth
  2025-10-17 15:12 ` [PULL 00/10] Various testing and s390x patches Richard Henderson
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2025-10-16 16:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Richard Henderson, Philippe Mathieu-Daudé

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

If a address_space_rw() call ever fails, break the loop and
return the PGM_ADDRESSING error (after triggering an access
exception).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20251008141410.99865-3-philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/s390x/mmu_helper.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 22d3d4a97df..3b1e75f7833 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -546,9 +546,15 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
 
         /* Copy data by stepping through the area page by page */
         for (i = 0; i < nr_pages; i++) {
+            MemTxResult res;
+
             currlen = MIN(len, TARGET_PAGE_SIZE - (laddr % TARGET_PAGE_SIZE));
-            address_space_rw(as, pages[i] | (laddr & ~TARGET_PAGE_MASK),
-                             attrs, hostbuf, currlen, is_write);
+            res = address_space_rw(as, pages[i] | (laddr & ~TARGET_PAGE_MASK),
+                                   attrs, hostbuf, currlen, is_write);
+            if (res != MEMTX_OK) {
+                ret = PGM_ADDRESSING;
+                break;
+            }
             laddr += currlen;
             hostbuf += currlen;
             len -= currlen;
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PULL 00/10] Various testing and s390x patches
  2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
                   ` (9 preceding siblings ...)
  2025-10-16 16:26 ` [PULL 10/10] target/s390x/mmu_helper: Do not ignore address_space_rw() errors Thomas Huth
@ 2025-10-17 15:12 ` Richard Henderson
  10 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2025-10-17 15:12 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel

On 10/16/25 09:25, Thomas Huth wrote:
>   Hi Richard!
> 
> The following changes since commit 8109ebdb95c45d9062c7e6e7beae0ee571fca4f8:
> 
>    Merge tag 'pull-loongarch-20251015' ofhttps://github.com/bibo-mao/qemu into staging (2025-10-15 14:49:51 -0700)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/thuth/qemu.git tags/pull-request-2025-10-16
> 
> for you to fetch changes up to d6f7f9254e333c56226bb7051e74ea57daea2fff:
> 
>    target/s390x/mmu_helper: Do not ignore address_space_rw() errors (2025-10-16 18:19:23 +0200)
> 
> ----------------------------------------------------------------
> * Improve cache handling for the msys2 CI and the functional asset cache
> * Clean ups for some minor issues in functional tests
> * Don't ignore errors of address_space_rw in s390x MMU code

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/10.2 as appropriate.

r~



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-10-17 15:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-16 16:25 [PULL 00/10] Various testing and s390x patches Thomas Huth
2025-10-16 16:25 ` [PULL 01/10] python/qemu: Replace some remaining "avocados" with "functional tests" Thomas Huth
2025-10-16 16:25 ` [PULL 02/10] tests/functional/aarch64: Drop some sbsaref_alpine tests Thomas Huth
2025-10-16 16:25 ` [PULL 03/10] gitlab: purge msys pacman cache Thomas Huth
2025-10-16 16:25 ` [PULL 04/10] tests/functional: Set current time stamp of assets when using them Thomas Huth
2025-10-16 16:25 ` [PULL 05/10] tests: Evict stale files in the functional download cache after a while Thomas Huth
2025-10-16 16:25 ` [PULL 06/10] tests/functional/alpha: Remove superfluous fetch() line from the clipper test Thomas Huth
2025-10-16 16:25 ` [PULL 07/10] tests/functional: remove use of getLogger in reverse debuging Thomas Huth
2025-10-16 16:25 ` [PULL 08/10] tests/functional: ensure GDB client is stopped on error Thomas Huth
2025-10-16 16:26 ` [PULL 09/10] target/s390x/mmu_helper: Simplify s390_cpu_virt_mem_rw() logic Thomas Huth
2025-10-16 16:26 ` [PULL 10/10] target/s390x/mmu_helper: Do not ignore address_space_rw() errors Thomas Huth
2025-10-17 15:12 ` [PULL 00/10] Various testing and s390x patches Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).