Re: [PATCH] f2fs/023: test sanity check condition w/ error injection

[Chao Yu <chao@kernel.org>]

Zorro,

On 11/1/25 23:23, Zorro Lang wrote:
> On Wed, Oct 29, 2025 at 07:54:13PM +0800, Chao Yu wrote:
>> After commit 5c1768b67250 ("f2fs: fix to do sanity check correctly on
>> i_inline_xattr_size"), f2fs should handle corrupted i_inline_xattr_size
>> correctly, let's add this regression testcase to check that.
>>
>> Cc: Jaegeuk Kim <jaegeuk@kernel.org>
>> Signed-off-by: Chao Yu <chao@kernel.org>
>> ---
> 
> Hi Chao,
> 
> Thanks for new test case for f2fs. Some review points as below :)
> 
>>  tests/f2fs/023     | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>  tests/f2fs/023.out |  2 ++
>>  2 files changed, 46 insertions(+)
>>  create mode 100755 tests/f2fs/023
>>  create mode 100644 tests/f2fs/023.out
>>
>> diff --git a/tests/f2fs/023 b/tests/f2fs/023
>> new file mode 100755
>> index 00000000..d1b7df32
>> --- /dev/null
>> +++ b/tests/f2fs/023
>> @@ -0,0 +1,44 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2025 Chao Yu <chao@kernel.org>
>> +#
>> +# FS QA Test No. f2fs/023
>> +#
>> +# This testcase tries to inject fault into inode.i_inline_xattr_size,
>> +# and check whether sanity check of f2fs can handle fault correctly.
>> +#
>> +
>> +. ./common/preamble
>> +_begin_fstest auto quick rw
>> +
>> +_fixed_by_kernel_commit 5c1768b67250 \
>> +        "f2fs: fix to do sanity check correctly on i_inline_xattr_size"
>> +
>> +_require_scratch_nocheck
>> +_require_inject_f2fs_command node i_inline
>> +_require_inject_f2fs_command node i_inline_xattr_size
>> +
>> +testfile=$SCRATCH_MNT/testfile
>> +
>> +# remove all mkfs options to avoid layout change of on-disk inode
>> +export MKFS_OPTIONS=""
>> +
>> +_scratch_mkfs "-O extra_attr,flexible_inline_xattr" >> $seqres.full
> 
> As you use specific mkfs options, so better to _fail "..." if mkfs fails.
> 
>> +_scratch_mount "-o inline_xattr_size=512" >>$seqres.full 2>&1
>> +touch $testfile
>> +_scratch_unmount
>> +
>> +# inject .i_inline field: clear F2FS_EXTRA_ATTR bit
>> +$F2FS_INJECT_PROG --node --mb i_inline --nid 4 --val 0x1 $SCRATCH_DEV \
>> +		>>$seqres.full || _fail "failed to inject i_inline"
>> +
>> +# inject .i_inline_xattr_size field from 512 to 2048
>> +$F2FS_INJECT_PROG --node --mb i_inline_xattr_size --nid 4 --val 2048 $SCRATCH_DEV \
>> +		>>$seqres.full || _fail "failed to inject i_inline_xattr_size"
> 
> I found my system doesn't support i_inline_xattr_size,
> 
>   # inject.f2fs -V
>  inject.f2fs 1.16.0 (2025-08-12)
> 
> I tried to removed above "_require_inject_f2fs_command node i_inline_xattr_size",
> but I found this test didn't exit from above _fail. The .full file got:
> 
>   [inject_inode:833] unknown or unsupported member "i_inline_xattr_size"
> 
> So are you sure the $F2FS_INJECT_PROG return error if "failed to inject i_inline_xattr_size" :)

Oops, I found that inject.f2fs missed to return correct value for error case,
let me fix this.

In order to keep compatibility for old inject.f2fs tool, let me check output
instead of return value from inject.f2fs.

Let me know if you have any other concern.

> 
>> +
>> +_scratch_mount
>> +
>> +getfattr -n user.test "$testfile" 2>&1 | awk -F ': ' '/Structure needs cleaning/ { print $NF }'
> 
> As you need "user" attribute, so you need "_require_attrs" (. ./common/attr), also
> the test group should have "attr".
> 
> If you don't need _getfattr, please use $GETFATTR_PROG at least.
> 
> And how about replace the "awk" with "_filter_scratch" (. ./common/filter)

Thanks for all your comments, will fix them all in v2. :)

Thankss,

> 
> Thanks,
> Zorro
> 
>> +
>> +status=0
>> +exit
>> diff --git a/tests/f2fs/023.out b/tests/f2fs/023.out
>> new file mode 100644
>> index 00000000..7f16f33f
>> --- /dev/null
>> +++ b/tests/f2fs/023.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 023
>> +Structure needs cleaning
>> -- 
>> 2.49.0
>>
>