[LTP] [PATCH 0/4] shell: fix regression since 1878502f6

[LTP] [PATCH 0/4] shell: fix regression since 1878502f6

[Petr Vorel]

Hi all,

various tests fails since 1878502f6:

TBROK: tst_brk can be called only with TBROK or TCONF

This should be merged before the release.
(or revert 1878502f6).

Kind regards,
Petr

Petr Vorel (4):
  tst_test.sh: Improve info on wrong tst_brk param
  commands: Fix tst_brk TFAIL
  kernel: Fix tst_brk TFAIL
  network: Fix tst_brk TFAIL

 testcases/commands/gzip/gzip_tests.sh         |  3 +-
 testcases/commands/mv/mv_tests.sh             |  3 +-
 .../cpuset/cpuset_regression_test.sh          | 18 +++++++----
 .../kernel/device-drivers/zram/zram01.sh      |  3 +-
 .../kernel/device-drivers/zram/zram_lib.sh    | 30 ++++++++++++-------
 .../fs/quota_remount/quota_remount_test01.sh  |  3 +-
 .../security/integrity/ima/tests/ima_tpm.sh   |  3 +-
 testcases/lib/tst_test.sh                     |  2 +-
 testcases/network/nfs/nfs_stress/nfs06.sh     |  5 +++-
 testcases/network/stress/route/route-lib.sh   |  8 +++--
 testcases/network/stress/ssh/ssh-stress.sh    | 16 +++++++---
 testcases/network/tcp_cmds/host/host01.sh     |  5 ++--
 .../network/tcp_cmds/ipneigh/ipneigh01.sh     | 21 ++++++++-----
 testcases/network/virt/virt_lib.sh            | 12 +++++---
 testcases/network/xinetd/xinetd_tests.sh      |  7 +++--
 15 files changed, 93 insertions(+), 46 deletions(-)

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/4] tst_test.sh: Improve info on wrong tst_brk param

[Petr Vorel]

Print wrong $res parameter in tst_brk helps searching wrong code.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_test.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 05a920156..0d2fccb95 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -135,7 +135,7 @@ tst_brk()
 	fi
 
 	if [ "$res" != TBROK -a "$res" != TCONF ]; then
-		tst_res TBROK "tst_brk can be called only with TBROK or TCONF"
+		tst_res TBROK "tst_brk can be called only with TBROK or TCONF ($res)"
 	else
 		tst_res "$res" "$@"
 	fi
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/4] commands: Fix tst_brk TFAIL

[Petr Vorel]

It needs to be replaced with tst_res TFAIL and return

Fixes: 1878502f6 ("tst_test.sh/tst_brk(): Allow only TBROK and TCONF")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/commands/gzip/gzip_tests.sh | 3 ++-
 testcases/commands/mv/mv_tests.sh     | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/testcases/commands/gzip/gzip_tests.sh b/testcases/commands/gzip/gzip_tests.sh
index fdc933ea2..3262c555b 100755
--- a/testcases/commands/gzip/gzip_tests.sh
+++ b/testcases/commands/gzip/gzip_tests.sh
@@ -82,7 +82,8 @@ test1()
 	gzip -r tst_gzip.tmp > tst_gzip.err 2>&1
 	if [ $? -ne 0 ]; then
 		cat tst_gzip.err
-		tst_brk TFAIL "Test #1: gzip -r failed"
+		tst_res TFAIL "Test #1: gzip -r failed"
+		return
 	fi
 
 	tst_res TINFO "Test #1: creating output file"
diff --git a/testcases/commands/mv/mv_tests.sh b/testcases/commands/mv/mv_tests.sh
index 91648dd8f..ae8b8701d 100755
--- a/testcases/commands/mv/mv_tests.sh
+++ b/testcases/commands/mv/mv_tests.sh
@@ -88,7 +88,8 @@ test1()
 	mv tst_mv.old tst_mv.new > tst_mv.err 2>&1
 	if [ $? -ne 0 ]; then
 		cat tst_mv.err
-		tst_brk TFAIL "Test #1: 'mv tst_mv.old tst_mv.new' failed"
+		tst_res TFAIL "Test #1: 'mv tst_mv.old tst_mv.new' failed"
+		return
 	fi
 
 	tst_res TINFO "Test #1: creating output file"
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/4] kernel: Fix tst_brk TFAIL

[Petr Vorel]

It needs to be replaced with tst_res TFAIL and return

Fixes: 1878502f6 ("tst_test.sh/tst_brk(): Allow only TBROK and TCONF")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../cpuset/cpuset_regression_test.sh          | 18 +++++++----
 .../kernel/device-drivers/zram/zram01.sh      |  3 +-
 .../kernel/device-drivers/zram/zram_lib.sh    | 30 ++++++++++++-------
 .../fs/quota_remount/quota_remount_test01.sh  |  3 +-
 .../security/integrity/ima/tests/ima_tpm.sh   |  3 +-
 5 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh b/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh
index a5757309f..bedc48110 100755
--- a/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh
+++ b/testcases/kernel/controllers/cpuset/cpuset_regression_test.sh
@@ -190,21 +190,27 @@ do_test()
 	ROD_SILENT mkdir ${root_cpuset_dir}/testdir
 
 	# Creat an exclusive cpuset.
-	echo 1 > ${root_cpuset_dir}/testdir/${cpu_exclusive}
-	[ $? -ne 0 ] && tst_brk TFAIL "'echo 1 > ${root_cpuset_dir}/testdir/${cpu_exclusive}' failed"
+	if ! echo 1 > ${root_cpuset_dir}/testdir/${cpu_exclusive}; then
+		tst_res TFAIL "'echo 1 > ${root_cpuset_dir}/testdir/${cpu_exclusive}' failed"
+		return
+	fi
 
 	cpu_exclusive_tmp=$(cat ${root_cpuset_dir}/testdir/${cpu_exclusive})
 	if [ "${cpu_exclusive_tmp}" != "1" ]; then
-		tst_brk TFAIL "${cpu_exclusive} is '${cpu_exclusive_tmp}', expected '1'"
+		tst_res TFAIL "${cpu_exclusive} is '${cpu_exclusive_tmp}', expected '1'"
+		return
 	fi
 
 	# This may trigger the kernel crash
-	echo 0 > ${root_cpuset_dir}/testdir/${cpus}
-	[ $? -ne 0 ] && tst_brk TFAIL "'echo 0 > ${root_cpuset_dir}/testdir/${cpus}' failed"
+	if ! echo 0 > ${root_cpuset_dir}/testdir/${cpus}; then
+		tst_res TFAIL "'echo 0 > ${root_cpuset_dir}/testdir/${cpus}' failed"
+		return
+	fi
 
 	cpus_value=$(cat ${root_cpuset_dir}/testdir/${cpus})
 	if [ "${cpus_value}" != "0" ]; then
-		tst_brk TFAIL "${cpus} is '${cpus_value}', expected '0'"
+		tst_res TFAIL "${cpus} is '${cpus_value}', expected '0'"
+		return
 	fi
 
 	tst_res TPASS "Bug is not reproducible"
diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
index 6bc305f2c..234bf06dd 100755
--- a/testcases/kernel/device-drivers/zram/zram01.sh
+++ b/testcases/kernel/device-drivers/zram/zram01.sh
@@ -82,7 +82,8 @@ zram_makefs()
 		mkfs.$fs /dev/zram$i > err.log 2>&1
 		if [ $? -ne 0 ]; then
 			cat err.log
-			tst_brk TFAIL "failed to make $fs on /dev/zram$i"
+			tst_res TFAIL "failed to make $fs on /dev/zram$i"
+			return
 		fi
 
 		i=$(($i + 1))
diff --git a/testcases/kernel/device-drivers/zram/zram_lib.sh b/testcases/kernel/device-drivers/zram/zram_lib.sh
index e94d7db11..e94f9244d 100755
--- a/testcases/kernel/device-drivers/zram/zram_lib.sh
+++ b/testcases/kernel/device-drivers/zram/zram_lib.sh
@@ -108,12 +108,16 @@ zram_max_streams()
 
 	for max_s in $zram_max_streams; do
 		local sys_path="/sys/block/zram${i}/max_comp_streams"
-		echo $max_s > $sys_path || \
-			tst_brk TFAIL "failed to set '$max_s' to $sys_path"
+		if ! echo $max_s > $sys_path; then
+			tst_res TFAIL "failed to set '$max_s' to $sys_path"
+			return
+		fi
 		local max_streams=$(cat $sys_path)
 
-		[ "$max_s" -ne "$max_streams" ] && \
-			tst_brk TFAIL "can't set max_streams '$max_s', get $max_stream"
+		if [ "$max_s" -ne "$max_streams" ]; then
+			tst_res TFAIL "can't set max_streams '$max_s', get $max_stream"
+			return
+		fi
 
 		i=$(($i + 1))
 		tst_res TINFO "$sys_path = '$max_streams'"
@@ -140,8 +144,10 @@ zram_compress_alg()
 	for i in $(seq $dev_start $dev_end); do
 		for alg in $algs; do
 			local sys_path="/sys/block/zram${i}/comp_algorithm"
-			echo "$alg" >  $sys_path || \
-				tst_brk TFAIL "can't set '$alg' to $sys_path"
+			if ! echo "$alg" >  $sys_path; then
+				tst_res TFAIL "can't set '$alg' to $sys_path"
+				return
+			fi
 			tst_res TINFO "$sys_path = '$alg'"
 		done
 	done
