Re: [PATCH v3 6/6] perf test: Extend perf kvm tests to check default event

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing explicit error handling for the `perf kvm record` command in `test_kvm_default_event` causes an ungraceful script abort on failure.
--

commit 0b371c3387f66fb432d87052c8a94cad8f1c734c
Author: Namhyung Kim <namhyung@kernel.org>

perf test: Extend perf kvm tests to check default event

This commit extends the perf kvm tests by adding a check to verify that
perf kvm record can successfully handle command line arguments like
'sleep 1'.

> diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
> index f88e859025c42..ad208abce0a99 100755
> --- a/tools/perf/tests/shell/kvm.sh
> +++ b/tools/perf/tests/shell/kvm.sh
[ ... ]
> @@ -123,6 +123,15 @@ test_kvm_stat_live() {
>  	echo "perf kvm stat live test [Success]"
>  }
>  
> +test_kvm_default_event() {
> +	echo "Testing perf kvm record default event with command line"
> +
> +	# Check if kvm record with default events handle command line arguments
> +	perf kvm record -p "${qemu_pid}" -o /dev/null sleep 1

[Severity: Low]
Since this script runs with set -e enabled globally, could a failure in this
perf command cause an immediate script abort rather than a graceful test
failure?

Other tests in this file appear to use a conditional error check pattern
(if ! ...; then err=1; return; fi) to catch failures and report them properly
without triggering the EXIT trap prematurely. Would it be better to wrap this
command with similar error handling?

> +
> +	echo "perf kvm record default event [Success]"
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701194153.401218-1-namhyung@kernel.org?part=6