On Thu, Mar 06, 2025 at 08:17:41AM +0000, Nirjhar Roy (IBM) wrote: > Silently executing scripts during sourcing common/rc doesn't look good > and also causes unnecessary script execution. Decouple init_rc() call > and call init_rc() explicitly where required. > > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@xxxxxxxxx> FWIW, I've just done somethign similar for check-parallel. I need to decouple common/config from common/rc and not run any code from either common/config or common/rc. I've included the patch below (it won't apply because there's all sorts of refactoring for test list and config-section parsing in the series before it), but it should give you an idea of how I think we should be separating one-off initialisation environment varaibles, common code inclusion and the repeated initialisation of section specific parameters.... ..... > diff --git a/soak b/soak > index d5c4229a..5734d854 100755 > --- a/soak > +++ b/soak > @@ -5,6 +5,7 @@ > > # get standard environment, filters and checks > . ./common/rc > +# ToDo: Do we need an init_rc() here? How is soak used? > . ./common/filter I've also go a patch series that removes all these old 2000-era SGI QE scripts that have not been used by anyone for the last 15 years. I did that to get rid of the technical debt that these scripts have gathered over years of neglect. They aren't used, we shouldn't even attempt to maintain them anymore. -Dave. -- Dave Chinner david@xxxxxxxxxxxxx fstests: separate sourcing common/rc and common/config from initialisation From: Dave Chinner <dchinner@xxxxxxxxxx> The sourcing of common/rc causes code to be run. init_rc is executed from common/rc, and it includes common/config which also runs a couple of initialisation functions. This is messy, because re-sourcing those files also does an awful lot more setup work than running those a couple of init functions. common/config only needs to be included once - everything that scripts then depend on should be exported by it, and hence it should only be included once from check/check-parallel to set up all the environmental parameters for the entire run. common/rc also only needs to be included once per context, but it does not need to directly include common config nor does it need to run init_rc in each individual test context. Seperate out this mess. Include common/config directly where needed and only use it to set up the environment. Move all the code that is in common/config to common/rc so that common/config is not needed for any purpose other than setting up the initial environment. Move the initialisation functions to the scripts that include common/config. Config file and config section parsing can be run directly from check and/or check-parallel; this is not needed for every context that needs to know how what XFS_MKFS_PROG is set to... Similarly, include common/rc only once, and only call init_rc or _source_specific_fs() from the contexts that actually need that code to be run. Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> --- check | 24 +++--- common/config | 218 -------------------------------------------------- common/preamble | 1 + common/rc | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++---- tests/generic/367 | 7 +- tests/generic/749 | 3 +- 6 files changed, 235 insertions(+), 252 deletions(-) diff --git a/check b/check index b4d31d138..b968a134a 100755 --- a/check +++ b/check @@ -43,10 +43,12 @@ timestamp=${TIMESTAMP:=false} rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist -# We need to include the test list processing first as argument parsing -# requires test list parsing and setup. +. ./common/config +. ./common/config-sections +. ./common/rc . ./common/test_list + usage() { echo "Usage: $0 [options] [testlist]"' @@ -183,11 +185,15 @@ while [ $# -gt 0 ]; do shift done -# we need common/rc, that also sources common/config. We need to source it -# after processing args, overlay needs FSTYP set before sourcing common/config -if ! . ./common/rc; then - echo "check: failed to source common/rc" - exit 1 +# now we have done argument parsing, overlay has FSTYP set and we can now +# start processing the config files and setting up devices. +_config_section_setup +_canonicalize_devices +init_rc + +if [ ! -z "$REPORT_LIST" ]; then + . ./common/report + _assert_report_list fi # If the test config specified a soak test duration, see if there are any @@ -601,10 +607,6 @@ function run_section() status=1 exit fi - # Previous FSTYP derived from TEST_DEV could be changed, source - # common/rc again with correct FSTYP to get FSTYP specific configs, - # e.g. common/xfs - . common/rc _tl_prepare_test_list elif [ "$OLD_TEST_FS_MOUNT_OPTS" != "$TEST_FS_MOUNT_OPTS" ]; then _test_unmount 2> /dev/null diff --git a/common/config b/common/config index 571e52a58..03970bf54 100644 --- a/common/config +++ b/common/config @@ -40,7 +40,6 @@ # . common/test_names -. common/config-sections # all tests should use a common language setting to prevent golden # output mismatches. @@ -348,220 +347,3 @@ if [ -x /usr/sbin/selinuxenabled ] && /usr/sbin/selinuxenabled; then export SELINUX_MOUNT_OPTIONS fi -_common_mount_opts() -{ - case $FSTYP in - 9p) - echo $PLAN9_MOUNT_OPTIONS - ;; - fuse) - echo $FUSE_MOUNT_OPTIONS - ;; - xfs) - echo $XFS_MOUNT_OPTIONS - ;; - udf) - echo $UDF_MOUNT_OPTIONS - ;; - nfs) - echo $NFS_MOUNT_OPTIONS - ;; - afs) - echo $AFS_MOUNT_OPTIONS - ;; - cifs) - echo $CIFS_MOUNT_OPTIONS - ;; - ceph) - echo $CEPHFS_MOUNT_OPTIONS - ;; - glusterfs) - echo $GLUSTERFS_MOUNT_OPTIONS - ;; - overlay) - echo $OVERLAY_MOUNT_OPTIONS - ;; - ext2|ext3|ext4) - # acls & xattrs aren't turned on by default on ext$FOO - echo "-o acl,user_xattr $EXT_MOUNT_OPTIONS" - ;; - f2fs) - echo "-o acl,user_xattr $F2FS_MOUNT_OPTIONS" - ;; - reiserfs) - # acls & xattrs aren't turned on by default on reiserfs - echo "-o acl,user_xattr $REISERFS_MOUNT_OPTIONS" - ;; - reiser4) - # acls & xattrs aren't supported by reiser4 - echo $REISER4_MOUNT_OPTIONS - ;; - gfs2) - # acls aren't turned on by default on gfs2 - echo "-o acl $GFS2_MOUNT_OPTIONS" - ;; - tmpfs) - # We need to specify the size at mount, use 1G by default - echo "-o size=1G $TMPFS_MOUNT_OPTIONS" - ;; - ubifs) - echo $UBIFS_MOUNT_OPTIONS - ;; - *) - ;; - esac -} - -_mount_opts() -{ - export MOUNT_OPTIONS=$(_common_mount_opts) -} - -_test_mount_opts() -{ - export TEST_FS_MOUNT_OPTS=$(_common_mount_opts) -} - -_mkfs_opts() -{ - case $FSTYP in - xfs) - export MKFS_OPTIONS=$XFS_MKFS_OPTIONS - ;; - udf) - [ ! -z "$udf_fsize" ] && \ - UDF_MKFS_OPTIONS="$UDF_MKFS_OPTIONS -s $udf_fsize" - export MKFS_OPTIONS=$UDF_MKFS_OPTIONS - ;; - nfs) - export MKFS_OPTIONS=$NFS_MKFS_OPTIONS - ;; - afs) - export MKFS_OPTIONS=$AFS_MKFS_OPTIONS - ;; - cifs) - export MKFS_OPTIONS=$CIFS_MKFS_OPTIONS - ;; - ceph) - export MKFS_OPTIONS=$CEPHFS_MKFS_OPTIONS - ;; - reiserfs) - export MKFS_OPTIONS="$REISERFS_MKFS_OPTIONS -q" - ;; - reiser4) - export MKFS_OPTIONS=$REISER4_MKFS_OPTIONS - ;; - gfs2) - export MKFS_OPTIONS="$GFS2_MKFS_OPTIONS -O -p lock_nolock" - ;; - jfs) - export MKFS_OPTIONS="$JFS_MKFS_OPTIONS -q" - ;; - f2fs) - export MKFS_OPTIONS="$F2FS_MKFS_OPTIONS" - ;; - btrfs) - export MKFS_OPTIONS="$BTRFS_MKFS_OPTIONS" - ;; - bcachefs) - export MKFS_OPTIONS=$BCACHEFS_MKFS_OPTIONS - ;; - *) - ;; - esac -} - -_fsck_opts() -{ - case $FSTYP in - ext2|ext3|ext4) - export FSCK_OPTIONS="-nf" - ;; - reiser*) - export FSCK_OPTIONS="--yes" - ;; - f2fs) - export FSCK_OPTIONS="" - ;; - *) - export FSCK_OPTIONS="-n" - ;; - esac -} - -# check necessary running dependences then source sepcific fs helpers -_source_specific_fs() -{ - local fs=$1 - - if [ -z "$fs" ];then - fs=$FSTYP - fi - - case "$fs" in - xfs) - [ "$XFS_LOGPRINT_PROG" = "" ] && _fatal "xfs_logprint not found" - [ "$XFS_REPAIR_PROG" = "" ] && _fatal "xfs_repair not found" - [ "$XFS_DB_PROG" = "" ] && _fatal "xfs_db not found" - [ "$MKFS_XFS_PROG" = "" ] && _fatal "mkfs_xfs not found" - [ "$XFS_INFO_PROG" = "" ] && _fatal "xfs_info not found" - - . ./common/xfs - ;; - udf) - [ "$MKFS_UDF_PROG" = "" ] && _fatal "mkfs_udf/mkudffs not found" - ;; - btrfs) - [ "$MKFS_BTRFS_PROG" = "" ] && _fatal "mkfs.btrfs not found" - - . ./common/btrfs - ;; - ext4) - [ "$MKFS_EXT4_PROG" = "" ] && _fatal "mkfs.ext4 not found" - . ./common/ext4 - ;; - ext2|ext3) - . ./common/ext4 - ;; - f2fs) - [ "$MKFS_F2FS_PROG" = "" ] && _fatal "mkfs.f2fs not found" - ;; - nfs) - . ./common/nfs - ;; - afs) - ;; - cifs) - ;; - 9p) - ;; - fuse) - ;; - ceph) - . ./common/ceph - ;; - glusterfs) - ;; - overlay) - . ./common/overlay - ;; - reiser4) - [ "$MKFS_REISER4_PROG" = "" ] && _fatal "mkfs.reiser4 not found" - ;; - pvfs2) - ;; - ubifs) - [ "$UBIUPDATEVOL_PROG" = "" ] && _fatal "ubiupdatevol not found" - . ./common/ubifs - ;; - esac -} - -_config_section_setup -_canonicalize_devices -# mkfs.xfs checks for TEST_DEV before permitting < 300M filesystems. TEST_DIR -# and QA_CHECK_FS are also checked by mkfs.xfs, but already exported elsewhere. -export TEST_DEV - -# make sure this script returns success -/bin/true diff --git a/common/preamble b/common/preamble index 0c9ee2e03..1f40dd5d1 100644 --- a/common/preamble +++ b/common/preamble @@ -50,6 +50,7 @@ _begin_fstest() _register_cleanup _cleanup . ./common/rc + _source_specific_fs $FSTYP # remove previous $seqres.full before test rm -f $seqres.full $seqres.hints diff --git a/common/rc b/common/rc index 056112714..01f8cba2e 100644 --- a/common/rc +++ b/common/rc @@ -2,10 +2,11 @@ # SPDX-License-Identifier: GPL-2.0+ # Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved. -. common/config - BC="$(type -P bc)" || BC= +# make sure we have a standard umask +umask 022 + # Don't use sync(1) directly if at all possible. In most cases we only need to # sync the fs under test, so we use syncfs if it is supported to prevent # disturbance of other tests that may be running concurrently. @@ -250,17 +251,6 @@ _log_err() echo "(see $seqres.full for details)" } -# make sure we have a standard umask -umask 022 - -# check for correct setup and source the $FSTYP specific functions now -_source_specific_fs $FSTYP - -if [ ! -z "$REPORT_LIST" ]; then - . ./common/report - _assert_report_list -fi - _get_filesize() { stat -c %s "$1" @@ -4895,6 +4885,8 @@ init_rc() exit 1 fi + _source_specific_fs $FSTYP + # if $TEST_DEV is not mounted, mount it now as XFS if [ -z "`_fs_type $TEST_DEV`" ] then @@ -4934,6 +4926,11 @@ init_rc() # it is supported. $XFS_IO_PROG -i -c quit 2>/dev/null && \ export XFS_IO_PROG="$XFS_IO_PROG -i" + + # mkfs.xfs checks for TEST_DEV before permitting < 300M filesystems. + # TEST_DIR and QA_CHECK_FS are also checked by mkfs.xfs, but already + # exported elsewhere. + export TEST_DEV } # get real device path name by following link @@ -5757,8 +5754,211 @@ _require_program() { _have_program "$1" || _notrun "$tag required" } -init_rc +_common_mount_opts() +{ + case $FSTYP in + 9p) + echo $PLAN9_MOUNT_OPTIONS + ;; + fuse) + echo $FUSE_MOUNT_OPTIONS + ;; + xfs) + echo $XFS_MOUNT_OPTIONS + ;; + udf) + echo $UDF_MOUNT_OPTIONS + ;; + nfs) + echo $NFS_MOUNT_OPTIONS + ;; + afs) + echo $AFS_MOUNT_OPTIONS + ;; + cifs) + echo $CIFS_MOUNT_OPTIONS + ;; + ceph) + echo $CEPHFS_MOUNT_OPTIONS + ;; + glusterfs) + echo $GLUSTERFS_MOUNT_OPTIONS + ;; + overlay) + echo $OVERLAY_MOUNT_OPTIONS + ;; + ext2|ext3|ext4) + # acls & xattrs aren't turned on by default on ext$FOO + echo "-o acl,user_xattr $EXT_MOUNT_OPTIONS" + ;; + f2fs) + echo "-o acl,user_xattr $F2FS_MOUNT_OPTIONS" + ;; + reiserfs) + # acls & xattrs aren't turned on by default on reiserfs + echo "-o acl,user_xattr $REISERFS_MOUNT_OPTIONS" + ;; + reiser4) + # acls & xattrs aren't supported by reiser4 + echo $REISER4_MOUNT_OPTIONS + ;; + gfs2) + # acls aren't turned on by default on gfs2 + echo "-o acl $GFS2_MOUNT_OPTIONS" + ;; + tmpfs) + # We need to specify the size at mount, use 1G by default + echo "-o size=1G $TMPFS_MOUNT_OPTIONS" + ;; + ubifs) + echo $UBIFS_MOUNT_OPTIONS + ;; + *) + ;; + esac +} -################################################################################ -# make sure this script returns success -/bin/true +_mount_opts() +{ + export MOUNT_OPTIONS=$(_common_mount_opts) +} + +_test_mount_opts() +{ + export TEST_FS_MOUNT_OPTS=$(_common_mount_opts) +} + +_mkfs_opts() +{ + case $FSTYP in + xfs) + export MKFS_OPTIONS=$XFS_MKFS_OPTIONS + ;; + udf) + [ ! -z "$udf_fsize" ] && \ + UDF_MKFS_OPTIONS="$UDF_MKFS_OPTIONS -s $udf_fsize" + export MKFS_OPTIONS=$UDF_MKFS_OPTIONS + ;; + nfs) + export MKFS_OPTIONS=$NFS_MKFS_OPTIONS + ;; + afs) + export MKFS_OPTIONS=$AFS_MKFS_OPTIONS + ;; + cifs) + export MKFS_OPTIONS=$CIFS_MKFS_OPTIONS + ;; + ceph) + export MKFS_OPTIONS=$CEPHFS_MKFS_OPTIONS + ;; + reiserfs) + export MKFS_OPTIONS="$REISERFS_MKFS_OPTIONS -q" + ;; + reiser4) + export MKFS_OPTIONS=$REISER4_MKFS_OPTIONS + ;; + gfs2) + export MKFS_OPTIONS="$GFS2_MKFS_OPTIONS -O -p lock_nolock" + ;; + jfs) + export MKFS_OPTIONS="$JFS_MKFS_OPTIONS -q" + ;; + f2fs) + export MKFS_OPTIONS="$F2FS_MKFS_OPTIONS" + ;; + btrfs) + export MKFS_OPTIONS="$BTRFS_MKFS_OPTIONS" + ;; + bcachefs) + export MKFS_OPTIONS=$BCACHEFS_MKFS_OPTIONS + ;; + *) + ;; + esac +} + +_fsck_opts() +{ + case $FSTYP in + ext2|ext3|ext4) + export FSCK_OPTIONS="-nf" + ;; + reiser*) + export FSCK_OPTIONS="--yes" + ;; + f2fs) + export FSCK_OPTIONS="" + ;; + *) + export FSCK_OPTIONS="-n" + ;; + esac +} + +# check necessary running dependences then source sepcific fs helpers +_source_specific_fs() +{ + local fs=$1 + + if [ -z "$fs" ];then + fs=$FSTYP + fi + + case "$fs" in + xfs) + [ "$XFS_LOGPRINT_PROG" = "" ] && _fatal "xfs_logprint not found" + [ "$XFS_REPAIR_PROG" = "" ] && _fatal "xfs_repair not found" + [ "$XFS_DB_PROG" = "" ] && _fatal "xfs_db not found" + [ "$MKFS_XFS_PROG" = "" ] && _fatal "mkfs_xfs not found" + [ "$XFS_INFO_PROG" = "" ] && _fatal "xfs_info not found" + + . ./common/xfs + ;; + udf) + [ "$MKFS_UDF_PROG" = "" ] && _fatal "mkfs_udf/mkudffs not found" + ;; + btrfs) + [ "$MKFS_BTRFS_PROG" = "" ] && _fatal "mkfs.btrfs not found" + + . ./common/btrfs + ;; + ext4) + [ "$MKFS_EXT4_PROG" = "" ] && _fatal "mkfs.ext4 not found" + . ./common/ext4 + ;; + ext2|ext3) + . ./common/ext4 + ;; + f2fs) + [ "$MKFS_F2FS_PROG" = "" ] && _fatal "mkfs.f2fs not found" + ;; + nfs) + . ./common/nfs + ;; + afs) + ;; + cifs) + ;; + 9p) + ;; + fuse) + ;; + ceph) + . ./common/ceph + ;; + glusterfs) + ;; + overlay) + . ./common/overlay + ;; + reiser4) + [ "$MKFS_REISER4_PROG" = "" ] && _fatal "mkfs.reiser4 not found" + ;; + pvfs2) + ;; + ubifs) + [ "$UBIUPDATEVOL_PROG" = "" ] && _fatal "ubiupdatevol not found" + . ./common/ubifs + ;; + esac +} diff --git a/tests/generic/367 b/tests/generic/367 index ed371a02b..1c9c66730 100755 --- a/tests/generic/367 +++ b/tests/generic/367 @@ -10,13 +10,12 @@ # i.e, with any file with allocated extents or delayed allocation. We also # check if the extsize value and the xflag bit actually got reflected after # setting/re-setting the extsize value. - -. ./common/config -. ./common/filter +# . ./common/preamble - _begin_fstest ioctl quick +. ./common/filter + [ "$FSTYP" = "xfs" ] && _fixed_by_kernel_commit 2a492ff66673 \ "xfs: Check for delayed allocations before setting extsize" diff --git a/tests/generic/749 b/tests/generic/749 index fc7477380..aac8da20d 100755 --- a/tests/generic/749 +++ b/tests/generic/749 @@ -14,11 +14,10 @@ # page, you should get a SIGBUS. This test verifies we zero-fill to page # boundary and ensures we get a SIGBUS if we write to data beyond the system # page size even if the block size is greater than the system page size. +# . ./common/preamble -. ./common/rc _begin_fstest auto quick prealloc -# Import common functions. . ./common/filter _require_scratch_nocheck