[PATCH 1/4] common/config: remove redundant export of F2FS_IO_PROG

[PATCH 1/4] common/config: remove redundant export of F2FS_IO_PROG

[Chao Yu]

F2FS_IO_PROG was been exported twice, remove the redudant one.

Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 common/config | 1 -
 1 file changed, 1 deletion(-)

diff --git a/common/config b/common/config
index 2afbda14..8d3047a5 100644
--- a/common/config
+++ b/common/config
@@ -191,7 +191,6 @@ export XFS_COPY_PROG="$(type -P xfs_copy)"
 export FSTRIM_PROG="$(type -P fstrim)"
 export DUMPE2FS_PROG="$(type -P dumpe2fs)"
 export RESIZE2FS_PROG="$(type -P resize2fs)"
-export F2FS_IO_PROG="$(type -P f2fs_io)"
 export FIO_PROG="$(type -P fio)"
 export FILEFRAG_PROG="$(type -P filefrag)"
 export E4DEFRAG_PROG="$(type -P e4defrag)"
-- 
2.48.1

[PATCH 2/4] common/config: export F2FS_INJECT_PROG

[Chao Yu]

export F2FS_INJECT_PROG w/ inject.f2fs, it can be used for fault injection.

Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 common/config | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/config b/common/config
index 8d3047a5..05f192ea 100644
--- a/common/config
+++ b/common/config
@@ -317,6 +317,7 @@ export MKFS_BTRFS_PROG=$(set_mkfs_prog_path_with_opts btrfs)
 export MKFS_F2FS_PROG=$(set_mkfs_prog_path_with_opts f2fs)
 export DUMP_F2FS_PROG=$(type -P dump.f2fs)
 export F2FS_IO_PROG=$(type -P f2fs_io)
+export F2FS_INJECT_PROG=$(type -P inject.f2fs)
 export BTRFS_UTIL_PROG=$(type -P btrfs)
 export BTRFS_SHOW_SUPER_PROG=$(type -P btrfs-show-super)
 export BTRFS_CONVERT_PROG=$(type -P btrfs-convert)
-- 
2.48.1

Re: [PATCH 2/4] common/config: export F2FS_INJECT_PROG

[David Disseldorp]

On Thu,  6 Mar 2025 16:18:07 +0800, Chao Yu wrote:

> export F2FS_INJECT_PROG w/ inject.f2fs, it can be used for fault injection.
> 
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  common/config | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/common/config b/common/config
> index 8d3047a5..05f192ea 100644
> --- a/common/config
> +++ b/common/config
> @@ -317,6 +317,7 @@ export MKFS_BTRFS_PROG=$(set_mkfs_prog_path_with_opts btrfs)
>  export MKFS_F2FS_PROG=$(set_mkfs_prog_path_with_opts f2fs)
>  export DUMP_F2FS_PROG=$(type -P dump.f2fs)
>  export F2FS_IO_PROG=$(type -P f2fs_io)
> +export F2FS_INJECT_PROG=$(type -P inject.f2fs)
>  export BTRFS_UTIL_PROG=$(type -P btrfs)
>  export BTRFS_SHOW_SUPER_PROG=$(type -P btrfs-show-super)
>  export BTRFS_CONVERT_PROG=$(type -P btrfs-convert)

Reviewed-by: David Disseldorp <ddiss@suse.de>

[PATCH 3/4] common/rc: support f2fs in _repair_scratch_fs

[Chao Yu]

Support f2fs in _repair_scratch_fs(), and use -f to repair the image
forcely.

Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 common/rc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/rc b/common/rc
index ca755055..bf24da4e 100644
--- a/common/rc
+++ b/common/rc
@@ -1500,6 +1500,9 @@ _repair_scratch_fs()
 	# want the test to fail:
 	_check_scratch_fs
 	;;
+    f2fs)
+	fsck -t $FSTYP -f $SCRATCH_DEV
+	;;
     *)
 	local dev=$SCRATCH_DEV
 	local fstyp=$FSTYP
-- 
2.48.1

Re: [PATCH 3/4] common/rc: support f2fs in _repair_scratch_fs

[David Disseldorp]

On Thu,  6 Mar 2025 16:18:08 +0800, Chao Yu wrote:

> Support f2fs in _repair_scratch_fs(), and use -f to repair the image
> forcely.
> 
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  common/rc | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index ca755055..bf24da4e 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1500,6 +1500,9 @@ _repair_scratch_fs()
>  	# want the test to fail:
>  	_check_scratch_fs
>  	;;
> +    f2fs)
> +	fsck -t $FSTYP -f $SCRATCH_DEV
> +	;;
>      *)
>  	local dev=$SCRATCH_DEV
>  	local fstyp=$FSTYP

Reviewed-by: David Disseldorp <ddiss@suse.de>

[PATCH 4/4] f2fs/009: detect and repair nlink corruption

[Chao Yu]

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
+
+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
-- 
2.48.1

Re: [PATCH 4/4] f2fs/009: detect and repair nlink corruption

[David Disseldorp]

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

Re: [PATCH 4/4] f2fs/009: detect and repair nlink corruption

[Chao Yu]

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
> 

Re: [PATCH 4/4] f2fs/009: detect and repair nlink corruption

[Zorro Lang]

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
> 
> 

Re: [PATCH 4/4] f2fs/009: detect and repair nlink corruption

[Chao Yu]

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
>>
>>
> 

Re: [PATCH 4/4] f2fs/009: detect and repair nlink corruption

[Zorro Lang]

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
> >>
> >>
> > 
> 

Re: [PATCH 4/4] f2fs/009: detect and repair nlink corruption

[Chao Yu]

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
>>>>
>>>>
>>>
>>
> 

Re: [PATCH 1/4] common/config: remove redundant export of F2FS_IO_PROG

[David Disseldorp]

On Thu,  6 Mar 2025 16:18:06 +0800, Chao Yu wrote:

> F2FS_IO_PROG was been exported twice, remove the redudant one.
> 
> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  common/config | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/common/config b/common/config
> index 2afbda14..8d3047a5 100644
> --- a/common/config
> +++ b/common/config
> @@ -191,7 +191,6 @@ export XFS_COPY_PROG="$(type -P xfs_copy)"
>  export FSTRIM_PROG="$(type -P fstrim)"
>  export DUMPE2FS_PROG="$(type -P dumpe2fs)"
>  export RESIZE2FS_PROG="$(type -P resize2fs)"
> -export F2FS_IO_PROG="$(type -P f2fs_io)"
>  export FIO_PROG="$(type -P fio)"
>  export FILEFRAG_PROG="$(type -P filefrag)"
>  export E4DEFRAG_PROG="$(type -P e4defrag)"

Looks good. @Zorro, might as well squash in removal of the duplicate
TIMEOUT_PROG export too.

Reviewed-by: David Disseldorp <ddiss@suse.de>

Re: [PATCH 1/4] common/config: remove redundant export of F2FS_IO_PROG

[Zorro Lang]

On Fri, Mar 07, 2025 at 02:56:29PM +1100, David Disseldorp wrote:
> On Thu,  6 Mar 2025 16:18:06 +0800, Chao Yu wrote:
> 
> > F2FS_IO_PROG was been exported twice, remove the redudant one.
> > 
> > Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> > Signed-off-by: Chao Yu <chao@kernel.org>
> > ---
> >  common/config | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/common/config b/common/config
> > index 2afbda14..8d3047a5 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -191,7 +191,6 @@ export XFS_COPY_PROG="$(type -P xfs_copy)"
> >  export FSTRIM_PROG="$(type -P fstrim)"
> >  export DUMPE2FS_PROG="$(type -P dumpe2fs)"
> >  export RESIZE2FS_PROG="$(type -P resize2fs)"
> > -export F2FS_IO_PROG="$(type -P f2fs_io)"
> >  export FIO_PROG="$(type -P fio)"
> >  export FILEFRAG_PROG="$(type -P filefrag)"
> >  export E4DEFRAG_PROG="$(type -P e4defrag)"
> 
> Looks good. @Zorro, might as well squash in removal of the duplicate
> TIMEOUT_PROG export too.

Sure, I roughly tried to find all duplication by:

  $ for prog in `sed -n "/export/s/export \(.*_PROG\)=.*/\1/p" common/config `;do n=`grep -E "\b$prog=" common/config|wc -l`;if [ $n -gt 1 ];then grep "\b$prog=" common/config;fi;done

Currently only found F2FS_IO_PROG and TIMEOUT_PROG. So I'll change this patch
commit to:

  common/config: remove redundant export variables

  F2FS_IO_PROG and TIMEOUT_PROG are exported twice, remove the
  redundant one.

