| Message ID | 20220922210931.23982-4-pvorel@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tst_test.sh: Fix filesystem support detection | expand |
Hi! > Fixes: 1f6bd6e66 ("tst_test.sh: Add $TST_ALL_FILESYSTEMS") > > Reported-by: Martin Doucha <mdoucha@suse.cz> > Signed-off-by: Petr Vorel <pvorel@suse.cz> Looks like this is the only real fix in the series, right? The actual change looks good, but I do wonder what exactly has been broken, git grpe TST_SKIP_FILESYSTEMS does not show any real tests that would use that variable.
> Hi! > > Fixes: 1f6bd6e66 ("tst_test.sh: Add $TST_ALL_FILESYSTEMS") > > Reported-by: Martin Doucha <mdoucha@suse.cz> > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > Looks like this is the only real fix in the series, right? > The actual change looks good, but I do wonder what exactly has been > broken, git grpe TST_SKIP_FILESYSTEMS does not show any real tests > that would use that variable. Broken were actually all shell tests which did not use TST_SKIP_FILESYSTEMS: e.g. all tests in net_stress.ipsec_* did run whole filesystem check: tst_supported_fs_types.c:93: TINFO: Kernel supports ext2 tst_supported_fs_types.c:55: TINFO: mkfs.ext2 does exist tst_supported_fs_types.c:93: TINFO: Kernel supports ext3 tst_supported_fs_types.c:55: TINFO: mkfs.ext3 does exist tst_supported_fs_types.c:93: TINFO: Kernel supports ext4 tst_supported_fs_types.c:55: TINFO: mkfs.ext4 does exist tst_supported_fs_types.c:93: TINFO: Kernel supports xfs tst_supported_fs_types.c:55: TINFO: mkfs.xfs does exist tst_supported_fs_types.c:93: TINFO: Kernel supports btrfs tst_supported_fs_types.c:55: TINFO: mkfs.btrfs does exist tst_supported_fs_types.c:93: TINFO: Kernel supports vfat tst_supported_fs_types.c:55: TINFO: mkfs.vfat does exist tst_supported_fs_types.c:93: TINFO: Kernel supports exfat tst_supported_fs_types.c:55: TINFO: mkfs.exfat does exist tst_supported_fs_types.c:123: TINFO: FUSE does support ntfs tst_supported_fs_types.c:55: TINFO: mkfs.ntfs does exist tst_supported_fs_types.c:93: TINFO: Kernel supports tmpfs tst_supported_fs_types.c:42: TINFO: mkfs is not needed for tmpfs sctp_ipsec 1 TINFO: timeout per run is 0h 5m 0s sctp_ipsec 1 TINFO: IPsec[ah/transport] sctp_ipsec 1 TINFO: run server 'netstress -T sctp -S 10.0.0.1 -D ltp_ns_veth1 -R 500000 -B /tmp/LTP_sctp_ipsec.hC471AeJ9L' ... instead checking just filesystem in TMPDIR due empty $TST_FS_TYPE (I should have quoted it). + there are IMA tests had this + specific problem. Kind regards, Petr
Hi! > Broken were actually all shell tests which did not use TST_SKIP_FILESYSTEMS: > e.g. all tests in net_stress.ipsec_* did run whole filesystem check: > > tst_supported_fs_types.c:93: TINFO: Kernel supports ext2 > tst_supported_fs_types.c:55: TINFO: mkfs.ext2 does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports ext3 > tst_supported_fs_types.c:55: TINFO: mkfs.ext3 does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports ext4 > tst_supported_fs_types.c:55: TINFO: mkfs.ext4 does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports xfs > tst_supported_fs_types.c:55: TINFO: mkfs.xfs does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports btrfs > tst_supported_fs_types.c:55: TINFO: mkfs.btrfs does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports vfat > tst_supported_fs_types.c:55: TINFO: mkfs.vfat does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports exfat > tst_supported_fs_types.c:55: TINFO: mkfs.exfat does exist > tst_supported_fs_types.c:123: TINFO: FUSE does support ntfs > tst_supported_fs_types.c:55: TINFO: mkfs.ntfs does exist > tst_supported_fs_types.c:93: TINFO: Kernel supports tmpfs > tst_supported_fs_types.c:42: TINFO: mkfs is not needed for tmpfs > sctp_ipsec 1 TINFO: timeout per run is 0h 5m 0s > sctp_ipsec 1 TINFO: IPsec[ah/transport] > sctp_ipsec 1 TINFO: run server 'netstress -T sctp -S 10.0.0.1 -D ltp_ns_veth1 -R 500000 -B /tmp/LTP_sctp_ipsec.hC471AeJ9L' > ... > > instead checking just filesystem in TMPDIR due empty $TST_FS_TYPE (I should have > quoted it). > > + there are IMA tests had this + specific problem. Ah right, so it did useless checks and printed out a lot of lines too. Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
> Hi! > > Broken were actually all shell tests which did not use TST_SKIP_FILESYSTEMS: > > e.g. all tests in net_stress.ipsec_* did run whole filesystem check: > > tst_supported_fs_types.c:93: TINFO: Kernel supports ext2 > > tst_supported_fs_types.c:55: TINFO: mkfs.ext2 does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports ext3 > > tst_supported_fs_types.c:55: TINFO: mkfs.ext3 does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports ext4 > > tst_supported_fs_types.c:55: TINFO: mkfs.ext4 does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports xfs > > tst_supported_fs_types.c:55: TINFO: mkfs.xfs does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports btrfs > > tst_supported_fs_types.c:55: TINFO: mkfs.btrfs does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports vfat > > tst_supported_fs_types.c:55: TINFO: mkfs.vfat does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports exfat > > tst_supported_fs_types.c:55: TINFO: mkfs.exfat does exist > > tst_supported_fs_types.c:123: TINFO: FUSE does support ntfs > > tst_supported_fs_types.c:55: TINFO: mkfs.ntfs does exist > > tst_supported_fs_types.c:93: TINFO: Kernel supports tmpfs > > tst_supported_fs_types.c:42: TINFO: mkfs is not needed for tmpfs > > sctp_ipsec 1 TINFO: timeout per run is 0h 5m 0s > > sctp_ipsec 1 TINFO: IPsec[ah/transport] > > sctp_ipsec 1 TINFO: run server 'netstress -T sctp -S 10.0.0.1 -D ltp_ns_veth1 -R 500000 -B /tmp/LTP_sctp_ipsec.hC471AeJ9L' > > ... > > instead checking just filesystem in TMPDIR due empty $TST_FS_TYPE (I should have > > quoted it). > > + there are IMA tests had this + specific problem. > Ah right, so it did useless checks and printed out a lot of lines too. Also IMA fixes patchset needs this, otherwise it gets: tst_supported_fs.c:134: TCONF: tmpfs is skipped > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> Thanks! Kind regards, Petr
Hi all, Cyril, Li, thanks a lot for your review, merged! Kind regards, Petr
diff --git a/lib/newlib_tests/shell/tst_skip_filesystems_skip.sh b/lib/newlib_tests/shell/tst_skip_filesystems_skip.sh deleted file mode 100755 index 6748d021d..000000000 --- a/lib/newlib_tests/shell/tst_skip_filesystems_skip.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/sh -# SPDX-License-Identifier: GPL-2.0-or-later -# Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> - -TST_MOUNT_DEVICE=1 -TST_NEEDS_ROOT=1 -TST_FS_TYPE=ext4 -TST_TESTFUNC=test -TST_SKIP_FILESYSTEMS="ext4" - -test() -{ - tst_res TFAIL "test should be skipped with TCONF" -} - -. tst_test.sh -tst_run diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh index 33aadea91..01c01b79b 100644 --- a/testcases/lib/tst_test.sh +++ b/testcases/lib/tst_test.sh @@ -702,12 +702,6 @@ tst_run() [ "$TST_FORMAT_DEVICE" = 1 -o "$TST_ALL_FILESYSTEMS" = 1 ] && TST_NEEDS_DEVICE=1 [ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1 - if [ "$TST_ALL_FILESYSTEMS" != 1 ]; then - if ! tst_supported_fs -s "$TST_SKIP_FILESYSTEMS" $TST_FS_TYPE > /dev/null; then - tst_brk TCONF "$TST_FS_TYPE is skipped by the test" - fi - fi - if [ "$TST_NEEDS_DEVICE" = 1 ]; then TST_DEVICE=$(tst_device acquire) @@ -735,6 +729,14 @@ tst_run() cd "$TST_TMPDIR" fi + if [ "$TST_ALL_FILESYSTEMS" != 1 -a "$TST_SKIP_FILESYSTEMS" ]; then + if ! tst_supported_fs -s "$TST_SKIP_FILESYSTEMS" -d . > /dev/null; then + tst_brk TCONF "filesystem is not supported by the test" + fi + + tst_res TINFO "filesystem is supported by the test" + fi + [ -n "$TST_NEEDS_CHECKPOINTS" ] && _tst_init_checkpoints TST_MNTPOINT="${TST_MNTPOINT:-$PWD/mntpoint}"
Filesystem detection of locally used filesystem was broken on tests which did not use TST_ALL_FILESYSTEMS as it 1) expected used filesystem is $TST_FS_TYPE 2) this variable was not yet set. Also this check makes sense only if test defines TST_SKIP_FILESYSTEMS (to align with the condition in do_test_setup() in C API). Move filesystem check after (optional) cd "$TST_TMPDIR" (TMPDIR can have different filesystem). Remove tst_skip_filesystems_skip.sh test, as filesystem on tmpfs would have to be detected before. Fixes: 1f6bd6e66 ("tst_test.sh: Add $TST_ALL_FILESYSTEMS") Reported-by: Martin Doucha <mdoucha@suse.cz> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- .../shell/tst_skip_filesystems_skip.sh | 17 ----------------- testcases/lib/tst_test.sh | 14 ++++++++------ 2 files changed, 8 insertions(+), 23 deletions(-) delete mode 100755 lib/newlib_tests/shell/tst_skip_filesystems_skip.sh