Re: [PATCH] ext4: regression test for "tune2fs -l" after ext4 shutdown
From: Zorro Lang <hidden>
Date: 2021-08-18 11:32:41
Also in:
linux-ext4
On Wed, Aug 18, 2021 at 04:40:56PM +0800, bxue@redhat.com wrote:
From: Boyang Xue <redacted> Regression test for: ext4: Fix tune2fs checksum failure for mounted filesystem
Better to specify the commit id number. I saw Ted has applied that patch: https://lore.kernel.org/linux-ext4/162895105421.460437.8931255765382647790.b4-ty@mit.edu/ (local) And maybe you can describe *a little* more in commit log.
quoted hunk ↗ jump to hunk
Signed-off-by: Boyang Xue <redacted> --- Hi, This is a new regression test for the patchext4: Fix tune2fs checksum failure for mounted filesystem Commit 81414b4dd48 ("ext4: remove redundant sb checksum recomputation") removed checksum recalculation after updating superblock free space / inode counters in ext4_fill_super() based on the fact that we will recalculate the checksum on superblock writeout. That is correct assumption but until the writeout happens (which can take a long time) the checksum is incorrect in the buffer cache and if tune2fs is called in that time window it will complain. So return back the checksum recalculation and add a comment explaining the tune2fs peculiarity. Fixes: 81414b4dd48f ("ext4: remove redundant sb checksum recomputation") Reported-by: Boyang Xue <bxue@xxxxxxxxxx> Signed-off-by: Jan Kara <jack@xxxxxxx>It's expected to fail on kernels from the kernel-5.11-rc1 to the latest version, where tune2fs fails with:tune2fs 1.46.2 (28-Feb-2021) tune2fs: Superblock checksum does not match superblock while trying to open /dev/loop0 Couldn't find valid filesystem superblock.Please help review this test, Thanks! -Boyang tests/ext4/309 | 42 ++++++++++++++++++++++++++++++++++++++++++ tests/ext4/309.out | 2 ++ 2 files changed, 44 insertions(+) create mode 100755 tests/ext4/309 create mode 100644 tests/ext4/309.outdiff --git a/tests/ext4/309 b/tests/ext4/309 new file mode 100755 index 00000000..ae335617 --- /dev/null +++ b/tests/ext4/309@@ -0,0 +1,42 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2021 YOUR NAME HERE. All Rights Reserved.
^^^^^^^^^^^^^^
Write your copyright
+#
+# FS QA Test 309
+#
+# Test that tune2fs doesn't fail after ext4 shutdown
+# Regression test for commit:
+# ext4: Fix tune2fs checksum failure for mounted filesystem
+#
+. ./common/preamble
+_begin_fstest auto rw quick
+
+_cleanup()
+{
+ _scratch_unmount
+}I think the umount isn't necessary, so the specific _cleanup isn't needed either.
+ +# Import common functions. +. ./common/filter
Do you use any filter helpers below?
+ +# real QA test starts here +_supported_fs ext4
I'm wondering if this case can be a generic case, there's nothing ext4 specified operations, except this line: "$TUNE2FS_PROG -l $SCRATCH_DEV" Hmm... if we can change this line to something likes _get_fs_super(), it might help to make this test to be a generic test.
+_require_scratch +_require_scratch_shutdown +_require_command "$TUNE2FS_PROG" tune2fs + +echo "Silence is golden" + +_scratch_mkfs >/dev/null 2>&1 +_scratch_mount +echo "ext4/309" > $SCRATCH_MNT/309.tmp
It's sure this case will be "ext4/309", although you use "309" won't affect anything.
+_scratch_shutdown +_scratch_cycle_mount +$TUNE2FS_PROG -l $SCRATCH_DEV >> $seqres.full 2>&1 +if [ $? -eq 0 ]; then + status=0 +else + status=1 +fi
Don't need to change the status value, how about write as: $TUNE2FS_PROG -l $SCRATCH_DEV >/dev/null The error output will break the golden image directly. ( cc ext4 mailist, to get more review) Thanks, Zorro
quoted hunk ↗ jump to hunk
+ +exitdiff --git a/tests/ext4/309.out b/tests/ext4/309.out new file mode 100644 index 00000000..56330d65 --- /dev/null +++ b/tests/ext4/309.out@@ -0,0 +1,2 @@ +QA output created by 309 +Silence is golden-- 2.27.0