Thanks,
Zorro

> 
> Reviewed-by: David Disseldorp <ddiss@suse.de>
> 

Re: [PATCH 1/4] common/config: remove redundant export of F2FS_IO_PROG

[Zorro Lang]

On Mon, Mar 10, 2025 at 03:16:36PM +0800, Zorro Lang wrote:
> On Fri, Mar 07, 2025 at 02:56:29PM +1100, David Disseldorp wrote:
> > On Thu,  6 Mar 2025 16:18:06 +0800, Chao Yu wrote:
> > 
> > > F2FS_IO_PROG was been exported twice, remove the redudant one.
> > > 
> > > Cc: Jaegeuk Kim <jaegeuk@kernel.org>
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > > ---
> > >  common/config | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/common/config b/common/config
> > > index 2afbda14..8d3047a5 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -191,7 +191,6 @@ export XFS_COPY_PROG="$(type -P xfs_copy)"
> > >  export FSTRIM_PROG="$(type -P fstrim)"
> > >  export DUMPE2FS_PROG="$(type -P dumpe2fs)"
> > >  export RESIZE2FS_PROG="$(type -P resize2fs)"
> > > -export F2FS_IO_PROG="$(type -P f2fs_io)"
> > >  export FIO_PROG="$(type -P fio)"
> > >  export FILEFRAG_PROG="$(type -P filefrag)"
> > >  export E4DEFRAG_PROG="$(type -P e4defrag)"
> > 
> > Looks good. @Zorro, might as well squash in removal of the duplicate
> > TIMEOUT_PROG export too.
> 
> Sure, I roughly tried to find all duplication by:
> 
>   $ for prog in `sed -n "/export/s/export \(.*_PROG\)=.*/\1/p" common/config `;do n=`grep -E "\b$prog=" common/config|wc -l`;if [ $n -gt 1 ];then grep "\b$prog=" common/config;fi;done
> 
> Currently only found F2FS_IO_PROG and TIMEOUT_PROG. So I'll change this patch
> commit to:
> 
>   common/config: remove redundant export variables
> 
>   F2FS_IO_PROG and TIMEOUT_PROG are exported twice, remove the
>   redundant one.

Hi Chao,

As you need to change patch 4/4 more. I thought you might change others, so
please change this patch refer to below:)

Thanks,
Zorro

Author: Chao Yu <chao@kernel.org>
Date:   Thu Mar 6 16:18:06 2025 +0800

    common/config: remove redundant export variables
    
    F2FS_IO_PROG and TIMEOUT_PROG are exported twice, remove the redudant
    one.
    
    Cc: Jaegeuk Kim <jaegeuk@kernel.org>
    Signed-off-by: Chao Yu <chao@kernel.org>
    Reviewed-by: David Disseldorp <ddiss@suse.de>


diff --git a/common/config b/common/config
index 2afbda141..aa5258250 100644
--- a/common/config
+++ b/common/config
@@ -191,7 +191,6 @@ export XFS_COPY_PROG="$(type -P xfs_copy)"
 export FSTRIM_PROG="$(type -P fstrim)"
 export DUMPE2FS_PROG="$(type -P dumpe2fs)"
 export RESIZE2FS_PROG="$(type -P resize2fs)"
-export F2FS_IO_PROG="$(type -P f2fs_io)"
 export FIO_PROG="$(type -P fio)"
 export FILEFRAG_PROG="$(type -P filefrag)"
 export E4DEFRAG_PROG="$(type -P e4defrag)"
@@ -220,7 +219,6 @@ export UBIUPDATEVOL_PROG="$(type -P ubiupdatevol)"
 export THIN_CHECK_PROG="$(type -P thin_check)"
 export PYTHON3_PROG="$(type -P python3)"
 export SQLITE3_PROG="$(type -P sqlite3)"
-export TIMEOUT_PROG="$(type -P timeout)"
 export SETCAP_PROG="$(type -P setcap)"
 export GETCAP_PROG="$(type -P getcap)"
 export CAPSH_PROG="$(type -P capsh)"

> 
> Thanks,
> Zorro
> 
> > 
> > Reviewed-by: David Disseldorp <ddiss@suse.de>
> > 

Re: [PATCH 1/4] common/config: remove redundant export of F2FS_IO_PROG

[Chao Yu]

On 3/10/25 16:02, Zorro Lang wrote:
> On Mon, Mar 10, 2025 at 03:16:36PM +0800, Zorro Lang wrote:
>> On Fri, Mar 07, 2025 at 02:56:29PM +1100, David Disseldorp wrote:
>>> On Thu,  6 Mar 2025 16:18:06 +0800, Chao Yu wrote:
>>>
>>>> F2FS_IO_PROG was been exported twice, remove the redudant one.
>>>>
>>>> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
>>>> Signed-off-by: Chao Yu <chao@kernel.org>
>>>> ---
>>>>  common/config | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/common/config b/common/config
>>>> index 2afbda14..8d3047a5 100644
>>>> --- a/common/config
>>>> +++ b/common/config
>>>> @@ -191,7 +191,6 @@ export XFS_COPY_PROG="$(type -P xfs_copy)"
>>>>  export FSTRIM_PROG="$(type -P fstrim)"
>>>>  export DUMPE2FS_PROG="$(type -P dumpe2fs)"
>>>>  export RESIZE2FS_PROG="$(type -P resize2fs)"
>>>> -export F2FS_IO_PROG="$(type -P f2fs_io)"
>>>>  export FIO_PROG="$(type -P fio)"
>>>>  export FILEFRAG_PROG="$(type -P filefrag)"
>>>>  export E4DEFRAG_PROG="$(type -P e4defrag)"
>>>
>>> Looks good. @Zorro, might as well squash in removal of the duplicate
>>> TIMEOUT_PROG export too.
>>
>> Sure, I roughly tried to find all duplication by:
>>
>>   $ for prog in `sed -n "/export/s/export \(.*_PROG\)=.*/\1/p" common/config `;do n=`grep -E "\b$prog=" common/config|wc -l`;if [ $n -gt 1 ];then grep "\b$prog=" common/config;fi;done
>>
>> Currently only found F2FS_IO_PROG and TIMEOUT_PROG. So I'll change this patch
>> commit to:
>>
>>   common/config: remove redundant export variables
>>
>>   F2FS_IO_PROG and TIMEOUT_PROG are exported twice, remove the
>>   redundant one.
> 
> Hi Chao,
> 
> As you need to change patch 4/4 more. I thought you might change others, so
> please change this patch refer to below:)

Zorro, yes, let me update all in v2, thank you!

Thanks,

> 
> Thanks,
> Zorro
> 
> Author: Chao Yu <chao@kernel.org>
> Date:   Thu Mar 6 16:18:06 2025 +0800
> 
>     common/config: remove redundant export variables
>     
>     F2FS_IO_PROG and TIMEOUT_PROG are exported twice, remove the redudant
>     one.
>     
>     Cc: Jaegeuk Kim <jaegeuk@kernel.org>
>     Signed-off-by: Chao Yu <chao@kernel.org>
>     Reviewed-by: David Disseldorp <ddiss@suse.de>
> 
> 
> diff --git a/common/config b/common/config
> index 2afbda141..aa5258250 100644
> --- a/common/config
> +++ b/common/config
> @@ -191,7 +191,6 @@ export XFS_COPY_PROG="$(type -P xfs_copy)"
>  export FSTRIM_PROG="$(type -P fstrim)"
>  export DUMPE2FS_PROG="$(type -P dumpe2fs)"
>  export RESIZE2FS_PROG="$(type -P resize2fs)"
> -export F2FS_IO_PROG="$(type -P f2fs_io)"
>  export FIO_PROG="$(type -P fio)"
>  export FILEFRAG_PROG="$(type -P filefrag)"
>  export E4DEFRAG_PROG="$(type -P e4defrag)"
> @@ -220,7 +219,6 @@ export UBIUPDATEVOL_PROG="$(type -P ubiupdatevol)"
>  export THIN_CHECK_PROG="$(type -P thin_check)"
>  export PYTHON3_PROG="$(type -P python3)"
>  export SQLITE3_PROG="$(type -P sqlite3)"
> -export TIMEOUT_PROG="$(type -P timeout)"
>  export SETCAP_PROG="$(type -P setcap)"
>  export GETCAP_PROG="$(type -P getcap)"
>  export CAPSH_PROG="$(type -P capsh)"
> 
>>
>> Thanks,
>> Zorro
>>
>>>
>>> Reviewed-by: David Disseldorp <ddiss@suse.de>
>>>
>