@@ -157,8 +163,10 @@ zram_set_disksizes()
 	tst_res TINFO "set disk size to zram device(s)"
 	for ds in $zram_sizes; do
 		local sys_path="/sys/block/zram${i}/disksize"
-		echo "$ds" >  $sys_path || \
-			tst_brk TFAIL "can't set '$ds' to $sys_path"
+		if ! echo "$ds" >  $sys_path; then
+			tst_res TFAIL "can't set '$ds' to $sys_path"
+			return
+		fi
 
 		i=$(($i + 1))
 		tst_res TINFO "$sys_path = '$ds'"
@@ -183,8 +191,10 @@ zram_set_memlimit()
 
 	for ds in $zram_mem_limits; do
 		local sys_path="/sys/block/zram${i}/mem_limit"
-		echo "$ds" >  $sys_path || \
-			tst_brk TFAIL "can't set '$ds' to $sys_path"
+		if ! echo "$ds" >  $sys_path; then
+			tst_res TFAIL "can't set '$ds' to $sys_path"
+			return
+		fi
 
 		i=$(($i + 1))
 		tst_res TINFO "$sys_path = '$ds'"
diff --git a/testcases/kernel/fs/quota_remount/quota_remount_test01.sh b/testcases/kernel/fs/quota_remount/quota_remount_test01.sh
index 25d9f8fcc..7e20b3608 100755
--- a/testcases/kernel/fs/quota_remount/quota_remount_test01.sh
+++ b/testcases/kernel/fs/quota_remount/quota_remount_test01.sh
@@ -67,7 +67,8 @@ do_test()
 	newblocks=$(get_blocks)
 
 	if [ $blocks -eq $newblocks ]; then
-	   tst_brk TFAIL "usage did not change after remount"
+	   tst_res TFAIL "usage did not change after remount"
+	   return
 	fi
 
 	tst_res TPASS "quota on remount passed"
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
index b675a20a1..2a28562e6 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
@@ -159,7 +159,8 @@ get_pcr10_aggregate()
 		$cmd > hash.txt 2>&1
 		ret=$?
 	elif [ $ret -ne 0 -a "$MISSING_EVMCTL" = 1 ]; then
-		tst_brk TFAIL "evmctl failed $msg"
+		tst_res TFAIL "evmctl failed $msg"
+		return
 	fi
 
 	[ $ret -ne 0 ] && tst_res TWARN "evmctl failed, trying to continue $msg"
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 4/4] network: Fix tst_brk TFAIL

[Petr Vorel]

It needs to be replaced with tst_res TFAIL and return

Fixes: 1878502f6 ("tst_test.sh/tst_brk(): Allow only TBROK and TCONF")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/network/nfs/nfs_stress/nfs06.sh     |  5 ++++-
 testcases/network/stress/route/route-lib.sh   |  8 +++++--
 testcases/network/stress/ssh/ssh-stress.sh    | 16 ++++++++++----
 testcases/network/tcp_cmds/host/host01.sh     |  5 +++--
 .../network/tcp_cmds/ipneigh/ipneigh01.sh     | 21 ++++++++++++-------
 testcases/network/virt/virt_lib.sh            | 12 +++++++----
 testcases/network/xinetd/xinetd_tests.sh      |  7 ++++---
 7 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/testcases/network/nfs/nfs_stress/nfs06.sh b/testcases/network/nfs/nfs_stress/nfs06.sh
index 560df05b1..09e55fe34 100755
--- a/testcases/network/nfs/nfs_stress/nfs06.sh
+++ b/testcases/network/nfs/nfs_stress/nfs06.sh
@@ -34,7 +34,10 @@ do_test()
 
 	tst_res TINFO "waiting for pids:$pids"
 	for p in $pids; do
-		wait $p || tst_brk TFAIL "fsstress process failed"
+		if ! wait $p; then
+			tst_res TFAIL "fsstress process failed"
+			return
+		fi
 		tst_res TINFO "fsstress '$p' completed"
 	done
 	pids=
diff --git a/testcases/network/stress/route/route-lib.sh b/testcases/network/stress/route/route-lib.sh
index 163c15423..29aa2e913 100644
--- a/testcases/network/stress/route/route-lib.sh
+++ b/testcases/network/stress/route/route-lib.sh
@@ -97,10 +97,14 @@ test_netlink()
 	tst_res TINFO "running $cmd $opt"
 	$cmd $opt || ret=$?
 	if [ "$ret" -ne 0 ]; then
-		[ $((ret & 3)) -ne 0 ] && \
-			tst_brk TFAIL "$cmd failed"
+		if [ $((ret & 3)) -ne 0 ]; then
+			tst_res TFAIL "$cmd failed"
+			return
+		fi
+
 		[ $((ret & 32)) -ne 0 ] && \
 			tst_brk TCONF "not supported configuration"
+
 		[ $((ret & 4)) -ne 0 ] && \
 			tst_res TWARN "$cmd has warnings"
 	fi
