Re: [PATCH v3 1/2] ext4: resize fs after resize_inode without e2fsck

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]





在 2022/7/16 2:08, Zorro Lang 写道:
On Thu, Jul 14, 2022 at 06:00:34PM -0400, Theodore Ts'o wrote:
On Thu, Jul 14, 2022 at 11:46:07PM +0800, Zorro Lang wrote:
On Wed, Jul 13, 2022 at 05:28:58PM +0800, Sun Ke wrote:
+
+# forget to run requested e2fsck after resize_inode
+$TUNE2FS_PROG -O ^resize_inode $SCRATCH_DEV | grep -w "e2fsck"
+
+_scratch_mount
+
+# resize fs will trigger NULL pointer in ext4_flex_group_add
+$RESIZE2FS_PROG $SCRATCH_DEV 1G >> $seqres.full 2>&1
+
+echo "Silence is golden"
   ...
diff --git a/tests/ext4/057.out b/tests/ext4/057.out
new file mode 100644
index 00000000..4784ad7e
--- /dev/null
+++ b/tests/ext4/057.out
@@ -0,0 +1,3 @@
+QA output created by 057
+Please run e2fsck -f on the filesystem.

If you hope to match this line, means this case isn't "Silence is golden".

I don't know why you'd to have this line, it looks not suit to be golden
image. If you'd like to make sure current ext4 supports "resize_inode"
feature, you can use:
   _require_scratch_ext4_feature resize_inode

That's not the problem.

The "tune2fs -O ^resize_inode" command is printing that message as a
reminder that it would be a Really Good idea to run e2fsck on the file
system, because tune2fs doesn't completely remove the resize inode
after turning off that feature.

The commit which this test is trying to verify is that the kernel
won't oops if the system adminsitrator ignores the rather explicit
request:

Please run e2fsck -f on the filesystem.

... and blithely mounts the file system without running fsck -f on the
file system first.  While it could be argued that a system
administrator which fails to follow instructions deserves everything
they get, we decided the as a quality of implementation issue, it
would be better if the kernel didn't dereference a NULL pointer in
that case.  :-)

The one thing I'll note is that it is possible that at some point in
the future, tune2fs could be improved so that it cleanly removes the
resize_inode when the resize inode feature is removed, so that running
"fsck.ext4 -f" is no longer necessary.  So if you want to future-proof

Good to know :)

the test so it doesn't fail once tune2fs is made more idiot-proof, it
might be better if the test did something like this:

mke2fs -t ext4 -O ^resize_inode /dev/vdc 512m
debugfs -w -R "set_super_value s_reserved_gdt_blocks 100" /dev/vdc

So make sure there're reserved GDT blocks, even if disable resize_inode
feature.

mount -t ext4 /dev/vdc /vdc
resize2fs /dev/vdc 1G

Thanks Ted! That's really helpful to get review points from ext4 expert.

Hi Ke, would you mind re-sending this case refer to above review points?
You can refer to below code, but I didn't test it, so please test and make
sure it works and can reproduce the bug. Feel free to improve it if something
wrong.

_require_command "$DEBUGFS_PROG" debugfs
...

MKFS_OPTIONS="-O ^resize_inode $MKFS_OPTIONS" _scratch_mkfs_sized $dev_size \
	>>$seqres.full 2>&1 || _fail "mkfs failed"
$DEBUGFS_PROG -w -R "set_super_value s_reserved_gdt_blocks 100" $SCRATCH_DEV \
	>>$seqres.full 2>&1
$DEBUGFS_PROG -R "show_super_stats -h" $SCRATCH_DEV 2>/dev/null | \
	grep "Reserved GDT blocks"
_scratch_mount
$RESIZE2FS_PROG $SCRATCH_DEV 1g >> $seqres.full 2>&1


Thanks,
Zorro

two questions.

1. The test run fsck automatically, and complain inconsistent,I think it need not run fsck.
2. It warn missing kernel fix, but the commit had merged.

##test log##
[root@testvm xfstests-dev]# ./check tests/ext4/057
FSTYP         -- ext4
PLATFORM -- Linux/x86_64 testvm 5.10.0-00009-g08870cf6d88a #35 SMP Fri Jul 22 15:50:00 CST 2022
MKFS_OPTIONS  -- /dev/sdb
MOUNT_OPTIONS -- -o acl,user_xattr /dev/sdb /mnt/scratch

ext4/057 5s ... _check_generic_filesystem: filesystem on /dev/sdb is inconsistent
(see /root/xfstests-dev/results//ext4/057.full for details)


HINT: You _MAY_ be missing kernel fix:
      b55c3cd102a6 ext4: add reserved GDT blocks check

Ran: ext4/057
Failures: ext4/057
Failed 1 of 1 tests



##test##
# Modify as appropriate.
_supported_fs ext4
_fixed_by_kernel_commit b55c3cd102a6 \
    "ext4: add reserved GDT blocks check"

_require_scratch
_require_command "$TUNE2FS_PROG" tune2fs
_require_command "$RESIZE2FS_PROG" resize2fs
_require_command "$DEBUGFS_PROG" debugfs
_require_scratch_size $((1024 * 1024)) #kB

# set fs size 512M
dev_size=$((512 * 1024 * 1024))
MKFS_OPTIONS="-O ^resize_inode $MKFS_OPTIONS" _scratch_mkfs_sized $dev_size \
    >>$seqres.full 2>&1 || _fail "mkfs failed"

$DEBUGFS_PROG -w -R "set_super_value s_reserved_gdt_blocks 100" $SCRATCH_DEV \
    >>$seqres.full 2>&1

$DEBUGFS_PROG -R "show_super_stats -h" $SCRATCH_DEV 2>/dev/null | \
    grep "Reserved GDT blocks"

_scratch_mount

# resize fs will trigger NULL pointer in ext4_flex_group_add
$RESIZE2FS_PROG $SCRATCH_DEV 1G >> $seqres.full 2>&1





Translating the above from commands suitable for manual trial using
"kvm-xfstests shell" to a proper xfstests script is left as an
exercise for the reader.  :-)

					- Ted


.




[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux