[PATCH] ext4/033: remove a portion of the test assuming resize2fs would fail

[PATCH] ext4/033: remove a portion of the test assuming resize2fs would fail

[Theodore Ts'o]

The ext4/033 test tries to test if resize2fs would fail when resizing
a file system to a size that the number of inodes exceeds the maximum
allowed inode size, 2**32-1.  This no longer takes place as of
e2fsprogs commit 623985ed7dd5 ("resize2fs: attempt to keep the # of
inodes valid by removing the last bg") which will allow resizing a
file system to 64TB by reducing the last block group so that file
system will be 64TB - 128MB, which is close enough for government
work.

Reported-by: Eric Whitney <enwlinux@gmail.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/ext4/033     | 8 --------
 tests/ext4/033.out | 1 -
 2 files changed, 9 deletions(-)

diff --git a/tests/ext4/033 b/tests/ext4/033
index 1bc14c03..0fd0ec90 100755
--- a/tests/ext4/033
+++ b/tests/ext4/033
@@ -66,14 +66,6 @@ _mount $DMHUGEDISK_DEV $SCRATCH_MNT
 echo "Initial fs dump" >> $seqres.full
 $DUMPE2FS_PROG -h $DMHUGEDISK_DEV >> $seqres.full 2>&1
 
-# This should fail, s_inodes_count would just overflow!
-echo "Resizing to inode limit + 1..."
-$RESIZE2FS_PROG $DMHUGEDISK_DEV $((limit_groups*group_blocks)) >> $seqres.full 2>&1
-if [ $? -eq 0 ]; then
-	echo "Resizing succeeded but it should fail!"
-	exit
-fi
-
 # This should succeed, we are maxing out inodes
 echo "Resizing to max group count..."
 $RESIZE2FS_PROG $DMHUGEDISK_DEV $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
diff --git a/tests/ext4/033.out b/tests/ext4/033.out
index 24c251cb..183a7492 100644
--- a/tests/ext4/033.out
+++ b/tests/ext4/033.out
@@ -1,6 +1,5 @@
 QA output created by 033
 Figure out block size
 Format huge device
-Resizing to inode limit + 1...
 Resizing to max group count...
 Resizing to device size...
-- 
2.31.0

Re: [PATCH] ext4/033: remove a portion of the test assuming resize2fs would fail

[Eryu Guan]

On Tue, Dec 14, 2021 at 10:54:09PM -0500, Theodore Ts'o wrote:
> The ext4/033 test tries to test if resize2fs would fail when resizing
> a file system to a size that the number of inodes exceeds the maximum
> allowed inode size, 2**32-1.  This no longer takes place as of
> e2fsprogs commit 623985ed7dd5 ("resize2fs: attempt to keep the # of
> inodes valid by removing the last bg") which will allow resizing a
> file system to 64TB by reducing the last block group so that file
> system will be 64TB - 128MB, which is close enough for government
> work.
> 
> Reported-by: Eric Whitney <enwlinux@gmail.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  tests/ext4/033     | 8 --------
>  tests/ext4/033.out | 1 -
>  2 files changed, 9 deletions(-)
> 
> diff --git a/tests/ext4/033 b/tests/ext4/033
> index 1bc14c03..0fd0ec90 100755
> --- a/tests/ext4/033
> +++ b/tests/ext4/033
> @@ -66,14 +66,6 @@ _mount $DMHUGEDISK_DEV $SCRATCH_MNT
>  echo "Initial fs dump" >> $seqres.full
>  $DUMPE2FS_PROG -h $DMHUGEDISK_DEV >> $seqres.full 2>&1
>  
> -# This should fail, s_inodes_count would just overflow!
> -echo "Resizing to inode limit + 1..."
> -$RESIZE2FS_PROG $DMHUGEDISK_DEV $((limit_groups*group_blocks)) >> $seqres.full 2>&1
> -if [ $? -eq 0 ]; then
> -	echo "Resizing succeeded but it should fail!"
> -	exit
> -fi

This portion of test is the key of the original regression test, if
we're going to remove this part, does it make sense to remove ext4/033
completely?

Thanks,
Eryu

> -
>  # This should succeed, we are maxing out inodes
>  echo "Resizing to max group count..."
>  $RESIZE2FS_PROG $DMHUGEDISK_DEV $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
> diff --git a/tests/ext4/033.out b/tests/ext4/033.out
> index 24c251cb..183a7492 100644
> --- a/tests/ext4/033.out
> +++ b/tests/ext4/033.out
> @@ -1,6 +1,5 @@
>  QA output created by 033
>  Figure out block size
>  Format huge device
> -Resizing to inode limit + 1...
>  Resizing to max group count...
>  Resizing to device size...
> -- 
> 2.31.0

Re: [PATCH] ext4/033: remove a portion of the test assuming resize2fs would fail

[Theodore Ts'o]

On Sun, Dec 19, 2021 at 11:16:45PM +0800, Eryu Guan wrote:
> 
> This portion of test is the key of the original regression test, if
> we're going to remove this part, does it make sense to remove ext4/033
> completely?

Hmm, good point.  It's nice to keep the regression test around, but we
can't use resize2fs to test the kernel's online resize code directly.
We can fix this by creating test program to execute the ioctl
directly.   Revised patch coming....

							- Ted

[PATCH] ext4/033: test EXT4_IOC_RESIZE_FS by calling the ioctl directly

[Theodore Ts'o]

E2fsprogs commits 4ea80d031c7e ("resize2fs: adjust new size of the
file system to allow a successful resize") and 50088b1996cc
("resize2fs: attempt to keep the # of inodes valid by removing the
last bg") will automatically reduce the requested new size of the file
system by up to a single block group to avoid overflowing the 32-bit
inode count.   This interferes with ext4/033's test of kernel commit
4f2f76f75143 ("ext4: Forbid overflowing inode count when # resizing".)

Address this by creating a new test program, ext4_resize which calls
the EXT4_IOC_RESIZE_FS ioctl directly so we can correctly test the
kernel's online resize code.

Reported-by: Eric Whitney <enwlinux@gmail.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 .gitignore        |  1 +
 src/Makefile      |  2 +-
 src/ext4_resize.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/033    | 16 ++++++++++-----
 4 files changed, 63 insertions(+), 6 deletions(-)
 create mode 100644 src/ext4_resize.c

diff --git a/.gitignore b/.gitignore
index 9e6d2fd5..65b93307 100644
--- a/.gitignore
+++ b/.gitignore
@@ -77,6 +77,7 @@ tags
 /src/dirperf
 /src/dirstress
 /src/e4compact
+/src/ext4_resize
 /src/fault
 /src/feature
 /src/fiemap-tester
diff --git a/src/Makefile b/src/Makefile
index 25ab061d..1737ed0e 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -31,7 +31,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
 	dio-invalidate-cache stat_test t_encrypted_d_revalidate \
 	attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \
 	fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
-	detached_mounts_propagation
+	detached_mounts_propagation ext4_resize
 
 EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
 	      btrfs_crc32c_forged_name.py
diff --git a/src/ext4_resize.c b/src/ext4_resize.c
new file mode 100644
index 00000000..1ac51e6f
--- /dev/null
+++ b/src/ext4_resize.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Test program which uses the raw ext4 resize_fs ioctl directly.
+ */
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <sys/mount.h>
+
+typedef unsigned long long __u64;
+
+#ifndef EXT4_IOC_RESIZE_FS
+#define EXT4_IOC_RESIZE_FS		_IOW('f', 16, __u64)
+#endif
+
+int main(int argc, char **argv)
+{
+	__u64	new_size;
+	int	error, fd;
+	char	*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;
+	}
+
+	new_size = strtoull(argv[2], &tmp, 10);
+	if ((errno) || (*tmp != '\0')) {
+		fprintf(stderr, "%s: invalid new size\n", argv[0]);
+		return 1;
+	}
+
+	error = ioctl(fd, EXT4_IOC_RESIZE_FS, &new_size);
+	if (error < 0) {
+		perror("EXT4_IOC_RESIZE_FS");
+		return 1;
+	}
+	return 0;
+}
diff --git a/tests/ext4/033 b/tests/ext4/033
index 1bc14c03..22041a17 100755
--- a/tests/ext4/033
+++ b/tests/ext4/033
@@ -5,7 +5,8 @@
 # FS QA Test 033
 #
 # Test s_inodes_count overflow for huge filesystems. This bug was fixed
-# by commit "ext4: Forbid overflowing inode count when resizing".
+# by commit 4f2f76f75143 ("ext4: Forbid overflowing inode count when
+# resizing".)
 #
 . ./common/preamble
 _begin_fstest auto ioctl resize
@@ -28,7 +29,9 @@ _supported_fs ext4
 _require_scratch_nocheck
 _require_dmhugedisk
 _require_dumpe2fs
-_require_command "$RESIZE2FS_PROG" resize2fs
+_require_test_program ext4_resize
+
+EXT4_RESIZE=$here/src/ext4_resize
 
 # Figure out whether device is large enough
 devsize=$(blockdev --getsize64 $SCRATCH_DEV)
@@ -68,7 +71,8 @@ $DUMPE2FS_PROG -h $DMHUGEDISK_DEV >> $seqres.full 2>&1
 
 # This should fail, s_inodes_count would just overflow!
 echo "Resizing to inode limit + 1..."
-$RESIZE2FS_PROG $DMHUGEDISK_DEV $((limit_groups*group_blocks)) >> $seqres.full 2>&1
+echo $EXT4_RESIZE $SCRATCH_MNT $((limit_groups*group_blocks)) >> $seqres.full 2>&1
+$EXT4_RESIZE $SCRATCH_MNT $((limit_groups*group_blocks)) >> $seqres.full 2>&1
 if [ $? -eq 0 ]; then
 	echo "Resizing succeeded but it should fail!"
 	exit
@@ -76,7 +80,8 @@ fi
 
 # This should succeed, we are maxing out inodes
 echo "Resizing to max group count..."
-$RESIZE2FS_PROG $DMHUGEDISK_DEV $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
+echo $EXT4_RESIZE $SCRATCH_MNT $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
+$EXT4_RESIZE $SCRATCH_MNT $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
 if [ $? -ne 0 ]; then
 	echo "Resizing failed!"
 	exit
@@ -87,7 +92,8 @@ $DUMPE2FS_PROG -h $DMHUGEDISK_DEV >> $seqres.full 2>&1
 
 # This should fail, s_inodes_count would overflow by quite a bit!
 echo "Resizing to device size..."
-$RESIZE2FS_PROG $DMHUGEDISK_DEV >> $seqres.full 2>&1
+echo $EXT4_RESIZE $SCRATCH_MNT $(((limit_groups + 16)*group_blocks)) >> $seqres.full 2>&1
+$EXT4_RESIZE $SCRATCH_MNT $(((limit_groups + 16)*group_blocks)) >> $seqres.full 2>&1
 if [ $? -eq 0 ]; then
 	echo "Resizing succeeded but it should fail!"
 	exit
-- 
2.31.0

Re: [PATCH] ext4/033: test EXT4_IOC_RESIZE_FS by calling the ioctl directly

[Zorro Lang]

On Mon, Dec 20, 2021 at 03:40:59PM -0500, Theodore Ts'o wrote:
> E2fsprogs commits 4ea80d031c7e ("resize2fs: adjust new size of the
> file system to allow a successful resize") and 50088b1996cc
> ("resize2fs: attempt to keep the # of inodes valid by removing the
> last bg") will automatically reduce the requested new size of the file
> system by up to a single block group to avoid overflowing the 32-bit
> inode count.   This interferes with ext4/033's test of kernel commit
> 4f2f76f75143 ("ext4: Forbid overflowing inode count when # resizing".)
> 
> Address this by creating a new test program, ext4_resize which calls
> the EXT4_IOC_RESIZE_FS ioctl directly so we can correctly test the
> kernel's online resize code.
> 
> Reported-by: Eric Whitney <enwlinux@gmail.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  .gitignore        |  1 +
>  src/Makefile      |  2 +-
>  src/ext4_resize.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/ext4/033    | 16 ++++++++++-----
>  4 files changed, 63 insertions(+), 6 deletions(-)
>  create mode 100644 src/ext4_resize.c
> 
> diff --git a/.gitignore b/.gitignore
> index 9e6d2fd5..65b93307 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -77,6 +77,7 @@ tags
>  /src/dirperf
>  /src/dirstress
>  /src/e4compact
> +/src/ext4_resize
>  /src/fault
>  /src/feature
>  /src/fiemap-tester
> diff --git a/src/Makefile b/src/Makefile
> index 25ab061d..1737ed0e 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -31,7 +31,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
>  	dio-invalidate-cache stat_test t_encrypted_d_revalidate \
>  	attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \
>  	fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
> -	detached_mounts_propagation
> +	detached_mounts_propagation ext4_resize
>  
>  EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
>  	      btrfs_crc32c_forged_name.py
> diff --git a/src/ext4_resize.c b/src/ext4_resize.c
> new file mode 100644
> index 00000000..1ac51e6f
> --- /dev/null
> +++ b/src/ext4_resize.c
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Test program which uses the raw ext4 resize_fs ioctl directly.
> + */
> +
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <sys/ioctl.h>
> +#include <sys/mount.h>
> +
> +typedef unsigned long long __u64;
> +
> +#ifndef EXT4_IOC_RESIZE_FS
> +#define EXT4_IOC_RESIZE_FS		_IOW('f', 16, __u64)
> +#endif

This patch looks good to me, I just want to ask if we'd better to try to include
ext2fs/ext2fs.h at here? And of course, check it in configure.ac.
The EXT4_IOC_RESIZE_FS looks like defined in ext2fs/ext2_fs.h which comes from
e2fsprogs-devel package. I can't find this definition from kernel-hearders package.
As you're the expert of this part, please correct me if it's wrong :)

Thanks,
Zorro

> +
> +int main(int argc, char **argv)
> +{
> +	__u64	new_size;
> +	int	error, fd;
> +	char	*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;
> +	}
> +
> +	new_size = strtoull(argv[2], &tmp, 10);
> +	if ((errno) || (*tmp != '\0')) {
> +		fprintf(stderr, "%s: invalid new size\n", argv[0]);
> +		return 1;
> +	}
> +
> +	error = ioctl(fd, EXT4_IOC_RESIZE_FS, &new_size);
> +	if (error < 0) {
> +		perror("EXT4_IOC_RESIZE_FS");
> +		return 1;
> +	}
> +	return 0;
> +}
> diff --git a/tests/ext4/033 b/tests/ext4/033
> index 1bc14c03..22041a17 100755
> --- a/tests/ext4/033
> +++ b/tests/ext4/033
> @@ -5,7 +5,8 @@
>  # FS QA Test 033
>  #
>  # Test s_inodes_count overflow for huge filesystems. This bug was fixed
> -# by commit "ext4: Forbid overflowing inode count when resizing".
> +# by commit 4f2f76f75143 ("ext4: Forbid overflowing inode count when
> +# resizing".)
>  #
>  . ./common/preamble
>  _begin_fstest auto ioctl resize
> @@ -28,7 +29,9 @@ _supported_fs ext4
>  _require_scratch_nocheck
>  _require_dmhugedisk
>  _require_dumpe2fs
> -_require_command "$RESIZE2FS_PROG" resize2fs
> +_require_test_program ext4_resize
> +
> +EXT4_RESIZE=$here/src/ext4_resize
>  
>  # Figure out whether device is large enough
>  devsize=$(blockdev --getsize64 $SCRATCH_DEV)
> @@ -68,7 +71,8 @@ $DUMPE2FS_PROG -h $DMHUGEDISK_DEV >> $seqres.full 2>&1
>  
>  # This should fail, s_inodes_count would just overflow!
>  echo "Resizing to inode limit + 1..."
> -$RESIZE2FS_PROG $DMHUGEDISK_DEV $((limit_groups*group_blocks)) >> $seqres.full 2>&1
> +echo $EXT4_RESIZE $SCRATCH_MNT $((limit_groups*group_blocks)) >> $seqres.full 2>&1
> +$EXT4_RESIZE $SCRATCH_MNT $((limit_groups*group_blocks)) >> $seqres.full 2>&1
>  if [ $? -eq 0 ]; then
>  	echo "Resizing succeeded but it should fail!"
>  	exit
> @@ -76,7 +80,8 @@ fi
>  
>  # This should succeed, we are maxing out inodes
>  echo "Resizing to max group count..."
> -$RESIZE2FS_PROG $DMHUGEDISK_DEV $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
> +echo $EXT4_RESIZE $SCRATCH_MNT $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
> +$EXT4_RESIZE $SCRATCH_MNT $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
>  if [ $? -ne 0 ]; then
>  	echo "Resizing failed!"
>  	exit
> @@ -87,7 +92,8 @@ $DUMPE2FS_PROG -h $DMHUGEDISK_DEV >> $seqres.full 2>&1
>  
>  # This should fail, s_inodes_count would overflow by quite a bit!
>  echo "Resizing to device size..."
> -$RESIZE2FS_PROG $DMHUGEDISK_DEV >> $seqres.full 2>&1
> +echo $EXT4_RESIZE $SCRATCH_MNT $(((limit_groups + 16)*group_blocks)) >> $seqres.full 2>&1
> +$EXT4_RESIZE $SCRATCH_MNT $(((limit_groups + 16)*group_blocks)) >> $seqres.full 2>&1
>  if [ $? -eq 0 ]; then
>  	echo "Resizing succeeded but it should fail!"
>  	exit
> -- 
> 2.31.0
> 

Re: [PATCH] ext4/033: test EXT4_IOC_RESIZE_FS by calling the ioctl directly

[Zorro Lang]

On Wed, Jan 05, 2022 at 11:57:43PM +0800, Zorro Lang wrote:
> On Mon, Dec 20, 2021 at 03:40:59PM -0500, Theodore Ts'o wrote:
> > E2fsprogs commits 4ea80d031c7e ("resize2fs: adjust new size of the
> > file system to allow a successful resize") and 50088b1996cc
> > ("resize2fs: attempt to keep the # of inodes valid by removing the
> > last bg") will automatically reduce the requested new size of the file
> > system by up to a single block group to avoid overflowing the 32-bit
> > inode count.   This interferes with ext4/033's test of kernel commit
> > 4f2f76f75143 ("ext4: Forbid overflowing inode count when # resizing".)
> > 
> > Address this by creating a new test program, ext4_resize which calls
> > the EXT4_IOC_RESIZE_FS ioctl directly so we can correctly test the
> > kernel's online resize code.
> > 
> > Reported-by: Eric Whitney <enwlinux@gmail.com>
> > Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> > ---
> >  .gitignore        |  1 +
> >  src/Makefile      |  2 +-
> >  src/ext4_resize.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/ext4/033    | 16 ++++++++++-----
> >  4 files changed, 63 insertions(+), 6 deletions(-)
> >  create mode 100644 src/ext4_resize.c
> > 
> > diff --git a/.gitignore b/.gitignore
> > index 9e6d2fd5..65b93307 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -77,6 +77,7 @@ tags
> >  /src/dirperf
> >  /src/dirstress
> >  /src/e4compact
> > +/src/ext4_resize
> >  /src/fault
> >  /src/feature
> >  /src/fiemap-tester
> > diff --git a/src/Makefile b/src/Makefile
> > index 25ab061d..1737ed0e 100644
> > --- a/src/Makefile
> > +++ b/src/Makefile
> > @@ -31,7 +31,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> >  	dio-invalidate-cache stat_test t_encrypted_d_revalidate \
> >  	attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \
> >  	fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
> > -	detached_mounts_propagation
> > +	detached_mounts_propagation ext4_resize
> >  
> >  EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
> >  	      btrfs_crc32c_forged_name.py
> > diff --git a/src/ext4_resize.c b/src/ext4_resize.c
> > new file mode 100644
> > index 00000000..1ac51e6f
> > --- /dev/null
> > +++ b/src/ext4_resize.c
> > @@ -0,0 +1,50 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Test program which uses the raw ext4 resize_fs ioctl directly.
> > + */
> > +
> > +#include <stdio.h>
> > +#include <fcntl.h>
> > +#include <errno.h>
> > +#include <unistd.h>
> > +#include <stdint.h>
> > +#include <stdlib.h>
> > +#include <sys/ioctl.h>
> > +#include <sys/mount.h>
> > +
> > +typedef unsigned long long __u64;
> > +
> > +#ifndef EXT4_IOC_RESIZE_FS
> > +#define EXT4_IOC_RESIZE_FS		_IOW('f', 16, __u64)
> > +#endif
> 
> This patch looks good to me, I just want to ask if we'd better to try to include
> ext2fs/ext2fs.h at here? And of course, check it in configure.ac.
> The EXT4_IOC_RESIZE_FS looks like defined in ext2fs/ext2_fs.h which comes from
> e2fsprogs-devel package. I can't find this definition from kernel-hearders package.
> As you're the expert of this part, please correct me if it's wrong :)

Oh, I just noticed that this patch has been merged. I have no idea why this email become
*unread* status in my mutt, cause I thought it's a new patch. Please ignore this review.

> 
> Thanks,
> Zorro
> 
> > +
> > +int main(int argc, char **argv)
> > +{
> > +	__u64	new_size;
> > +	int	error, fd;
> > +	char	*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;
> > +	}
> > +
> > +	new_size = strtoull(argv[2], &tmp, 10);
> > +	if ((errno) || (*tmp != '\0')) {
> > +		fprintf(stderr, "%s: invalid new size\n", argv[0]);
> > +		return 1;
> > +	}
> > +
> > +	error = ioctl(fd, EXT4_IOC_RESIZE_FS, &new_size);
> > +	if (error < 0) {
> > +		perror("EXT4_IOC_RESIZE_FS");
> > +		return 1;
> > +	}
> > +	return 0;
> > +}
> > diff --git a/tests/ext4/033 b/tests/ext4/033
> > index 1bc14c03..22041a17 100755
> > --- a/tests/ext4/033
> > +++ b/tests/ext4/033
> > @@ -5,7 +5,8 @@
> >  # FS QA Test 033
> >  #
> >  # Test s_inodes_count overflow for huge filesystems. This bug was fixed
> > -# by commit "ext4: Forbid overflowing inode count when resizing".
> > +# by commit 4f2f76f75143 ("ext4: Forbid overflowing inode count when
> > +# resizing".)
> >  #
> >  . ./common/preamble
> >  _begin_fstest auto ioctl resize
> > @@ -28,7 +29,9 @@ _supported_fs ext4
> >  _require_scratch_nocheck
> >  _require_dmhugedisk
> >  _require_dumpe2fs
> > -_require_command "$RESIZE2FS_PROG" resize2fs
> > +_require_test_program ext4_resize
> > +
> > +EXT4_RESIZE=$here/src/ext4_resize
> >  
> >  # Figure out whether device is large enough
> >  devsize=$(blockdev --getsize64 $SCRATCH_DEV)
> > @@ -68,7 +71,8 @@ $DUMPE2FS_PROG -h $DMHUGEDISK_DEV >> $seqres.full 2>&1
> >  
> >  # This should fail, s_inodes_count would just overflow!
> >  echo "Resizing to inode limit + 1..."
> > -$RESIZE2FS_PROG $DMHUGEDISK_DEV $((limit_groups*group_blocks)) >> $seqres.full 2>&1
> > +echo $EXT4_RESIZE $SCRATCH_MNT $((limit_groups*group_blocks)) >> $seqres.full 2>&1
> > +$EXT4_RESIZE $SCRATCH_MNT $((limit_groups*group_blocks)) >> $seqres.full 2>&1
> >  if [ $? -eq 0 ]; then
> >  	echo "Resizing succeeded but it should fail!"
> >  	exit
> > @@ -76,7 +80,8 @@ fi
> >  
> >  # This should succeed, we are maxing out inodes
> >  echo "Resizing to max group count..."
> > -$RESIZE2FS_PROG $DMHUGEDISK_DEV $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
> > +echo $EXT4_RESIZE $SCRATCH_MNT $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
> > +$EXT4_RESIZE $SCRATCH_MNT $(((limit_groups-1)*group_blocks)) >> $seqres.full 2>&1
> >  if [ $? -ne 0 ]; then
> >  	echo "Resizing failed!"
> >  	exit
> > @@ -87,7 +92,8 @@ $DUMPE2FS_PROG -h $DMHUGEDISK_DEV >> $seqres.full 2>&1
> >  
> >  # This should fail, s_inodes_count would overflow by quite a bit!
> >  echo "Resizing to device size..."
> > -$RESIZE2FS_PROG $DMHUGEDISK_DEV >> $seqres.full 2>&1
> > +echo $EXT4_RESIZE $SCRATCH_MNT $(((limit_groups + 16)*group_blocks)) >> $seqres.full 2>&1
> > +$EXT4_RESIZE $SCRATCH_MNT $(((limit_groups + 16)*group_blocks)) >> $seqres.full 2>&1
> >  if [ $? -eq 0 ]; then
> >  	echo "Resizing succeeded but it should fail!"
> >  	exit
> > -- 
> > 2.31.0
> > 

Re: [PATCH] ext4/033: test EXT4_IOC_RESIZE_FS by calling the ioctl directly

[Theodore Ts'o]

On Thu, Jan 06, 2022 at 12:06:19AM +0800, Zorro Lang wrote:
> > This patch looks good to me, I just want to ask if we'd better to try to include
> > ext2fs/ext2fs.h at here? And of course, check it in configure.ac.
> > The EXT4_IOC_RESIZE_FS looks like defined in ext2fs/ext2_fs.h which comes from
> > e2fsprogs-devel package. I can't find this definition from kernel-hearders package.
> > As you're the expert of this part, please correct me if it's wrong :)

We're not depending on ext2fs/ext2_fs.h and hence the e2fsprogs-devel
(or libext2fs-dev package if you're using Debian/Ubuntu) anywhere else
in the xfstests-dev.  It's not like the code points for
EXT4_IOC_RESIZE_FS are going to change, so we just use constructs
like:

#ifndef EXT4_IOC_RESIZE_FS
#define EXT4_IOC_RESIZE_FS           _IOW('f', 16, __u64)
#endif

in xfstests-dev/src/*.c as needed.

There's no real upside in adding a dependency which makes it harder
for developers to compile xfstests.  (Trivia note: I created
xfstests-bld several years ago because back then, Debian didn't
include some of the internal header files from xfsprogs which xfstests
needed.)

Cheers,

						- Ted

Re: [PATCH] ext4/033: test EXT4_IOC_RESIZE_FS by calling the ioctl directly

[Zorro Lang]

On Wed, Jan 05, 2022 at 03:02:58PM -0500, Theodore Ts'o wrote:
> On Thu, Jan 06, 2022 at 12:06:19AM +0800, Zorro Lang wrote:
> > > This patch looks good to me, I just want to ask if we'd better to try to include
> > > ext2fs/ext2fs.h at here? And of course, check it in configure.ac.
> > > The EXT4_IOC_RESIZE_FS looks like defined in ext2fs/ext2_fs.h which comes from
> > > e2fsprogs-devel package. I can't find this definition from kernel-hearders package.
> > > As you're the expert of this part, please correct me if it's wrong :)
> 
> We're not depending on ext2fs/ext2_fs.h and hence the e2fsprogs-devel
> (or libext2fs-dev package if you're using Debian/Ubuntu) anywhere else
> in the xfstests-dev.  It's not like the code points for
> EXT4_IOC_RESIZE_FS are going to change, so we just use constructs
> like:
> 
> #ifndef EXT4_IOC_RESIZE_FS
> #define EXT4_IOC_RESIZE_FS           _IOW('f', 16, __u64)
> #endif
> 
> in xfstests-dev/src/*.c as needed.
> 
> There's no real upside in adding a dependency which makes it harder
> for developers to compile xfstests.  (Trivia note: I created
> xfstests-bld several years ago because back then, Debian didn't
> include some of the internal header files from xfsprogs which xfstests
> needed.)

Thanks for your kindly explanation.

> 
> Cheers,
> 
> 						- Ted
>