diff --git a/testcases/network/stress/ssh/ssh-stress.sh b/testcases/network/stress/ssh/ssh-stress.sh
index e7c4d45ce..c27c27a28 100755
--- a/testcases/network/stress/ssh/ssh-stress.sh
+++ b/testcases/network/stress/ssh/ssh-stress.sh
@@ -93,8 +93,10 @@ IdentityFile $TST_TMPDIR/id_rsa\n\" > $RHOST_SSH_CONF"
 
 test_ssh_connectivity()
 {
-	tst_rhost_run -c "$RHOST_SSH 'true >/dev/null 2>&1' >/dev/null"
-	[ $? -ne 0 ] && tst_brk TFAIL "SSH not reachable"
+	if ! tst_rhost_run -c "$RHOST_SSH 'true >/dev/null 2>&1' >/dev/null"; then
+		tst_res TFAIL "SSH not reachable"
+		return
+	fi
 }
 
 test1()
@@ -121,7 +123,10 @@ test1()
 		[ $? -ne 0 ] && num=$((num + 1))
 	done
 
-	[ $num -ne 0 ] && tst_brk TFAIL "$num ssh processes died unexpectedly during execution"
+	if [ $num -ne 0 ]; then
+		tst_res TFAIL "$num ssh processes died unexpectedly during execution"
+		return
+	fi
 
 	test_ssh_connectivity
 
@@ -216,7 +221,10 @@ test3()
 
 	# Setup an ssh tunnel from the remote host to testhost
 	tst_rhost_run -c "$RHOST_SSH -f -N -L $lport:$rhost:$port </dev/null >/dev/null 2>&1"
-	[ "$?" -ne 0 ] && tst_brk TFAIL "Failed to create an SSH session with port forwarding"
+	if [ "$?" -ne 0 ]; then
+		tst_res TFAIL "Failed to create an SSH session with port forwarding"
+		return
+	fi
 	RHOST_PIDS=$(tst_rhost_run -c "pgrep -f '^ssh .*$lport:$rhost:$port'")
 
 	# Start the TCP traffic clients
diff --git a/testcases/network/tcp_cmds/host/host01.sh b/testcases/network/tcp_cmds/host/host01.sh
index 6a4067495..9ed44bae3 100755
--- a/testcases/network/tcp_cmds/host/host01.sh
+++ b/testcases/network/tcp_cmds/host/host01.sh
@@ -21,11 +21,12 @@ do_test()
 	if addr=$(host $lhost); then
 		addr=$(echo "$addr" | grep address | tail -1 | awk '{print $NF}')
 		if [ -z "$addr" ]; then
-			tst_brk TFAIL "empty address"
+			tst_res TFAIL "empty address"
+			return
 		fi
 		EXPECT_PASS host $addr \>/dev/null
 	else
-		tst_brk TFAIL "host $lhost on local machine failed"
+		tst_res TFAIL "host $lhost on local machine failed"
 	fi
 }
 
diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
index e67ff5cc8..4b818357e 100755
--- a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
+++ b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
@@ -57,8 +57,10 @@ do_test()
 
 	for i in $(seq 1 $NUMLOOPS); do
 
-		ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) -I $(tst_iface) > /dev/null || \
-			tst_brk TFAIL "cannot ping $(tst_ipaddr rhost)"
+		if ! ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) -I $(tst_iface) > /dev/null; then
+			tst_res TFAIL "cannot ping $(tst_ipaddr rhost)"
+			return
+		fi
 
 		local k
 		local ret=1
@@ -66,19 +68,22 @@ do_test()
 			$SHOW_CMD | grep -q $(tst_ipaddr rhost)
 			if [ $? -eq 0 ]; then
 				ret=0
-				break;
+				break
 			fi
 			tst_sleep 100ms
 		done
 
-		[ "$ret" -ne 0 ] && \
-			tst_brk TFAIL "$entry_name entry '$(tst_ipaddr rhost)' not listed"
+		if [ "$ret" -ne 0 ]; then
+			tst_res TFAIL "$entry_name entry '$(tst_ipaddr rhost)' not listed"
+			return
+		fi
 
 		$DEL_CMD
 
-		$SHOW_CMD | grep -q "$(tst_ipaddr rhost).*$(tst_hwaddr rhost)" && \
-			tst_brk TFAIL "'$DEL_CMD' failed, entry has " \
-				"$(tst_hwaddr rhost)' $i/$NUMLOOPS"
+		if $SHOW_CMD | grep -q "$(tst_ipaddr rhost).*$(tst_hwaddr rhost)"; then
+			tst_res TFAIL "'$DEL_CMD' failed, entry has $(tst_hwaddr rhost)' $i/$NUMLOOPS"
+			return
+		fi
 	done
 
 	tst_res TPASS "verified adding/removing $entry_name cache entry"
