Re: [Devel] [PATCH hci-8.1 v2 4/7] selftests/zram: fail when a step fails

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests/zram: unsupported lzo or missing mkfs now fails instead of skipping
--

commit 59f3632881739f08c8d19cb91d8f89fa150024b0
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

selftests/zram: fail when a step fails

This commit makes zram_lib.sh set ERR_CODE when a step prints FAIL. zram01.sh
and zram02.sh now exit 1 on failure. zram.sh now exits with the worst result
of the two tests: a failure over a skip over a pass.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-134205

> diff --git a/tools/testing/selftests/zram/zram01.sh b/tools/testing/selftests/zram/zram01.sh
> --- a/tools/testing/selftests/zram/zram01.sh
> +++ b/tools/testing/selftests/zram/zram01.sh

[ ... ]

> @@ -70,6 +73,7 @@ zram_cleanup
>
>  if [ $ERR_CODE -ne 0 ]; then
>  	echo "$TCID : [FAIL]"
> +	exit 1
>  else
>  	echo "$TCID : [PASS]"
>  fi

[Severity: Low]
This isn't a bug, but is it intended that this exit 1 also covers failures
caused by a missing capability?

Before this patch the FAIL lines in zram_lib.sh were only printed and the
test still passed. After this patch, a missing capability makes zram01.sh
exit 1, and zram.sh then exits non-zero. The two cases below are examples.

The first is zram_compress_alg(). zram01.sh hard-codes zram_algs="lzo":

> diff --git a/tools/testing/selftests/zram/zram_lib.sh b/tools/testing/selftests/zram/zram_lib.sh
> --- a/tools/testing/selftests/zram/zram_lib.sh
> +++ b/tools/testing/selftests/zram/zram_lib.sh

[ ... ]

> @@ -156,7 +158,7 @@ zram_compress_alg()
>  	for alg in $zram_algs; do
>  		local sys_path="/sys/block/zram${i}/comp_algorithm"
>  		echo "$alg" >	$sys_path || \
> -			echo "FAIL can't set '$alg' to $sys_path"
> +			{ echo "FAIL can't set '$alg' to $sys_path"; ERR_CODE=-1; }

If the running kernel does not provide lzo, this write fails and the test
now reports FAIL.

Would a skip (ksft_skip, exit 4) fit better here than a failure?

The second is zram_makefs(), which falls back to ext2 when mkfs.$fs is
not found:

> @@ -248,6 +253,7 @@ zram_makefs()
>  		if [ $? -ne 0 ]; then
>  			cat err.log
>  			echo "FAIL failed to make $fs on /dev/zram$i"
> +			ERR_CODE=-1
>  		fi

On a host with neither mkfs.ext4 nor mkfs.ext2, the mkfs command fails and
the test now fails.

Should this case skip rather than fail?

The only ksft_skip exits I found are in check_prereqs() (not root) and
zram_load() (module or zram-control problems). Nothing skips for an
unsupported algorithm or a missing mkfs tool.

The commit message says only that "A step that reports FAIL now fails its
test". It does not say that missing capabilities should now fail rather
than skip.

The commit message also does not mention the incidental typo fixes in
zram_lib.sh: $1 changed to $i in the mkswap and swapon messages, and
$max_stream changed to $max_streams in zram_max_streams().

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261027155236.1226178-1-eva.kurchatova@virtuozzo.com?part=4

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel