Skip to contentSkip to conversation

tests/ext4: Ensure resizes with sparse_super2 are handled correctly

PATCH Superseded Ojaswin Mujoo 3 participants 9 messages Feb 7, 2022 – Feb 21, 2022

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Ojaswin Mujoo

author
#0 lore raw

cover tests/ext4: Ensure resizes with sparse_super2 are handled correctly

As detailed in the patch [1], kernel currently does not support resizes with sparse_super2 enabled. Before the above patch, if we used the EXT4_IOC_RESIZE_FS ioctl directly, wiht sparse_super2 enabled, the kernel used to still try the resize and ultimatley leave the fs in an inconsistent state. This also led to corruption and kernel BUGs.

This patchset adds a test for ext4 to ensure that the kernel handles resizes with sparse_super2 correctly, and returns -EOPNOTSUPP.

Summary:

Patch 1: Fix the src/ext4_resize.c script to return accurate error codes. Patch 2: Add the ext4 test for checking resize functionality

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b1489186cc8391e0c1e342f9fbc3eedf6b944c61

Ojaswin Mujoo (2):
src/ext4_resize.c: Refactor code and ensure accurate errno is returned
ext4: Test to ensure resize with sparse_super2 is handled correctly

 src/ext4_resize.c  |  46 +++++++++++++-------
 tests/ext4/056     | 102 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/056.out |   2 +
 3 files changed, 136 insertions(+), 14 deletions(-)
 create mode 100755 tests/ext4/056
 create mode 100644 tests/ext4/056.out
Signature · 1 line
2.27.0
author
#1 lore raw

PATCH 1/2 src/ext4_resize.c: Refactor code and ensure accurate errno is returned

The current implementation of ext4_resize returned 1 whenever there was an error. Modify this to return the correct error code. This is important for tests that rely on correct error reporting, by the kernel, for ext4 resize functionality.

Additionaly, perform some code cleanup.

Signed-off-by Ojaswin Mujoo <[email protected]>
---
 src/ext4_resize.c | 46 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 14 deletions(-)
src/ext4_resize.c +32 -14
index 39e16529..78b66432 100644
@@ -10,6 +10,7 @@
 #include <unistd.h>
 #include <stdint.h>
 #include <stdlib.h>
+#include <string.h>
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 

@@ -19,33 +20,50 @@ typedef unsigned long long __u64;
 #define EXT4_IOC_RESIZE_FS		_IOW('f', 16, __u64)
 #endif
 
+#define pr_error(fmt, ...) do { \
+	fprintf (stderr, "ext4_resize.c: " fmt, ##__VA_ARGS__); \
+} while (0)
+
+static void usage(void)
+{
+	fprintf(stdout, "\nUsage: ext4_resize [mnt_path] [new_size(blocks)]\n");
+}
+
 int main(int argc, char **argv)
 {
 	__u64	new_size;
 	int	error, fd;
-	char	*tmp = NULL;
+	char	*mnt_dir = NULL, *tmp = NULL;
 
 	if (argc != 3) {
-		fputs("insufficient arguments\n", stderr);
-		return 1;
-	}
-	fd = open(argv[1], O_RDONLY);
-	if (!fd) {
-		perror(argv[1]);
-		return 1;
+		error = EINVAL;
+		pr_error("insufficient arguments\n");
+		usage();
+		return error;
 	}
 
+	mnt_dir = argv[1];
+
 	errno = 0;
 	new_size = strtoull(argv[2], &tmp, 10);
 	if ((errno) || (*tmp != '\0')) {
-		fprintf(stderr, "%s: invalid new size\n", argv[0]);
-		return 1;
+		error = errno;
+		pr_error("invalid new size\n");
+		return error;
+	}
+
+	fd = open(mnt_dir, O_RDONLY);
+	if (fd < 0) {
+		error = errno;
+		pr_error("open() failed with error: %s\n", strerror(error));
+		return error;
 	}
 
-	error = ioctl(fd, EXT4_IOC_RESIZE_FS, &new_size);
-	if (error < 0) {
-		perror("EXT4_IOC_RESIZE_FS");
-		return 1;
+	if(ioctl(fd, EXT4_IOC_RESIZE_FS, &new_size) < 0) {
+		error = errno;
+		pr_error("EXT4_IOC_RESIZE_FS ioctl() failed with error: %s\n", strerror(error));
+		return error;
 	}
+
 	return 0;
 }
Signature · 1 line
2.27.0
Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] src/ext4_resize.c: Refactor code and ensure accurate errno is returned

Quoted 6 lines · Ojaswin Mujoo

The current implementation of ext4_resize returned 1 whenever there was an error. Modify this to return the correct error code. This is important for tests that rely on correct error reporting, by the kernel, for ext4 resize functionality.