diff --git a/testcases/network/virt/virt_lib.sh b/testcases/network/virt/virt_lib.sh
index d7e694eb7..0aadbfdb9 100644
--- a/testcases/network/virt/virt_lib.sh
+++ b/testcases/network/virt/virt_lib.sh
@@ -174,8 +174,10 @@ virt_multiple_add_test()
 	tst_res TINFO "add $NS_TIMES $virt_type, then delete"
 
 	for i in $(seq $start_id $max); do
-		virt_add ltp_v$i id $i $opt || \
-			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
+		if ! virt_add ltp_v$i id $i $opt; then
+			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
+			return
+		fi
 		ROD_SILENT "ip link set ltp_v$i up"
 	done
 
@@ -196,8 +198,10 @@ virt_add_delete_test()
 	tst_res TINFO "add/del $virt_type $NS_TIMES times"
 
 	for i in $(seq 0 $max); do
-		virt_add ltp_v0 $opt || \
-			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
+		if ! virt_add ltp_v0 $opt; then
+			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
+			return
+		fi
 		ROD_SILENT "ip link set ltp_v0 up"
 		ROD_SILENT "ip link delete ltp_v0"
 	done
diff --git a/testcases/network/xinetd/xinetd_tests.sh b/testcases/network/xinetd/xinetd_tests.sh
index 505dae5d7..25ec91d26 100755
--- a/testcases/network/xinetd/xinetd_tests.sh
+++ b/testcases/network/xinetd/xinetd_tests.sh
@@ -91,9 +91,10 @@ xinetd_test()
 
 	for a in $check_addr; do
 		p=$(echo $pattern | sed "s/ADDR/$a/")
-		echo '\x04' | telnet $a 2>&1 | grep -qiE "$p"
-		[ $? -ne 0 ] && \
-			tst_brk TFAIL "not expected output for 'telnet $a'"
+		if ! echo '\x04' | telnet $a 2>&1 | grep -qiE "$p"; then
+			tst_res TFAIL "not expected output for 'telnet $a'"
+			return
+		fi
 	done
 	tst_res TPASS "expected output with telnet $desc"
 }
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 0/4] shell: fix regression since 1878502f6

[Li Wang]

On Wed, Jan 24, 2024 at 12:27 AM Petr Vorel <pvorel@suse.cz> wrote:

> Hi all,
>
> various tests fails since 1878502f6:
>
> TBROK: tst_brk can be called only with TBROK or TCONF
>
> This should be merged before the release.
> (or revert 1878502f6).


Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/4] kernel: Fix tst_brk TFAIL

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
> index 6bc305f2c..234bf06dd 100755
> --- a/testcases/kernel/device-drivers/zram/zram01.sh
> +++ b/testcases/kernel/device-drivers/zram/zram01.sh
> @@ -82,7 +82,8 @@ zram_makefs()
>  		mkfs.$fs /dev/zram$i > err.log 2>&1
>  		if [ $? -ne 0 ]; then
>  			cat err.log
> -			tst_brk TFAIL "failed to make $fs on /dev/zram$i"
> +			tst_res TFAIL "failed to make $fs on /dev/zram$i"
> +			return

I do not think that this one is correct, unlike other tests the steps in
zram depends on each other if this fails and we attempt to continue we
will get failues from zram_mount and zram_fill_fs as well.

The zram tests are unfortunatelly messy in a sense that a testcase is
a setup for the next one so we really need to exit the whole test if one
of the testcases fails. I guess the clearest solution would be TFAIL
followed by a TBROK in this case. Something as:

	tst_res TFAIL "Failed to make $fs on /dev/zram$i"
	tst_brk TBROK "Can't continue with mounting the FS"


> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> index b675a20a1..2a28562e6 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> @@ -159,7 +159,8 @@ get_pcr10_aggregate()
>  		$cmd > hash.txt 2>&1
>  		ret=$?
>  	elif [ $ret -ne 0 -a "$MISSING_EVMCTL" = 1 ]; then
> -		tst_brk TFAIL "evmctl failed $msg"
> +		tst_res TFAIL "evmctl failed $msg"
> +		return
>  	fi

Again here, I'm not sure if it's safe to continue with the rest of the
test.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/4] kernel: Fix tst_brk TFAIL

[Petr Vorel]

> Hi!
> > diff --git a/testcases/kernel/device-drivers/zram/zram01.sh b/testcases/kernel/device-drivers/zram/zram01.sh
> > index 6bc305f2c..234bf06dd 100755
> > --- a/testcases/kernel/device-drivers/zram/zram01.sh
> > +++ b/testcases/kernel/device-drivers/zram/zram01.sh
> > @@ -82,7 +82,8 @@ zram_makefs()
> >  		mkfs.$fs /dev/zram$i > err.log 2>&1
> >  		if [ $? -ne 0 ]; then
> >  			cat err.log
> > -			tst_brk TFAIL "failed to make $fs on /dev/zram$i"
> > +			tst_res TFAIL "failed to make $fs on /dev/zram$i"
> > +			return

