[PATCH 0/2] tests/ext4: Ensure resizes with sparse_super2 are handled correctly

[PATCH 0/2] tests/ext4: Ensure resizes with sparse_super2 are handled correctly

[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

-- 
2.27.0

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

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

Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 src/ext4_resize.c | 46 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/src/ext4_resize.c b/src/ext4_resize.c
index 39e16529..78b66432 100644
--- a/src/ext4_resize.c
+++ b/src/ext4_resize.c
@@ -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;
 }
-- 
2.27.0

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

[Ritesh Harjani]

On 22/02/07 01:55PM, Ojaswin Mujoo wrote:
> 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 <riteshh@linux.ibm.com>

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

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

[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 <ojaswin@linux.ibm.com>
---
 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

diff --git a/tests/ext4/056 b/tests/ext4/056
new file mode 100755
index 00000000..9185621d
--- /dev/null
+++ b/tests/ext4/056
@@ -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
diff --git a/tests/ext4/056.out b/tests/ext4/056.out
new file mode 100644
index 00000000..41706284
--- /dev/null
+++ b/tests/ext4/056.out
@@ -0,0 +1,2 @@
+QA output created by 056
+Test Succeeded!
-- 
2.27.0

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

[Ritesh Harjani]

On 22/02/07 01:55PM, Ojaswin Mujoo wrote:
> 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

Thanks for the patch. Few nits below.

>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> ---
>  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
>
> diff --git a/tests/ext4/056 b/tests/ext4/056
> new file mode 100755
> index 00000000..9185621d
> --- /dev/null
> +++ b/tests/ext4/056
> @@ -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."

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.


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

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

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

> +
> +	_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
> diff --git a/tests/ext4/056.out b/tests/ext4/056.out
> new file mode 100644
> index 00000000..41706284
> --- /dev/null
> +++ b/tests/ext4/056.out
> @@ -0,0 +1,2 @@
> +QA output created by 056
> +Test Succeeded!
> --
> 2.27.0
>

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

[Ojaswin Mujoo]

Hi Ritesh,

Thanks for the review. 

On Sat, Feb 19, 2022 at 12:52:11PM +0530, Ritesh Harjani wrote:
> On 22/02/07 01:55PM, Ojaswin Mujoo wrote:
> > 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
> 
> Thanks for the patch. Few nits below.
> 
> >
> > Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> > ---
> >  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
> >
> > diff --git a/tests/ext4/056 b/tests/ext4/056
> > new file mode 100755
> > index 00000000..9185621d
> > --- /dev/null
> > +++ b/tests/ext4/056
> > @@ -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."
> 
> 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.
> 
> 
> > +	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?
> 
> > +
> > +	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.
> 
> > +	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

> > +
> > +	_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
> > diff --git a/tests/ext4/056.out b/tests/ext4/056.out
> > new file mode 100644
> > index 00000000..41706284
> > --- /dev/null
> > +++ b/tests/ext4/056.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 056
> > +Test Succeeded!
> > --
> > 2.27.0
> >

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

[Eryu Guan]

On Mon, Feb 07, 2022 at 01:55:34PM +0530, Ojaswin Mujoo wrote:
> 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 <ojaswin@linux.ibm.com>
> ---
>  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
> 
> diff --git a/tests/ext4/056 b/tests/ext4/056
> new file mode 100755
> index 00000000..9185621d
> --- /dev/null
> +++ b/tests/ext4/056
> @@ -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
> +}

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

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

> +
> +	_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."

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

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

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

> +
> +exit
> diff --git a/tests/ext4/056.out b/tests/ext4/056.out
> new file mode 100644
> index 00000000..41706284
> --- /dev/null
> +++ b/tests/ext4/056.out
> @@ -0,0 +1,2 @@
> +QA output created by 056
> +Test Succeeded!
> -- 
> 2.27.0

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

[Ojaswin Mujoo]

Hi Eryu

Thanks for the review.

On Mon, Feb 21, 2022 at 12:24:56AM +0800, Eryu Guan wrote:
> On Mon, Feb 07, 2022 at 01:55:34PM +0530, Ojaswin Mujoo wrote:
> > 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 <ojaswin@linux.ibm.com>
> > ---
> >  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
> > 
> > diff --git a/tests/ext4/056 b/tests/ext4/056
> > new file mode 100755
> > index 00000000..9185621d
> > --- /dev/null
> > +++ b/tests/ext4/056
> > @@ -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
> > +}
> 
> Leading under score is used for common functions, local functions don't
> need it.
> 
Got it.
> > +
> > +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.
> > +
> > +	_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."
> 
> I don't think _log is needed here, just echo message to stdout and this
> will break golden image and fail the test.
> 
Noted.
> > +	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
> 
> 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

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

Thanks and regards,
Ojaswin
> 
> > +
> > +exit
> > diff --git a/tests/ext4/056.out b/tests/ext4/056.out
> > new file mode 100644
> > index 00000000..41706284
> > --- /dev/null
> > +++ b/tests/ext4/056.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 056
> > +Test Succeeded!
> > -- 
> > 2.27.0

Re: [PATCH 0/2] tests/ext4: Ensure resizes with sparse_super2 are handled correctly

[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

On Mon, Feb 07, 2022 at 01:55:32PM +0530, Ojaswin Mujoo wrote:
> 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
> 
> -- 
> 2.27.0
>