| Message ID | 20250306081809.2397527-4-chao@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] common/config: remove redundant export of F2FS_IO_PROG | expand |
Thanks for submitting the new test. It looks good aside from a few minor things below... On Thu, 6 Mar 2025 16:18:09 +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> > --- > tests/f2fs/009 | 142 +++++++++++++++++++++++++++++++++++++++++++++ > tests/f2fs/009.out | 2 + > 2 files changed, 144 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..8f6a3e11 > --- /dev/null > +++ b/tests/f2fs/009 > @@ -0,0 +1,142 @@ > +#! /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 > + > +_require_scratch You should probably check for F2FS_INJECT_PROG and skip if not present. Is it dependent on CONFIG_F2FS_FAULT_INJECTION? If so it'd be nice if you could check for that too. Flag the fix for the regression via e.g.: _fixed_by_git_commit f2fs-tools 958cd6e > + > +dir=$SCRATCH_MNT I think it'd be easier to follow if you dropped this alias and just used $SCRATCH_MNT directly. > +filename=$dir/foo > +hardlink=$dir/bar > + > +for ((i=0;i<14;i++)) do > + echo "round: " $i >> $seqres.full > + > + _scratch_mkfs "-f" >> $seqres.full > + _scratch_mount >> $seqres.full > + > + if [ $i == 0 ]; then > + touch $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` Use "stat -c '%i'" instead of piping into awk. Also, it looks like this is called in every round, so just move it after the elifs. > + nlink=0 > + elif [ $i == 1 ]; then > + mkdir $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=1 > + elif [ $i == 2 ]; then > + mknod $filename c 9 0 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=0 > + elif [ $i == 3 ]; then > + mknod $filename b 8 0 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=0 > + elif [ $i == 4 ]; then > + mkfifo $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=0 > + elif [ $i == 5 ]; then > + socket -s $filename >> $seqres.full 2>&1 & > + pid=$! > + sleep 2 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + kill $pid >> $seqres.full 2>&1 > + nlink=0 > + elif [ $i == 6 ]; then > + ln -s $dir/empty $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=0 > + elif [ $i == 7 ]; then > + # orphan inode > + touch $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + $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 $dir/ >> $seqres.full > + sleep 6 > + nlink=1 > + elif [ $i == 8 ]; then > + # hardlink on file > + touch $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 9 ]; then > + # hardlink on charactor > + mknod $filename c 9 0 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 10 ]; then > + # hardlink on blockdev > + mknod $filename b 8 0 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 11 ]; then > + # hardlink on pipe > + mkfifo $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 12 ]; then > + # hardlink on socket > + socket -s $filename >> $seqres.full 2>&1 & > + pid=$! > + sleep 2 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + kill $pid >> $seqres.full 2>&1 > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 13 ]; then > + # hardlink on symlink > + ln -s $dir/empty $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + fi > + > + if [ $i != 7 ]; then > + stat $dir/* >> $seqres.full > + fi > + 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 > + if [ $? != 0 ]; then > + exit > + fi xfstests failures are normally triggered via golden output mismatch instead of explicit status checks or _fail calls... Can you filter the inject / repair output so that it ends up in the golden output for comparison? > + export FSCK_OPTIONS="-f" > + _repair_scratch_fs >> $seqres.full > + if [ $? != 1 ]; then > + _fail "fsck can not detect and repair zero nlink corruption "$i > + exit > + fi > + > + export FSCK_OPTIONS="" > + _check_scratch_fs >> $seqres.full > + if [ $? != 0 ]; then > + _fail "fsck hasn't fixed nlink corruption "$i > + exit > + fi > + > + _scratch_mount >> $seqres.full > + _scratch_unmount > +done > + > +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
On Thu, Mar 06, 2025 at 04:18:09PM +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> > --- Hi Chao, I planned to merge patch 1~3 at first. But as you need to change this patch more, and might need to change other patches, so how about you send v2 with all this patchset. I'll wait your v2 :) Some review points as below... > tests/f2fs/009 | 142 +++++++++++++++++++++++++++++++++++++++++++++ > tests/f2fs/009.out | 2 + > 2 files changed, 144 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..8f6a3e11 > --- /dev/null > +++ b/tests/f2fs/009 > @@ -0,0 +1,142 @@ > +#! /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 > + > +_require_scratch > + > +dir=$SCRATCH_MNT > +filename=$dir/foo > +hardlink=$dir/bar > + > +for ((i=0;i<14;i++)) do > + echo "round: " $i >> $seqres.full > + > + _scratch_mkfs "-f" >> $seqres.full Can we give "-f" to _scratch_mkfs "f2fs" part? Likes: diff --git a/common/rc b/common/rc index bf24da4eb..cafe2f3dd 100644 --- a/common/rc +++ b/common/rc @@ -993,7 +993,7 @@ _scratch_mkfs() mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \"" ;; f2fs) - mkfs_cmd="$MKFS_F2FS_PROG" + mkfs_cmd="$MKFS_F2FS_PROG -f" mkfs_filter="cat" ;; > + _scratch_mount >> $seqres.full > + > + if [ $i == 0 ]; then > + touch $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=0 > + elif [ $i == 1 ]; then > + mkdir $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=1 > + elif [ $i == 2 ]; then > + mknod $filename c 9 0 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=0 > + elif [ $i == 3 ]; then > + mknod $filename b 8 0 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=0 > + elif [ $i == 4 ]; then > + mkfifo $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=0 > + elif [ $i == 5 ]; then > + socket -s $filename >> $seqres.full 2>&1 & > + pid=$! > + sleep 2 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + kill $pid >> $seqres.full 2>&1 > + nlink=0 > + elif [ $i == 6 ]; then > + ln -s $dir/empty $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + nlink=0 > + elif [ $i == 7 ]; then > + # orphan inode > + touch $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + $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 $dir/ >> $seqres.full > + sleep 6 > + nlink=1 > + elif [ $i == 8 ]; then > + # hardlink on file > + touch $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 9 ]; then > + # hardlink on charactor > + mknod $filename c 9 0 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 10 ]; then > + # hardlink on blockdev > + mknod $filename b 8 0 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 11 ]; then > + # hardlink on pipe > + mkfifo $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 12 ]; then > + # hardlink on socket > + socket -s $filename >> $seqres.full 2>&1 & > + pid=$! > + sleep 2 > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + kill $pid >> $seqres.full 2>&1 > + ln $filename $hardlink > + nlink=0 > + elif [ $i == 13 ]; then > + # hardlink on symlink > + ln -s $dir/empty $filename > + ino=`stat $filename | awk '/Inode:/ {print $4}'` > + ln $filename $hardlink > + nlink=0 > + fi > + > + if [ $i != 7 ]; then > + stat $dir/* >> $seqres.full > + fi > + 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 > + if [ $? != 0 ]; then > + exit > + fi > + > + export FSCK_OPTIONS="-f" You've set below code in _repair_scratch_fs(): f2fs) fsck -t $FSTYP -f $SCRATCH_DEV ;; The FSCK_OPTIONS="-f" is useless. > + _repair_scratch_fs >> $seqres.full > + if [ $? != 1 ]; then What does $?=1 mean? Does $?=1 mean finding corruption and fixed, $?=0 mean no corruption? If you want to detect there's a corruption, then fix it, then check if it's fixed. How about calling _check_scratch_fs at first to get the corruption error you expect, then call _repair_scratch_fs to fix it. Then call _check_scratch_fs to make sure the corruption is fixed? Something likes (just a rough example) _check_scratch_fs >>$seqres.full 2>&1 && _fail "can't find corruption" _repair_scratch_fs >> $seqres.full _check_scratch_fs > + _fail "fsck can not detect and repair zero nlink corruption "$i > + exit > + fi > + > + export FSCK_OPTIONS="" > + _check_scratch_fs >> $seqres.full I think _check_scratch_fs outputs nothing if run passed, right? _check_scratch_fs calls _check_generic_filesystem for f2fs, the FSCK_OPTIONS is "null" by default, so above FSCK_OPTIONS="" is useless too. > + if [ $? != 0 ]; then > + _fail "fsck hasn't fixed nlink corruption "$i > + exit > + fi > + > + _scratch_mount >> $seqres.full ">> $seqres.full" isn't necessary. > + _scratch_unmount > +done > + > +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 > -- > 2.48.1 > >
On 3/7/25 12:02, David Disseldorp wrote: > Thanks for submitting the new test. It looks good aside from a few minor > things below... > > On Thu, 6 Mar 2025 16:18:09 +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> >> --- >> tests/f2fs/009 | 142 +++++++++++++++++++++++++++++++++++++++++++++ >> tests/f2fs/009.out | 2 + >> 2 files changed, 144 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..8f6a3e11 >> --- /dev/null >> +++ b/tests/f2fs/009 >> @@ -0,0 +1,142 @@ >> +#! /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 >> + >> +_require_scratch > > You should probably check for F2FS_INJECT_PROG and skip if not present. > Is it dependent on CONFIG_F2FS_FAULT_INJECTION? If so it'd be nice if It isn't dependent on CONFIG_F2FS_FAULT_INJECTION. > you could check for that too. > > Flag the fix for the regression via e.g.: > _fixed_by_git_commit f2fs-tools 958cd6e > >> + >> +dir=$SCRATCH_MNT > > I think it'd be easier to follow if you dropped this alias and just used > $SCRATCH_MNT directly. > >> +filename=$dir/foo >> +hardlink=$dir/bar >> + >> +for ((i=0;i<14;i++)) do >> + echo "round: " $i >> $seqres.full >> + >> + _scratch_mkfs "-f" >> $seqres.full >> + _scratch_mount >> $seqres.full >> + >> + if [ $i == 0 ]; then >> + touch $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` > > Use "stat -c '%i'" instead of piping into awk. Also, it looks like this > is called in every round, so just move it after the elifs. > >> + nlink=0 >> + elif [ $i == 1 ]; then >> + mkdir $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=1 >> + elif [ $i == 2 ]; then >> + mknod $filename c 9 0 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=0 >> + elif [ $i == 3 ]; then >> + mknod $filename b 8 0 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=0 >> + elif [ $i == 4 ]; then >> + mkfifo $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=0 >> + elif [ $i == 5 ]; then >> + socket -s $filename >> $seqres.full 2>&1 & >> + pid=$! >> + sleep 2 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + kill $pid >> $seqres.full 2>&1 >> + nlink=0 >> + elif [ $i == 6 ]; then >> + ln -s $dir/empty $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=0 >> + elif [ $i == 7 ]; then >> + # orphan inode >> + touch $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + $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 $dir/ >> $seqres.full >> + sleep 6 >> + nlink=1 >> + elif [ $i == 8 ]; then >> + # hardlink on file >> + touch $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 9 ]; then >> + # hardlink on charactor >> + mknod $filename c 9 0 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 10 ]; then >> + # hardlink on blockdev >> + mknod $filename b 8 0 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 11 ]; then >> + # hardlink on pipe >> + mkfifo $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 12 ]; then >> + # hardlink on socket >> + socket -s $filename >> $seqres.full 2>&1 & >> + pid=$! >> + sleep 2 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + kill $pid >> $seqres.full 2>&1 >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 13 ]; then >> + # hardlink on symlink >> + ln -s $dir/empty $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + fi >> + >> + if [ $i != 7 ]; then >> + stat $dir/* >> $seqres.full >> + fi >> + 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 >> + if [ $? != 0 ]; then >> + exit >> + fi > > xfstests failures are normally triggered via golden output mismatch > instead of explicit status checks or _fail calls... Can you filter the > inject / repair output so that it ends up in the golden output for > comparison? I tried to avoid filting inject/repair outputs and compare the output w/ golden output in 009.out, since I'm not sure whether the output will change or not later in f2fs-tools... Will fix all others according to your comments, thanks a lot for the comments. Thanks, > >> + export FSCK_OPTIONS="-f" >> + _repair_scratch_fs >> $seqres.full >> + if [ $? != 1 ]; then >> + _fail "fsck can not detect and repair zero nlink corruption "$i >> + exit >> + fi >> + >> + export FSCK_OPTIONS="" >> + _check_scratch_fs >> $seqres.full >> + if [ $? != 0 ]; then >> + _fail "fsck hasn't fixed nlink corruption "$i >> + exit >> + fi >> + >> + _scratch_mount >> $seqres.full >> + _scratch_unmount >> +done >> + >> +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 >
On 3/10/25 16:00, Zorro Lang wrote: > On Thu, Mar 06, 2025 at 04:18:09PM +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> >> --- > > Hi Chao, > > I planned to merge patch 1~3 at first. But as you need to change this > patch more, and might need to change other patches, so how about you > send v2 with all this patchset. I'll wait your v2 :) > > Some review points as below... > >> tests/f2fs/009 | 142 +++++++++++++++++++++++++++++++++++++++++++++ >> tests/f2fs/009.out | 2 + >> 2 files changed, 144 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..8f6a3e11 >> --- /dev/null >> +++ b/tests/f2fs/009 >> @@ -0,0 +1,142 @@ >> +#! /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 >> + >> +_require_scratch >> + >> +dir=$SCRATCH_MNT >> +filename=$dir/foo >> +hardlink=$dir/bar >> + >> +for ((i=0;i<14;i++)) do >> + echo "round: " $i >> $seqres.full >> + >> + _scratch_mkfs "-f" >> $seqres.full > > Can we give "-f" to _scratch_mkfs "f2fs" part? Likes: Looks better, > > diff --git a/common/rc b/common/rc > index bf24da4eb..cafe2f3dd 100644 > --- a/common/rc > +++ b/common/rc > @@ -993,7 +993,7 @@ _scratch_mkfs() > mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \"" > ;; > f2fs) > - mkfs_cmd="$MKFS_F2FS_PROG" > + mkfs_cmd="$MKFS_F2FS_PROG -f" > mkfs_filter="cat" > ;; > > > >> + _scratch_mount >> $seqres.full >> + >> + if [ $i == 0 ]; then >> + touch $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=0 >> + elif [ $i == 1 ]; then >> + mkdir $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=1 >> + elif [ $i == 2 ]; then >> + mknod $filename c 9 0 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=0 >> + elif [ $i == 3 ]; then >> + mknod $filename b 8 0 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=0 >> + elif [ $i == 4 ]; then >> + mkfifo $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=0 >> + elif [ $i == 5 ]; then >> + socket -s $filename >> $seqres.full 2>&1 & >> + pid=$! >> + sleep 2 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + kill $pid >> $seqres.full 2>&1 >> + nlink=0 >> + elif [ $i == 6 ]; then >> + ln -s $dir/empty $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + nlink=0 >> + elif [ $i == 7 ]; then >> + # orphan inode >> + touch $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + $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 $dir/ >> $seqres.full >> + sleep 6 >> + nlink=1 >> + elif [ $i == 8 ]; then >> + # hardlink on file >> + touch $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 9 ]; then >> + # hardlink on charactor >> + mknod $filename c 9 0 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 10 ]; then >> + # hardlink on blockdev >> + mknod $filename b 8 0 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 11 ]; then >> + # hardlink on pipe >> + mkfifo $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 12 ]; then >> + # hardlink on socket >> + socket -s $filename >> $seqres.full 2>&1 & >> + pid=$! >> + sleep 2 >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + kill $pid >> $seqres.full 2>&1 >> + ln $filename $hardlink >> + nlink=0 >> + elif [ $i == 13 ]; then >> + # hardlink on symlink >> + ln -s $dir/empty $filename >> + ino=`stat $filename | awk '/Inode:/ {print $4}'` >> + ln $filename $hardlink >> + nlink=0 >> + fi >> + >> + if [ $i != 7 ]; then >> + stat $dir/* >> $seqres.full >> + fi >> + 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 >> + if [ $? != 0 ]; then >> + exit >> + fi >> + >> + export FSCK_OPTIONS="-f" > > You've set below code in _repair_scratch_fs(): > > f2fs) > fsck -t $FSTYP -f $SCRATCH_DEV > ;; > > The FSCK_OPTIONS="-f" is useless. > >> + _repair_scratch_fs >> $seqres.full >> + if [ $? != 1 ]; then > > What does $?=1 mean? Does $?=1 mean finding corruption and fixed, $?=0 mean no corruption? That's correct. > > If you want to detect there's a corruption, then fix it, then check if it's fixed. How about > calling _check_scratch_fs at first to get the corruption error you expect, then call > _repair_scratch_fs to fix it. Then call _check_scratch_fs to make sure the corruption is > fixed? > > Something likes (just a rough example) > > _check_scratch_fs >>$seqres.full 2>&1 && _fail "can't find corruption" You mean this? export FSCK_OPTIONS="--dry-run" _check_scratch_fs >>$seqres.full 2>&1 || _fail "can't find corruption" We need to export FSCK_OPTIONS w/ "--dry-run", otherwise _check_scratch_fs will be stuck once it detects corruption. > _repair_scratch_fs >> $seqres.full > _check_scratch_fs > >> + _fail "fsck can not detect and repair zero nlink corruption "$i >> + exit >> + fi >> + >> + export FSCK_OPTIONS="" >> + _check_scratch_fs >> $seqres.full > > I think _check_scratch_fs outputs nothing if run passed, right? > > _check_scratch_fs calls _check_generic_filesystem for f2fs, the FSCK_OPTIONS > is "null" by default, so above FSCK_OPTIONS="" is useless too. > >> + if [ $? != 0 ]; then >> + _fail "fsck hasn't fixed nlink corruption "$i >> + exit >> + fi >> + >> + _scratch_mount >> $seqres.full > > ">> $seqres.full" isn't necessary. Will update according to you comments, thanks a lot. Thanks, > >> + _scratch_unmount >> +done >> + >> +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 >> -- >> 2.48.1 >> >> >
On Mon, Mar 10, 2025 at 05:59:23PM +0800, Chao Yu wrote: > On 3/10/25 16:00, Zorro Lang wrote: > > On Thu, Mar 06, 2025 at 04:18:09PM +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> > >> --- > > [snip] > >> + $F2FS_INJECT_PROG --node --mb i_links --nid $ino --val $nlink $SCRATCH_DEV >> $seqres.full > >> + if [ $? != 0 ]; then > >> + exit > >> + fi > >> + > >> + export FSCK_OPTIONS="-f" > > > > You've set below code in _repair_scratch_fs(): > > > > f2fs) > > fsck -t $FSTYP -f $SCRATCH_DEV > > ;; > > > > The FSCK_OPTIONS="-f" is useless. > > > >> + _repair_scratch_fs >> $seqres.full > >> + if [ $? != 1 ]; then > > > > What does $?=1 mean? Does $?=1 mean finding corruption and fixed, $?=0 mean no corruption? > > That's correct. > > > > > If you want to detect there's a corruption, then fix it, then check if it's fixed. How about > > calling _check_scratch_fs at first to get the corruption error you expect, then call > > _repair_scratch_fs to fix it. Then call _check_scratch_fs to make sure the corruption is > > fixed? > > > > Something likes (just a rough example) > > > > _check_scratch_fs >>$seqres.full 2>&1 && _fail "can't find corruption" > > You mean this? > > export FSCK_OPTIONS="--dry-run" > _check_scratch_fs >>$seqres.full 2>&1 || _fail "can't find corruption" No, > > We need to export FSCK_OPTIONS w/ "--dry-run", otherwise _check_scratch_fs > will be stuck once it detects corruption. If so, you might need to give _check_scratch_fs (and _check_test_fs) a f2fs specific handling. Due to _check_scratch_fs aims to do "check" only, _repair_scratch_fs aims to do "repair", they have different target. When we call _check_scratch_fs, we hope it reports pass or corruption then return, neither "repair" nor "stuck". So if I understand correct, you might need: _check_scratch_fs() { case $FSTYP in ... f2fs) FSCK_OPTIONS="--dry-run" _check_generic_filesystem $device ;; ... } Or you have any better way to do f2fs check :) Thanks, Zorro > > > _repair_scratch_fs >> $seqres.full > > _check_scratch_fs > > > >> + _fail "fsck can not detect and repair zero nlink corruption "$i > >> + exit > >> + fi > >> + > >> + export FSCK_OPTIONS="" > >> + _check_scratch_fs >> $seqres.full > > > > I think _check_scratch_fs outputs nothing if run passed, right? > > > > _check_scratch_fs calls _check_generic_filesystem for f2fs, the FSCK_OPTIONS > > is "null" by default, so above FSCK_OPTIONS="" is useless too. > > > >> + if [ $? != 0 ]; then > >> + _fail "fsck hasn't fixed nlink corruption "$i > >> + exit > >> + fi > >> + > >> + _scratch_mount >> $seqres.full > > > > ">> $seqres.full" isn't necessary. > > Will update according to you comments, thanks a lot. > > Thanks, > > > > >> + _scratch_unmount > >> +done > >> + > >> +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 > >> -- > >> 2.48.1 > >> > >> > > >
On 3/11/25 03:48, Zorro Lang wrote: > On Mon, Mar 10, 2025 at 05:59:23PM +0800, Chao Yu wrote: >> On 3/10/25 16:00, Zorro Lang wrote: >>> On Thu, Mar 06, 2025 at 04:18:09PM +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> >>>> --- >>> > > [snip] > >>>> + $F2FS_INJECT_PROG --node --mb i_links --nid $ino --val $nlink $SCRATCH_DEV >> $seqres.full >>>> + if [ $? != 0 ]; then >>>> + exit >>>> + fi >>>> + >>>> + export FSCK_OPTIONS="-f" >>> >>> You've set below code in _repair_scratch_fs(): >>> >>> f2fs) >>> fsck -t $FSTYP -f $SCRATCH_DEV >>> ;; >>> >>> The FSCK_OPTIONS="-f" is useless. >>> >>>> + _repair_scratch_fs >> $seqres.full >>>> + if [ $? != 1 ]; then >>> >>> What does $?=1 mean? Does $?=1 mean finding corruption and fixed, $?=0 mean no corruption? >> >> That's correct. >> >>> >>> If you want to detect there's a corruption, then fix it, then check if it's fixed. How about >>> calling _check_scratch_fs at first to get the corruption error you expect, then call >>> _repair_scratch_fs to fix it. Then call _check_scratch_fs to make sure the corruption is >>> fixed? >>> >>> Something likes (just a rough example) >>> >>> _check_scratch_fs >>$seqres.full 2>&1 && _fail "can't find corruption" >> >> You mean this? >> >> export FSCK_OPTIONS="--dry-run" >> _check_scratch_fs >>$seqres.full 2>&1 || _fail "can't find corruption" > > No, > >> >> We need to export FSCK_OPTIONS w/ "--dry-run", otherwise _check_scratch_fs >> will be stuck once it detects corruption. > > If so, you might need to give _check_scratch_fs (and _check_test_fs) a f2fs > specific handling. Due to _check_scratch_fs aims to do "check" only, > _repair_scratch_fs aims to do "repair", they have different target. When > we call _check_scratch_fs, we hope it reports pass or corruption then return, > neither "repair" nor "stuck". So if I understand correct, you might need: Thank you for the explanation, now it's clear to me for use of those functions. > > _check_scratch_fs() > { > case $FSTYP in > ... > f2fs) > FSCK_OPTIONS="--dry-run" _check_generic_filesystem $device It seems not work for me, due to fsck can not accept long parameter started w/ --? I didn't dig into this now. fsck -t f2fs --dry-run /dev/vdb fsck from util-linux 2.40.2 fsck -t f2fs /dev/vdb -- --dry-run fsck from util-linux 2.40.2 Info: Dry run Info: MKFS version ... Done: 0.071639 secs So I used this in v2: + f2fs) + if [ "$FSCK_OPTIONS" == "--dry-run" ]; then + fsck -t $FSTYP $device -- $FSCK_OPTIONS >> $seqres.full 2>&1 + else + _check_generic_filesystem $device + fi + ;; Any suggestion? What about introducing _check_f2fs_filesystem and reuse codes in _check_generic_filesystem as much as possible, then replace fsck w/ $F2FS_FSCK_PROG in order to use --dry-run. Thanks, > ;; > ... > } > > Or you have any better way to do f2fs check :) > > Thanks, > Zorro > >> >>> _repair_scratch_fs >> $seqres.full >>> _check_scratch_fs >>> >>>> + _fail "fsck can not detect and repair zero nlink corruption "$i >>>> + exit >>>> + fi >>>> + >>>> + export FSCK_OPTIONS="" >>>> + _check_scratch_fs >> $seqres.full >>> >>> I think _check_scratch_fs outputs nothing if run passed, right? >>> >>> _check_scratch_fs calls _check_generic_filesystem for f2fs, the FSCK_OPTIONS >>> is "null" by default, so above FSCK_OPTIONS="" is useless too. >>> >>>> + if [ $? != 0 ]; then >>>> + _fail "fsck hasn't fixed nlink corruption "$i >>>> + exit >>>> + fi >>>> + >>>> + _scratch_mount >> $seqres.full >>> >>> ">> $seqres.full" isn't necessary. >> >> Will update according to you comments, thanks a lot. >> >> Thanks, >> >>> >>>> + _scratch_unmount >>>> +done >>>> + >>>> +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 >>>> -- >>>> 2.48.1 >>>> >>>> >>> >> >
diff --git a/tests/f2fs/009 b/tests/f2fs/009 new file mode 100755 index 00000000..8f6a3e11 --- /dev/null +++ b/tests/f2fs/009 @@ -0,0 +1,142 @@ +#! /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 + +_require_scratch + +dir=$SCRATCH_MNT +filename=$dir/foo +hardlink=$dir/bar + +for ((i=0;i<14;i++)) do + echo "round: " $i >> $seqres.full + + _scratch_mkfs "-f" >> $seqres.full + _scratch_mount >> $seqres.full + + if [ $i == 0 ]; then + touch $filename + ino=`stat $filename | awk '/Inode:/ {print $4}'` + nlink=0 + elif [ $i == 1 ]; then + mkdir $filename + ino=`stat $filename | awk '/Inode:/ {print $4}'` + nlink=1 + elif [ $i == 2 ]; then + mknod $filename c 9 0 + ino=`stat $filename | awk '/Inode:/ {print $4}'` + nlink=0 + elif [ $i == 3 ]; then + mknod $filename b 8 0 + ino=`stat $filename | awk '/Inode:/ {print $4}'` + nlink=0 + elif [ $i == 4 ]; then + mkfifo $filename + ino=`stat $filename | awk '/Inode:/ {print $4}'` + nlink=0 + elif [ $i == 5 ]; then + socket -s $filename >> $seqres.full 2>&1 & + pid=$! + sleep 2 + ino=`stat $filename | awk '/Inode:/ {print $4}'` + kill $pid >> $seqres.full 2>&1 + nlink=0 + elif [ $i == 6 ]; then + ln -s $dir/empty $filename + ino=`stat $filename | awk '/Inode:/ {print $4}'` + nlink=0 + elif [ $i == 7 ]; then + # orphan inode + touch $filename + ino=`stat $filename | awk '/Inode:/ {print $4}'` + $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 $dir/ >> $seqres.full + sleep 6 + nlink=1 + elif [ $i == 8 ]; then + # hardlink on file + touch $filename + ino=`stat $filename | awk '/Inode:/ {print $4}'` + ln $filename $hardlink + nlink=0 + elif [ $i == 9 ]; then + # hardlink on charactor + mknod $filename c 9 0 + ino=`stat $filename | awk '/Inode:/ {print $4}'` + ln $filename $hardlink + nlink=0 + elif [ $i == 10 ]; then + # hardlink on blockdev + mknod $filename b 8 0 + ino=`stat $filename | awk '/Inode:/ {print $4}'` + ln $filename $hardlink + nlink=0 + elif [ $i == 11 ]; then + # hardlink on pipe + mkfifo $filename + ino=`stat $filename | awk '/Inode:/ {print $4}'` + ln $filename $hardlink + nlink=0 + elif [ $i == 12 ]; then + # hardlink on socket + socket -s $filename >> $seqres.full 2>&1 & + pid=$! + sleep 2 + ino=`stat $filename | awk '/Inode:/ {print $4}'` + kill $pid >> $seqres.full 2>&1 + ln $filename $hardlink + nlink=0 + elif [ $i == 13 ]; then + # hardlink on symlink + ln -s $dir/empty $filename + ino=`stat $filename | awk '/Inode:/ {print $4}'` + ln $filename $hardlink + nlink=0 + fi + + if [ $i != 7 ]; then + stat $dir/* >> $seqres.full + fi + 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 + if [ $? != 0 ]; then + exit + fi + + export FSCK_OPTIONS="-f" + _repair_scratch_fs >> $seqres.full + if [ $? != 1 ]; then + _fail "fsck can not detect and repair zero nlink corruption "$i + exit + fi + + export FSCK_OPTIONS="" + _check_scratch_fs >> $seqres.full + if [ $? != 0 ]; then + _fail "fsck hasn't fixed nlink corruption "$i + exit + fi + + _scratch_mount >> $seqres.full + _scratch_unmount +done + +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> --- tests/f2fs/009 | 142 +++++++++++++++++++++++++++++++++++++++++++++ tests/f2fs/009.out | 2 + 2 files changed, 144 insertions(+) create mode 100755 tests/f2fs/009 create mode 100644 tests/f2fs/009.out