* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.