> I do not think that this one is correct, unlike other tests the steps in
> zram depends on each other if this fails and we attempt to continue we
> will get failues from zram_mount and zram_fill_fs as well.

> The zram tests are unfortunatelly messy in a sense that a testcase is
> a setup for the next one so we really need to exit the whole test if one
> of the testcases fails. I guess the clearest solution would be TFAIL
> followed by a TBROK in this case. Something as:

> 	tst_res TFAIL "Failed to make $fs on /dev/zram$i"
> 	tst_brk TBROK "Can't continue with mounting the FS"

You're right! I'll just slightly modify
	tst_brk TBROK "Can't continue with mounting $fs"


> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > index b675a20a1..2a28562e6 100755
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_tpm.sh
> > @@ -159,7 +159,8 @@ get_pcr10_aggregate()
> >  		$cmd > hash.txt 2>&1
> >  		ret=$?
> >  	elif [ $ret -ne 0 -a "$MISSING_EVMCTL" = 1 ]; then
> > -		tst_brk TFAIL "evmctl failed $msg"
> > +		tst_res TFAIL "evmctl failed $msg"
> > +		return
> >  	fi

> Again here, I'm not sure if it's safe to continue with the rest of the
> test.

Yes, this is safe, because the output of get_pcr10_aggregate() is later checked:

	get_pcr10_aggregate > tmp.txt
	pcr_aggregate="$(cat tmp.txt)"
	if [ -z "$pcr_aggregate" ]; then
		return
	fi

I made ima_tpm.sh quite hard to understand. I guess soon I should drop support
for older evmctl to make test easier to understand.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/4] kernel: Fix tst_brk TFAIL

[Cyril Hrubis]

Hi!
> > 	tst_res TFAIL "Failed to make $fs on /dev/zram$i"
> > 	tst_brk TBROK "Can't continue with mounting the FS"
> 
> You're right! I'll just slightly modify
> 	tst_brk TBROK "Can't continue with mounting $fs"

You can add my Reviewed-by: with this change.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] network: Fix tst_brk TFAIL

[Martin Doucha]

Hi,
two comments below.

