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 7/7] tools/vmtb: Extend IGT and WSIM executors
Date: Thu, 8 Oct 2026 14:59:59 +0200	[thread overview]
Message-ID: <c48614db-7dfc-45a7-9fa8-c440ae21e3ee@linux.intel.com> (raw)
In-Reply-To: <20260928072600.426902-8-adam.miszczak@linux.intel.com>

LGTM,

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

On 9/28/2026 9:26 AM, Adam Miszczak wrote:
> Introduce an optional 'env_vars' param (list) on IgtExecutor
> that injects 'igt_runner --environment KEY=VALUE' options,
> allowing environment variables to be passed to IGT test processes.
>
> Cache GemWsim ProcessResult (like IgtExecutor) so status()/wait()
> results are reused instead of re-fetched.
> Add a common clear_results() to both: GemWsim and IgtExecutor.
>
> Additionally, align test_migration with executors' changes:
> - use common IGT/WSIM clear_results() instead of instance checks.
> - call check_results() before failing so the IGT or WSIM
>    error output is logged on unexpected workload exit.
>
> Signed-off-by: Adam Miszczak <adam.miszczak@linux.intel.com>
> ---
>   tools/vmtb/bench/executors/gem_wsim.py | 38 +++++++++++++++++++++-----
>   tools/vmtb/bench/executors/igt.py      | 10 ++++++-
>   tools/vmtb/vmm_flows/test_migration.py | 22 +++++++++------
>   3 files changed, 54 insertions(+), 16 deletions(-)
>
> diff --git a/tools/vmtb/bench/executors/gem_wsim.py b/tools/vmtb/bench/executors/gem_wsim.py
> index 0cdfcc7e4..0510a9f9a 100644
> --- a/tools/vmtb/bench/executors/gem_wsim.py
> +++ b/tools/vmtb/bench/executors/gem_wsim.py
> @@ -8,7 +8,7 @@ from pathlib import Path
>   
>   from bench import exceptions
>   from bench.executors.shell import ShellExecutor
> -from bench.machines.machine_interface import DEFAULT_TIMEOUT, MachineInterface
> +from bench.machines.machine_interface import (DEFAULT_TIMEOUT, MachineInterface, ProcessResult)
>   
>   logger = logging.getLogger('GemWsim')
>   
> @@ -44,26 +44,45 @@ class GemWsim(ShellExecutor):
>   
>           wsim_path = Path(target.get_igt_config().test_dir) / 'benchmarks' / 'gem_wsim'
>           command = f'{wsim_path} -w {workload} -c {num_clients} -r {num_repeats}'
> +        self.proc_result = ProcessResult()
>   
>           super().__init__(target, command, timeout)
>   
>       def __str__(self) -> str:
>           return f'gem_wsim({self.machine_id}:{self.pid})'
>   
> +    # ShellExecutor overrides
> +    def status(self) -> ProcessResult:
> +        if self.proc_result.exited:
> +            return self.proc_result
> +
> +        self.proc_result = super().status()
> +        return self.proc_result
> +
> +    def wait(self) -> ProcessResult:
> +        if self.proc_result.exited:
> +            return self.proc_result
> +
> +        self.proc_result = super().wait()
> +        return self.proc_result
> +
> +    # IGT/WSIM specific methods
>       def is_running(self) -> bool:
>           return not self.status().exited
>   
>       def wait_results(self) -> GemWsimResult:
> -        proc_result = self.wait()
> -        if proc_result.exit_code == 0:
> -            logger.info('%s: %s', self, proc_result.stdout)
> +        if not self.proc_result.exited:
> +            self.proc_result = self.wait()
> +
> +        if self.proc_result.exit_code == 0:
> +            logger.info('%s: %s', self, self.proc_result.stdout)
>               # Try parse output ex.: 19.449s elapsed (102.836 workloads/s)
>               pattern = r'(?P<elapsed>\d+(\.\d*)?|\.\d+)s elapsed \((?P<wps>\d+(\.\d*)?|\.\d+) workloads/s\)'
> -            match = re.search(pattern, proc_result.stdout, re.MULTILINE)
> +            match = re.search(pattern, self.proc_result.stdout, re.MULTILINE)
>               if match:
>                   return GemWsimResult(float(match.group('elapsed')), float(match.group('wps')))
> -        raise exceptions.GemWsimError(f'{self}: exit_code: {proc_result.exit_code}'
> -                                      f' stdout: {proc_result.stdout} stderr: {proc_result.stderr}')
> +        raise exceptions.GemWsimError(f'{self}: exit_code: {self.proc_result.exit_code}'
> +                                      f' stdout: {self.proc_result.stdout} stderr: {self.proc_result.stderr}')
>   
>       def check_results(self) -> bool:
>           """Verify gem_wsim execution results. Return True for workload success, False on fail."""
> @@ -78,6 +97,11 @@ class GemWsim(ShellExecutor):
>                         self.target, wl_results.elapsed_sec, wl_results.workloads_per_sec)
>           return True
>   
> +    def clear_results(self) -> None:
> +        """Clear gem_wsim results cache."""
> +        self.proc_result = ProcessResult()
> +        self.machine_id = str(self.target)
> +
>   
>   def gem_wsim_parallel_exec_and_check(vms: typing.List[MachineInterface], workload: str, iterations: int,
>                                        expected: typing.Optional[GemWsimResult] = None) -> GemWsimResult:
> diff --git a/tools/vmtb/bench/executors/igt.py b/tools/vmtb/bench/executors/igt.py
> index f443fb99e..f4587c1fb 100644
> --- a/tools/vmtb/bench/executors/igt.py
> +++ b/tools/vmtb/bench/executors/igt.py
> @@ -119,6 +119,7 @@ class IgtExecutor(ExecutorInterface):
>       def __init__(self, target: MachineInterface,
>                    test: typing.Union[str, IgtType],
>                    num_repeats: int = 1,
> +                 env_vars: typing.Optional[typing.List[str]] = None,
>                    timeout: int = DEFAULT_TIMEOUT) -> None:
>           self.igt_config = target.get_igt_config()
>   
> @@ -126,7 +127,9 @@ class IgtExecutor(ExecutorInterface):
>           #ld_library_path = f'LD_LIBRARY_PATH={igt_config.lib_dir}'
>           runner = posixpath.join(self.igt_config.tool_dir, 'igt_runner')
>           testlist = '/tmp/igt_executor.testlist'
> -        command = f'{runner} {self.igt_config.options} ' \
> +        # Set test environment variables via igt_runner: --environment KEY=VALUE (or KEY)
> +        env_options = ''.join(f'--environment {env} ' for env in env_vars) if env_vars else ''
> +        command = f'{runner} {self.igt_config.options} {env_options}' \
>                     f'--test-list {testlist} {self.igt_config.test_dir} {self.igt_config.result_dir}'
>           self.results: typing.Dict[str, typing.Any] = {}
>           self.target: MachineInterface = target
> @@ -176,6 +179,11 @@ class IgtExecutor(ExecutorInterface):
>           logger.error("[%s] IGT failed: %s", self.target, self.proc_result)
>           return False
>   
> +    def clear_results(self) -> None:
> +        """Clear IGT results cache."""
> +        self.proc_result = ProcessResult()
> +        self.results.clear()
> +
>       def get_results_log(self) -> typing.Dict:
>           # Results are cached
>           if self.results:
> diff --git a/tools/vmtb/vmm_flows/test_migration.py b/tools/vmtb/vmm_flows/test_migration.py
> index 8a3f10d52..c13350fd2 100644
> --- a/tools/vmtb/vmm_flows/test_migration.py
> +++ b/tools/vmtb/vmm_flows/test_migration.py
> @@ -233,7 +233,9 @@ class BaseTestBusyMigration:
>           logger.debug("Execute migration in-flight workload on source VM")
>           migration_wl = run_source_workload
>           time.sleep(IGT_INIT_DELAY)
> -        assert migration_wl.is_running(), 'IGT/wsim migration workload is not running on source VM'
> +        if not migration_wl.is_running():
> +            migration_wl.check_results()
> +            assert False, 'IGT/wsim migration workload is not running on source VM'
>   
>           # Pause VM and save snapshot
>           logger.debug("Pause execution and save source VM state")
> @@ -267,10 +269,7 @@ class BaseTestBusyMigration:
>   
>           # Patch the source IgtExecutor/GemWsim instance with the current VM
>           migration_wl.target = vm_dst
> -        if isinstance(migration_wl, IgtExecutor):
> -            # Clear IGT test results cache - remove post-save source VM results
> -            # TODO: implement common IgtExecutor/GemWsim results clear interface to avoid instance type check
> -            migration_wl.results.clear()
> +        migration_wl.clear_results()
>   
>           # Load the source state snapshot
>           logger.debug("Restore source state on the destination VM")
> @@ -281,7 +280,10 @@ class BaseTestBusyMigration:
>           sync_value = random.randint(1, 0xFFFF)
>           assert vm_dst.ga.sync(sync_value)['return'] == sync_value
>   
> -        assert migration_wl.is_running(), 'IGT/wsim migration workload is not running on destination VM'
> +        if not migration_wl.is_running():
> +            migration_wl.check_results()
> +            assert False, 'IGT/wsim migration workload is not running on destination VM'
> +
>           time.sleep(IGT_RESTORE_DELAY)
>   
>           assert migration_wl.check_results(), 'VF migration workload failed on destination VM (post-restore)'
> @@ -824,7 +826,9 @@ class BaseTestDoubleMigration:
>           logger.debug("Execute throughout-migration workload on source VM")
>           migration_wl: GemWsim = run_source_workload
>           time.sleep(IGT_INIT_DELAY)
> -        assert migration_wl.is_running(), 'IGT/wsim migration workload is not running on source VM'
> +        if not migration_wl.is_running():
> +            migration_wl.check_results()
> +            assert False, 'IGT/wsim migration workload is not running on source VM'
>   
>           # Pause source VM and save snapshot
>           logger.debug("Pause execution and save source VM state (snapshot #1)")
> @@ -1113,7 +1117,9 @@ def test_2vm_pause_resume(create_1host_2vm):
>   
>       # Allow wsim WL to run some time
>       time.sleep(IGT_INIT_DELAY)
> -    assert gem_wsim_vm0.is_running()
> +    if not gem_wsim_vm0.is_running():
> +        gem_wsim_vm0.check_results()
> +        assert False, 'IGT/wsim workload is not running on VM0'
>   
>       logger.debug("Submit IGT WL (gem_spin_batch) on VM1")
>       igt_vm1 = IgtExecutor(vm1, IgtType.SPIN_BATCH)

  reply	other threads:[~2026-10-08 13:00 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
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 [this message]
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=c48614db-7dfc-45a7-9fa8-c440ae21e3ee@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.