All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Bernatowicz, Marcin" <marcin.bernatowicz@linux.intel.com>
To: Adam Miszczak <adam.miszczak@linux.intel.com>,
	igt-dev@lists.freedesktop.org
Cc: kamil.konieczny@linux.intel.com
Subject: Re: [PATCH i-g-t 5/7] tools/vmtb: Add pre-test environment cleanup
Date: Thu, 8 Oct 2026 15:55:50 +0200	[thread overview]
Message-ID: <2965da2b-6567-4574-9b86-a6a547f32fd8@linux.intel.com> (raw)
In-Reply-To: <20260928072600.426902-6-adam.miszczak@linux.intel.com>

LGTM,

Reviewed-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>

On 9/28/2026 9:25 AM, Adam Miszczak wrote:
> Introduce 'EnvironmentCleaner' to ensure a clean DUT state before each
> test by removing leftovers from crashed, failed, or timed-out runs.
> The cleaner handles:
> - terminating orphaned QEMU processes (SIGTERM, then SIGKILL if needed)
> - removing stale QEMU sockets (Guest Agent, QMP monitor)
> - optionally removing leftover qcow2 guest images (disabled by default)
> Wire EnvironmentCleaner().cleanup() into the VM setup fixtures in
> vmm_flows/conftest.py so cleanup runs before every test setup phase.
>
> Consolidate multi-step SR-IOV configuration teardown into a single
> reset_sriov_config() function that removes VFs and restores
> auto provisioning via debugfs.
> Call reset_sriov_config() at test setup when leftover VFs are
> detected, guarding against dirty state from crashed runs.
>
> Drop the explicit provisioning attributes clearing
> (clear_provisioning_attributes/reset_provisioning) and the unused
> cancel_work() teardown step — restore_auto_provisioning is sufficient.
> Remove implicit VF removal from create_vf() (handled by the setup check).
>
> Signed-off-by: Adam Miszczak <adam.miszczak@linux.intel.com>
> ---
>   tools/vmtb/bench/drivers/driver_interface.py |  1 +
>   tools/vmtb/bench/drivers/xe.py               |  1 +
>   tools/vmtb/bench/helpers/cleanup.py          | 91 ++++++++++++++++++++
>   tools/vmtb/bench/machines/physical/device.py | 39 ++-------
>   tools/vmtb/vmm_flows/conftest.py             | 34 ++++++--
>   5 files changed, 126 insertions(+), 40 deletions(-)
>   create mode 100644 tools/vmtb/bench/helpers/cleanup.py
>
> diff --git a/tools/vmtb/bench/drivers/driver_interface.py b/tools/vmtb/bench/drivers/driver_interface.py
> index 9d1e5ab00..498afa9af 100644
> --- a/tools/vmtb/bench/drivers/driver_interface.py
> +++ b/tools/vmtb/bench/drivers/driver_interface.py
> @@ -146,6 +146,7 @@ class PfDriverInterface(DriverInterface, abc.ABC):
>       def restore_auto_provisioning(self) -> None:
>           raise NotImplementedError
>   
> +    # Deprecated - not used on xe
>       @abc.abstractmethod
>       def cancel_work(self) -> None:
>           raise NotImplementedError
> diff --git a/tools/vmtb/bench/drivers/xe.py b/tools/vmtb/bench/drivers/xe.py
> index 479c1dd87..45182aad5 100644
> --- a/tools/vmtb/bench/drivers/xe.py
> +++ b/tools/vmtb/bench/drivers/xe.py
> @@ -62,6 +62,7 @@ class XeDriver(PfDriverInterface):
>           path = self.debugfs_path / 'sriov' / 'restore_auto_provisioning'
>           self.write_debugfs(str(path), str(1))
>   
> +    # Deprecated - not used on xe
>       def cancel_work(self) -> None:
>           # Function to cancel all remaing work on GPU (for test cleanup).
>           # Forcing reset (debugfs/gtM/force_reset_sync) shouldn't be used to idle GPU.
> diff --git a/tools/vmtb/bench/helpers/cleanup.py b/tools/vmtb/bench/helpers/cleanup.py
> new file mode 100644
> index 000000000..f1d3e3f76
> --- /dev/null
> +++ b/tools/vmtb/bench/helpers/cleanup.py
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: MIT
> +# Copyright © 2026 Intel Corporation
> +
> +import logging
> +import subprocess
> +import time
> +from pathlib import Path
> +
> +logger = logging.getLogger('Cleanup')
> +
> +
> +class EnvironmentCleaner:
> +    """Pre-test environment cleanup utilities.
> +
> +    Ensures that leftovers from crashed, failed, or timed-out tests
> +    are removed before the next test setup phase begins.
> +    """
> +
> +    QEMU_PROCESS_NAME = 'qemu-system-x86_64'
> +    SOCKET_PATTERNS = ['qga*.sock', 'mon*.sock']
> +    QEMU_IMAGE_PATTERN = 'vm*_*_image.qcow2'
> +
> +    def __init__(self, work_dir: str = '.', remove_qemu_images: bool = False) -> None:
> +        self.work_dir = Path(work_dir)
> +        self.remove_qemu_images = remove_qemu_images
> +
> +    def cleanup(self) -> None:
> +        """Perform full pre-test environment cleanup.
> +        Handles:
> +        - Terminating orphaned QEMU processes
> +        - Removing stale QEMU sockets (Guest Agent, QMP monitor)
> +        - Removing leftover QEMU qcow2 images (optionally)
> +        """
> +        logger.debug("Pre-test environment cleanup")
> +        self.terminate_orphaned_qemu_processes()
> +        self.remove_stale_sockets()
> +        if self.remove_qemu_images:
> +            self.remove_leftover_qemu_images()
> +
> +    def terminate_orphaned_qemu_processes(self) -> None:
> +        """Find and terminate any leftover QEMU processes from previous test runs.
> +           Try SIGTERM first, then SIGKILL if necessary."""
> +        for signal in ('TERM', 'KILL'):
> +            pids = self._find_qemu_pids()
> +            if not pids:
> +                return
> +            for pid_str in pids:
> +                logger.debug("Terminating (SIG%s) orphaned QEMU process (PID: %s)", signal, pid_str)
> +                try:
> +                    subprocess.run(['kill', f'-{signal}', pid_str], check=False)
> +                except Exception as exc:
> +                    logger.warning("Failed to %s QEMU PID %s: %s", signal, pid_str, exc)
> +            time.sleep(1)  # Give the processes a moment to terminate
> +
> +    def _find_qemu_pids(self) -> list[str] | None:
> +        """Return list of QEMU PIDs, or None if none found or pgrep unavailable."""
> +        try:
> +            result = subprocess.run(
> +                ['pgrep', '-f', self.QEMU_PROCESS_NAME],
> +                capture_output=True, text=True, check=False)
> +        except FileNotFoundError:
> +            logger.warning("pgrep not available, skipping orphan QEMU check")
> +            return None
> +
> +        if result.returncode != 0:
> +            return None
> +
> +        return [pid for pid in result.stdout.strip().split('\n') if pid] or None
> +
> +    def remove_stale_sockets(self) -> None:
> +        """Remove leftover QEMU Guest Agent and QMP monitor sockets."""
> +        for pattern in self.SOCKET_PATTERNS:
> +            for sock_path in Path('/tmp').glob(pattern):
> +                try:
> +                    if not sock_path.is_socket():
> +                        continue
> +                    logger.debug("Removing stale VMTB/QEMU socket: %s", sock_path)
> +                    sock_path.unlink()
> +                except OSError as exc:
> +                    logger.warning("Failed to remove stale socket %s: %s", sock_path, exc)
> +
> +    def remove_leftover_qemu_images(self) -> None:
> +        """Remove leftover QEMU qcow2 images from previous test runs."""
> +        for image_path in self.work_dir.glob(self.QEMU_IMAGE_PATTERN):
> +            try:
> +                if not image_path.is_file():
> +                    continue
> +                logger.debug("Removing leftover VMTB/QEMU guest image: %s", image_path)
> +                image_path.unlink()
> +            except OSError as exc:
> +                logger.warning("Failed to remove leftover image %s: %s", image_path, exc)
> diff --git a/tools/vmtb/bench/machines/physical/device.py b/tools/vmtb/bench/machines/physical/device.py
> index b4eebc245..7cbbee277 100644
> --- a/tools/vmtb/bench/machines/physical/device.py
> +++ b/tools/vmtb/bench/machines/physical/device.py
> @@ -91,9 +91,6 @@ class Device(DeviceInterface):
>           Disable SRIOV drivers autoprobe to allow VFIO driver override for VFs.
>           """
>           logger.info("[%s] Enable %s VFs", self.pci_info.bdf, num)
> -        if self.get_current_vfs() != 0:
> -            self.remove_vfs()
> -
>           self.numvf = num
>   
>           # Disable driver autoprobe to avoid driver load on VF (override to vfio is required)
> @@ -233,36 +230,6 @@ class Device(DeviceInterface):
>               self.driver.set_contexts_quota(fn_num, gt_num, resources_config.vfContexts)
>               self.driver.set_doorbells_quota(fn_num, gt_num, resources_config.vfDoorbells)
>   
> -    def clear_provisioning_attributes(self, num_vfs: int) -> None:
> -        """Clear provisioning attributes for a requested number of VFs."""
> -        # Provisioning config are likely wiped out by (xe) debugfs/restore_auto_provisioning,
> -        # but explicit clear shouldn't harm.
> -        self.driver.set_bulk_sched_priority(SchedulingPriority.LOW)
> -        self.driver.set_bulk_exec_quantum_ms(0)
> -        self.driver.set_bulk_preempt_timeout_us(0)
> -
> -        for gt_num in range(self.get_num_gts()):
> -            self.driver.set_pf_policy_reset_engine(gt_num, 0)
> -            self.driver.set_doorbells_quota(0, gt_num, 0)
> -            # PF contexts cannot be set from sysfs
> -
> -            for vf_num in range(1, num_vfs + 1):
> -                self.driver.set_contexts_quota(vf_num, gt_num, 0)
> -                self.driver.set_doorbells_quota(vf_num, gt_num, 0)
> -                if not self.is_gt_media_type(gt_num):
> -                    self.driver.set_vram_quota(vf_num, 0)
> -                    self.driver.set_ggtt_quota(vf_num, gt_num, 0)
> -
> -    def reset_provisioning(self, num_vfs: int) -> None:
> -        """Clear provisioning config for a given number of VFs and restore auto provisioning mode."""
> -        logger.info("[%s] Reset %sxVF provisioning configuration", self.pci_info.bdf, num_vfs)
> -        self.clear_provisioning_attributes(num_vfs)
> -        self.driver.restore_auto_provisioning()
> -
> -    def cancel_work(self) -> None:
> -        """Drop and reset remaining GPU execution at exit."""
> -        self.driver.cancel_work()
> -
>       def get_scheduling_priority(self, fn_num: int) -> SchedulingPriority:
>           """Get scheduling priority for a given VF or PF."""
>           return self.driver.get_sched_priority(fn_num)
> @@ -274,3 +241,9 @@ class Device(DeviceInterface):
>       def set_pf_scheduling_priority(self, val: SchedulingPriority) -> None:
>           """Set scheduling priority for PF only. High prioritizes PF execution over VFs."""
>           self.driver.set_pf_sched_priority(val)
> +
> +    def reset_sriov_config(self) -> None:
> +        """Clear setup - remove enabled VFs and restore auto provisioning mode."""
> +        logger.info("[%s] Reset VFs configuration", self.pci_info.bdf)
> +        self.remove_vfs()
> +        self.driver.restore_auto_provisioning()
> diff --git a/tools/vmtb/vmm_flows/conftest.py b/tools/vmtb/vmm_flows/conftest.py
> index ba6487c84..df395794c 100644
> --- a/tools/vmtb/vmm_flows/conftest.py
> +++ b/tools/vmtb/vmm_flows/conftest.py
> @@ -16,6 +16,7 @@ from bench.configurators.vgpu_profile_config import (VfProvisioningMode,
>                                                        VfSchedulingMode,
>                                                        VgpuProfileConfigurator)
>   from bench.configurators.vmtb_config import VmtbConfigurator
> +from bench.helpers.cleanup import EnvironmentCleaner
>   from bench.helpers.log import HOST_DMESG_FILE
>   from bench.machines.host import Device, Host
>   from bench.machines.virtual.vm import VirtualMachine
> @@ -203,10 +204,7 @@ class VmmTestingSetup:
>           except Exception as exc:
>               logger.error("Error on test teardown (%s)", exc)
>           finally:
> -            num_vfs = self.get_dut().get_current_vfs()
> -            self.get_dut().remove_vfs()
> -            self.get_dut().reset_provisioning(num_vfs)
> -            self.get_dut().cancel_work()
> +            self.get_dut().reset_sriov_config()
>   
>               if self.testing_config.unload_host_drivers_on_teardown:
>                   self.host.unload_drivers()
> @@ -256,6 +254,9 @@ def fixture_setup_vms(get_vmtb_config, get_cmdline_config, get_host, request):
>       tc: VmmTestingConfig = request.param
>       logger.debug(repr(tc))
>   
> +    # Pre-test cleanup: remove leftovers from crashed/failed/timed-out tests
> +    EnvironmentCleaner().cleanup()
> +
>       host: Host = get_host
>       ts: VmmTestingSetup = VmmTestingSetup(get_vmtb_config, get_cmdline_config, host, tc)
>   
> @@ -263,6 +264,10 @@ def fixture_setup_vms(get_vmtb_config, get_cmdline_config, get_host, request):
>       num_vfs = ts.testing_config.num_vfs
>       num_vms = ts.get_num_vms()
>   
> +    if device.get_current_vfs() != 0:
> +        logger.debug("Test environment is not clean - remove VFs remains")
> +        device.reset_sriov_config()
> +
>       logger.info('[Test setup: %sVF-%sVM]', num_vfs, num_vms)
>   
>       if ts.testing_config.provisioning_mode is VfProvisioningMode.VGPU_PROFILE:
> @@ -283,9 +288,6 @@ def fixture_setup_vms(get_vmtb_config, get_cmdline_config, get_host, request):
>           if tc.wa_reduce_vf_lmem and device.has_lmem():
>               ts.vgpu_profile.resources.vfLmem = org_vgpu_profile_vfLmem
>   
> -    else:
> -        device.driver.restore_auto_provisioning()
> -
>       assert num_vfs <= ts.total_vfs, f'Requested number of VFs ({num_vfs}) exceeds total VFs ({ts.total_vfs})'
>       device.create_vf(num_vfs)
>   
> @@ -320,9 +322,18 @@ def fixture_setup_vms(get_vmtb_config, get_cmdline_config, get_host, request):
>   @pytest.fixture(scope='function')
>   def create_1host_1vm(get_vmtb_config, get_cmdline_config, get_host):
>       num_vfs, num_vms = 1, 1
> +
> +    # Pre-test cleanup: remove leftovers from crashed/failed/timed-out tests
> +    EnvironmentCleaner().cleanup()
> +
>       ts: VmmTestingSetup = VmmTestingSetup(get_vmtb_config, get_cmdline_config, get_host,
>                                             VmmTestingConfig(num_vfs, num_vms))
>   
> +    device: Device = ts.get_dut()
> +    if device.get_current_vfs() != 0:
> +        logger.debug("Test environment is not clean - remove VFs remains")
> +        device.reset_sriov_config()
> +
>       logger.info('[Test setup: %sVF-%sVM]', num_vfs, num_vms)
>       logger.debug(repr(ts.testing_config))
>   
> @@ -336,9 +347,18 @@ def create_1host_1vm(get_vmtb_config, get_cmdline_config, get_host):
>   @pytest.fixture(scope='function')
>   def create_1host_2vm(get_vmtb_config, get_cmdline_config, get_host):
>       num_vfs, num_vms = 2, 2
> +
> +    # Pre-test cleanup: remove leftovers from crashed/failed/timed-out tests
> +    EnvironmentCleaner().cleanup()
> +
>       ts: VmmTestingSetup = VmmTestingSetup(get_vmtb_config, get_cmdline_config, get_host,
>                                             VmmTestingConfig(num_vfs, num_vms))
>   
> +    device: Device = ts.get_dut()
> +    if device.get_current_vfs() != 0:
> +        logger.debug("Test environment is not clean - remove VFs remains")
> +        device.reset_sriov_config()
> +
>       logger.info('[Test setup: %sVF-%sVM]', num_vfs, num_vms)
>       logger.debug(repr(ts.testing_config))
>   

  reply	other threads:[~2026-10-08 13:57 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28  7:25 [PATCH i-g-t 0/7] tools/vmtb: Sync VM Test Bench core with current xe SR-IOV Adam Miszczak
2026-09-28  7:25 ` [PATCH i-g-t 1/7] tools/vmtb: Introduce asynchronous VM state save/load functions Adam Miszczak
2026-10-08 13:53   ` Bernatowicz, Marcin
2026-09-28  7:25 ` [PATCH i-g-t 2/7] tools/vmtb: Rework VF scheduler provisioning Adam Miszczak
2026-10-08 13:54   ` Bernatowicz, Marcin
2026-09-28  7:25 ` [PATCH i-g-t 3/7] tools/vmtb: Align driver/device layer with current xe SR-IOV interfaces Adam Miszczak
2026-10-08 13:54   ` Bernatowicz, Marcin
2026-09-28  7:25 ` [PATCH i-g-t 4/7] tools/vmtb: Fix guest timeout handling and PCI BDF matching Adam Miszczak
2026-10-08 13:55   ` Bernatowicz, Marcin
2026-09-28  7:25 ` [PATCH i-g-t 5/7] tools/vmtb: Add pre-test environment cleanup Adam Miszczak
2026-10-08 13:55   ` Bernatowicz, Marcin [this message]
2026-09-28  7:25 ` [PATCH i-g-t 6/7] tools/vmtb: Add VM dmesg wait support Adam Miszczak
2026-10-08 13:56   ` Bernatowicz, Marcin
2026-09-28  7:26 ` [PATCH i-g-t 7/7] tools/vmtb: Extend IGT and WSIM executors Adam Miszczak
2026-10-08 12:59   ` Bernatowicz, Marcin
2026-09-28 20:28 ` ✓ i915.CI.BAT: success for tools/vmtb: Sync VM Test Bench core with current xe SR-IOV Patchwork
2026-09-28 20:47 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-29  2:19 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-09-29  4:08 ` ✗ i915.CI.Full: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2965da2b-6567-4574-9b86-a6a547f32fd8@linux.intel.com \
    --to=marcin.bernatowicz@linux.intel.com \
    --cc=adam.miszczak@linux.intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=kamil.konieczny@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.