Additionaly, perform some code cleanup.

Thanks for fixing the error return codes. This looks good to me.

Feel free to add -

Reviewed-by Ritesh Harjani <[email protected]>

Stats on running this stats on my dev machine

<on old kernels>
===================

qemu@qemu:~/src/tools/xfstests-dev$ sudo ./check -s ext4_4k ext4/056
SECTION       -- ext4_4k
FSTYP         -- ext4
PLATFORM      -- Linux/ppc64le qemu 5.4.0-100-generic #113-Ubuntu SMP Thu Feb 3 18:43:11 UTC 2022
MKFS_OPTIONS  -- -I 256 -O 64bit -F -b 4096 /dev/loop10
MOUNT_OPTIONS -- -o data=ordered /dev/loop10 /mnt1/scratch

ext4/056 [failed, exit status 1]- output mismatch (see /home/qemu/src/tools/xfstests-dev/results//ext4_4k/ext4/056.out.bad)

    --- tests/ext4/056.out      2022-02-19 06:55:22.233659113 +0000
    +++ /home/qemu/src/tools/xfstests-dev/results//ext4_4k/ext4/056.out.bad     2022-02-19 06:57:48.931542566 +0000
    @@ -1,2 +1,3 @@
     QA output created by 056
    -Test Succeeded!
    +_check_generic_filesystem: filesystem on /dev/loop10 is inconsistent
    +(see /home/qemu/src/tools/xfstests-dev/results//ext4_4k/ext4/056.full for details)
    ...
    (Run 'diff -u /home/qemu/src/tools/xfstests-dev/tests/ext4/056.out /home/qemu/src/tools/xfstests-dev/results//ext4_4k/ext4/056.out.bad'  to see the entire diff)

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

SECTION -- ext4_4k
=========================
Ran: ext4/056
Failures: ext4/056
Failed 1 of 1 tests

<on 5.16.0-rc4>
===================

qemu@qemu:~/src/tools/xfstests-dev$ sudo ./check -s ext4_4k -i 10 ext4/056
SECTION       -- ext4_4k
FSTYP         -- ext4
PLATFORM      -- Linux/ppc64le qemu 5.16.0-rc4+ #6 SMP Sat Jan 29 22:07:24 UTC 2022
MKFS_OPTIONS  -- -I 256 -O 64bit -F -b 4096 /dev/loop10
MOUNT_OPTIONS -- -o data=ordered /dev/loop10 /mnt1/scratch

ext4/056 9s ... 11s
Ran: ext4/056
Passed all 1 tests

-ritesh

author
#3 lore raw

PATCH 2/2 ext4: Test to ensure resize with sparse_super2 is handled correctly

In reply to Ojaswin Mujoo

Kernel currently doesn't support resize of EXT4 mounted
with sparse_super2 option enabled. Earlier, it used to leave the resize incomplete and the fs would be left in an inconsistent state, however commit b1489186cc83[1] fixed this to avoid the fs corruption by clearly returning -ENOTSUPP.

Test to ensure that kernel handles resizing with sparse_super2 correctly. Run resize for multiple iterations because this leads to kernel crash due to fs corruption, which we want to detect.

Related commit in mainline:

[1] commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61

ext4: add check to prevent attempting to resize an fs with sparse_super2

Signed-off-by Ojaswin Mujoo <[email protected]>
---
 tests/ext4/056     | 102 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/056.out |   2 +
 2 files changed, 104 insertions(+)
 create mode 100755 tests/ext4/056
 create mode 100644 tests/ext4/056.out
tests/ext4/056 new file +102 -0
new file mode 100755
index 00000000..9185621d
@@ -0,0 +1,102 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 IBM. All Rights Reserved.
+#
+# We don't currently support resize of EXT4 filesystems mounted
+# with sparse_super2 option enabled. Earlier, kernel used to leave the resize
+# incomplete and the fs would be left into an incomplete state, however commit
+# b1489186cc83 fixed this to avoid the fs corruption by clearly returning
+# -ENOTSUPP.
+#
+# This test ensures that kernel handles resizing with sparse_super2 correctly
+#
+# Related commit in mainline:
+#
+# commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61
+# ext4: add check to prevent attempting to resize an fs with sparse_super2
+#
+
+. ./common/preamble
+_begin_fstest auto ioctl resize quick
+
+# real QA test starts here
+
+INITIAL_FS_SIZE=1G
+RESIZED_FS_SIZE=$((2*1024*1024*1024))  # 2G
+ONLINE_RESIZE_BLOCK_LIMIT=$((256*1024*1024))
+
+_supported_fs ext4
+_require_scratch_size $(($RESIZED_FS_SIZE/1024))
+_require_test_program "ext4_resize"
+
+_log()
+{
+	echo "$seq: $*" >> $seqres.full 2>&1
+}
+
+do_resize()
+{
+
+	$MKFS_PROG `_scratch_mkfs_options -t ext4 -E resize=$ONLINE_RESIZE_BLOCK_LIMIT \
+		-O sparse_super2` $INITIAL_FS_SIZE >> $seqres.full 2>&1 \
+		|| _fail "$MKFS_PROG failed. Exiting"
+
+	_scratch_mount || _fail "Failed to mount scratch partition. Exiting"
+
+	BS=$(_get_block_size $SCRATCH_MNT)
+	NEW_BLOCKS=$(($RESIZED_FS_SIZE/$BS))
+
+	local RESIZE_RET
+	local EOPNOTSUPP=95
+
+	$here/src/ext4_resize $SCRATCH_MNT $NEW_BLOCKS >> $seqres.full 2>&1
+	RESIZE_RET=$?
+
+	# Use $RESIZE_RET for logging
+	if [ $RESIZE_RET = 0 ]
+	then
+		_log "Resizing succeeded but FS might still be corrupt."
+	elif [ $RESIZE_RET = $EOPNOTSUPP ]
+	then
+		_log "Resize operation not supported with sparse_super2"
+		_log "Threw expected error $RESIZE_RET (EOPNOTSUPP)"
+
+	else
+		_log "Output of resize = $RESIZE_RET. Expected $EOPNOTSUPP (EOPNOTSUPP)"
+		_log "You might be missing kernel patch b1489186cc83"
+	fi
+
+	# unount after ioctl sometimes fails with "device busy" so add a small delay
+	sleep 0.1
+
+	_scratch_unmount >> $seqres.full 2>&1 || _fail "$UMOUNT_PROG failed. Exiting"
+}
+
+run_test()
+{
+	local FSCK_RET
+	local ITERS=8
+
+	for i in $(seq 1 $ITERS)
+	do
+		_log "----------- Iteration: $i ------------"
+		do_resize
+	done
+
+	_log "-------- Iterations Complete ---------"
+	_log "Checking if FS is in consistent state"
+	_check_scratch_fs
+	FSCK_RET=$?
+
+	return $FSCK_RET
+}
+
+run_test
+status=$?
+
+if [ "$status" -eq "0" ]
+then
+	echo "Test Succeeded!" | tee -a $seqres.full
+fi
+
+exit
tests/ext4/056.out new file +2 -0
new file mode 100644
index 00000000..41706284
@@ -0,0 +1,2 @@
+QA output created by 056
+Test Succeeded!
Signature · 1 line
2.27.0
#4 lore raw

Re: [PATCH 2/2] ext4: Test to ensure resize with sparse_super2 is handled correctly

Quoted 10 lines · Ojaswin Mujoo

Kernel currently doesn't support resize of EXT4 mounted
with sparse_super2 option enabled. Earlier, it used to leave the resize incomplete and the fs would be left in an inconsistent state, however commit b1489186cc83[1] fixed this to avoid the fs corruption by clearly returning -ENOTSUPP.

Test to ensure that kernel handles resizing with sparse_super2 correctly. Run resize for multiple iterations because this leads to kernel crash due to fs corruption, which we want to detect.

Quoted 5 lines

Related commit in mainline:

[1] commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61

ext4: add check to prevent attempting to resize an fs with sparse_super2

Thanks for the patch. Few nits below.

Quoted 63 lines
Signed-off-by Ojaswin Mujoo <[email protected]>
---
 tests/ext4/056     | 102 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/056.out |   2 +
 2 files changed, 104 insertions(+)
 create mode 100755 tests/ext4/056
 create mode 100644 tests/ext4/056.out
tests/ext4/056 new file +48 -0
new file mode 100755
index 00000000..9185621d
@@ -0,0 +1,102 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 IBM. All Rights Reserved.
+#
+# We don't currently support resize of EXT4 filesystems mounted
+# with sparse_super2 option enabled. Earlier, kernel used to leave the resize
+# incomplete and the fs would be left into an incomplete state, however commit
+# b1489186cc83 fixed this to avoid the fs corruption by clearly returning
+# -ENOTSUPP.
+#
+# This test ensures that kernel handles resizing with sparse_super2 correctly
+#
+# Related commit in mainline:
+#
+# commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61
+# ext4: add check to prevent attempting to resize an fs with sparse_super2
+#
+
+. ./common/preamble
+_begin_fstest auto ioctl resize quick
+
+# real QA test starts here
+
+INITIAL_FS_SIZE=1G
+RESIZED_FS_SIZE=$((2*1024*1024*1024))  # 2G
+ONLINE_RESIZE_BLOCK_LIMIT=$((256*1024*1024))
+
+_supported_fs ext4
+_require_scratch_size $(($RESIZED_FS_SIZE/1024))
+_require_test_program "ext4_resize"
+
+_log()
+{
+	echo "$seq: $*" >> $seqres.full 2>&1
+}
+
+do_resize()
+{
+
+	$MKFS_PROG `_scratch_mkfs_options -t ext4 -E resize=$ONLINE_RESIZE_BLOCK_LIMIT \
+		-O sparse_super2` $INITIAL_FS_SIZE >> $seqres.full 2>&1 \
+		|| _fail "$MKFS_PROG failed. Exiting"
+
+	_scratch_mount || _fail "Failed to mount scratch partition. Exiting"
+
+	BS=$(_get_block_size $SCRATCH_MNT)
+	NEW_BLOCKS=$(($RESIZED_FS_SIZE/$BS))
+

tests/ext4/056 line 58

Quoted 10 lines
tests/ext4/056 new file +10 -0
+	local RESIZE_RET
+	local EOPNOTSUPP=95
+
+	$here/src/ext4_resize $SCRATCH_MNT $NEW_BLOCKS >> $seqres.full 2>&1
+	RESIZE_RET=$?
+
+	# Use $RESIZE_RET for logging
+	if [ $RESIZE_RET = 0 ]
+	then
+		_log "Resizing succeeded but FS might still be corrupt."

IMO, this above logs is not required. Putting iteration count is more than enough. You could log more info if the resize fails. But above log is somewhat confusing to me.

tests/ext4/056 line 62

Quoted 4 lines
tests/ext4/056 new file +4 -0
+	elif [ $RESIZE_RET = $EOPNOTSUPP ]
+	then
+		_log "Resize operation not supported with sparse_super2"
+		_log "Threw expected error $RESIZE_RET (EOPNOTSUPP)"

If it fails with EOPNOTSUPP, do we still need to iterate in do_resize() 8 times?

tests/ext4/056 line 66

Quoted 4 lines
tests/ext4/056 new file +4 -0
+
+	else
+		_log "Output of resize = $RESIZE_RET. Expected $EOPNOTSUPP (EOPNOTSUPP)"
+		_log "You might be missing kernel patch b1489186cc83"

Not sure if we should pin point to a particular patch in this case. It could be that we add some features later and then some of those doesn't again support resize feature where it should return EOPNOTSUPP, but this test could capture that. So, I feel above may not be required.

tests/ext4/056 line 70

Quoted 4 lines
tests/ext4/056 new file +4 -0
+	fi
+
+	# unount after ioctl sometimes fails with "device busy" so add a small delay
+	sleep 0.1

Let's not add this sleep for EOPNOTSUPP case.

Quoted 43 lines
tests/ext4/056 new file +32 -0
+
+	_scratch_unmount >> $seqres.full 2>&1 || _fail "$UMOUNT_PROG failed. Exiting"
+}
+
+run_test()
+{
+	local FSCK_RET
+	local ITERS=8
+
+	for i in $(seq 1 $ITERS)
+	do
+		_log "----------- Iteration: $i ------------"
+		do_resize
+	done
+
+	_log "-------- Iterations Complete ---------"
+	_log "Checking if FS is in consistent state"
+	_check_scratch_fs
+	FSCK_RET=$?
+
+	return $FSCK_RET
+}
+
+run_test
+status=$?
+
+if [ "$status" -eq "0" ]
+then
+	echo "Test Succeeded!" | tee -a $seqres.full
+fi
+
+exit
tests/ext4/056.out new file +2 -1
new file mode 100644
index 00000000..41706284
@@ -0,0 +1,2 @@
+QA output created by 056
+Test Succeeded!
--

2.27.0

author
#5 lore raw

Re: [PATCH 2/2] ext4: Test to ensure resize with sparse_super2 is handled correctly

Hi Ritesh,

Thanks for the review.

Quoted 116 lines · Ritesh Harjani
Quoted 10 lines · Ojaswin Mujoo

Kernel currently doesn't support resize of EXT4 mounted
with sparse_super2 option enabled. Earlier, it used to leave the resize incomplete and the fs would be left in an inconsistent state, however commit b1489186cc83[1] fixed this to avoid the fs corruption by clearly returning -ENOTSUPP.

Test to ensure that kernel handles resizing with sparse_super2 correctly. Run resize for multiple iterations because this leads to kernel crash due to fs corruption, which we want to detect.

Quoted 5 lines

Related commit in mainline:

[1] commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61

ext4: add check to prevent attempting to resize an fs with sparse_super2

Thanks for the patch. Few nits below.

Quoted 63 lines
Signed-off-by Ojaswin Mujoo <[email protected]>
---
 tests/ext4/056     | 102 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/056.out |   2 +
 2 files changed, 104 insertions(+)
 create mode 100755 tests/ext4/056
 create mode 100644 tests/ext4/056.out
tests/ext4/056 new file +48 -0
new file mode 100755
index 00000000..9185621d
@@ -0,0 +1,102 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 IBM. All Rights Reserved.
+#
+# We don't currently support resize of EXT4 filesystems mounted
+# with sparse_super2 option enabled. Earlier, kernel used to leave the resize
+# incomplete and the fs would be left into an incomplete state, however commit
+# b1489186cc83 fixed this to avoid the fs corruption by clearly returning
+# -ENOTSUPP.
+#
+# This test ensures that kernel handles resizing with sparse_super2 correctly
+#
+# Related commit in mainline:
+#
+# commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61
+# ext4: add check to prevent attempting to resize an fs with sparse_super2
+#
+
+. ./common/preamble
+_begin_fstest auto ioctl resize quick
+
+# real QA test starts here
+
+INITIAL_FS_SIZE=1G
+RESIZED_FS_SIZE=$((2*1024*1024*1024))  # 2G
+ONLINE_RESIZE_BLOCK_LIMIT=$((256*1024*1024))
+
+_supported_fs ext4
+_require_scratch_size $(($RESIZED_FS_SIZE/1024))
+_require_test_program "ext4_resize"
+
+_log()
+{
+	echo "$seq: $*" >> $seqres.full 2>&1
+}
+
+do_resize()
+{
+
+	$MKFS_PROG `_scratch_mkfs_options -t ext4 -E resize=$ONLINE_RESIZE_BLOCK_LIMIT \
+		-O sparse_super2` $INITIAL_FS_SIZE >> $seqres.full 2>&1 \
+		|| _fail "$MKFS_PROG failed. Exiting"
+
+	_scratch_mount || _fail "Failed to mount scratch partition. Exiting"
+
+	BS=$(_get_block_size $SCRATCH_MNT)
+	NEW_BLOCKS=$(($RESIZED_FS_SIZE/$BS))
+

tests/ext4/056 line 58

Quoted 10 lines
tests/ext4/056 new file +10 -0
+	local RESIZE_RET
+	local EOPNOTSUPP=95
+
+	$here/src/ext4_resize $SCRATCH_MNT $NEW_BLOCKS >> $seqres.full 2>&1
+	RESIZE_RET=$?
+
+	# Use $RESIZE_RET for logging
+	if [ $RESIZE_RET = 0 ]
+	then
+		_log "Resizing succeeded but FS might still be corrupt."

IMO, this above logs is not required. Putting iteration count is more than enough. You could log more info if the resize fails. But above log is somewhat confusing to me.

tests/ext4/056 line 62

Quoted 4 lines
tests/ext4/056 new file +4 -0
+	elif [ $RESIZE_RET = $EOPNOTSUPP ]
+	then
+		_log "Resize operation not supported with sparse_super2"
+		_log "Threw expected error $RESIZE_RET (EOPNOTSUPP)"

If it fails with EOPNOTSUPP, do we still need to iterate in do_resize() 8 times?

tests/ext4/056 line 66

Quoted 4 lines
tests/ext4/056 new file +4 -0
+
+	else
+		_log "Output of resize = $RESIZE_RET. Expected $EOPNOTSUPP (EOPNOTSUPP)"
+		_log "You might be missing kernel patch b1489186cc83"

Not sure if we should pin point to a particular patch in this case. It could be that we add some features later and then some of those doesn't again support resize feature where it should return EOPNOTSUPP, but this test could capture that. So, I feel above may not be required.

Quoted 7 lines

tests/ext4/056 line 70

Quoted 4 lines
tests/ext4/056 new file +4 -0
+	fi
+
+	# unount after ioctl sometimes fails with "device busy" so add a small delay
+	sleep 0.1

Let's not add this sleep for EOPNOTSUPP case.

I've noted all the above points and I agree on them. I'll work on these and send out a v2.

Thanks and regards,
Ojaswin

Quoted 43 lines
Quoted 43 lines
tests/ext4/056 new file +32 -0
+
+	_scratch_unmount >> $seqres.full 2>&1 || _fail "$UMOUNT_PROG failed. Exiting"
+}
+
+run_test()
+{
+	local FSCK_RET
+	local ITERS=8
+
+	for i in $(seq 1 $ITERS)
+	do
+		_log "----------- Iteration: $i ------------"
+		do_resize
+	done
+
+	_log "-------- Iterations Complete ---------"
+	_log "Checking if FS is in consistent state"
+	_check_scratch_fs
+	FSCK_RET=$?
+
+	return $FSCK_RET
+}
+
+run_test
+status=$?
+
+if [ "$status" -eq "0" ]
+then
+	echo "Test Succeeded!" | tee -a $seqres.full
+fi
+
+exit
tests/ext4/056.out new file +2 -1
new file mode 100644
index 00000000..41706284
@@ -0,0 +1,2 @@
+QA output created by 056
+Test Succeeded!
--

2.27.0

#6 lore raw

Re: [PATCH 2/2] ext4: Test to ensure resize with sparse_super2 is handled correctly

In reply to Ojaswin Mujoo
Quoted 55 lines · Ojaswin Mujoo

Kernel currently doesn't support resize of EXT4 mounted
with sparse_super2 option enabled. Earlier, it used to leave the resize incomplete and the fs would be left in an inconsistent state, however commit b1489186cc83[1] fixed this to avoid the fs corruption by clearly returning -ENOTSUPP.

Test to ensure that kernel handles resizing with sparse_super2 correctly. Run resize for multiple iterations because this leads to kernel crash due to fs corruption, which we want to detect.

Related commit in mainline:

[1] commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61

ext4: add check to prevent attempting to resize an fs with sparse_super2

Signed-off-by Ojaswin Mujoo <[email protected]>
---
 tests/ext4/056     | 102 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/056.out |   2 +
 2 files changed, 104 insertions(+)
 create mode 100755 tests/ext4/056
 create mode 100644 tests/ext4/056.out
tests/ext4/056 new file +25 -0
new file mode 100755
index 00000000..9185621d
@@ -0,0 +1,102 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 IBM. All Rights Reserved.
+#
+# We don't currently support resize of EXT4 filesystems mounted
+# with sparse_super2 option enabled. Earlier, kernel used to leave the resize
+# incomplete and the fs would be left into an incomplete state, however commit
+# b1489186cc83 fixed this to avoid the fs corruption by clearly returning
+# -ENOTSUPP.
+#
+# This test ensures that kernel handles resizing with sparse_super2 correctly
+#
+# Related commit in mainline:
+#
+# commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61
+# ext4: add check to prevent attempting to resize an fs with sparse_super2
+#
+
+. ./common/preamble
+_begin_fstest auto ioctl resize quick
+
+# real QA test starts here
+
+INITIAL_FS_SIZE=1G
+RESIZED_FS_SIZE=$((2*1024*1024*1024))  # 2G

tests/ext4/056 line 35

Quoted 10 lines
tests/ext4/056 new file +10 -0
+ONLINE_RESIZE_BLOCK_LIMIT=$((256*1024*1024))
+
+_supported_fs ext4
+_require_scratch_size $(($RESIZED_FS_SIZE/1024))
+_require_test_program "ext4_resize"
+
+_log()
+{
+	echo "$seq: $*" >> $seqres.full 2>&1
+}

Leading under score is used for common functions, local functions don't need it.

tests/ext4/056 line 42

Quoted 7 lines
tests/ext4/056 new file +7 -0
+
+do_resize()
+{
+
+	$MKFS_PROG `_scratch_mkfs_options -t ext4 -E resize=$ONLINE_RESIZE_BLOCK_LIMIT \
+		-O sparse_super2` $INITIAL_FS_SIZE >> $seqres.full 2>&1 \
+		|| _fail "$MKFS_PROG failed. Exiting"

I think we could use _mkfs_dev here.

Quoted 6 lines
tests/ext4/056 new file +6 -0
+
+	_scratch_mount || _fail "Failed to mount scratch partition. Exiting"
+
+	BS=$(_get_block_size $SCRATCH_MNT)
+	NEW_BLOCKS=$(($RESIZED_FS_SIZE/$BS))
+

tests/ext4/056 line 58

Quoted 10 lines
tests/ext4/056 new file +10 -0
+	local RESIZE_RET
+	local EOPNOTSUPP=95
+
+	$here/src/ext4_resize $SCRATCH_MNT $NEW_BLOCKS >> $seqres.full 2>&1
+	RESIZE_RET=$?
+
+	# Use $RESIZE_RET for logging
+	if [ $RESIZE_RET = 0 ]
+	then
+		_log "Resizing succeeded but FS might still be corrupt."

I don't think _log is needed here, just echo message to stdout and this will break golden image and fail the test.

Quoted 32 lines
tests/ext4/056 new file +32 -0
+	elif [ $RESIZE_RET = $EOPNOTSUPP ]
+	then
+		_log "Resize operation not supported with sparse_super2"
+		_log "Threw expected error $RESIZE_RET (EOPNOTSUPP)"
+
+	else
+		_log "Output of resize = $RESIZE_RET. Expected $EOPNOTSUPP (EOPNOTSUPP)"
+		_log "You might be missing kernel patch b1489186cc83"
+	fi
+
+	# unount after ioctl sometimes fails with "device busy" so add a small delay
+	sleep 0.1
+
+	_scratch_unmount >> $seqres.full 2>&1 || _fail "$UMOUNT_PROG failed. Exiting"
+}
+
+run_test()
+{
+	local FSCK_RET
+	local ITERS=8
+
+	for i in $(seq 1 $ITERS)
+	do
+		_log "----------- Iteration: $i ------------"
+		do_resize
+	done
+
+	_log "-------- Iterations Complete ---------"
+	_log "Checking if FS is in consistent state"
+	_check_scratch_fs
+	FSCK_RET=$?
+

tests/ext4/056 line 100

Quoted 10 lines
tests/ext4/056 new file +10 -0
+	return $FSCK_RET
+}
+
+run_test
+status=$?
+
+if [ "$status" -eq "0" ]
+then
+	echo "Test Succeeded!" | tee -a $seqres.full
+fi

This is not needed, just echo "Silence is golden" at the beginning of the test, so any additional output will fail the test.

Thanks,
Eryu

Quoted 12 lines
tests/ext4/056 new file +2 -0
+
+exit
tests/ext4/056.out new file +2 -0
new file mode 100644
index 00000000..41706284
@@ -0,0 +1,2 @@
+QA output created by 056
+Test Succeeded!
Signature · 1 line
2.27.0
author
#7 lore raw

Re: [PATCH 2/2] ext4: Test to ensure resize with sparse_super2 is handled correctly

Hi Eryu

Thanks for the review.

Quoted 61 lines · Eryu Guan
Quoted 60 lines · Ojaswin Mujoo

Kernel currently doesn't support resize of EXT4 mounted
with sparse_super2 option enabled. Earlier, it used to leave the resize incomplete and the fs would be left in an inconsistent state, however commit b1489186cc83[1] fixed this to avoid the fs corruption by clearly returning -ENOTSUPP.

Test to ensure that kernel handles resizing with sparse_super2 correctly. Run resize for multiple iterations because this leads to kernel crash due to fs corruption, which we want to detect.

Related commit in mainline:

[1] commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61

ext4: add check to prevent attempting to resize an fs with sparse_super2

Signed-off-by Ojaswin Mujoo <[email protected]>
---
 tests/ext4/056     | 102 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/056.out |   2 +
 2 files changed, 104 insertions(+)
 create mode 100755 tests/ext4/056
 create mode 100644 tests/ext4/056.out
tests/ext4/056 new file +30 -0
new file mode 100755
index 00000000..9185621d
@@ -0,0 +1,102 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 IBM. All Rights Reserved.
+#
+# We don't currently support resize of EXT4 filesystems mounted
+# with sparse_super2 option enabled. Earlier, kernel used to leave the resize
+# incomplete and the fs would be left into an incomplete state, however commit
+# b1489186cc83 fixed this to avoid the fs corruption by clearly returning
+# -ENOTSUPP.
+#
+# This test ensures that kernel handles resizing with sparse_super2 correctly
+#
+# Related commit in mainline:
+#
+# commit b1489186cc8391e0c1e342f9fbc3eedf6b944c61
+# ext4: add check to prevent attempting to resize an fs with sparse_super2
+#
+
+. ./common/preamble
+_begin_fstest auto ioctl resize quick
+
+# real QA test starts here
+
+INITIAL_FS_SIZE=1G
+RESIZED_FS_SIZE=$((2*1024*1024*1024))  # 2G
+ONLINE_RESIZE_BLOCK_LIMIT=$((256*1024*1024))
+
+_supported_fs ext4
+_require_scratch_size $(($RESIZED_FS_SIZE/1024))
+_require_test_program "ext4_resize"
Quoted 9 lines

tests/ext4/056 line 35

Quoted 5 lines
tests/ext4/056 new file +5 -0
+
+_log()
+{
+	echo "$seq: $*" >> $seqres.full 2>&1
+}

Leading under score is used for common functions, local functions don't need it.

Got it.

Quoted 10 lines

tests/ext4/056 line 42

Quoted 7 lines
tests/ext4/056 new file +7 -0
+
+do_resize()
+{
+
+	$MKFS_PROG `_scratch_mkfs_options -t ext4 -E resize=$ONLINE_RESIZE_BLOCK_LIMIT \
+		-O sparse_super2` $INITIAL_FS_SIZE >> $seqres.full 2>&1 \
+		|| _fail "$MKFS_PROG failed. Exiting"

I think we could use _mkfs_dev here.

Oh right, I missed that. I'll fix this, thanks.

Quoted 11 lines
Quoted 11 lines
tests/ext4/056 new file +11 -0
+
+	_scratch_mount || _fail "Failed to mount scratch partition. Exiting"
+
+	BS=$(_get_block_size $SCRATCH_MNT)
+	NEW_BLOCKS=$(($RESIZED_FS_SIZE/$BS))
+
+	local RESIZE_RET
+	local EOPNOTSUPP=95
+
+	$here/src/ext4_resize $SCRATCH_MNT $NEW_BLOCKS >> $seqres.full 2>&1
+	RESIZE_RET=$?
Quoted 9 lines

tests/ext4/056 line 58

Quoted 5 lines
tests/ext4/056 new file +5 -0
+
+	# Use $RESIZE_RET for logging
+	if [ $RESIZE_RET = 0 ]
+	then
+		_log "Resizing succeeded but FS might still be corrupt."

I don't think _log is needed here, just echo message to stdout and this will break golden image and fail the test.

Noted.

Quoted 46 lines
Quoted 32 lines
tests/ext4/056 new file +32 -0
+	elif [ $RESIZE_RET = $EOPNOTSUPP ]
+	then
+		_log "Resize operation not supported with sparse_super2"
+		_log "Threw expected error $RESIZE_RET (EOPNOTSUPP)"
+
+	else
+		_log "Output of resize = $RESIZE_RET. Expected $EOPNOTSUPP (EOPNOTSUPP)"
+		_log "You might be missing kernel patch b1489186cc83"
+	fi
+
+	# unount after ioctl sometimes fails with "device busy" so add a small delay
+	sleep 0.1
+
+	_scratch_unmount >> $seqres.full 2>&1 || _fail "$UMOUNT_PROG failed. Exiting"
+}
+
+run_test()
+{
+	local FSCK_RET
+	local ITERS=8
+
+	for i in $(seq 1 $ITERS)
+	do
+		_log "----------- Iteration: $i ------------"
+		do_resize
+	done
+
+	_log "-------- Iterations Complete ---------"
+	_log "Checking if FS is in consistent state"
+	_check_scratch_fs
+	FSCK_RET=$?
+

tests/ext4/056 line 100

Quoted 10 lines
tests/ext4/056 new file +10 -0
+	return $FSCK_RET
+}
+
+run_test
+status=$?
+
+if [ "$status" -eq "0" ]
+then
+	echo "Test Succeeded!" | tee -a $seqres.full
+fi

This is not needed, just echo "Silence is golden" at the beginning of the test, so any additional output will fail the test.

Quoted 2 lines

Thanks,
Eryu

Got it, thanks for the suggestions. I'll work on those and send a v2.

Thanks and regards,
Ojaswin

Quoted 13 lines
Quoted 13 lines
tests/ext4/056 new file +2 -0
+
+exit
tests/ext4/056.out new file +2 -0
new file mode 100644
index 00000000..41706284
@@ -0,0 +1,2 @@
+QA output created by 056
+Test Succeeded!
Signature · 1 line
2.27.0
author
#8 lore raw
In reply to Ojaswin Mujoo

Hello,

Just wanted to send a gentle reminder to look into this patchset and let me know if anyone has any suggestions or review comments.

Thank you for your time,
Ojaswin

Quoted 31 lines · Ojaswin Mujoo

As detailed in the patch [1], kernel currently does not support resizes with sparse_super2 enabled. Before the above patch, if we used the EXT4_IOC_RESIZE_FS ioctl directly, wiht sparse_super2 enabled, the kernel used to still try the resize and ultimatley leave the fs in an inconsistent state. This also led to corruption and kernel BUGs.

This patchset adds a test for ext4 to ensure that the kernel handles resizes with sparse_super2 correctly, and returns -EOPNOTSUPP.

Summary:

Patch 1: Fix the src/ext4_resize.c script to return accurate error codes. Patch 2: Add the ext4 test for checking resize functionality

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b1489186cc8391e0c1e342f9fbc3eedf6b944c61

Ojaswin Mujoo (2):
src/ext4_resize.c: Refactor code and ensure accurate errno is returned
ext4: Test to ensure resize with sparse_super2 is handled correctly

 src/ext4_resize.c  |  46 +++++++++++++-------
 tests/ext4/056     | 102 +++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/056.out |   2 +
 3 files changed, 136 insertions(+), 14 deletions(-)
 create mode 100755 tests/ext4/056
 create mode 100644 tests/ext4/056.out
Signature · 1 line
2.27.0