Re: [PATCH 4/4] perf test: Simplify perf kvm record/report tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The synchronous `perf kvm record` command is not wrapped in an error-checking conditional, causing the script to unexpectedly abort via `set -e` upon failure.
--

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

perf test: Simplify perf kvm record/report tests

This commit simplifies the perf kvm tests by passing sleep directly as a
command line argument to perf kvm record, rather than running it in the
background and manually killing it.

> diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
> index f88e859025c42..4ca10388748de 100755
> --- a/tools/perf/tests/shell/kvm.sh
> +++ b/tools/perf/tests/shell/kvm.sh
[ ... ]
> @@ -63,13 +63,7 @@ test_kvm_record_report() {
>  	echo "Recording kvm profile for pid ${qemu_pid}..."
>  	# Use --host to avoid needing guest symbols/mounts for this simple test
>  	# We just want to verify the command runs and produces data
> -	# We run in background and kill it because 'perf kvm record' appends options
> -	# after the command, which breaks 'sleep' (e.g. it gets '-e cycles').
> -	perf kvm --host record -p "${qemu_pid}" -o "${perfdata}" &
> -	rec_pid=$!
> -	sleep 1
> -	kill -INT "${rec_pid}"
> -	wait "${rec_pid}" || true
> +	perf kvm --host record -p "${qemu_pid}" -o "${perfdata}" sleep 1

[Severity: Medium]
If this command fails, will the script abort immediately due to set -e being
enabled globally?

The previous implementation guarded against non-zero exit codes by using
wait "${rec_pid}" || true. Without an error-checking conditional or an
appended || true, a failure during this synchronous execution could cause the
script to instantly terminate.

Would this bypass the test's intended logic of recording the failure (err=1)
and accidentally skip subsequent tests like test_kvm_buildid_list?

>  
>  	echo "Reporting kvm profile..."
>  	# Check for some standard output from report

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