* [PATCH v1 0/2] common: Move exit related functions to common/exit
@ 2025-04-23 6:41 Nirjhar Roy (IBM)
2025-04-23 6:41 ` [PATCH v1 1/2] common: Move exit related functions to a common/exit Nirjhar Roy (IBM)
2025-04-23 6:41 ` [PATCH v1 2/2] check: Replace exit with _exit in check Nirjhar Roy (IBM)
0 siblings, 2 replies; 9+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-23 6:41 UTC (permalink / raw)
To: fstests
Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
nirjhar.roy.lists
This patch series moves all the exit related functions to a separate file -
common/exit. This will remove the dependency to source non-related files to use
these exit related functions. Thanks to Dave for suggesting this[1]. The second
patch replaces exit with _exit in check file - I missed replacing them in [2].
[1] https://lore.kernel.org/all/Z_UJ7XcpmtkPRhTr@dread.disaster.area/
[2] https://lore.kernel.org/all/48dacdf636be19ae8bff66cc3852d27e28030613.1744181682.git.nirjhar.roy.lists@gmail.com/
Nirjhar Roy (IBM) (2):
common: Move exit related functions to a common/exit
check: Replace exit with _exit in check
check | 40 ++++++++++++++++-----------------------
common/btrfs | 2 +-
common/ceph | 2 ++
common/config | 17 +----------------
common/dump | 1 +
common/exit | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
common/ext4 | 2 +-
common/populate | 2 +-
common/preamble | 1 +
common/punch | 6 +-----
common/rc | 29 +---------------------------
common/repair | 1 +
common/xfs | 1 +
13 files changed, 78 insertions(+), 76 deletions(-)
create mode 100644 common/exit
--
2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 1/2] common: Move exit related functions to a common/exit
2025-04-23 6:41 [PATCH v1 0/2] common: Move exit related functions to common/exit Nirjhar Roy (IBM)
@ 2025-04-23 6:41 ` Nirjhar Roy (IBM)
2025-04-23 14:18 ` Zorro Lang
2025-04-23 6:41 ` [PATCH v1 2/2] check: Replace exit with _exit in check Nirjhar Roy (IBM)
1 sibling, 1 reply; 9+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-23 6:41 UTC (permalink / raw)
To: fstests
Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
nirjhar.roy.lists
Introduce a new file common/exit that will contain all the exit
related functions. This will remove the dependencies these functions
have on other non-related helper files and they can be indepedently
sourced. This was suggested by Dave Chinner[1].
[1] https://lore.kernel.org/linux-xfs/Z_UJ7XcpmtkPRhTr@dread.disaster.area/
Suggested-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
check | 1 +
common/btrfs | 2 +-
common/ceph | 2 ++
common/config | 17 +----------------
common/dump | 1 +
common/exit | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
common/ext4 | 2 +-
common/populate | 2 +-
common/preamble | 1 +
common/punch | 6 +-----
common/rc | 29 +---------------------------
common/repair | 1 +
common/xfs | 1 +
13 files changed, 63 insertions(+), 52 deletions(-)
create mode 100644 common/exit
diff --git a/check b/check
index 9451c350..67355c52 100755
--- a/check
+++ b/check
@@ -51,6 +51,7 @@ rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
SRC_GROUPS="generic"
export SRC_DIR="tests"
+. common/exit
usage()
{
diff --git a/common/btrfs b/common/btrfs
index 3725632c..9e91ee71 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -1,7 +1,7 @@
#
# Common btrfs specific functions
#
-
+. common/exit
. common/module
# The recommended way to execute simple "btrfs" command.
diff --git a/common/ceph b/common/ceph
index df7a6814..89e36403 100644
--- a/common/ceph
+++ b/common/ceph
@@ -2,6 +2,8 @@
# CephFS specific common functions.
#
+. common/exit
+
# _ceph_create_file_layout <filename> <stripe unit> <stripe count> <object size>
# This function creates a new empty file and sets the file layout according to
# parameters. It will exit if the file already exists.
diff --git a/common/config b/common/config
index eada3971..6a60d144 100644
--- a/common/config
+++ b/common/config
@@ -38,7 +38,7 @@
# - this script shouldn't make any assertions about filesystem
# validity or mountedness.
#
-
+. common/exit
. common/test_names
# all tests should use a common language setting to prevent golden
@@ -96,15 +96,6 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
-# This functions sets the exit code to status and then exits. Don't use
-# exit directly, as it might not set the value of "$status" correctly, which is
-# used as an exit code in the trap handler routine set up by the check script.
-_exit()
-{
- test -n "$1" && status="$1"
- exit "$status"
-}
-
# Handle mkfs.$fstyp which does (or does not) require -f to overwrite
set_mkfs_prog_path_with_opts()
{
@@ -121,12 +112,6 @@ set_mkfs_prog_path_with_opts()
fi
}
-_fatal()
-{
- echo "$*"
- _exit 1
-}
-
export MKFS_PROG="$(type -P mkfs)"
[ "$MKFS_PROG" = "" ] && _fatal "mkfs not found"
diff --git a/common/dump b/common/dump
index 09859006..4701a956 100644
--- a/common/dump
+++ b/common/dump
@@ -3,6 +3,7 @@
# Copyright (c) 2000-2002,2005 Silicon Graphics, Inc. All Rights Reserved.
#
# Functions useful for xfsdump/xfsrestore tests
+. common/exit
# --- initializations ---
rm -f $seqres.full
diff --git a/common/exit b/common/exit
new file mode 100644
index 00000000..ad7e7498
--- /dev/null
+++ b/common/exit
@@ -0,0 +1,50 @@
+##/bin/bash
+
+# This functions sets the exit code to status and then exits. Don't use
+# exit directly, as it might not set the value of "$status" correctly, which is
+# used as an exit code in the trap handler routine set up by the check script.
+_exit()
+{
+ test -n "$1" && status="$1"
+ exit "$status"
+}
+
+_fatal()
+{
+ echo "$*"
+ _exit 1
+}
+
+_die()
+{
+ echo $@
+ _exit 1
+}
+
+die_now()
+{
+ _exit 1
+}
+
+# just plain bail out
+#
+_fail()
+{
+ echo "$*" | tee -a $seqres.full
+ echo "(see $seqres.full for details)"
+ _exit 1
+}
+
+# bail out, setting up .notrun file. Need to kill the filesystem check files
+# here, otherwise they are set incorrectly for the next test.
+#
+_notrun()
+{
+ echo "$*" > $seqres.notrun
+ echo "$seq not run: $*"
+ rm -f ${RESULT_DIR}/require_test*
+ rm -f ${RESULT_DIR}/require_scratch*
+
+ _exit 0
+}
+
diff --git a/common/ext4 b/common/ext4
index f88fa532..ab566c41 100644
--- a/common/ext4
+++ b/common/ext4
@@ -1,7 +1,7 @@
#
# ext4 specific common functions
#
-
+. common/exit
__generate_ext4_report_vars() {
__generate_blockdev_report_vars TEST_LOGDEV
__generate_blockdev_report_vars SCRATCH_LOGDEV
diff --git a/common/populate b/common/populate
index 50dc75d3..a17acc9e 100644
--- a/common/populate
+++ b/common/populate
@@ -4,7 +4,7 @@
#
# Routines for populating a scratch fs, and helpers to exercise an FS
# once it's been fuzzed.
-
+. common/exit
. ./common/quota
_require_populate_commands() {
diff --git a/common/preamble b/common/preamble
index ba029a34..0f306412 100644
--- a/common/preamble
+++ b/common/preamble
@@ -3,6 +3,7 @@
# Copyright (c) 2021 Oracle. All Rights Reserved.
# Boilerplate fstests functionality
+. common/exit
# Standard cleanup function. Individual tests can override this.
_cleanup()
diff --git a/common/punch b/common/punch
index 64d665d8..637f463f 100644
--- a/common/punch
+++ b/common/punch
@@ -3,6 +3,7 @@
# Copyright (c) 2007 Silicon Graphics, Inc. All Rights Reserved.
#
# common functions for excersizing hole punches with extent size hints etc.
+. common/exit
_spawn_test_file() {
echo "# spawning test file with $*"
@@ -222,11 +223,6 @@ _filter_bmap()
_coalesce_extents
}
-die_now()
-{
- _exit 1
-}
-
# test the different corner cases for zeroing a range:
#
# 1. into a hole
diff --git a/common/rc b/common/rc
index 9bed6dad..945f5134 100644
--- a/common/rc
+++ b/common/rc
@@ -2,6 +2,7 @@
# SPDX-License-Identifier: GPL-2.0+
# Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved.
+. common/exit
. common/config
BC="$(type -P bc)" || BC=
@@ -1798,28 +1799,6 @@ _do()
return $ret
}
-# bail out, setting up .notrun file. Need to kill the filesystem check files
-# here, otherwise they are set incorrectly for the next test.
-#
-_notrun()
-{
- echo "$*" > $seqres.notrun
- echo "$seq not run: $*"
- rm -f ${RESULT_DIR}/require_test*
- rm -f ${RESULT_DIR}/require_scratch*
-
- _exit 0
-}
-
-# just plain bail out
-#
-_fail()
-{
- echo "$*" | tee -a $seqres.full
- echo "(see $seqres.full for details)"
- _exit 1
-}
-
#
# Tests whether $FSTYP should be exclude from this test.
#
@@ -3835,12 +3814,6 @@ _link_out_file()
_link_out_file_named $seqfull.out "$features"
}
-_die()
-{
- echo $@
- _exit 1
-}
-
# convert urandom incompressible data to compressible text data
_ddt()
{
diff --git a/common/repair b/common/repair
index fd206f8e..db6a1b5c 100644
--- a/common/repair
+++ b/common/repair
@@ -3,6 +3,7 @@
# Copyright (c) 2000-2002 Silicon Graphics, Inc. All Rights Reserved.
#
# Functions useful for xfs_repair tests
+. common/exit
_zero_position()
{
diff --git a/common/xfs b/common/xfs
index 96c15f3c..c236146c 100644
--- a/common/xfs
+++ b/common/xfs
@@ -1,6 +1,7 @@
#
# XFS specific common functions.
#
+. common/exit
__generate_xfs_report_vars() {
__generate_blockdev_report_vars TEST_RTDEV
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 2/2] check: Replace exit with _exit in check
2025-04-23 6:41 [PATCH v1 0/2] common: Move exit related functions to common/exit Nirjhar Roy (IBM)
2025-04-23 6:41 ` [PATCH v1 1/2] common: Move exit related functions to a common/exit Nirjhar Roy (IBM)
@ 2025-04-23 6:41 ` Nirjhar Roy (IBM)
1 sibling, 0 replies; 9+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-23 6:41 UTC (permalink / raw)
To: fstests
Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
nirjhar.roy.lists
Some of the "status=<val>;exit" and "exit <val>" were not
replace with _exit <val>. Doing it now.
Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
check | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/check b/check
index 67355c52..30d44c0e 100755
--- a/check
+++ b/check
@@ -122,7 +122,7 @@ examples:
check -X .exclude -g auto
check -E ~/.xfstests.exclude
'
- exit 1
+ _exit 1
}
get_sub_group_list()
@@ -232,7 +232,7 @@ _prepare_test_list()
list=$(get_group_list $group)
if [ -z "$list" ]; then
echo "Group \"$group\" is empty or not defined?"
- exit 1
+ _exit 1
fi
for t in $list; do
@@ -317,14 +317,14 @@ while [ $# -gt 0 ]; do
-r)
if $exact_order; then
echo "Cannot specify -r and --exact-order."
- exit 1
+ _exit 1
fi
randomize=true
;;
--exact-order)
if $randomize; then
echo "Cannnot specify --exact-order and -r."
- exit 1
+ _exit 1
fi
exact_order=true
;;
@@ -362,7 +362,7 @@ done
# after processing args, overlay needs FSTYP set before sourcing common/config
if ! . ./common/rc; then
echo "check: failed to source common/rc"
- exit 1
+ _exit 1
fi
init_rc
@@ -374,8 +374,7 @@ if [ -n "$SOAK_DURATION" ]; then
sed -e 's/^\([.0-9]*\)\([a-z]\)*/\1 \2/g' | \
$AWK_PROG -f $here/src/soak_duration.awk)"
if [ $? -ne 0 ]; then
- status=1
- exit 1
+ _exit 1
fi
fi
@@ -386,8 +385,7 @@ if [ -n "$FUZZ_REWRITE_DURATION" ]; then
sed -e 's/^\([.0-9]*\)\([a-z]\)*/\1 \2/g' | \
$AWK_PROG -f $here/src/soak_duration.awk)"
if [ $? -ne 0 ]; then
- status=1
- exit 1
+ _exit 1
fi
fi
@@ -405,8 +403,7 @@ if $have_test_arg; then
while [ $# -gt 0 ]; do
case "$1" in
-*) echo "Arguments before tests, please!"
- status=1
- exit $status
+ _exit 1
;;
*) # Expand test pattern (e.g. xfs/???, *fs/001)
list=$(cd $SRC_DIR; echo $1)
@@ -439,7 +436,7 @@ fi
if [ `id -u` -ne 0 ]
then
echo "check: QA must be run as root"
- exit 1
+ _exit 1
fi
_wipe_counters()
@@ -768,8 +765,7 @@ function run_section()
mkdir -p $RESULT_BASE
if [ ! -d $RESULT_BASE ]; then
echo "failed to create results directory $RESULT_BASE"
- status=1
- exit
+ _exit 1
fi
if $OPTIONS_HAVE_SECTIONS; then
@@ -785,8 +781,7 @@ function run_section()
echo "our local _test_mkfs routine ..."
cat $tmp.err
echo "check: failed to mkfs \$TEST_DEV using specified options"
- status=1
- exit
+ _exit 1
fi
# Previous FSTYP derived from TEST_DEV could be changed, source
# common/rc again with correct FSTYP to get FSTYP specific configs,
@@ -830,8 +825,7 @@ function run_section()
echo "our local _scratch_mkfs routine ..."
cat $tmp.err
echo "check: failed to mkfs \$SCRATCH_DEV using specified options"
- status=1
- exit
+ _exit 1
fi
# call the overridden mount - make sure the FS mounts with
@@ -841,8 +835,7 @@ function run_section()
echo "our local mount routine ..."
cat $tmp.err
echo "check: failed to mount \$SCRATCH_DEV using specified options"
- status=1
- exit
+ _exit 1
else
_scratch_unmount
fi
@@ -1105,12 +1098,10 @@ for ((iters = 0; iters < $iterations; iters++)) do
run_section $section
if [ "$sum_bad" != 0 ] && [ "$istop" = true ]; then
interrupt=false
- status=`expr $sum_bad != 0`
- exit
+ _exit `expr $sum_bad != 0`
fi
done
done
interrupt=false
-status=`expr $sum_bad != 0`
-exit
+_exit `expr $sum_bad != 0`
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit
2025-04-23 6:41 ` [PATCH v1 1/2] common: Move exit related functions to a common/exit Nirjhar Roy (IBM)
@ 2025-04-23 14:18 ` Zorro Lang
2025-04-24 9:09 ` Nirjhar Roy (IBM)
0 siblings, 1 reply; 9+ messages in thread
From: Zorro Lang @ 2025-04-23 14:18 UTC (permalink / raw)
To: Nirjhar Roy (IBM)
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang, david
On Wed, Apr 23, 2025 at 06:41:34AM +0000, Nirjhar Roy (IBM) wrote:
> Introduce a new file common/exit that will contain all the exit
> related functions. This will remove the dependencies these functions
> have on other non-related helper files and they can be indepedently
> sourced. This was suggested by Dave Chinner[1].
>
> [1] https://lore.kernel.org/linux-xfs/Z_UJ7XcpmtkPRhTr@dread.disaster.area/
> Suggested-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> ---
> check | 1 +
> common/btrfs | 2 +-
> common/ceph | 2 ++
> common/config | 17 +----------------
> common/dump | 1 +
> common/exit | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
> common/ext4 | 2 +-
> common/populate | 2 +-
> common/preamble | 1 +
> common/punch | 6 +-----
> common/rc | 29 +---------------------------
> common/repair | 1 +
> common/xfs | 1 +
I think if you define exit helpers in common/exit, and import common/exit
in common/config, then you don't need to source it(common/exit) in other
common files (.e.g common/xfs, common/rc, etc). Due to when we call the
helpers in these common files, the process should already imported
common/rc -> common/config -> common/exit. right?
Thanks,
Zorro
> 13 files changed, 63 insertions(+), 52 deletions(-)
> create mode 100644 common/exit
>
> diff --git a/check b/check
> index 9451c350..67355c52 100755
> --- a/check
> +++ b/check
> @@ -51,6 +51,7 @@ rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
>
> SRC_GROUPS="generic"
> export SRC_DIR="tests"
> +. common/exit
>
> usage()
> {
> diff --git a/common/btrfs b/common/btrfs
> index 3725632c..9e91ee71 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -1,7 +1,7 @@
> #
> # Common btrfs specific functions
> #
> -
> +. common/exit
> . common/module
>
> # The recommended way to execute simple "btrfs" command.
> diff --git a/common/ceph b/common/ceph
> index df7a6814..89e36403 100644
> --- a/common/ceph
> +++ b/common/ceph
> @@ -2,6 +2,8 @@
> # CephFS specific common functions.
> #
>
> +. common/exit
> +
> # _ceph_create_file_layout <filename> <stripe unit> <stripe count> <object size>
> # This function creates a new empty file and sets the file layout according to
> # parameters. It will exit if the file already exists.
> diff --git a/common/config b/common/config
> index eada3971..6a60d144 100644
> --- a/common/config
> +++ b/common/config
> @@ -38,7 +38,7 @@
> # - this script shouldn't make any assertions about filesystem
> # validity or mountedness.
> #
> -
> +. common/exit
> . common/test_names
>
> # all tests should use a common language setting to prevent golden
> @@ -96,15 +96,6 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
>
> export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
>
> -# This functions sets the exit code to status and then exits. Don't use
> -# exit directly, as it might not set the value of "$status" correctly, which is
> -# used as an exit code in the trap handler routine set up by the check script.
> -_exit()
> -{
> - test -n "$1" && status="$1"
> - exit "$status"
> -}
> -
> # Handle mkfs.$fstyp which does (or does not) require -f to overwrite
> set_mkfs_prog_path_with_opts()
> {
> @@ -121,12 +112,6 @@ set_mkfs_prog_path_with_opts()
> fi
> }
>
> -_fatal()
> -{
> - echo "$*"
> - _exit 1
> -}
> -
> export MKFS_PROG="$(type -P mkfs)"
> [ "$MKFS_PROG" = "" ] && _fatal "mkfs not found"
>
> diff --git a/common/dump b/common/dump
> index 09859006..4701a956 100644
> --- a/common/dump
> +++ b/common/dump
> @@ -3,6 +3,7 @@
> # Copyright (c) 2000-2002,2005 Silicon Graphics, Inc. All Rights Reserved.
> #
> # Functions useful for xfsdump/xfsrestore tests
> +. common/exit
>
> # --- initializations ---
> rm -f $seqres.full
> diff --git a/common/exit b/common/exit
> new file mode 100644
> index 00000000..ad7e7498
> --- /dev/null
> +++ b/common/exit
> @@ -0,0 +1,50 @@
> +##/bin/bash
> +
> +# This functions sets the exit code to status and then exits. Don't use
> +# exit directly, as it might not set the value of "$status" correctly, which is
> +# used as an exit code in the trap handler routine set up by the check script.
> +_exit()
> +{
> + test -n "$1" && status="$1"
> + exit "$status"
> +}
> +
> +_fatal()
> +{
> + echo "$*"
> + _exit 1
> +}
> +
> +_die()
> +{
> + echo $@
> + _exit 1
> +}
> +
> +die_now()
> +{
> + _exit 1
> +}
> +
> +# just plain bail out
> +#
> +_fail()
> +{
> + echo "$*" | tee -a $seqres.full
> + echo "(see $seqres.full for details)"
> + _exit 1
> +}
> +
> +# bail out, setting up .notrun file. Need to kill the filesystem check files
> +# here, otherwise they are set incorrectly for the next test.
> +#
> +_notrun()
> +{
> + echo "$*" > $seqres.notrun
> + echo "$seq not run: $*"
> + rm -f ${RESULT_DIR}/require_test*
> + rm -f ${RESULT_DIR}/require_scratch*
> +
> + _exit 0
> +}
> +
> diff --git a/common/ext4 b/common/ext4
> index f88fa532..ab566c41 100644
> --- a/common/ext4
> +++ b/common/ext4
> @@ -1,7 +1,7 @@
> #
> # ext4 specific common functions
> #
> -
> +. common/exit
> __generate_ext4_report_vars() {
> __generate_blockdev_report_vars TEST_LOGDEV
> __generate_blockdev_report_vars SCRATCH_LOGDEV
> diff --git a/common/populate b/common/populate
> index 50dc75d3..a17acc9e 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -4,7 +4,7 @@
> #
> # Routines for populating a scratch fs, and helpers to exercise an FS
> # once it's been fuzzed.
> -
> +. common/exit
> . ./common/quota
>
> _require_populate_commands() {
> diff --git a/common/preamble b/common/preamble
> index ba029a34..0f306412 100644
> --- a/common/preamble
> +++ b/common/preamble
> @@ -3,6 +3,7 @@
> # Copyright (c) 2021 Oracle. All Rights Reserved.
>
> # Boilerplate fstests functionality
> +. common/exit
>
> # Standard cleanup function. Individual tests can override this.
> _cleanup()
> diff --git a/common/punch b/common/punch
> index 64d665d8..637f463f 100644
> --- a/common/punch
> +++ b/common/punch
> @@ -3,6 +3,7 @@
> # Copyright (c) 2007 Silicon Graphics, Inc. All Rights Reserved.
> #
> # common functions for excersizing hole punches with extent size hints etc.
> +. common/exit
>
> _spawn_test_file() {
> echo "# spawning test file with $*"
> @@ -222,11 +223,6 @@ _filter_bmap()
> _coalesce_extents
> }
>
> -die_now()
> -{
> - _exit 1
> -}
> -
> # test the different corner cases for zeroing a range:
> #
> # 1. into a hole
> diff --git a/common/rc b/common/rc
> index 9bed6dad..945f5134 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2,6 +2,7 @@
> # SPDX-License-Identifier: GPL-2.0+
> # Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved.
>
> +. common/exit
> . common/config
>
> BC="$(type -P bc)" || BC=
> @@ -1798,28 +1799,6 @@ _do()
> return $ret
> }
>
> -# bail out, setting up .notrun file. Need to kill the filesystem check files
> -# here, otherwise they are set incorrectly for the next test.
> -#
> -_notrun()
> -{
> - echo "$*" > $seqres.notrun
> - echo "$seq not run: $*"
> - rm -f ${RESULT_DIR}/require_test*
> - rm -f ${RESULT_DIR}/require_scratch*
> -
> - _exit 0
> -}
> -
> -# just plain bail out
> -#
> -_fail()
> -{
> - echo "$*" | tee -a $seqres.full
> - echo "(see $seqres.full for details)"
> - _exit 1
> -}
> -
> #
> # Tests whether $FSTYP should be exclude from this test.
> #
> @@ -3835,12 +3814,6 @@ _link_out_file()
> _link_out_file_named $seqfull.out "$features"
> }
>
> -_die()
> -{
> - echo $@
> - _exit 1
> -}
> -
> # convert urandom incompressible data to compressible text data
> _ddt()
> {
> diff --git a/common/repair b/common/repair
> index fd206f8e..db6a1b5c 100644
> --- a/common/repair
> +++ b/common/repair
> @@ -3,6 +3,7 @@
> # Copyright (c) 2000-2002 Silicon Graphics, Inc. All Rights Reserved.
> #
> # Functions useful for xfs_repair tests
> +. common/exit
>
> _zero_position()
> {
> diff --git a/common/xfs b/common/xfs
> index 96c15f3c..c236146c 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -1,6 +1,7 @@
> #
> # XFS specific common functions.
> #
> +. common/exit
>
> __generate_xfs_report_vars() {
> __generate_blockdev_report_vars TEST_RTDEV
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit
2025-04-23 14:18 ` Zorro Lang
@ 2025-04-24 9:09 ` Nirjhar Roy (IBM)
2025-04-25 11:27 ` Zorro Lang
0 siblings, 1 reply; 9+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-24 9:09 UTC (permalink / raw)
To: Zorro Lang
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang, david
On 4/23/25 19:48, Zorro Lang wrote:
> On Wed, Apr 23, 2025 at 06:41:34AM +0000, Nirjhar Roy (IBM) wrote:
>> Introduce a new file common/exit that will contain all the exit
>> related functions. This will remove the dependencies these functions
>> have on other non-related helper files and they can be indepedently
>> sourced. This was suggested by Dave Chinner[1].
>>
>> [1] https://lore.kernel.org/linux-xfs/Z_UJ7XcpmtkPRhTr@dread.disaster.area/
>> Suggested-by: Dave Chinner <david@fromorbit.com>
>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>> ---
>> check | 1 +
>> common/btrfs | 2 +-
>> common/ceph | 2 ++
>> common/config | 17 +----------------
>> common/dump | 1 +
>> common/exit | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
>> common/ext4 | 2 +-
>> common/populate | 2 +-
>> common/preamble | 1 +
>> common/punch | 6 +-----
>> common/rc | 29 +---------------------------
>> common/repair | 1 +
>> common/xfs | 1 +
> I think if you define exit helpers in common/exit, and import common/exit
> in common/config, then you don't need to source it(common/exit) in other
> common files (.e.g common/xfs, common/rc, etc). Due to when we call the
> helpers in these common files, the process should already imported
> common/rc -> common/config -> common/exit. right?
Oh, right. I can remove the redundant imports from
common/{btrfs,ceph,dump,ext4,populate,preamble,punch,rc,repair,xfs} in
v2. I will keep ". common/exit" only in common/config and check. The
reason for me to keep it in check is that before common/rc is sourced in
check, we might need _exit() (which is present is common/exit). Do you
agree?
--NR
>
> Thanks,
> Zorro
>
>> 13 files changed, 63 insertions(+), 52 deletions(-)
>> create mode 100644 common/exit
>>
>> diff --git a/check b/check
>> index 9451c350..67355c52 100755
>> --- a/check
>> +++ b/check
>> @@ -51,6 +51,7 @@ rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
>>
>> SRC_GROUPS="generic"
>> export SRC_DIR="tests"
>> +. common/exit
>>
>> usage()
>> {
>> diff --git a/common/btrfs b/common/btrfs
>> index 3725632c..9e91ee71 100644
>> --- a/common/btrfs
>> +++ b/common/btrfs
>> @@ -1,7 +1,7 @@
>> #
>> # Common btrfs specific functions
>> #
>> -
>> +. common/exit
>> . common/module
>>
>> # The recommended way to execute simple "btrfs" command.
>> diff --git a/common/ceph b/common/ceph
>> index df7a6814..89e36403 100644
>> --- a/common/ceph
>> +++ b/common/ceph
>> @@ -2,6 +2,8 @@
>> # CephFS specific common functions.
>> #
>>
>> +. common/exit
>> +
>> # _ceph_create_file_layout <filename> <stripe unit> <stripe count> <object size>
>> # This function creates a new empty file and sets the file layout according to
>> # parameters. It will exit if the file already exists.
>> diff --git a/common/config b/common/config
>> index eada3971..6a60d144 100644
>> --- a/common/config
>> +++ b/common/config
>> @@ -38,7 +38,7 @@
>> # - this script shouldn't make any assertions about filesystem
>> # validity or mountedness.
>> #
>> -
>> +. common/exit
>> . common/test_names
>>
>> # all tests should use a common language setting to prevent golden
>> @@ -96,15 +96,6 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
>>
>> export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
>>
>> -# This functions sets the exit code to status and then exits. Don't use
>> -# exit directly, as it might not set the value of "$status" correctly, which is
>> -# used as an exit code in the trap handler routine set up by the check script.
>> -_exit()
>> -{
>> - test -n "$1" && status="$1"
>> - exit "$status"
>> -}
>> -
>> # Handle mkfs.$fstyp which does (or does not) require -f to overwrite
>> set_mkfs_prog_path_with_opts()
>> {
>> @@ -121,12 +112,6 @@ set_mkfs_prog_path_with_opts()
>> fi
>> }
>>
>> -_fatal()
>> -{
>> - echo "$*"
>> - _exit 1
>> -}
>> -
>> export MKFS_PROG="$(type -P mkfs)"
>> [ "$MKFS_PROG" = "" ] && _fatal "mkfs not found"
>>
>> diff --git a/common/dump b/common/dump
>> index 09859006..4701a956 100644
>> --- a/common/dump
>> +++ b/common/dump
>> @@ -3,6 +3,7 @@
>> # Copyright (c) 2000-2002,2005 Silicon Graphics, Inc. All Rights Reserved.
>> #
>> # Functions useful for xfsdump/xfsrestore tests
>> +. common/exit
>>
>> # --- initializations ---
>> rm -f $seqres.full
>> diff --git a/common/exit b/common/exit
>> new file mode 100644
>> index 00000000..ad7e7498
>> --- /dev/null
>> +++ b/common/exit
>> @@ -0,0 +1,50 @@
>> +##/bin/bash
>> +
>> +# This functions sets the exit code to status and then exits. Don't use
>> +# exit directly, as it might not set the value of "$status" correctly, which is
>> +# used as an exit code in the trap handler routine set up by the check script.
>> +_exit()
>> +{
>> + test -n "$1" && status="$1"
>> + exit "$status"
>> +}
>> +
>> +_fatal()
>> +{
>> + echo "$*"
>> + _exit 1
>> +}
>> +
>> +_die()
>> +{
>> + echo $@
>> + _exit 1
>> +}
>> +
>> +die_now()
>> +{
>> + _exit 1
>> +}
>> +
>> +# just plain bail out
>> +#
>> +_fail()
>> +{
>> + echo "$*" | tee -a $seqres.full
>> + echo "(see $seqres.full for details)"
>> + _exit 1
>> +}
>> +
>> +# bail out, setting up .notrun file. Need to kill the filesystem check files
>> +# here, otherwise they are set incorrectly for the next test.
>> +#
>> +_notrun()
>> +{
>> + echo "$*" > $seqres.notrun
>> + echo "$seq not run: $*"
>> + rm -f ${RESULT_DIR}/require_test*
>> + rm -f ${RESULT_DIR}/require_scratch*
>> +
>> + _exit 0
>> +}
>> +
>> diff --git a/common/ext4 b/common/ext4
>> index f88fa532..ab566c41 100644
>> --- a/common/ext4
>> +++ b/common/ext4
>> @@ -1,7 +1,7 @@
>> #
>> # ext4 specific common functions
>> #
>> -
>> +. common/exit
>> __generate_ext4_report_vars() {
>> __generate_blockdev_report_vars TEST_LOGDEV
>> __generate_blockdev_report_vars SCRATCH_LOGDEV
>> diff --git a/common/populate b/common/populate
>> index 50dc75d3..a17acc9e 100644
>> --- a/common/populate
>> +++ b/common/populate
>> @@ -4,7 +4,7 @@
>> #
>> # Routines for populating a scratch fs, and helpers to exercise an FS
>> # once it's been fuzzed.
>> -
>> +. common/exit
>> . ./common/quota
>>
>> _require_populate_commands() {
>> diff --git a/common/preamble b/common/preamble
>> index ba029a34..0f306412 100644
>> --- a/common/preamble
>> +++ b/common/preamble
>> @@ -3,6 +3,7 @@
>> # Copyright (c) 2021 Oracle. All Rights Reserved.
>>
>> # Boilerplate fstests functionality
>> +. common/exit
>>
>> # Standard cleanup function. Individual tests can override this.
>> _cleanup()
>> diff --git a/common/punch b/common/punch
>> index 64d665d8..637f463f 100644
>> --- a/common/punch
>> +++ b/common/punch
>> @@ -3,6 +3,7 @@
>> # Copyright (c) 2007 Silicon Graphics, Inc. All Rights Reserved.
>> #
>> # common functions for excersizing hole punches with extent size hints etc.
>> +. common/exit
>>
>> _spawn_test_file() {
>> echo "# spawning test file with $*"
>> @@ -222,11 +223,6 @@ _filter_bmap()
>> _coalesce_extents
>> }
>>
>> -die_now()
>> -{
>> - _exit 1
>> -}
>> -
>> # test the different corner cases for zeroing a range:
>> #
>> # 1. into a hole
>> diff --git a/common/rc b/common/rc
>> index 9bed6dad..945f5134 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2,6 +2,7 @@
>> # SPDX-License-Identifier: GPL-2.0+
>> # Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved.
>>
>> +. common/exit
>> . common/config
>>
>> BC="$(type -P bc)" || BC=
>> @@ -1798,28 +1799,6 @@ _do()
>> return $ret
>> }
>>
>> -# bail out, setting up .notrun file. Need to kill the filesystem check files
>> -# here, otherwise they are set incorrectly for the next test.
>> -#
>> -_notrun()
>> -{
>> - echo "$*" > $seqres.notrun
>> - echo "$seq not run: $*"
>> - rm -f ${RESULT_DIR}/require_test*
>> - rm -f ${RESULT_DIR}/require_scratch*
>> -
>> - _exit 0
>> -}
>> -
>> -# just plain bail out
>> -#
>> -_fail()
>> -{
>> - echo "$*" | tee -a $seqres.full
>> - echo "(see $seqres.full for details)"
>> - _exit 1
>> -}
>> -
>> #
>> # Tests whether $FSTYP should be exclude from this test.
>> #
>> @@ -3835,12 +3814,6 @@ _link_out_file()
>> _link_out_file_named $seqfull.out "$features"
>> }
>>
>> -_die()
>> -{
>> - echo $@
>> - _exit 1
>> -}
>> -
>> # convert urandom incompressible data to compressible text data
>> _ddt()
>> {
>> diff --git a/common/repair b/common/repair
>> index fd206f8e..db6a1b5c 100644
>> --- a/common/repair
>> +++ b/common/repair
>> @@ -3,6 +3,7 @@
>> # Copyright (c) 2000-2002 Silicon Graphics, Inc. All Rights Reserved.
>> #
>> # Functions useful for xfs_repair tests
>> +. common/exit
>>
>> _zero_position()
>> {
>> diff --git a/common/xfs b/common/xfs
>> index 96c15f3c..c236146c 100644
>> --- a/common/xfs
>> +++ b/common/xfs
>> @@ -1,6 +1,7 @@
>> #
>> # XFS specific common functions.
>> #
>> +. common/exit
>>
>> __generate_xfs_report_vars() {
>> __generate_blockdev_report_vars TEST_RTDEV
>> --
>> 2.34.1
>>
--
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit
2025-04-24 9:09 ` Nirjhar Roy (IBM)
@ 2025-04-25 11:27 ` Zorro Lang
2025-04-25 12:03 ` Nirjhar Roy (IBM)
0 siblings, 1 reply; 9+ messages in thread
From: Zorro Lang @ 2025-04-25 11:27 UTC (permalink / raw)
To: Nirjhar Roy (IBM)
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang, david
On Thu, Apr 24, 2025 at 02:39:39PM +0530, Nirjhar Roy (IBM) wrote:
>
> On 4/23/25 19:48, Zorro Lang wrote:
> > On Wed, Apr 23, 2025 at 06:41:34AM +0000, Nirjhar Roy (IBM) wrote:
> > > Introduce a new file common/exit that will contain all the exit
> > > related functions. This will remove the dependencies these functions
> > > have on other non-related helper files and they can be indepedently
> > > sourced. This was suggested by Dave Chinner[1].
> > >
> > > [1] https://lore.kernel.org/linux-xfs/Z_UJ7XcpmtkPRhTr@dread.disaster.area/
> > > Suggested-by: Dave Chinner <david@fromorbit.com>
> > > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> > > ---
> > > check | 1 +
> > > common/btrfs | 2 +-
> > > common/ceph | 2 ++
> > > common/config | 17 +----------------
> > > common/dump | 1 +
> > > common/exit | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > common/ext4 | 2 +-
> > > common/populate | 2 +-
> > > common/preamble | 1 +
> > > common/punch | 6 +-----
> > > common/rc | 29 +---------------------------
> > > common/repair | 1 +
> > > common/xfs | 1 +
> > I think if you define exit helpers in common/exit, and import common/exit
> > in common/config, then you don't need to source it(common/exit) in other
> > common files (.e.g common/xfs, common/rc, etc). Due to when we call the
> > helpers in these common files, the process should already imported
> > common/rc -> common/config -> common/exit. right?
>
> Oh, right. I can remove the redundant imports from
> common/{btrfs,ceph,dump,ext4,populate,preamble,punch,rc,repair,xfs} in v2. I
> will keep ". common/exit" only in common/config and check. The reason for me
> to keep it in check is that before common/rc is sourced in check, we might
> need _exit() (which is present is common/exit). Do you agree?
I thought "check" might not need that either. I didn't give it a test, but I found
before importing common/rc, there're only command arguments initialization, and
"check" calls "exit" directly if the initialization fails (except you want to call
_exit, but I didn't see you change that).
Thanks,
Zorro
>
> --NR
>
> >
> > Thanks,
> > Zorro
> >
> > > 13 files changed, 63 insertions(+), 52 deletions(-)
> > > create mode 100644 common/exit
> > >
> > > diff --git a/check b/check
> > > index 9451c350..67355c52 100755
> > > --- a/check
> > > +++ b/check
> > > @@ -51,6 +51,7 @@ rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
> > > SRC_GROUPS="generic"
> > > export SRC_DIR="tests"
> > > +. common/exit
> > > usage()
> > > {
> > > diff --git a/common/btrfs b/common/btrfs
> > > index 3725632c..9e91ee71 100644
> > > --- a/common/btrfs
> > > +++ b/common/btrfs
> > > @@ -1,7 +1,7 @@
> > > #
> > > # Common btrfs specific functions
> > > #
> > > -
> > > +. common/exit
> > > . common/module
> > > # The recommended way to execute simple "btrfs" command.
> > > diff --git a/common/ceph b/common/ceph
> > > index df7a6814..89e36403 100644
> > > --- a/common/ceph
> > > +++ b/common/ceph
> > > @@ -2,6 +2,8 @@
> > > # CephFS specific common functions.
> > > #
> > > +. common/exit
> > > +
> > > # _ceph_create_file_layout <filename> <stripe unit> <stripe count> <object size>
> > > # This function creates a new empty file and sets the file layout according to
> > > # parameters. It will exit if the file already exists.
> > > diff --git a/common/config b/common/config
> > > index eada3971..6a60d144 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -38,7 +38,7 @@
> > > # - this script shouldn't make any assertions about filesystem
> > > # validity or mountedness.
> > > #
> > > -
> > > +. common/exit
> > > . common/test_names
> > > # all tests should use a common language setting to prevent golden
> > > @@ -96,15 +96,6 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
> > > export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
> > > -# This functions sets the exit code to status and then exits. Don't use
> > > -# exit directly, as it might not set the value of "$status" correctly, which is
> > > -# used as an exit code in the trap handler routine set up by the check script.
> > > -_exit()
> > > -{
> > > - test -n "$1" && status="$1"
> > > - exit "$status"
> > > -}
> > > -
> > > # Handle mkfs.$fstyp which does (or does not) require -f to overwrite
> > > set_mkfs_prog_path_with_opts()
> > > {
> > > @@ -121,12 +112,6 @@ set_mkfs_prog_path_with_opts()
> > > fi
> > > }
> > > -_fatal()
> > > -{
> > > - echo "$*"
> > > - _exit 1
> > > -}
> > > -
> > > export MKFS_PROG="$(type -P mkfs)"
> > > [ "$MKFS_PROG" = "" ] && _fatal "mkfs not found"
> > > diff --git a/common/dump b/common/dump
> > > index 09859006..4701a956 100644
> > > --- a/common/dump
> > > +++ b/common/dump
> > > @@ -3,6 +3,7 @@
> > > # Copyright (c) 2000-2002,2005 Silicon Graphics, Inc. All Rights Reserved.
> > > #
> > > # Functions useful for xfsdump/xfsrestore tests
> > > +. common/exit
> > > # --- initializations ---
> > > rm -f $seqres.full
> > > diff --git a/common/exit b/common/exit
> > > new file mode 100644
> > > index 00000000..ad7e7498
> > > --- /dev/null
> > > +++ b/common/exit
> > > @@ -0,0 +1,50 @@
> > > +##/bin/bash
> > > +
> > > +# This functions sets the exit code to status and then exits. Don't use
> > > +# exit directly, as it might not set the value of "$status" correctly, which is
> > > +# used as an exit code in the trap handler routine set up by the check script.
> > > +_exit()
> > > +{
> > > + test -n "$1" && status="$1"
> > > + exit "$status"
> > > +}
> > > +
> > > +_fatal()
> > > +{
> > > + echo "$*"
> > > + _exit 1
> > > +}
> > > +
> > > +_die()
> > > +{
> > > + echo $@
> > > + _exit 1
> > > +}
> > > +
> > > +die_now()
> > > +{
> > > + _exit 1
> > > +}
> > > +
> > > +# just plain bail out
> > > +#
> > > +_fail()
> > > +{
> > > + echo "$*" | tee -a $seqres.full
> > > + echo "(see $seqres.full for details)"
> > > + _exit 1
> > > +}
> > > +
> > > +# bail out, setting up .notrun file. Need to kill the filesystem check files
> > > +# here, otherwise they are set incorrectly for the next test.
> > > +#
> > > +_notrun()
> > > +{
> > > + echo "$*" > $seqres.notrun
> > > + echo "$seq not run: $*"
> > > + rm -f ${RESULT_DIR}/require_test*
> > > + rm -f ${RESULT_DIR}/require_scratch*
> > > +
> > > + _exit 0
> > > +}
> > > +
> > > diff --git a/common/ext4 b/common/ext4
> > > index f88fa532..ab566c41 100644
> > > --- a/common/ext4
> > > +++ b/common/ext4
> > > @@ -1,7 +1,7 @@
> > > #
> > > # ext4 specific common functions
> > > #
> > > -
> > > +. common/exit
> > > __generate_ext4_report_vars() {
> > > __generate_blockdev_report_vars TEST_LOGDEV
> > > __generate_blockdev_report_vars SCRATCH_LOGDEV
> > > diff --git a/common/populate b/common/populate
> > > index 50dc75d3..a17acc9e 100644
> > > --- a/common/populate
> > > +++ b/common/populate
> > > @@ -4,7 +4,7 @@
> > > #
> > > # Routines for populating a scratch fs, and helpers to exercise an FS
> > > # once it's been fuzzed.
> > > -
> > > +. common/exit
> > > . ./common/quota
> > > _require_populate_commands() {
> > > diff --git a/common/preamble b/common/preamble
> > > index ba029a34..0f306412 100644
> > > --- a/common/preamble
> > > +++ b/common/preamble
> > > @@ -3,6 +3,7 @@
> > > # Copyright (c) 2021 Oracle. All Rights Reserved.
> > > # Boilerplate fstests functionality
> > > +. common/exit
> > > # Standard cleanup function. Individual tests can override this.
> > > _cleanup()
> > > diff --git a/common/punch b/common/punch
> > > index 64d665d8..637f463f 100644
> > > --- a/common/punch
> > > +++ b/common/punch
> > > @@ -3,6 +3,7 @@
> > > # Copyright (c) 2007 Silicon Graphics, Inc. All Rights Reserved.
> > > #
> > > # common functions for excersizing hole punches with extent size hints etc.
> > > +. common/exit
> > > _spawn_test_file() {
> > > echo "# spawning test file with $*"
> > > @@ -222,11 +223,6 @@ _filter_bmap()
> > > _coalesce_extents
> > > }
> > > -die_now()
> > > -{
> > > - _exit 1
> > > -}
> > > -
> > > # test the different corner cases for zeroing a range:
> > > #
> > > # 1. into a hole
> > > diff --git a/common/rc b/common/rc
> > > index 9bed6dad..945f5134 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -2,6 +2,7 @@
> > > # SPDX-License-Identifier: GPL-2.0+
> > > # Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved.
> > > +. common/exit
> > > . common/config
> > > BC="$(type -P bc)" || BC=
> > > @@ -1798,28 +1799,6 @@ _do()
> > > return $ret
> > > }
> > > -# bail out, setting up .notrun file. Need to kill the filesystem check files
> > > -# here, otherwise they are set incorrectly for the next test.
> > > -#
> > > -_notrun()
> > > -{
> > > - echo "$*" > $seqres.notrun
> > > - echo "$seq not run: $*"
> > > - rm -f ${RESULT_DIR}/require_test*
> > > - rm -f ${RESULT_DIR}/require_scratch*
> > > -
> > > - _exit 0
> > > -}
> > > -
> > > -# just plain bail out
> > > -#
> > > -_fail()
> > > -{
> > > - echo "$*" | tee -a $seqres.full
> > > - echo "(see $seqres.full for details)"
> > > - _exit 1
> > > -}
> > > -
> > > #
> > > # Tests whether $FSTYP should be exclude from this test.
> > > #
> > > @@ -3835,12 +3814,6 @@ _link_out_file()
> > > _link_out_file_named $seqfull.out "$features"
> > > }
> > > -_die()
> > > -{
> > > - echo $@
> > > - _exit 1
> > > -}
> > > -
> > > # convert urandom incompressible data to compressible text data
> > > _ddt()
> > > {
> > > diff --git a/common/repair b/common/repair
> > > index fd206f8e..db6a1b5c 100644
> > > --- a/common/repair
> > > +++ b/common/repair
> > > @@ -3,6 +3,7 @@
> > > # Copyright (c) 2000-2002 Silicon Graphics, Inc. All Rights Reserved.
> > > #
> > > # Functions useful for xfs_repair tests
> > > +. common/exit
> > > _zero_position()
> > > {
> > > diff --git a/common/xfs b/common/xfs
> > > index 96c15f3c..c236146c 100644
> > > --- a/common/xfs
> > > +++ b/common/xfs
> > > @@ -1,6 +1,7 @@
> > > #
> > > # XFS specific common functions.
> > > #
> > > +. common/exit
> > > __generate_xfs_report_vars() {
> > > __generate_blockdev_report_vars TEST_RTDEV
> > > --
> > > 2.34.1
> > >
> --
> Nirjhar Roy
> Linux Kernel Developer
> IBM, Bangalore
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit
2025-04-25 11:27 ` Zorro Lang
@ 2025-04-25 12:03 ` Nirjhar Roy (IBM)
2025-04-25 13:36 ` Zorro Lang
0 siblings, 1 reply; 9+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-25 12:03 UTC (permalink / raw)
To: Zorro Lang
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang, david
On 4/25/25 16:57, Zorro Lang wrote:
> On Thu, Apr 24, 2025 at 02:39:39PM +0530, Nirjhar Roy (IBM) wrote:
>> On 4/23/25 19:48, Zorro Lang wrote:
>>> On Wed, Apr 23, 2025 at 06:41:34AM +0000, Nirjhar Roy (IBM) wrote:
>>>> Introduce a new file common/exit that will contain all the exit
>>>> related functions. This will remove the dependencies these functions
>>>> have on other non-related helper files and they can be indepedently
>>>> sourced. This was suggested by Dave Chinner[1].
>>>>
>>>> [1] https://lore.kernel.org/linux-xfs/Z_UJ7XcpmtkPRhTr@dread.disaster.area/
>>>> Suggested-by: Dave Chinner <david@fromorbit.com>
>>>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>>>> ---
>>>> check | 1 +
>>>> common/btrfs | 2 +-
>>>> common/ceph | 2 ++
>>>> common/config | 17 +----------------
>>>> common/dump | 1 +
>>>> common/exit | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>> common/ext4 | 2 +-
>>>> common/populate | 2 +-
>>>> common/preamble | 1 +
>>>> common/punch | 6 +-----
>>>> common/rc | 29 +---------------------------
>>>> common/repair | 1 +
>>>> common/xfs | 1 +
>>> I think if you define exit helpers in common/exit, and import common/exit
>>> in common/config, then you don't need to source it(common/exit) in other
>>> common files (.e.g common/xfs, common/rc, etc). Due to when we call the
>>> helpers in these common files, the process should already imported
>>> common/rc -> common/config -> common/exit. right?
>> Oh, right. I can remove the redundant imports from
>> common/{btrfs,ceph,dump,ext4,populate,preamble,punch,rc,repair,xfs} in v2. I
>> will keep ". common/exit" only in common/config and check. The reason for me
>> to keep it in check is that before common/rc is sourced in check, we might
>> need _exit() (which is present is common/exit). Do you agree?
> I thought "check" might not need that either. I didn't give it a test, but I found
> before importing common/rc, there're only command arguments initialization, and
> "check" calls "exit" directly if the initialization fails (except you want to call
> _exit, but I didn't see you change that).
Yes, I have changed the exit() to _exit() in "check" in the next patch
[1] of this series. Can you please take a look at that patch[1] and
suggest whether I should have ". common/exit" in "check" or not?
[1]
https://lore.kernel.org/all/7d8587b8342ee2cbe226fb691b372ac7df5fdb71.1745390030.git.nirjhar.roy.lists@gmail.com/
--NR
>
> Thanks,
> Zorro
>
>> --NR
>>
>>> Thanks,
>>> Zorro
>>>
>>>> 13 files changed, 63 insertions(+), 52 deletions(-)
>>>> create mode 100644 common/exit
>>>>
>>>> diff --git a/check b/check
>>>> index 9451c350..67355c52 100755
>>>> --- a/check
>>>> +++ b/check
>>>> @@ -51,6 +51,7 @@ rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
>>>> SRC_GROUPS="generic"
>>>> export SRC_DIR="tests"
>>>> +. common/exit
>>>> usage()
>>>> {
>>>> diff --git a/common/btrfs b/common/btrfs
>>>> index 3725632c..9e91ee71 100644
>>>> --- a/common/btrfs
>>>> +++ b/common/btrfs
>>>> @@ -1,7 +1,7 @@
>>>> #
>>>> # Common btrfs specific functions
>>>> #
>>>> -
>>>> +. common/exit
>>>> . common/module
>>>> # The recommended way to execute simple "btrfs" command.
>>>> diff --git a/common/ceph b/common/ceph
>>>> index df7a6814..89e36403 100644
>>>> --- a/common/ceph
>>>> +++ b/common/ceph
>>>> @@ -2,6 +2,8 @@
>>>> # CephFS specific common functions.
>>>> #
>>>> +. common/exit
>>>> +
>>>> # _ceph_create_file_layout <filename> <stripe unit> <stripe count> <object size>
>>>> # This function creates a new empty file and sets the file layout according to
>>>> # parameters. It will exit if the file already exists.
>>>> diff --git a/common/config b/common/config
>>>> index eada3971..6a60d144 100644
>>>> --- a/common/config
>>>> +++ b/common/config
>>>> @@ -38,7 +38,7 @@
>>>> # - this script shouldn't make any assertions about filesystem
>>>> # validity or mountedness.
>>>> #
>>>> -
>>>> +. common/exit
>>>> . common/test_names
>>>> # all tests should use a common language setting to prevent golden
>>>> @@ -96,15 +96,6 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
>>>> export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
>>>> -# This functions sets the exit code to status and then exits. Don't use
>>>> -# exit directly, as it might not set the value of "$status" correctly, which is
>>>> -# used as an exit code in the trap handler routine set up by the check script.
>>>> -_exit()
>>>> -{
>>>> - test -n "$1" && status="$1"
>>>> - exit "$status"
>>>> -}
>>>> -
>>>> # Handle mkfs.$fstyp which does (or does not) require -f to overwrite
>>>> set_mkfs_prog_path_with_opts()
>>>> {
>>>> @@ -121,12 +112,6 @@ set_mkfs_prog_path_with_opts()
>>>> fi
>>>> }
>>>> -_fatal()
>>>> -{
>>>> - echo "$*"
>>>> - _exit 1
>>>> -}
>>>> -
>>>> export MKFS_PROG="$(type -P mkfs)"
>>>> [ "$MKFS_PROG" = "" ] && _fatal "mkfs not found"
>>>> diff --git a/common/dump b/common/dump
>>>> index 09859006..4701a956 100644
>>>> --- a/common/dump
>>>> +++ b/common/dump
>>>> @@ -3,6 +3,7 @@
>>>> # Copyright (c) 2000-2002,2005 Silicon Graphics, Inc. All Rights Reserved.
>>>> #
>>>> # Functions useful for xfsdump/xfsrestore tests
>>>> +. common/exit
>>>> # --- initializations ---
>>>> rm -f $seqres.full
>>>> diff --git a/common/exit b/common/exit
>>>> new file mode 100644
>>>> index 00000000..ad7e7498
>>>> --- /dev/null
>>>> +++ b/common/exit
>>>> @@ -0,0 +1,50 @@
>>>> +##/bin/bash
>>>> +
>>>> +# This functions sets the exit code to status and then exits. Don't use
>>>> +# exit directly, as it might not set the value of "$status" correctly, which is
>>>> +# used as an exit code in the trap handler routine set up by the check script.
>>>> +_exit()
>>>> +{
>>>> + test -n "$1" && status="$1"
>>>> + exit "$status"
>>>> +}
>>>> +
>>>> +_fatal()
>>>> +{
>>>> + echo "$*"
>>>> + _exit 1
>>>> +}
>>>> +
>>>> +_die()
>>>> +{
>>>> + echo $@
>>>> + _exit 1
>>>> +}
>>>> +
>>>> +die_now()
>>>> +{
>>>> + _exit 1
>>>> +}
>>>> +
>>>> +# just plain bail out
>>>> +#
>>>> +_fail()
>>>> +{
>>>> + echo "$*" | tee -a $seqres.full
>>>> + echo "(see $seqres.full for details)"
>>>> + _exit 1
>>>> +}
>>>> +
>>>> +# bail out, setting up .notrun file. Need to kill the filesystem check files
>>>> +# here, otherwise they are set incorrectly for the next test.
>>>> +#
>>>> +_notrun()
>>>> +{
>>>> + echo "$*" > $seqres.notrun
>>>> + echo "$seq not run: $*"
>>>> + rm -f ${RESULT_DIR}/require_test*
>>>> + rm -f ${RESULT_DIR}/require_scratch*
>>>> +
>>>> + _exit 0
>>>> +}
>>>> +
>>>> diff --git a/common/ext4 b/common/ext4
>>>> index f88fa532..ab566c41 100644
>>>> --- a/common/ext4
>>>> +++ b/common/ext4
>>>> @@ -1,7 +1,7 @@
>>>> #
>>>> # ext4 specific common functions
>>>> #
>>>> -
>>>> +. common/exit
>>>> __generate_ext4_report_vars() {
>>>> __generate_blockdev_report_vars TEST_LOGDEV
>>>> __generate_blockdev_report_vars SCRATCH_LOGDEV
>>>> diff --git a/common/populate b/common/populate
>>>> index 50dc75d3..a17acc9e 100644
>>>> --- a/common/populate
>>>> +++ b/common/populate
>>>> @@ -4,7 +4,7 @@
>>>> #
>>>> # Routines for populating a scratch fs, and helpers to exercise an FS
>>>> # once it's been fuzzed.
>>>> -
>>>> +. common/exit
>>>> . ./common/quota
>>>> _require_populate_commands() {
>>>> diff --git a/common/preamble b/common/preamble
>>>> index ba029a34..0f306412 100644
>>>> --- a/common/preamble
>>>> +++ b/common/preamble
>>>> @@ -3,6 +3,7 @@
>>>> # Copyright (c) 2021 Oracle. All Rights Reserved.
>>>> # Boilerplate fstests functionality
>>>> +. common/exit
>>>> # Standard cleanup function. Individual tests can override this.
>>>> _cleanup()
>>>> diff --git a/common/punch b/common/punch
>>>> index 64d665d8..637f463f 100644
>>>> --- a/common/punch
>>>> +++ b/common/punch
>>>> @@ -3,6 +3,7 @@
>>>> # Copyright (c) 2007 Silicon Graphics, Inc. All Rights Reserved.
>>>> #
>>>> # common functions for excersizing hole punches with extent size hints etc.
>>>> +. common/exit
>>>> _spawn_test_file() {
>>>> echo "# spawning test file with $*"
>>>> @@ -222,11 +223,6 @@ _filter_bmap()
>>>> _coalesce_extents
>>>> }
>>>> -die_now()
>>>> -{
>>>> - _exit 1
>>>> -}
>>>> -
>>>> # test the different corner cases for zeroing a range:
>>>> #
>>>> # 1. into a hole
>>>> diff --git a/common/rc b/common/rc
>>>> index 9bed6dad..945f5134 100644
>>>> --- a/common/rc
>>>> +++ b/common/rc
>>>> @@ -2,6 +2,7 @@
>>>> # SPDX-License-Identifier: GPL-2.0+
>>>> # Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved.
>>>> +. common/exit
>>>> . common/config
>>>> BC="$(type -P bc)" || BC=
>>>> @@ -1798,28 +1799,6 @@ _do()
>>>> return $ret
>>>> }
>>>> -# bail out, setting up .notrun file. Need to kill the filesystem check files
>>>> -# here, otherwise they are set incorrectly for the next test.
>>>> -#
>>>> -_notrun()
>>>> -{
>>>> - echo "$*" > $seqres.notrun
>>>> - echo "$seq not run: $*"
>>>> - rm -f ${RESULT_DIR}/require_test*
>>>> - rm -f ${RESULT_DIR}/require_scratch*
>>>> -
>>>> - _exit 0
>>>> -}
>>>> -
>>>> -# just plain bail out
>>>> -#
>>>> -_fail()
>>>> -{
>>>> - echo "$*" | tee -a $seqres.full
>>>> - echo "(see $seqres.full for details)"
>>>> - _exit 1
>>>> -}
>>>> -
>>>> #
>>>> # Tests whether $FSTYP should be exclude from this test.
>>>> #
>>>> @@ -3835,12 +3814,6 @@ _link_out_file()
>>>> _link_out_file_named $seqfull.out "$features"
>>>> }
>>>> -_die()
>>>> -{
>>>> - echo $@
>>>> - _exit 1
>>>> -}
>>>> -
>>>> # convert urandom incompressible data to compressible text data
>>>> _ddt()
>>>> {
>>>> diff --git a/common/repair b/common/repair
>>>> index fd206f8e..db6a1b5c 100644
>>>> --- a/common/repair
>>>> +++ b/common/repair
>>>> @@ -3,6 +3,7 @@
>>>> # Copyright (c) 2000-2002 Silicon Graphics, Inc. All Rights Reserved.
>>>> #
>>>> # Functions useful for xfs_repair tests
>>>> +. common/exit
>>>> _zero_position()
>>>> {
>>>> diff --git a/common/xfs b/common/xfs
>>>> index 96c15f3c..c236146c 100644
>>>> --- a/common/xfs
>>>> +++ b/common/xfs
>>>> @@ -1,6 +1,7 @@
>>>> #
>>>> # XFS specific common functions.
>>>> #
>>>> +. common/exit
>>>> __generate_xfs_report_vars() {
>>>> __generate_blockdev_report_vars TEST_RTDEV
>>>> --
>>>> 2.34.1
>>>>
>> --
>> Nirjhar Roy
>> Linux Kernel Developer
>> IBM, Bangalore
>>
--
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit
2025-04-25 12:03 ` Nirjhar Roy (IBM)
@ 2025-04-25 13:36 ` Zorro Lang
2025-04-28 8:39 ` Nirjhar Roy (IBM)
0 siblings, 1 reply; 9+ messages in thread
From: Zorro Lang @ 2025-04-25 13:36 UTC (permalink / raw)
To: Nirjhar Roy (IBM)
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang, david
On Fri, Apr 25, 2025 at 05:33:24PM +0530, Nirjhar Roy (IBM) wrote:
>
> On 4/25/25 16:57, Zorro Lang wrote:
> > On Thu, Apr 24, 2025 at 02:39:39PM +0530, Nirjhar Roy (IBM) wrote:
> > > On 4/23/25 19:48, Zorro Lang wrote:
> > > > On Wed, Apr 23, 2025 at 06:41:34AM +0000, Nirjhar Roy (IBM) wrote:
> > > > > Introduce a new file common/exit that will contain all the exit
> > > > > related functions. This will remove the dependencies these functions
> > > > > have on other non-related helper files and they can be indepedently
> > > > > sourced. This was suggested by Dave Chinner[1].
> > > > >
> > > > > [1] https://lore.kernel.org/linux-xfs/Z_UJ7XcpmtkPRhTr@dread.disaster.area/
> > > > > Suggested-by: Dave Chinner <david@fromorbit.com>
> > > > > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> > > > > ---
> > > > > check | 1 +
> > > > > common/btrfs | 2 +-
> > > > > common/ceph | 2 ++
> > > > > common/config | 17 +----------------
> > > > > common/dump | 1 +
> > > > > common/exit | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > common/ext4 | 2 +-
> > > > > common/populate | 2 +-
> > > > > common/preamble | 1 +
> > > > > common/punch | 6 +-----
> > > > > common/rc | 29 +---------------------------
> > > > > common/repair | 1 +
> > > > > common/xfs | 1 +
> > > > I think if you define exit helpers in common/exit, and import common/exit
> > > > in common/config, then you don't need to source it(common/exit) in other
> > > > common files (.e.g common/xfs, common/rc, etc). Due to when we call the
> > > > helpers in these common files, the process should already imported
> > > > common/rc -> common/config -> common/exit. right?
> > > Oh, right. I can remove the redundant imports from
> > > common/{btrfs,ceph,dump,ext4,populate,preamble,punch,rc,repair,xfs} in v2. I
> > > will keep ". common/exit" only in common/config and check. The reason for me
> > > to keep it in check is that before common/rc is sourced in check, we might
> > > need _exit() (which is present is common/exit). Do you agree?
> > I thought "check" might not need that either. I didn't give it a test, but I found
> > before importing common/rc, there're only command arguments initialization, and
> > "check" calls "exit" directly if the initialization fails (except you want to call
> > _exit, but I didn't see you change that).
>
> Yes, I have changed the exit() to _exit() in "check" in the next patch [1]
> of this series. Can you please take a look at that patch[1] and suggest
> whether I should have ". common/exit" in "check" or not?
>
>
> [1] https://lore.kernel.org/all/7d8587b8342ee2cbe226fb691b372ac7df5fdb71.1745390030.git.nirjhar.roy.lists@gmail.com/
Oh, as "check" has:
if $OPTIONS_HAVE_SECTIONS; then
trap "_summary; exit \$status" 0 1 2 3 15
else
trap "_wrapup; exit \$status" 0 1 2 3 15
fi
So I think it makes sense to use _exit() to deal with status variable :)
>
> --NR
>
> >
> > Thanks,
> > Zorro
> >
> > > --NR
> > >
> > > > Thanks,
> > > > Zorro
> > > >
> > > > > 13 files changed, 63 insertions(+), 52 deletions(-)
> > > > > create mode 100644 common/exit
> > > > >
> > > > > diff --git a/check b/check
> > > > > index 9451c350..67355c52 100755
> > > > > --- a/check
> > > > > +++ b/check
> > > > > @@ -51,6 +51,7 @@ rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
> > > > > SRC_GROUPS="generic"
> > > > > export SRC_DIR="tests"
> > > > > +. common/exit
> > > > > usage()
> > > > > {
> > > > > diff --git a/common/btrfs b/common/btrfs
> > > > > index 3725632c..9e91ee71 100644
> > > > > --- a/common/btrfs
> > > > > +++ b/common/btrfs
> > > > > @@ -1,7 +1,7 @@
> > > > > #
> > > > > # Common btrfs specific functions
> > > > > #
> > > > > -
> > > > > +. common/exit
> > > > > . common/module
> > > > > # The recommended way to execute simple "btrfs" command.
> > > > > diff --git a/common/ceph b/common/ceph
> > > > > index df7a6814..89e36403 100644
> > > > > --- a/common/ceph
> > > > > +++ b/common/ceph
> > > > > @@ -2,6 +2,8 @@
> > > > > # CephFS specific common functions.
> > > > > #
> > > > > +. common/exit
> > > > > +
> > > > > # _ceph_create_file_layout <filename> <stripe unit> <stripe count> <object size>
> > > > > # This function creates a new empty file and sets the file layout according to
> > > > > # parameters. It will exit if the file already exists.
> > > > > diff --git a/common/config b/common/config
> > > > > index eada3971..6a60d144 100644
> > > > > --- a/common/config
> > > > > +++ b/common/config
> > > > > @@ -38,7 +38,7 @@
> > > > > # - this script shouldn't make any assertions about filesystem
> > > > > # validity or mountedness.
> > > > > #
> > > > > -
> > > > > +. common/exit
> > > > > . common/test_names
> > > > > # all tests should use a common language setting to prevent golden
> > > > > @@ -96,15 +96,6 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
> > > > > export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
> > > > > -# This functions sets the exit code to status and then exits. Don't use
> > > > > -# exit directly, as it might not set the value of "$status" correctly, which is
> > > > > -# used as an exit code in the trap handler routine set up by the check script.
> > > > > -_exit()
> > > > > -{
> > > > > - test -n "$1" && status="$1"
> > > > > - exit "$status"
> > > > > -}
> > > > > -
> > > > > # Handle mkfs.$fstyp which does (or does not) require -f to overwrite
> > > > > set_mkfs_prog_path_with_opts()
> > > > > {
> > > > > @@ -121,12 +112,6 @@ set_mkfs_prog_path_with_opts()
> > > > > fi
> > > > > }
> > > > > -_fatal()
> > > > > -{
> > > > > - echo "$*"
> > > > > - _exit 1
> > > > > -}
> > > > > -
> > > > > export MKFS_PROG="$(type -P mkfs)"
> > > > > [ "$MKFS_PROG" = "" ] && _fatal "mkfs not found"
> > > > > diff --git a/common/dump b/common/dump
> > > > > index 09859006..4701a956 100644
> > > > > --- a/common/dump
> > > > > +++ b/common/dump
> > > > > @@ -3,6 +3,7 @@
> > > > > # Copyright (c) 2000-2002,2005 Silicon Graphics, Inc. All Rights Reserved.
> > > > > #
> > > > > # Functions useful for xfsdump/xfsrestore tests
> > > > > +. common/exit
> > > > > # --- initializations ---
> > > > > rm -f $seqres.full
> > > > > diff --git a/common/exit b/common/exit
> > > > > new file mode 100644
> > > > > index 00000000..ad7e7498
> > > > > --- /dev/null
> > > > > +++ b/common/exit
> > > > > @@ -0,0 +1,50 @@
> > > > > +##/bin/bash
> > > > > +
> > > > > +# This functions sets the exit code to status and then exits. Don't use
> > > > > +# exit directly, as it might not set the value of "$status" correctly, which is
> > > > > +# used as an exit code in the trap handler routine set up by the check script.
> > > > > +_exit()
> > > > > +{
> > > > > + test -n "$1" && status="$1"
> > > > > + exit "$status"
> > > > > +}
> > > > > +
> > > > > +_fatal()
> > > > > +{
> > > > > + echo "$*"
> > > > > + _exit 1
> > > > > +}
> > > > > +
> > > > > +_die()
> > > > > +{
> > > > > + echo $@
> > > > > + _exit 1
> > > > > +}
> > > > > +
> > > > > +die_now()
> > > > > +{
> > > > > + _exit 1
> > > > > +}
> > > > > +
> > > > > +# just plain bail out
> > > > > +#
> > > > > +_fail()
> > > > > +{
> > > > > + echo "$*" | tee -a $seqres.full
> > > > > + echo "(see $seqres.full for details)"
> > > > > + _exit 1
> > > > > +}
> > > > > +
> > > > > +# bail out, setting up .notrun file. Need to kill the filesystem check files
> > > > > +# here, otherwise they are set incorrectly for the next test.
> > > > > +#
> > > > > +_notrun()
> > > > > +{
> > > > > + echo "$*" > $seqres.notrun
> > > > > + echo "$seq not run: $*"
> > > > > + rm -f ${RESULT_DIR}/require_test*
> > > > > + rm -f ${RESULT_DIR}/require_scratch*
> > > > > +
> > > > > + _exit 0
> > > > > +}
> > > > > +
> > > > > diff --git a/common/ext4 b/common/ext4
> > > > > index f88fa532..ab566c41 100644
> > > > > --- a/common/ext4
> > > > > +++ b/common/ext4
> > > > > @@ -1,7 +1,7 @@
> > > > > #
> > > > > # ext4 specific common functions
> > > > > #
> > > > > -
> > > > > +. common/exit
> > > > > __generate_ext4_report_vars() {
> > > > > __generate_blockdev_report_vars TEST_LOGDEV
> > > > > __generate_blockdev_report_vars SCRATCH_LOGDEV
> > > > > diff --git a/common/populate b/common/populate
> > > > > index 50dc75d3..a17acc9e 100644
> > > > > --- a/common/populate
> > > > > +++ b/common/populate
> > > > > @@ -4,7 +4,7 @@
> > > > > #
> > > > > # Routines for populating a scratch fs, and helpers to exercise an FS
> > > > > # once it's been fuzzed.
> > > > > -
> > > > > +. common/exit
> > > > > . ./common/quota
> > > > > _require_populate_commands() {
> > > > > diff --git a/common/preamble b/common/preamble
> > > > > index ba029a34..0f306412 100644
> > > > > --- a/common/preamble
> > > > > +++ b/common/preamble
> > > > > @@ -3,6 +3,7 @@
> > > > > # Copyright (c) 2021 Oracle. All Rights Reserved.
> > > > > # Boilerplate fstests functionality
> > > > > +. common/exit
> > > > > # Standard cleanup function. Individual tests can override this.
> > > > > _cleanup()
> > > > > diff --git a/common/punch b/common/punch
> > > > > index 64d665d8..637f463f 100644
> > > > > --- a/common/punch
> > > > > +++ b/common/punch
> > > > > @@ -3,6 +3,7 @@
> > > > > # Copyright (c) 2007 Silicon Graphics, Inc. All Rights Reserved.
> > > > > #
> > > > > # common functions for excersizing hole punches with extent size hints etc.
> > > > > +. common/exit
> > > > > _spawn_test_file() {
> > > > > echo "# spawning test file with $*"
> > > > > @@ -222,11 +223,6 @@ _filter_bmap()
> > > > > _coalesce_extents
> > > > > }
> > > > > -die_now()
> > > > > -{
> > > > > - _exit 1
> > > > > -}
> > > > > -
> > > > > # test the different corner cases for zeroing a range:
> > > > > #
> > > > > # 1. into a hole
> > > > > diff --git a/common/rc b/common/rc
> > > > > index 9bed6dad..945f5134 100644
> > > > > --- a/common/rc
> > > > > +++ b/common/rc
> > > > > @@ -2,6 +2,7 @@
> > > > > # SPDX-License-Identifier: GPL-2.0+
> > > > > # Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved.
> > > > > +. common/exit
> > > > > . common/config
> > > > > BC="$(type -P bc)" || BC=
> > > > > @@ -1798,28 +1799,6 @@ _do()
> > > > > return $ret
> > > > > }
> > > > > -# bail out, setting up .notrun file. Need to kill the filesystem check files
> > > > > -# here, otherwise they are set incorrectly for the next test.
> > > > > -#
> > > > > -_notrun()
> > > > > -{
> > > > > - echo "$*" > $seqres.notrun
> > > > > - echo "$seq not run: $*"
> > > > > - rm -f ${RESULT_DIR}/require_test*
> > > > > - rm -f ${RESULT_DIR}/require_scratch*
> > > > > -
> > > > > - _exit 0
> > > > > -}
> > > > > -
> > > > > -# just plain bail out
> > > > > -#
> > > > > -_fail()
> > > > > -{
> > > > > - echo "$*" | tee -a $seqres.full
> > > > > - echo "(see $seqres.full for details)"
> > > > > - _exit 1
> > > > > -}
> > > > > -
> > > > > #
> > > > > # Tests whether $FSTYP should be exclude from this test.
> > > > > #
> > > > > @@ -3835,12 +3814,6 @@ _link_out_file()
> > > > > _link_out_file_named $seqfull.out "$features"
> > > > > }
> > > > > -_die()
> > > > > -{
> > > > > - echo $@
> > > > > - _exit 1
> > > > > -}
> > > > > -
> > > > > # convert urandom incompressible data to compressible text data
> > > > > _ddt()
> > > > > {
> > > > > diff --git a/common/repair b/common/repair
> > > > > index fd206f8e..db6a1b5c 100644
> > > > > --- a/common/repair
> > > > > +++ b/common/repair
> > > > > @@ -3,6 +3,7 @@
> > > > > # Copyright (c) 2000-2002 Silicon Graphics, Inc. All Rights Reserved.
> > > > > #
> > > > > # Functions useful for xfs_repair tests
> > > > > +. common/exit
> > > > > _zero_position()
> > > > > {
> > > > > diff --git a/common/xfs b/common/xfs
> > > > > index 96c15f3c..c236146c 100644
> > > > > --- a/common/xfs
> > > > > +++ b/common/xfs
> > > > > @@ -1,6 +1,7 @@
> > > > > #
> > > > > # XFS specific common functions.
> > > > > #
> > > > > +. common/exit
> > > > > __generate_xfs_report_vars() {
> > > > > __generate_blockdev_report_vars TEST_RTDEV
> > > > > --
> > > > > 2.34.1
> > > > >
> > > --
> > > Nirjhar Roy
> > > Linux Kernel Developer
> > > IBM, Bangalore
> > >
> --
> Nirjhar Roy
> Linux Kernel Developer
> IBM, Bangalore
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit
2025-04-25 13:36 ` Zorro Lang
@ 2025-04-28 8:39 ` Nirjhar Roy (IBM)
0 siblings, 0 replies; 9+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-28 8:39 UTC (permalink / raw)
To: Zorro Lang
Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong,
zlang, david
On 4/25/25 19:06, Zorro Lang wrote:
> On Fri, Apr 25, 2025 at 05:33:24PM +0530, Nirjhar Roy (IBM) wrote:
>> On 4/25/25 16:57, Zorro Lang wrote:
>>> On Thu, Apr 24, 2025 at 02:39:39PM +0530, Nirjhar Roy (IBM) wrote:
>>>> On 4/23/25 19:48, Zorro Lang wrote:
>>>>> On Wed, Apr 23, 2025 at 06:41:34AM +0000, Nirjhar Roy (IBM) wrote:
>>>>>> Introduce a new file common/exit that will contain all the exit
>>>>>> related functions. This will remove the dependencies these functions
>>>>>> have on other non-related helper files and they can be indepedently
>>>>>> sourced. This was suggested by Dave Chinner[1].
>>>>>>
>>>>>> [1] https://lore.kernel.org/linux-xfs/Z_UJ7XcpmtkPRhTr@dread.disaster.area/
>>>>>> Suggested-by: Dave Chinner <david@fromorbit.com>
>>>>>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>>>>>> ---
>>>>>> check | 1 +
>>>>>> common/btrfs | 2 +-
>>>>>> common/ceph | 2 ++
>>>>>> common/config | 17 +----------------
>>>>>> common/dump | 1 +
>>>>>> common/exit | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> common/ext4 | 2 +-
>>>>>> common/populate | 2 +-
>>>>>> common/preamble | 1 +
>>>>>> common/punch | 6 +-----
>>>>>> common/rc | 29 +---------------------------
>>>>>> common/repair | 1 +
>>>>>> common/xfs | 1 +
>>>>> I think if you define exit helpers in common/exit, and import common/exit
>>>>> in common/config, then you don't need to source it(common/exit) in other
>>>>> common files (.e.g common/xfs, common/rc, etc). Due to when we call the
>>>>> helpers in these common files, the process should already imported
>>>>> common/rc -> common/config -> common/exit. right?
>>>> Oh, right. I can remove the redundant imports from
>>>> common/{btrfs,ceph,dump,ext4,populate,preamble,punch,rc,repair,xfs} in v2. I
>>>> will keep ". common/exit" only in common/config and check. The reason for me
>>>> to keep it in check is that before common/rc is sourced in check, we might
>>>> need _exit() (which is present is common/exit). Do you agree?
>>> I thought "check" might not need that either. I didn't give it a test, but I found
>>> before importing common/rc, there're only command arguments initialization, and
>>> "check" calls "exit" directly if the initialization fails (except you want to call
>>> _exit, but I didn't see you change that).
>> Yes, I have changed the exit() to _exit() in "check" in the next patch [1]
>> of this series. Can you please take a look at that patch[1] and suggest
>> whether I should have ". common/exit" in "check" or not?
>>
>>
>> [1] https://lore.kernel.org/all/7d8587b8342ee2cbe226fb691b372ac7df5fdb71.1745390030.git.nirjhar.roy.lists@gmail.com/
> Oh, as "check" has:
>
> if $OPTIONS_HAVE_SECTIONS; then
> trap "_summary; exit \$status" 0 1 2 3 15
> else
> trap "_wrapup; exit \$status" 0 1 2 3 15
> fi
>
> So I think it makes sense to use _exit() to deal with status variable :)
Oh, right. Yes, I can replace this "exit \$status" with "_exit". I will
make the changes in v2. Any thoughts on the next patch[2]?
[2]
https://lore.kernel.org/all/7d8587b8342ee2cbe226fb691b372ac7df5fdb71.1745390030.git.nirjhar.roy.lists@gmail.com/
--NR
>
>> --NR
>>
>>> Thanks,
>>> Zorro
>>>
>>>> --NR
>>>>
>>>>> Thanks,
>>>>> Zorro
>>>>>
>>>>>> 13 files changed, 63 insertions(+), 52 deletions(-)
>>>>>> create mode 100644 common/exit
>>>>>>
>>>>>> diff --git a/check b/check
>>>>>> index 9451c350..67355c52 100755
>>>>>> --- a/check
>>>>>> +++ b/check
>>>>>> @@ -51,6 +51,7 @@ rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
>>>>>> SRC_GROUPS="generic"
>>>>>> export SRC_DIR="tests"
>>>>>> +. common/exit
>>>>>> usage()
>>>>>> {
>>>>>> diff --git a/common/btrfs b/common/btrfs
>>>>>> index 3725632c..9e91ee71 100644
>>>>>> --- a/common/btrfs
>>>>>> +++ b/common/btrfs
>>>>>> @@ -1,7 +1,7 @@
>>>>>> #
>>>>>> # Common btrfs specific functions
>>>>>> #
>>>>>> -
>>>>>> +. common/exit
>>>>>> . common/module
>>>>>> # The recommended way to execute simple "btrfs" command.
>>>>>> diff --git a/common/ceph b/common/ceph
>>>>>> index df7a6814..89e36403 100644
>>>>>> --- a/common/ceph
>>>>>> +++ b/common/ceph
>>>>>> @@ -2,6 +2,8 @@
>>>>>> # CephFS specific common functions.
>>>>>> #
>>>>>> +. common/exit
>>>>>> +
>>>>>> # _ceph_create_file_layout <filename> <stripe unit> <stripe count> <object size>
>>>>>> # This function creates a new empty file and sets the file layout according to
>>>>>> # parameters. It will exit if the file already exists.
>>>>>> diff --git a/common/config b/common/config
>>>>>> index eada3971..6a60d144 100644
>>>>>> --- a/common/config
>>>>>> +++ b/common/config
>>>>>> @@ -38,7 +38,7 @@
>>>>>> # - this script shouldn't make any assertions about filesystem
>>>>>> # validity or mountedness.
>>>>>> #
>>>>>> -
>>>>>> +. common/exit
>>>>>> . common/test_names
>>>>>> # all tests should use a common language setting to prevent golden
>>>>>> @@ -96,15 +96,6 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
>>>>>> export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
>>>>>> -# This functions sets the exit code to status and then exits. Don't use
>>>>>> -# exit directly, as it might not set the value of "$status" correctly, which is
>>>>>> -# used as an exit code in the trap handler routine set up by the check script.
>>>>>> -_exit()
>>>>>> -{
>>>>>> - test -n "$1" && status="$1"
>>>>>> - exit "$status"
>>>>>> -}
>>>>>> -
>>>>>> # Handle mkfs.$fstyp which does (or does not) require -f to overwrite
>>>>>> set_mkfs_prog_path_with_opts()
>>>>>> {
>>>>>> @@ -121,12 +112,6 @@ set_mkfs_prog_path_with_opts()
>>>>>> fi
>>>>>> }
>>>>>> -_fatal()
>>>>>> -{
>>>>>> - echo "$*"
>>>>>> - _exit 1
>>>>>> -}
>>>>>> -
>>>>>> export MKFS_PROG="$(type -P mkfs)"
>>>>>> [ "$MKFS_PROG" = "" ] && _fatal "mkfs not found"
>>>>>> diff --git a/common/dump b/common/dump
>>>>>> index 09859006..4701a956 100644
>>>>>> --- a/common/dump
>>>>>> +++ b/common/dump
>>>>>> @@ -3,6 +3,7 @@
>>>>>> # Copyright (c) 2000-2002,2005 Silicon Graphics, Inc. All Rights Reserved.
>>>>>> #
>>>>>> # Functions useful for xfsdump/xfsrestore tests
>>>>>> +. common/exit
>>>>>> # --- initializations ---
>>>>>> rm -f $seqres.full
>>>>>> diff --git a/common/exit b/common/exit
>>>>>> new file mode 100644
>>>>>> index 00000000..ad7e7498
>>>>>> --- /dev/null
>>>>>> +++ b/common/exit
>>>>>> @@ -0,0 +1,50 @@
>>>>>> +##/bin/bash
>>>>>> +
>>>>>> +# This functions sets the exit code to status and then exits. Don't use
>>>>>> +# exit directly, as it might not set the value of "$status" correctly, which is
>>>>>> +# used as an exit code in the trap handler routine set up by the check script.
>>>>>> +_exit()
>>>>>> +{
>>>>>> + test -n "$1" && status="$1"
>>>>>> + exit "$status"
>>>>>> +}
>>>>>> +
>>>>>> +_fatal()
>>>>>> +{
>>>>>> + echo "$*"
>>>>>> + _exit 1
>>>>>> +}
>>>>>> +
>>>>>> +_die()
>>>>>> +{
>>>>>> + echo $@
>>>>>> + _exit 1
>>>>>> +}
>>>>>> +
>>>>>> +die_now()
>>>>>> +{
>>>>>> + _exit 1
>>>>>> +}
>>>>>> +
>>>>>> +# just plain bail out
>>>>>> +#
>>>>>> +_fail()
>>>>>> +{
>>>>>> + echo "$*" | tee -a $seqres.full
>>>>>> + echo "(see $seqres.full for details)"
>>>>>> + _exit 1
>>>>>> +}
>>>>>> +
>>>>>> +# bail out, setting up .notrun file. Need to kill the filesystem check files
>>>>>> +# here, otherwise they are set incorrectly for the next test.
>>>>>> +#
>>>>>> +_notrun()
>>>>>> +{
>>>>>> + echo "$*" > $seqres.notrun
>>>>>> + echo "$seq not run: $*"
>>>>>> + rm -f ${RESULT_DIR}/require_test*
>>>>>> + rm -f ${RESULT_DIR}/require_scratch*
>>>>>> +
>>>>>> + _exit 0
>>>>>> +}
>>>>>> +
>>>>>> diff --git a/common/ext4 b/common/ext4
>>>>>> index f88fa532..ab566c41 100644
>>>>>> --- a/common/ext4
>>>>>> +++ b/common/ext4
>>>>>> @@ -1,7 +1,7 @@
>>>>>> #
>>>>>> # ext4 specific common functions
>>>>>> #
>>>>>> -
>>>>>> +. common/exit
>>>>>> __generate_ext4_report_vars() {
>>>>>> __generate_blockdev_report_vars TEST_LOGDEV
>>>>>> __generate_blockdev_report_vars SCRATCH_LOGDEV
>>>>>> diff --git a/common/populate b/common/populate
>>>>>> index 50dc75d3..a17acc9e 100644
>>>>>> --- a/common/populate
>>>>>> +++ b/common/populate
>>>>>> @@ -4,7 +4,7 @@
>>>>>> #
>>>>>> # Routines for populating a scratch fs, and helpers to exercise an FS
>>>>>> # once it's been fuzzed.
>>>>>> -
>>>>>> +. common/exit
>>>>>> . ./common/quota
>>>>>> _require_populate_commands() {
>>>>>> diff --git a/common/preamble b/common/preamble
>>>>>> index ba029a34..0f306412 100644
>>>>>> --- a/common/preamble
>>>>>> +++ b/common/preamble
>>>>>> @@ -3,6 +3,7 @@
>>>>>> # Copyright (c) 2021 Oracle. All Rights Reserved.
>>>>>> # Boilerplate fstests functionality
>>>>>> +. common/exit
>>>>>> # Standard cleanup function. Individual tests can override this.
>>>>>> _cleanup()
>>>>>> diff --git a/common/punch b/common/punch
>>>>>> index 64d665d8..637f463f 100644
>>>>>> --- a/common/punch
>>>>>> +++ b/common/punch
>>>>>> @@ -3,6 +3,7 @@
>>>>>> # Copyright (c) 2007 Silicon Graphics, Inc. All Rights Reserved.
>>>>>> #
>>>>>> # common functions for excersizing hole punches with extent size hints etc.
>>>>>> +. common/exit
>>>>>> _spawn_test_file() {
>>>>>> echo "# spawning test file with $*"
>>>>>> @@ -222,11 +223,6 @@ _filter_bmap()
>>>>>> _coalesce_extents
>>>>>> }
>>>>>> -die_now()
>>>>>> -{
>>>>>> - _exit 1
>>>>>> -}
>>>>>> -
>>>>>> # test the different corner cases for zeroing a range:
>>>>>> #
>>>>>> # 1. into a hole
>>>>>> diff --git a/common/rc b/common/rc
>>>>>> index 9bed6dad..945f5134 100644
>>>>>> --- a/common/rc
>>>>>> +++ b/common/rc
>>>>>> @@ -2,6 +2,7 @@
>>>>>> # SPDX-License-Identifier: GPL-2.0+
>>>>>> # Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved.
>>>>>> +. common/exit
>>>>>> . common/config
>>>>>> BC="$(type -P bc)" || BC=
>>>>>> @@ -1798,28 +1799,6 @@ _do()
>>>>>> return $ret
>>>>>> }
>>>>>> -# bail out, setting up .notrun file. Need to kill the filesystem check files
>>>>>> -# here, otherwise they are set incorrectly for the next test.
>>>>>> -#
>>>>>> -_notrun()
>>>>>> -{
>>>>>> - echo "$*" > $seqres.notrun
>>>>>> - echo "$seq not run: $*"
>>>>>> - rm -f ${RESULT_DIR}/require_test*
>>>>>> - rm -f ${RESULT_DIR}/require_scratch*
>>>>>> -
>>>>>> - _exit 0
>>>>>> -}
>>>>>> -
>>>>>> -# just plain bail out
>>>>>> -#
>>>>>> -_fail()
>>>>>> -{
>>>>>> - echo "$*" | tee -a $seqres.full
>>>>>> - echo "(see $seqres.full for details)"
>>>>>> - _exit 1
>>>>>> -}
>>>>>> -
>>>>>> #
>>>>>> # Tests whether $FSTYP should be exclude from this test.
>>>>>> #
>>>>>> @@ -3835,12 +3814,6 @@ _link_out_file()
>>>>>> _link_out_file_named $seqfull.out "$features"
>>>>>> }
>>>>>> -_die()
>>>>>> -{
>>>>>> - echo $@
>>>>>> - _exit 1
>>>>>> -}
>>>>>> -
>>>>>> # convert urandom incompressible data to compressible text data
>>>>>> _ddt()
>>>>>> {
>>>>>> diff --git a/common/repair b/common/repair
>>>>>> index fd206f8e..db6a1b5c 100644
>>>>>> --- a/common/repair
>>>>>> +++ b/common/repair
>>>>>> @@ -3,6 +3,7 @@
>>>>>> # Copyright (c) 2000-2002 Silicon Graphics, Inc. All Rights Reserved.
>>>>>> #
>>>>>> # Functions useful for xfs_repair tests
>>>>>> +. common/exit
>>>>>> _zero_position()
>>>>>> {
>>>>>> diff --git a/common/xfs b/common/xfs
>>>>>> index 96c15f3c..c236146c 100644
>>>>>> --- a/common/xfs
>>>>>> +++ b/common/xfs
>>>>>> @@ -1,6 +1,7 @@
>>>>>> #
>>>>>> # XFS specific common functions.
>>>>>> #
>>>>>> +. common/exit
>>>>>> __generate_xfs_report_vars() {
>>>>>> __generate_blockdev_report_vars TEST_RTDEV
>>>>>> --
>>>>>> 2.34.1
>>>>>>
>>>> --
>>>> Nirjhar Roy
>>>> Linux Kernel Developer
>>>> IBM, Bangalore
>>>>
>> --
>> Nirjhar Roy
>> Linux Kernel Developer
>> IBM, Bangalore
>>
--
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-04-28 8:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23 6:41 [PATCH v1 0/2] common: Move exit related functions to common/exit Nirjhar Roy (IBM)
2025-04-23 6:41 ` [PATCH v1 1/2] common: Move exit related functions to a common/exit Nirjhar Roy (IBM)
2025-04-23 14:18 ` Zorro Lang
2025-04-24 9:09 ` Nirjhar Roy (IBM)
2025-04-25 11:27 ` Zorro Lang
2025-04-25 12:03 ` Nirjhar Roy (IBM)
2025-04-25 13:36 ` Zorro Lang
2025-04-28 8:39 ` Nirjhar Roy (IBM)
2025-04-23 6:41 ` [PATCH v1 2/2] check: Replace exit with _exit in check Nirjhar Roy (IBM)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).