Re: [PATCH v4 3/9] KVM: selftests: Add timeout option in selftests runner

On Wed, Jul 29, 2026 at 12:39:06PM -0700, Sean Christopherson wrote:
> On Tue, Mar 31, 2026, Vipin Sharma wrote:
> > Add a command line argument in KVM selftest runner to limit amount of
> > time (seconds) given to a test for execution. Kill the test if it
> > exceeds the given timeout. Define a new SelftestStatus.TIMED_OUT to
> > denote a selftest final result. Add terminal color for status messages
> > of timed out tests.
> > 
> > Set the default value of 120 seconds for all tests.
> > 
> > Signed-off-by: Vipin Sharma <[email protected]>
> > ---
> >  .../testing/selftests/kvm/runner/__main__.py  |  9 +++++-
> >  .../testing/selftests/kvm/runner/selftest.py  | 29 ++++++++++++-------
> >  .../selftests/kvm/runner/test_runner.py       |  2 +-
> >  3 files changed, 27 insertions(+), 13 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/runner/__main__.py b/tools/testing/selftests/kvm/runner/__main__.py
> > index 9b6c78e69c64..82c543d11c34 100644
> > --- a/tools/testing/selftests/kvm/runner/__main__.py
> > +++ b/tools/testing/selftests/kvm/runner/__main__.py
> > @@ -37,6 +37,11 @@ def cli():
> >                          default=".",
> >                          help="Finds the test executables in the given path. Default is the current directory.")
> >  
> > +    parser.add_argument("--timeout",
> > +                        default=120,
> > +                        type=int,
> > +                        help="Timeout, in seconds, before runner kills the running test. (Default: 120 seconds)")
> > +
> >      return parser.parse_args()
> >  
> >  
> > @@ -44,6 +49,7 @@ def setup_logging():
> >      class TerminalColorFormatter(logging.Formatter):
> >          reset = "\033[0m"
> >          red_bold = "\033[31;1m"
> > +        red = "\033[31;1m"
> >          green = "\033[32m"
> >          yellow = "\033[33m"
> >          blue = "\033[34m"
> > @@ -52,7 +58,8 @@ def setup_logging():
> >              SelftestStatus.PASSED: green,
> >              SelftestStatus.NO_RUN: blue,
> >              SelftestStatus.SKIPPED: yellow,
> > -            SelftestStatus.FAILED: red_bold
> > +            SelftestStatus.FAILED: red_bold,
> > +            SelftestStatus.TIMED_OUT: red
> 
> Responding here because it's easier to see...
> 
> In my experience, uniquely identifying segfaults can be super helpful, e.g. (with
> the stderr/stdout changes too).

Sounds good to me, patch below looks fine.

> 
> diff --git a/tools/testing/selftests/kvm/runner/__main__.py b/tools/testing/selftests/kvm/runner/__main__.py
> index 13f4cbea1fa7..93ce46e25caf 100644
> --- a/tools/testing/selftests/kvm/runner/__main__.py
> +++ b/tools/testing/selftests/kvm/runner/__main__.py
> @@ -122,6 +122,7 @@ def setup_logging(args):
>              SelftestStatus.NO_RUN: blue,
>              SelftestStatus.SKIPPED: yellow,
>              SelftestStatus.FAILED: red_bold,
> +            SelftestStatus.SEGFAULT: red_bold,
>              SelftestStatus.TIMED_OUT: red
>          }
>  
> diff --git a/tools/testing/selftests/kvm/runner/selftest.py b/tools/testing/selftests/kvm/runner/selftest.py
> index 76c3e6d463ee..9ccdeef62a3e 100644
> --- a/tools/testing/selftests/kvm/runner/selftest.py
> +++ b/tools/testing/selftests/kvm/runner/selftest.py
> @@ -18,7 +18,8 @@ class SelftestStatus(enum.IntEnum):
>      NO_RUN = 22
>      SKIPPED = 23
>      FAILED = 24
> -    TIMED_OUT = 25
> +    SEGFAULT = 25
> +    TIMED_OUT = 26
>  
>      def __str__(self):
>          return str.__str__(self.name)
> @@ -63,6 +64,8 @@ class Selftest:
>                  self.status = SelftestStatus.PASSED
>              elif proc.returncode == 4:
>                  self.status = SelftestStatus.SKIPPED
> +            elif proc.returncode == 139:
> +                self.state = SelftestStatus.SEGFAULT
>              else:
>                  self.status = SelftestStatus.FAILED
>          except subprocess.TimeoutExpired as e:
> diff --git a/tools/testing/selftests/kvm/runner/test_runner.py b/tools/testing/selftests/kvm/runner/test_runner.py
> index 66a3c3711a8a..f06965c68520 100644
> --- a/tools/testing/selftests/kvm/runner/test_runner.py
> +++ b/tools/testing/selftests/kvm/runner/test_runner.py
> @@ -21,6 +21,7 @@ class TestRunner:
>          self.print_stds = {
>              SelftestStatus.PASSED: args.print_passed,
>              SelftestStatus.FAILED: args.print_failed,
> +            SelftestStatus.SEGFAULT: args.print_failed,
>              SelftestStatus.SKIPPED: args.print_skipped,
>              SelftestStatus.TIMED_OUT: args.print_timed_out,
>              SelftestStatus.NO_RUN: args.print_no_run
> @@ -38,6 +39,7 @@ class TestRunner:
>          print(f"\r\033[1mTotal: {self.tests_ran}/{len(self.tests)}" \
>                  f"\033[32;1m Passed: {self.status[SelftestStatus.PASSED]}" \
>                  f"\033[31;1m Failed: {self.status[SelftestStatus.FAILED]}" \
> +                f"\033[31;1m Segfaults: {self.status[SelftestStatus.SEGFAULT]}" \
>                  f"\033[33;1m Skipped: {self.status[SelftestStatus.SKIPPED]}"\
>                  f"\033[91;1m Timed Out: {self.status[SelftestStatus.TIMED_OUT]}"\
>                  f"\033[34;1m No Run: {self.status[SelftestStatus.NO_RUN]}\033[0m",
> @@ -47,10 +49,14 @@ class TestRunner:
>          print_level = self.print_stds.get(test_result.status, "full")
>  
>          print("\033[2K", end="\r", flush=True)
> -        if (print_level == "full" or print_level == "stdout"):
> +        if (print_level == "full"):
>              logger.info("*** stdout ***\n" + test_result.stdout)
> -        if (print_level == "full" or print_level == "stderr"):
>              logger.info("*** stderr ***\n" + test_result.stderr)
> +        elif (print_level == "stdout"):
> +            logger.info(test_result.stdout)
> +        elif (print_level == "stderr"):
> +            logger.info(test_result.stderr)
> +
>          if (print_level != "off"):
>              logger.log(test_result.status, f"[{test_result.status.name}] {test_result.test_path}")