On 23. 01. 24 17:26, Petr Vorel wrote:
> It needs to be replaced with tst_res TFAIL and return
> 
> Fixes: 1878502f6 ("tst_test.sh/tst_brk(): Allow only TBROK and TCONF")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   testcases/network/nfs/nfs_stress/nfs06.sh     |  5 ++++-
>   testcases/network/stress/route/route-lib.sh   |  8 +++++--
>   testcases/network/stress/ssh/ssh-stress.sh    | 16 ++++++++++----
>   testcases/network/tcp_cmds/host/host01.sh     |  5 +++--
>   .../network/tcp_cmds/ipneigh/ipneigh01.sh     | 21 ++++++++++++-------
>   testcases/network/virt/virt_lib.sh            | 12 +++++++----
>   testcases/network/xinetd/xinetd_tests.sh      |  7 ++++---
>   7 files changed, 50 insertions(+), 24 deletions(-)
> 
> diff --git a/testcases/network/nfs/nfs_stress/nfs06.sh b/testcases/network/nfs/nfs_stress/nfs06.sh
> index 560df05b1..09e55fe34 100755
> --- a/testcases/network/nfs/nfs_stress/nfs06.sh
> +++ b/testcases/network/nfs/nfs_stress/nfs06.sh
> @@ -34,7 +34,10 @@ do_test()
>   
>   	tst_res TINFO "waiting for pids:$pids"
>   	for p in $pids; do
> -		wait $p || tst_brk TFAIL "fsstress process failed"
> +		if ! wait $p; then
> +			tst_res TFAIL "fsstress process failed"
> +			return
> +		fi
>   		tst_res TINFO "fsstress '$p' completed"
>   	done
>   	pids=
> diff --git a/testcases/network/stress/route/route-lib.sh b/testcases/network/stress/route/route-lib.sh
> index 163c15423..29aa2e913 100644
> --- a/testcases/network/stress/route/route-lib.sh
> +++ b/testcases/network/stress/route/route-lib.sh
> @@ -97,10 +97,14 @@ test_netlink()
>   	tst_res TINFO "running $cmd $opt"
>   	$cmd $opt || ret=$?
>   	if [ "$ret" -ne 0 ]; then
> -		[ $((ret & 3)) -ne 0 ] && \
> -			tst_brk TFAIL "$cmd failed"
> +		if [ $((ret & 3)) -ne 0 ]; then
> +			tst_res TFAIL "$cmd failed"
> +			return
> +		fi
> +
>   		[ $((ret & 32)) -ne 0 ] && \
>   			tst_brk TCONF "not supported configuration"
> +
>   		[ $((ret & 4)) -ne 0 ] && \
>   			tst_res TWARN "$cmd has warnings"
>   	fi
> diff --git a/testcases/network/stress/ssh/ssh-stress.sh b/testcases/network/stress/ssh/ssh-stress.sh
> index e7c4d45ce..c27c27a28 100755
> --- a/testcases/network/stress/ssh/ssh-stress.sh
> +++ b/testcases/network/stress/ssh/ssh-stress.sh
> @@ -93,8 +93,10 @@ IdentityFile $TST_TMPDIR/id_rsa\n\" > $RHOST_SSH_CONF"
>   
>   test_ssh_connectivity()
>   {
> -	tst_rhost_run -c "$RHOST_SSH 'true >/dev/null 2>&1' >/dev/null"
> -	[ $? -ne 0 ] && tst_brk TFAIL "SSH not reachable"
> +	if ! tst_rhost_run -c "$RHOST_SSH 'true >/dev/null 2>&1' >/dev/null"; then
> +		tst_res TFAIL "SSH not reachable"
> +		return
> +	fi
>   }
>   
>   test1()
> @@ -121,7 +123,10 @@ test1()
>   		[ $? -ne 0 ] && num=$((num + 1))
>   	done
>   
> -	[ $num -ne 0 ] && tst_brk TFAIL "$num ssh processes died unexpectedly during execution"
> +	if [ $num -ne 0 ]; then
> +		tst_res TFAIL "$num ssh processes died unexpectedly during execution"
> +		return
> +	fi
>   
>   	test_ssh_connectivity
>   
> @@ -216,7 +221,10 @@ test3()
>   
>   	# Setup an ssh tunnel from the remote host to testhost
>   	tst_rhost_run -c "$RHOST_SSH -f -N -L $lport:$rhost:$port </dev/null >/dev/null 2>&1"
> -	[ "$?" -ne 0 ] && tst_brk TFAIL "Failed to create an SSH session with port forwarding"
> +	if [ "$?" -ne 0 ]; then
> +		tst_res TFAIL "Failed to create an SSH session with port forwarding"
> +		return
> +	fi
>   	RHOST_PIDS=$(tst_rhost_run -c "pgrep -f '^ssh .*$lport:$rhost:$port'")
>   
>   	# Start the TCP traffic clients
> diff --git a/testcases/network/tcp_cmds/host/host01.sh b/testcases/network/tcp_cmds/host/host01.sh
> index 6a4067495..9ed44bae3 100755
> --- a/testcases/network/tcp_cmds/host/host01.sh
> +++ b/testcases/network/tcp_cmds/host/host01.sh
> @@ -21,11 +21,12 @@ do_test()
>   	if addr=$(host $lhost); then
>   		addr=$(echo "$addr" | grep address | tail -1 | awk '{print $NF}')
>   		if [ -z "$addr" ]; then
> -			tst_brk TFAIL "empty address"
> +			tst_res TFAIL "empty address"
> +			return
>   		fi
>   		EXPECT_PASS host $addr \>/dev/null
>   	else
> -		tst_brk TFAIL "host $lhost on local machine failed"
> +		tst_res TFAIL "host $lhost on local machine failed"
>   	fi
>   }
>   
> diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> index e67ff5cc8..4b818357e 100755
> --- a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> +++ b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> @@ -57,8 +57,10 @@ do_test()
>   
>   	for i in $(seq 1 $NUMLOOPS); do
>   
> -		ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) -I $(tst_iface) > /dev/null || \
> -			tst_brk TFAIL "cannot ping $(tst_ipaddr rhost)"
> +		if ! ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) -I $(tst_iface) > /dev/null; then
> +			tst_res TFAIL "cannot ping $(tst_ipaddr rhost)"
> +			return
> +		fi
>   
>   		local k
>   		local ret=1
> @@ -66,19 +68,22 @@ do_test()
>   			$SHOW_CMD | grep -q $(tst_ipaddr rhost)
>   			if [ $? -eq 0 ]; then
>   				ret=0
> -				break;
> +				break
>   			fi
>   			tst_sleep 100ms
>   		done
>   
> -		[ "$ret" -ne 0 ] && \
> -			tst_brk TFAIL "$entry_name entry '$(tst_ipaddr rhost)' not listed"
> +		if [ "$ret" -ne 0 ]; then
> +			tst_res TFAIL "$entry_name entry '$(tst_ipaddr rhost)' not listed"
> +			return
> +		fi
>   
>   		$DEL_CMD
>   
> -		$SHOW_CMD | grep -q "$(tst_ipaddr rhost).*$(tst_hwaddr rhost)" && \
> -			tst_brk TFAIL "'$DEL_CMD' failed, entry has " \
> -				"$(tst_hwaddr rhost)' $i/$NUMLOOPS"
> +		if $SHOW_CMD | grep -q "$(tst_ipaddr rhost).*$(tst_hwaddr rhost)"; then
> +			tst_res TFAIL "'$DEL_CMD' failed, entry has $(tst_hwaddr rhost)' $i/$NUMLOOPS"
> +			return
> +		fi
>   	done
>   
>   	tst_res TPASS "verified adding/removing $entry_name cache entry"
> diff --git a/testcases/network/virt/virt_lib.sh b/testcases/network/virt/virt_lib.sh
> index d7e694eb7..0aadbfdb9 100644
> --- a/testcases/network/virt/virt_lib.sh
> +++ b/testcases/network/virt/virt_lib.sh
> @@ -174,8 +174,10 @@ virt_multiple_add_test()
>   	tst_res TINFO "add $NS_TIMES $virt_type, then delete"
>   
>   	for i in $(seq $start_id $max); do
> -		virt_add ltp_v$i id $i $opt || \
> -			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
> +		if ! virt_add ltp_v$i id $i $opt; then
> +			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
> +			return

