| Message ID | 20250325125824.3367060-6-chao@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,1/6] common/config: remove redundant export variables | expand |
On Tue, Mar 25, 2025 at 08:58:24PM +0800, Chao Yu wrote: > This is a regression test to check whether fsck can handle corrupted > nlinks correctly, it uses inject.f2fs to inject nlinks w/ wrong value, > and expects fsck.f2fs can detect such corruption and do the repair. > > Cc: Jaegeuk Kim <jaegeuk@kernel.org> > Signed-off-by: Chao Yu <chao@kernel.org> > --- > v5: > - clean up codes suggested by Dave. > tests/f2fs/009 | 141 +++++++++++++++++++++++++++++++++++++++++++++ > tests/f2fs/009.out | 2 + > 2 files changed, 143 insertions(+) > create mode 100755 tests/f2fs/009 > create mode 100644 tests/f2fs/009.out > > diff --git a/tests/f2fs/009 b/tests/f2fs/009 > new file mode 100755 > index 00000000..864fdcfb > --- /dev/null > +++ b/tests/f2fs/009 > @@ -0,0 +1,141 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2025 Chao Yu. All Rights Reserved. > +# > +# FS QA Test No. f2fs/009 > +# > +# This is a regression test to check whether fsck can handle corrupted > +# nlinks correctly, it uses inject.f2fs to inject nlinks w/ wrong value, > +# and expects fsck.f2fs can detect such corruption and do the repair. > +# > +. ./common/preamble > +_begin_fstest auto quick > + > +if [ ! -x "$(type -P socket)" ]; then > + _notrun "Couldn't find socket" > +fi Perhaps something like: _require_command $(type -P socket) socket would be more consistent with all the other code that checks for installed utilities that a test requires? > +_require_scratch > +_require_command "$F2FS_INJECT_PROG" inject.f2fs > + > +_fixed_by_git_commit f2fs-tools 958cd6e \ > + "fsck.f2fs: support to repair corrupted i_links" > + > +filename=$SCRATCH_MNT/foo > +hardlink=$SCRATCH_MNT/bar > + > +_cleanup() > +{ > + if [ -n "$pid" ]; then > + kill $pid &> /dev/null > + wait > + fi > + cd / > + rm -r -f $tmp.* > +} > + > +_inject_and_check() Single leading "_" is reserved for fstests functions, not for local test functions. Just call this one "inject_and_test", because that is what it does, and call this one: > +inject_and_check() > +{ > + local nlink=$1 > + local create_hardlink=$2 > + local ino=$3 > + > + if [ -z $ino ]; then > + ino=`stat -c '%i' $filename` > + fi > + > + if [ $create_hardlink == 1 ]; then > + ln $filename $hardlink > + fi > + > + _inject_and_check $nlink $ino > +} something like check_links() Otherwise this is a good improvement. -Dave.
On Wed, Mar 26, 2025 at 10:20:30AM +1100, Dave Chinner wrote: > On Tue, Mar 25, 2025 at 08:58:24PM +0800, Chao Yu wrote: > > This is a regression test to check whether fsck can handle corrupted > > nlinks correctly, it uses inject.f2fs to inject nlinks w/ wrong value, > > and expects fsck.f2fs can detect such corruption and do the repair. > > > > Cc: Jaegeuk Kim <jaegeuk@kernel.org> > > Signed-off-by: Chao Yu <chao@kernel.org> > > --- > > v5: > > - clean up codes suggested by Dave. > > tests/f2fs/009 | 141 +++++++++++++++++++++++++++++++++++++++++++++ > > tests/f2fs/009.out | 2 + > > 2 files changed, 143 insertions(+) > > create mode 100755 tests/f2fs/009 > > create mode 100644 tests/f2fs/009.out > > > > diff --git a/tests/f2fs/009 b/tests/f2fs/009 > > new file mode 100755 > > index 00000000..864fdcfb > > --- /dev/null > > +++ b/tests/f2fs/009 > > @@ -0,0 +1,141 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2025 Chao Yu. All Rights Reserved. > > +# > > +# FS QA Test No. f2fs/009 > > +# > > +# This is a regression test to check whether fsck can handle corrupted > > +# nlinks correctly, it uses inject.f2fs to inject nlinks w/ wrong value, > > +# and expects fsck.f2fs can detect such corruption and do the repair. > > +# > > +. ./common/preamble > > +_begin_fstest auto quick > > + > > +if [ ! -x "$(type -P socket)" ]; then > > + _notrun "Couldn't find socket" > > +fi > > Perhaps something like: > > _require_command $(type -P socket) socket Good point! Maybe double quotation marks -- "$(type -P socket)" is helpful, due to if socket isn't installed, there will be only one argument. > > would be more consistent with all the other code that checks for > installed utilities that a test requires? > > > +_require_scratch > > +_require_command "$F2FS_INJECT_PROG" inject.f2fs > > + > > +_fixed_by_git_commit f2fs-tools 958cd6e \ > > + "fsck.f2fs: support to repair corrupted i_links" > > + > > +filename=$SCRATCH_MNT/foo > > +hardlink=$SCRATCH_MNT/bar > > + > > +_cleanup() > > +{ > > + if [ -n "$pid" ]; then > > + kill $pid &> /dev/null > > + wait > > + fi > > + cd / > > + rm -r -f $tmp.* > > +} > > + > > +_inject_and_check() > > Single leading "_" is reserved for fstests functions, not for local > test functions. > > Just call this one "inject_and_test", because that is what it does, > and call this one: > > > +inject_and_check() > > +{ > > + local nlink=$1 > > + local create_hardlink=$2 > > + local ino=$3 > > + > > + if [ -z $ino ]; then > > + ino=`stat -c '%i' $filename` > > + fi > > + > > + if [ $create_hardlink == 1 ]; then > > + ln $filename $hardlink > > + fi > > + > > + _inject_and_check $nlink $ino > > +} > > something like check_links() > > Otherwise this is a good improvement. Hi Chao, if you agree with all these changes, and don't need to change more, I can help to merge this patchset with above changes. Or you'd like to send a new version? Thanks, Zorro > > -Dave. > -- > Dave Chinner > david@fromorbit.com >
On 3/26/25 08:47, Zorro Lang wrote: > On Wed, Mar 26, 2025 at 10:20:30AM +1100, Dave Chinner wrote: >> On Tue, Mar 25, 2025 at 08:58:24PM +0800, Chao Yu wrote: >>> This is a regression test to check whether fsck can handle corrupted >>> nlinks correctly, it uses inject.f2fs to inject nlinks w/ wrong value, >>> and expects fsck.f2fs can detect such corruption and do the repair. >>> >>> Cc: Jaegeuk Kim <jaegeuk@kernel.org> >>> Signed-off-by: Chao Yu <chao@kernel.org> >>> --- >>> v5: >>> - clean up codes suggested by Dave. >>> tests/f2fs/009 | 141 +++++++++++++++++++++++++++++++++++++++++++++ >>> tests/f2fs/009.out | 2 + >>> 2 files changed, 143 insertions(+) >>> create mode 100755 tests/f2fs/009 >>> create mode 100644 tests/f2fs/009.out >>> >>> diff --git a/tests/f2fs/009 b/tests/f2fs/009 >>> new file mode 100755 >>> index 00000000..864fdcfb >>> --- /dev/null >>> +++ b/tests/f2fs/009 >>> @@ -0,0 +1,141 @@ >>> +#! /bin/bash >>> +# SPDX-License-Identifier: GPL-2.0 >>> +# Copyright (c) 2025 Chao Yu. All Rights Reserved. >>> +# >>> +# FS QA Test No. f2fs/009 >>> +# >>> +# This is a regression test to check whether fsck can handle corrupted >>> +# nlinks correctly, it uses inject.f2fs to inject nlinks w/ wrong value, >>> +# and expects fsck.f2fs can detect such corruption and do the repair. >>> +# >>> +. ./common/preamble >>> +_begin_fstest auto quick >>> + >>> +if [ ! -x "$(type -P socket)" ]; then >>> + _notrun "Couldn't find socket" >>> +fi >> >> Perhaps something like: >> >> _require_command $(type -P socket) socket > > Good point! Maybe double quotation marks -- "$(type -P socket)" is > helpful, due to if socket isn't installed, there will be only one > argument. > >> >> would be more consistent with all the other code that checks for Agreed. >> installed utilities that a test requires? >> >>> +_require_scratch >>> +_require_command "$F2FS_INJECT_PROG" inject.f2fs >>> + >>> +_fixed_by_git_commit f2fs-tools 958cd6e \ >>> + "fsck.f2fs: support to repair corrupted i_links" >>> + >>> +filename=$SCRATCH_MNT/foo >>> +hardlink=$SCRATCH_MNT/bar >>> + >>> +_cleanup() >>> +{ >>> + if [ -n "$pid" ]; then >>> + kill $pid &> /dev/null >>> + wait >>> + fi >>> + cd / >>> + rm -r -f $tmp.* >>> +} >>> + >>> +_inject_and_check() >> >> Single leading "_" is reserved for fstests functions, not for local >> test functions. Oh, got it. >> >> Just call this one "inject_and_test", because that is what it does, >> and call this one: >> >>> +inject_and_check() >>> +{ >>> + local nlink=$1 >>> + local create_hardlink=$2 >>> + local ino=$3 >>> + >>> + if [ -z $ino ]; then >>> + ino=`stat -c '%i' $filename` >>> + fi >>> + >>> + if [ $create_hardlink == 1 ]; then >>> + ln $filename $hardlink >>> + fi >>> + >>> + _inject_and_check $nlink $ino >>> +} >> >> something like check_links() >> >> Otherwise this is a good improvement. Thanks Dave for all your review and suggestion! > > Hi Chao, if you agree with all these changes, and don't need to change more, I can > help to merge this patchset with above changes. Or you'd like to send a new version? Zorro, I'm fine w/ all the changes, I'm appreciate for that if you can help to update the patch! Thanks, > > Thanks, > Zorro > >> >> -Dave. >> -- >> Dave Chinner >> david@fromorbit.com >> >
diff --git a/tests/f2fs/009 b/tests/f2fs/009 new file mode 100755 index 00000000..864fdcfb --- /dev/null +++ b/tests/f2fs/009 @@ -0,0 +1,141 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2025 Chao Yu. All Rights Reserved. +# +# FS QA Test No. f2fs/009 +# +# This is a regression test to check whether fsck can handle corrupted +# nlinks correctly, it uses inject.f2fs to inject nlinks w/ wrong value, +# and expects fsck.f2fs can detect such corruption and do the repair. +# +. ./common/preamble +_begin_fstest auto quick + +if [ ! -x "$(type -P socket)" ]; then + _notrun "Couldn't find socket" +fi + +_require_scratch +_require_command "$F2FS_INJECT_PROG" inject.f2fs + +_fixed_by_git_commit f2fs-tools 958cd6e \ + "fsck.f2fs: support to repair corrupted i_links" + +filename=$SCRATCH_MNT/foo +hardlink=$SCRATCH_MNT/bar + +_cleanup() +{ + if [ -n "$pid" ]; then + kill $pid &> /dev/null + wait + fi + cd / + rm -r -f $tmp.* +} + +_inject_and_check() +{ + local nlink=$1 + local ino=$2 + + echo "ino:"$ino >> $seqres.full + echo "nlink:"$nlink >> $seqres.full + + _scratch_unmount + + $F2FS_INJECT_PROG --node --mb i_links --nid $ino --val $nlink $SCRATCH_DEV \ + >> $seqres.full || _fail "fail to inject" + + _check_scratch_fs >> $seqres.full 2>&1 && _fail "can't find corruption" + _repair_scratch_fs >> $seqres.full + _check_scratch_fs >> $seqres.full 2>&1 || _fail "fsck can't fix corruption" + + _scratch_mount + _scratch_unmount + + _scratch_mkfs >> $seqres.full + _scratch_mount +} + +inject_and_check() +{ + local nlink=$1 + local create_hardlink=$2 + local ino=$3 + + if [ -z $ino ]; then + ino=`stat -c '%i' $filename` + fi + + if [ $create_hardlink == 1 ]; then + ln $filename $hardlink + fi + + _inject_and_check $nlink $ino +} + +_scratch_mkfs >> $seqres.full +_scratch_mount + +touch $filename +inject_and_check 0 0 + +mkdir $filename +inject_and_check 1 0 + +mknod $filename c 9 0 +inject_and_check 0 0 + +mknod $filename b 8 0 +inject_and_check 0 0 + +mkfifo $filename +inject_and_check 0 0 + +socket -s $filename >> $seqres.full 2>&1 & +pid=$! +sleep 2 +ino=`stat -c '%i' $filename` +kill $pid >> $seqres.full 2>&1 +inject_and_check 0 0 $ino + +ln -s $SCRATCH_MNT/empty $filename +inject_and_check 0 0 + +touch $filename +ino=`stat -c '%i' $filename` +$F2FS_IO_PROG write 1 0 1 zero atomic_commit $filename 5000 >> $seqres.full 2>&1 & +stat $filename >> $seqres.full +rm $filename +$F2FS_IO_PROG shutdown 1 $SCRATCH_MNT/ >> $seqres.full +sleep 6 +inject_and_check 1 0 $ino + +# hardlink +touch $filename +inject_and_check 0 1 + +mknod $filename c 9 0 +inject_and_check 0 1 + +mknod $filename b 8 0 +inject_and_check 0 1 + +mkfifo $filename +inject_and_check 0 1 + +socket -s $filename >> $seqres.full 2>&1 & +pid=$! +sleep 2 +ino=`stat -c '%i' $filename` +kill $pid >> $seqres.full 2>&1 +inject_and_check 0 1 $ino + +ln -s $SCRATCH_MNT/empty $filename +inject_and_check 0 1 + +echo "Silence is golden" + +status=0 +exit diff --git a/tests/f2fs/009.out b/tests/f2fs/009.out new file mode 100644 index 00000000..7e977155 --- /dev/null +++ b/tests/f2fs/009.out @@ -0,0 +1,2 @@ +QA output created by 009 +Silence is golden
This is a regression test to check whether fsck can handle corrupted nlinks correctly, it uses inject.f2fs to inject nlinks w/ wrong value, and expects fsck.f2fs can detect such corruption and do the repair. Cc: Jaegeuk Kim <jaegeuk@kernel.org> Signed-off-by: Chao Yu <chao@kernel.org> --- v5: - clean up codes suggested by Dave. tests/f2fs/009 | 141 +++++++++++++++++++++++++++++++++++++++++++++ tests/f2fs/009.out | 2 + 2 files changed, 143 insertions(+) create mode 100755 tests/f2fs/009 create mode 100644 tests/f2fs/009.out