This is a library function so tst_brk TBROK is probably the correct fix 
here. Also, the message should say ltp_v$i instead of ltp_v0, but that's 
for a separate patch.

> +		fi
>   		ROD_SILENT "ip link set ltp_v$i up"
>   	done
>   
> @@ -196,8 +198,10 @@ virt_add_delete_test()
>   	tst_res TINFO "add/del $virt_type $NS_TIMES times"
>   
>   	for i in $(seq 0 $max); do
> -		virt_add ltp_v0 $opt || \
> -			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
> +		if ! virt_add ltp_v0 $opt; then
> +			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
> +			return

Same here.

> +		fi
>   		ROD_SILENT "ip link set ltp_v0 up"
>   		ROD_SILENT "ip link delete ltp_v0"
>   	done
> diff --git a/testcases/network/xinetd/xinetd_tests.sh b/testcases/network/xinetd/xinetd_tests.sh
> index 505dae5d7..25ec91d26 100755
> --- a/testcases/network/xinetd/xinetd_tests.sh
> +++ b/testcases/network/xinetd/xinetd_tests.sh
> @@ -91,9 +91,10 @@ xinetd_test()
>   
>   	for a in $check_addr; do
>   		p=$(echo $pattern | sed "s/ADDR/$a/")
> -		echo '\x04' | telnet $a 2>&1 | grep -qiE "$p"
> -		[ $? -ne 0 ] && \
> -			tst_brk TFAIL "not expected output for 'telnet $a'"
> +		if ! echo '\x04' | telnet $a 2>&1 | grep -qiE "$p"; then
> +			tst_res TFAIL "not expected output for 'telnet $a'"
> +			return
> +		fi
>   	done
>   	tst_res TPASS "expected output with telnet $desc"
>   }

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/4] network: Fix tst_brk TFAIL

[Petr Vorel]

Hi Martin,

> Hi,
> two comments below.

...
> > +++ b/testcases/network/virt/virt_lib.sh
> > @@ -174,8 +174,10 @@ virt_multiple_add_test()
> >   	tst_res TINFO "add $NS_TIMES $virt_type, then delete"
> >   	for i in $(seq $start_id $max); do
> > -		virt_add ltp_v$i id $i $opt || \
> > -			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
> > +		if ! virt_add ltp_v$i id $i $opt; then
> > +			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
> > +			return

> This is a library function so tst_brk TBROK is probably the correct fix
> here. Also, the message should say ltp_v$i instead of ltp_v0, but that's for
> a separate patch.

Thanks! tst_brk TBROK makes sense here.

ltp_v$i fixed in separate commit with your credit (merged now).

> > +		fi
> >   		ROD_SILENT "ip link set ltp_v$i up"
> >   	done
> > @@ -196,8 +198,10 @@ virt_add_delete_test()
> >   	tst_res TINFO "add/del $virt_type $NS_TIMES times"
> >   	for i in $(seq 0 $max); do
> > -		virt_add ltp_v0 $opt || \
> > -			tst_brk TFAIL "failed to create 'ltp_v0 $opt'"
> > +		if ! virt_add ltp_v0 $opt; then
> > +			tst_res TFAIL "failed to create 'ltp_v0 $opt'"
> > +			return

> Same here.

Yes (tst_brk TBROK).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/4] kernel: Fix tst_brk TFAIL

[Petr Vorel]

Hi Cyril, all,

> Hi!
> > > 	tst_res TFAIL "Failed to make $fs on /dev/zram$i"
> > > 	tst_brk TBROK "Can't continue with mounting the FS"

> > You're right! I'll just slightly modify
> > 	tst_brk TBROK "Can't continue with mounting $fs"

> You can add my Reviewed-by: with this change.

I checked that the other functions should not depend on each other, so I dared
to merge the patchset. We still have few days to check the result in CI if I'm
wrong.

Thanks!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp