* [PATCH v3 0/3] Addition of new tests for extsize hints
@ 2024-11-21 5:09 Nirjhar Roy
2024-11-21 5:09 ` [PATCH v3 1/3] common/rc,xfs/207: Add a common helper function to check xflag bits Nirjhar Roy
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Nirjhar Roy @ 2024-11-21 5:09 UTC (permalink / raw)
To: fstests; +Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang,
nirjhar
This test checks the behaviour of xfs/ext4 filesystems when extsize
hint is set on files with inode size as 0, non-empty
files with allocated and delalloc extents and so on.
This test set also tests one of the scenarios where setting extsize
hints on files with delayed allocation incorrectly succeeds and there is
a recent patch series[1] that got merged[2] which fixes this.
Currently this test only runs in xfs but there is a patch series[3] in review
that adds support for the extsize hints for ext4 as well.
I have tested it on ppc64le (with page size 64k) and x86_64 (with page size 4k).
The block sizes I have tested with are 2k, 4k, 8k, 16k, 32k, 64k with extsize being
twice of the block size or 4 * block size(if the default extsize is 2 * blocksize).
I have also tested with configurations where I added extszinherit mkfs option
on the root. I had to enable CONFIG_TRANSPARENT_HUGEPAGE to enable block size
greater than 4k on x86_64.
Changes since [v2]
- Renamed _test_fsx_xflags_field() to _test_fsxattr_xflag().
- Changed the definition of _test_fsx_xflags_field (now renamed to _test_fsxattr_xflag)
to accept xflag names (like extsize, cowextsize) instead of single characters (like 'e') and
modified the callers of this function accordingly (in xfs/207 and generic/366)
- Some changes in the comments in the setup() function of tests/generic/366.
- Added another helper function _require_scratch_extsize().
- Replaced the usage of _require_scratch() with _require_scratch_extsize() in tests/generic/366.
- Fixed some indentation issues in the test scripts(xfs/207, generic/366) and the commit messages.
Changes since [v1]
- Making the definition of _test_xfs_xflags_field() more compact and renamed
_test_xfs_xflags_field() to _test_fsx_xflags_field() since I moved it to common/rc.
- Removed the explicit import of common/xfs from the test script.
- Renamed the test file from generic/365 to generic/366 since generic/365 was taken.
- Made the following functions in tests/generic/366 accept a second parameter (extsize)
a) check_extsz_xflag_across_remount()
b) check_extsz_and_xflag()
c) read_file_extsize()
d) filter_extsz
This will help filter out any extsize that the test wants. This is required because, this
version no longer assumes any particular fixed value of the default extsize.
- Removed the check for xflags in test_data_allocated() and test_data_delayed() test
- Added an extra line in output that denotes that we are checking that the extent size
hasn't changed if the xfs_io command fails (in test_data_allocated() and test_data_delayed() test).
- Changed the new extsize to 4*blocksize if default extsize size is 2*blocksize to make
sure that the new extsize is not the same as the default extsize so that we can observe it changing.
[1] https://lore.kernel.org/all/20241015094509.678082-1-ojaswin@linux.ibm.com/
[2] kernel commit - 2a492ff66673
[3] https://lore.kernel.org/linux-ext4/cover.1726034272.git.ojaswin@linux.ibm.com/
[v1] https://lore.kernel.org/all/cover.1729624806.git.nirjhar@linux.ibm.com/
[v2] https://lore.kernel.org/all/cover.1731597226.git.nirjhar@linux.ibm.com/
Nirjhar Roy (3):
common/rc,xfs/207: Add a common helper function to check xflag bits
common/rc: Add a new _require_scratch_extsize helper function
generic: Addition of new tests for extsize hints
common/rc | 24 ++++++
tests/generic/366 | 175 ++++++++++++++++++++++++++++++++++++++++++
tests/generic/366.out | 26 +++++++
tests/xfs/207 | 15 +---
4 files changed, 229 insertions(+), 11 deletions(-)
create mode 100755 tests/generic/366
create mode 100644 tests/generic/366.out
--
2.43.5
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v3 1/3] common/rc,xfs/207: Add a common helper function to check xflag bits 2024-11-21 5:09 [PATCH v3 0/3] Addition of new tests for extsize hints Nirjhar Roy @ 2024-11-21 5:09 ` Nirjhar Roy 2024-11-21 17:20 ` Darrick J. Wong 2024-11-21 5:09 ` [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function Nirjhar Roy 2024-11-21 5:09 ` [PATCH v3 3/3] generic: Addition of new tests for extsize hints Nirjhar Roy 2 siblings, 1 reply; 15+ messages in thread From: Nirjhar Roy @ 2024-11-21 5:09 UTC (permalink / raw) To: fstests; +Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, nirjhar This patch defines a common helper function to test whether any of fsxattr xflags field is set or not. We will use this helper in an upcoming patch for checking extsize (e) flag. Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> --- common/rc | 7 +++++++ tests/xfs/207 | 15 ++++----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/common/rc b/common/rc index 2af26f23..cccc98f5 100644 --- a/common/rc +++ b/common/rc @@ -41,6 +41,13 @@ _md5_checksum() md5sum $1 | cut -d ' ' -f1 } +# Check whether a fsxattr xflags name ($2) field is set on a given file ($1). +# e.g, fsxattr.xflags = 0x80000800 [extsize, has-xattr] +_test_fsxattr_xflag() +{ + grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") +} + # Write a byte into a range of a file _pwrite_byte() { local pattern="$1" diff --git a/tests/xfs/207 b/tests/xfs/207 index bbe21307..394e7e55 100755 --- a/tests/xfs/207 +++ b/tests/xfs/207 @@ -21,15 +21,6 @@ _require_cp_reflink _require_xfs_io_command "fiemap" _require_xfs_io_command "cowextsize" -# Takes the fsxattr.xflags line, -# i.e. fsxattr.xflags = 0x0 [--------------C-] -# and tests whether a flag character is set -test_xflag() -{ - local flg=$1 - grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset" -} - echo "Format and mount" _scratch_mkfs > $seqres.full 2>&1 _scratch_mount >> $seqres.full 2>&1 @@ -65,14 +56,16 @@ echo "Set cowextsize and check flag" $XFS_IO_PROG -c "cowextsize 1048576" $testdir/file3 | _filter_scratch _scratch_cycle_mount -$XFS_IO_PROG -c "stat" $testdir/file3 | grep 'fsxattr.xflags' | test_xflag "C" +_test_fsxattr_xflag "$testdir/file3" "cowextsize" && echo "C flag set" || \ + echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch echo "Unset cowextsize and check flag" $XFS_IO_PROG -c "cowextsize 0" $testdir/file3 | _filter_scratch _scratch_cycle_mount -$XFS_IO_PROG -c "stat" $testdir/file3 | grep 'fsxattr.xflags' | test_xflag "C" +_test_fsxattr_xflag "$testdir/file3" "cowextsize" && echo "C flag set" || \ + echo "C flag unset" $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch status=0 -- 2.43.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] common/rc,xfs/207: Add a common helper function to check xflag bits 2024-11-21 5:09 ` [PATCH v3 1/3] common/rc,xfs/207: Add a common helper function to check xflag bits Nirjhar Roy @ 2024-11-21 17:20 ` Darrick J. Wong 2024-11-21 18:30 ` Nirjhar Roy 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2024-11-21 17:20 UTC (permalink / raw) To: Nirjhar Roy; +Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang On Thu, Nov 21, 2024 at 10:39:10AM +0530, Nirjhar Roy wrote: > This patch defines a common helper function to test whether any of > fsxattr xflags field is set or not. We will use this helper in > an upcoming patch for checking extsize (e) flag. > > Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> Looks good to me now, Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > common/rc | 7 +++++++ > tests/xfs/207 | 15 ++++----------- > 2 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/common/rc b/common/rc > index 2af26f23..cccc98f5 100644 > --- a/common/rc > +++ b/common/rc > @@ -41,6 +41,13 @@ _md5_checksum() > md5sum $1 | cut -d ' ' -f1 > } > > +# Check whether a fsxattr xflags name ($2) field is set on a given file ($1). > +# e.g, fsxattr.xflags = 0x80000800 [extsize, has-xattr] > +_test_fsxattr_xflag() > +{ > + grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") > +} > + > # Write a byte into a range of a file > _pwrite_byte() { > local pattern="$1" > diff --git a/tests/xfs/207 b/tests/xfs/207 > index bbe21307..394e7e55 100755 > --- a/tests/xfs/207 > +++ b/tests/xfs/207 > @@ -21,15 +21,6 @@ _require_cp_reflink > _require_xfs_io_command "fiemap" > _require_xfs_io_command "cowextsize" > > -# Takes the fsxattr.xflags line, > -# i.e. fsxattr.xflags = 0x0 [--------------C-] > -# and tests whether a flag character is set > -test_xflag() > -{ > - local flg=$1 > - grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset" > -} > - > echo "Format and mount" > _scratch_mkfs > $seqres.full 2>&1 > _scratch_mount >> $seqres.full 2>&1 > @@ -65,14 +56,16 @@ echo "Set cowextsize and check flag" > $XFS_IO_PROG -c "cowextsize 1048576" $testdir/file3 | _filter_scratch > _scratch_cycle_mount > > -$XFS_IO_PROG -c "stat" $testdir/file3 | grep 'fsxattr.xflags' | test_xflag "C" > +_test_fsxattr_xflag "$testdir/file3" "cowextsize" && echo "C flag set" || \ > + echo "C flag unset" > $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch > > echo "Unset cowextsize and check flag" > $XFS_IO_PROG -c "cowextsize 0" $testdir/file3 | _filter_scratch > _scratch_cycle_mount > > -$XFS_IO_PROG -c "stat" $testdir/file3 | grep 'fsxattr.xflags' | test_xflag "C" > +_test_fsxattr_xflag "$testdir/file3" "cowextsize" && echo "C flag set" || \ > + echo "C flag unset" > $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch > > status=0 > -- > 2.43.5 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] common/rc,xfs/207: Add a common helper function to check xflag bits 2024-11-21 17:20 ` Darrick J. Wong @ 2024-11-21 18:30 ` Nirjhar Roy 0 siblings, 0 replies; 15+ messages in thread From: Nirjhar Roy @ 2024-11-21 18:30 UTC (permalink / raw) To: Darrick J. Wong Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang On 11/21/24 22:50, Darrick J. Wong wrote: > On Thu, Nov 21, 2024 at 10:39:10AM +0530, Nirjhar Roy wrote: >> This patch defines a common helper function to test whether any of >> fsxattr xflags field is set or not. We will use this helper in >> an upcoming patch for checking extsize (e) flag. >> >> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> >> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> > Looks good to me now, > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> > > --D Thank you. --NR > >> --- >> common/rc | 7 +++++++ >> tests/xfs/207 | 15 ++++----------- >> 2 files changed, 11 insertions(+), 11 deletions(-) >> >> diff --git a/common/rc b/common/rc >> index 2af26f23..cccc98f5 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -41,6 +41,13 @@ _md5_checksum() >> md5sum $1 | cut -d ' ' -f1 >> } >> >> +# Check whether a fsxattr xflags name ($2) field is set on a given file ($1). >> +# e.g, fsxattr.xflags = 0x80000800 [extsize, has-xattr] >> +_test_fsxattr_xflag() >> +{ >> + grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") >> +} >> + >> # Write a byte into a range of a file >> _pwrite_byte() { >> local pattern="$1" >> diff --git a/tests/xfs/207 b/tests/xfs/207 >> index bbe21307..394e7e55 100755 >> --- a/tests/xfs/207 >> +++ b/tests/xfs/207 >> @@ -21,15 +21,6 @@ _require_cp_reflink >> _require_xfs_io_command "fiemap" >> _require_xfs_io_command "cowextsize" >> >> -# Takes the fsxattr.xflags line, >> -# i.e. fsxattr.xflags = 0x0 [--------------C-] >> -# and tests whether a flag character is set >> -test_xflag() >> -{ >> - local flg=$1 >> - grep -q "\[.*${flg}.*\]" && echo "$flg flag set" || echo "$flg flag unset" >> -} >> - >> echo "Format and mount" >> _scratch_mkfs > $seqres.full 2>&1 >> _scratch_mount >> $seqres.full 2>&1 >> @@ -65,14 +56,16 @@ echo "Set cowextsize and check flag" >> $XFS_IO_PROG -c "cowextsize 1048576" $testdir/file3 | _filter_scratch >> _scratch_cycle_mount >> >> -$XFS_IO_PROG -c "stat" $testdir/file3 | grep 'fsxattr.xflags' | test_xflag "C" >> +_test_fsxattr_xflag "$testdir/file3" "cowextsize" && echo "C flag set" || \ >> + echo "C flag unset" >> $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch >> >> echo "Unset cowextsize and check flag" >> $XFS_IO_PROG -c "cowextsize 0" $testdir/file3 | _filter_scratch >> _scratch_cycle_mount >> >> -$XFS_IO_PROG -c "stat" $testdir/file3 | grep 'fsxattr.xflags' | test_xflag "C" >> +_test_fsxattr_xflag "$testdir/file3" "cowextsize" && echo "C flag set" || \ >> + echo "C flag unset" >> $XFS_IO_PROG -c "cowextsize" $testdir/file3 | _filter_scratch >> >> status=0 >> -- >> 2.43.5 >> >> -- --- Nirjhar Roy Linux Kernel Developer IBM, Bangalore ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function 2024-11-21 5:09 [PATCH v3 0/3] Addition of new tests for extsize hints Nirjhar Roy 2024-11-21 5:09 ` [PATCH v3 1/3] common/rc,xfs/207: Add a common helper function to check xflag bits Nirjhar Roy @ 2024-11-21 5:09 ` Nirjhar Roy 2024-11-21 7:53 ` Ritesh Harjani 2024-11-21 5:09 ` [PATCH v3 3/3] generic: Addition of new tests for extsize hints Nirjhar Roy 2 siblings, 1 reply; 15+ messages in thread From: Nirjhar Roy @ 2024-11-21 5:09 UTC (permalink / raw) To: fstests; +Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, nirjhar _require_scratch_extsize helper function will be used in the the next patch to make the test run only on filesystems with extsize support. Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> --- common/rc | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/common/rc b/common/rc index cccc98f5..995979e9 100644 --- a/common/rc +++ b/common/rc @@ -48,6 +48,23 @@ _test_fsxattr_xflag() grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") } +# This test requires extsize support on the filesystem +_require_scratch_extsize() +{ + _require_scratch + _scratch_mkfs > /dev/null + _scratch_mount + local filename=$SCRATCH_MNT/$RANDOM + local blksz=$(_get_block_size $SCRATCH_MNT) + local extsz=$(( blksz*2 )) + local res=$($XFS_IO_PROG -c "open -f $filename" -c "extsize $extsz" \ + -c "extsize") + _scratch_unmount + grep -q "\[$extsz\] $filename" <(echo $res) || \ + _notrun "this test requires extsize support on the filesystem" +} + + # Write a byte into a range of a file _pwrite_byte() { local pattern="$1" -- 2.43.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function 2024-11-21 5:09 ` [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function Nirjhar Roy @ 2024-11-21 7:53 ` Ritesh Harjani 2024-11-21 18:33 ` Nirjhar Roy 0 siblings, 1 reply; 15+ messages in thread From: Ritesh Harjani @ 2024-11-21 7:53 UTC (permalink / raw) To: Nirjhar Roy, fstests Cc: linux-ext4, linux-xfs, ojaswin, djwong, zlang, nirjhar Nirjhar Roy <nirjhar@linux.ibm.com> writes: > _require_scratch_extsize helper function will be used in the > the next patch to make the test run only on filesystems with > extsize support. > > Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> > --- > common/rc | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/common/rc b/common/rc > index cccc98f5..995979e9 100644 > --- a/common/rc > +++ b/common/rc > @@ -48,6 +48,23 @@ _test_fsxattr_xflag() > grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") > } > > +# This test requires extsize support on the filesystem > +_require_scratch_extsize() > +{ > + _require_scratch _require_xfs_io_command "extsize" ^^^ Don't we need this too? > + _scratch_mkfs > /dev/null > + _scratch_mount > + local filename=$SCRATCH_MNT/$RANDOM > + local blksz=$(_get_block_size $SCRATCH_MNT) > + local extsz=$(( blksz*2 )) > + local res=$($XFS_IO_PROG -c "open -f $filename" -c "extsize $extsz" \ > + -c "extsize") > + _scratch_unmount > + grep -q "\[$extsz\] $filename" <(echo $res) || \ > + _notrun "this test requires extsize support on the filesystem" Why grep when we can simply just check the return value of previous xfs_io command? > +} > + > + ^^ Extra newline. > # Write a byte into a range of a file > _pwrite_byte() { > local pattern="$1" > -- > 2.43.5 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function 2024-11-21 7:53 ` Ritesh Harjani @ 2024-11-21 18:33 ` Nirjhar Roy 2024-11-21 18:52 ` Ritesh Harjani 0 siblings, 1 reply; 15+ messages in thread From: Nirjhar Roy @ 2024-11-21 18:33 UTC (permalink / raw) To: Ritesh Harjani (IBM), fstests Cc: linux-ext4, linux-xfs, ojaswin, djwong, zlang On 11/21/24 13:23, Ritesh Harjani (IBM) wrote: > Nirjhar Roy <nirjhar@linux.ibm.com> writes: > >> _require_scratch_extsize helper function will be used in the >> the next patch to make the test run only on filesystems with >> extsize support. >> >> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> >> --- >> common/rc | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/common/rc b/common/rc >> index cccc98f5..995979e9 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -48,6 +48,23 @@ _test_fsxattr_xflag() >> grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") >> } >> >> +# This test requires extsize support on the filesystem >> +_require_scratch_extsize() >> +{ >> + _require_scratch > _require_xfs_io_command "extsize" > > ^^^ Don't we need this too? Yes, good point. I will add this in the next revision. > >> + _scratch_mkfs > /dev/null >> + _scratch_mount >> + local filename=$SCRATCH_MNT/$RANDOM >> + local blksz=$(_get_block_size $SCRATCH_MNT) >> + local extsz=$(( blksz*2 )) >> + local res=$($XFS_IO_PROG -c "open -f $filename" -c "extsize $extsz" \ >> + -c "extsize") >> + _scratch_unmount >> + grep -q "\[$extsz\] $filename" <(echo $res) || \ >> + _notrun "this test requires extsize support on the filesystem" > Why grep when we can simply just check the return value of previous xfs_io command? No, I don't think we can rely on the return value of xfs_io. For ex, let's look at the following set of commands which are ran on an ext4 system: root@AMARPC: /mnt1/test$ xfs_io -V xfs_io version 5.13.0 root@AMARPC: /mnt1/test$ touch new root@AMARPC: /mnt1/test$ xfs_io -c "extsize 8k" new foreign file active, extsize command is for XFS filesystems only root@AMARPC: /mnt1/test$ echo "$?" 0 This incorrect return value might have been fixed in some later versions of xfs_io but there are still versions where we can't solely rely on the return value. > >> +} >> + >> + > ^^ Extra newline. Noted. I will fix this. --NR > >> # Write a byte into a range of a file >> _pwrite_byte() { >> local pattern="$1" >> -- >> 2.43.5 -- --- Nirjhar Roy Linux Kernel Developer IBM, Bangalore ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function 2024-11-21 18:33 ` Nirjhar Roy @ 2024-11-21 18:52 ` Ritesh Harjani 2024-11-22 16:04 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Ritesh Harjani @ 2024-11-21 18:52 UTC (permalink / raw) To: Nirjhar Roy, fstests; +Cc: linux-ext4, linux-xfs, ojaswin, djwong, zlang Nirjhar Roy <nirjhar@linux.ibm.com> writes: > On 11/21/24 13:23, Ritesh Harjani (IBM) wrote: >> Nirjhar Roy <nirjhar@linux.ibm.com> writes: >> >>> _require_scratch_extsize helper function will be used in the >>> the next patch to make the test run only on filesystems with >>> extsize support. >>> >>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >>> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> >>> --- >>> common/rc | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/common/rc b/common/rc >>> index cccc98f5..995979e9 100644 >>> --- a/common/rc >>> +++ b/common/rc >>> @@ -48,6 +48,23 @@ _test_fsxattr_xflag() >>> grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") >>> } >>> >>> +# This test requires extsize support on the filesystem >>> +_require_scratch_extsize() >>> +{ >>> + _require_scratch >> _require_xfs_io_command "extsize" >> >> ^^^ Don't we need this too? > Yes, good point. I will add this in the next revision. >> >>> + _scratch_mkfs > /dev/null >>> + _scratch_mount >>> + local filename=$SCRATCH_MNT/$RANDOM >>> + local blksz=$(_get_block_size $SCRATCH_MNT) >>> + local extsz=$(( blksz*2 )) >>> + local res=$($XFS_IO_PROG -c "open -f $filename" -c "extsize $extsz" \ >>> + -c "extsize") >>> + _scratch_unmount >>> + grep -q "\[$extsz\] $filename" <(echo $res) || \ >>> + _notrun "this test requires extsize support on the filesystem" >> Why grep when we can simply just check the return value of previous xfs_io command? > No, I don't think we can rely on the return value of xfs_io. For ex, > let's look at the following set of commands which are ran on an ext4 system: > > root@AMARPC: /mnt1/test$ xfs_io -V > xfs_io version 5.13.0 > root@AMARPC: /mnt1/test$ touch new > root@AMARPC: /mnt1/test$ xfs_io -c "extsize 8k" new > foreign file active, extsize command is for XFS filesystems only > root@AMARPC: /mnt1/test$ echo "$?" > 0 > This incorrect return value might have been fixed in some later versions > of xfs_io but there are still versions where we can't solely rely on the > return value. Ok. That's bad, we then have to rely on grep. Sure, thanks for checking and confirming that. -ritesh ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function 2024-11-21 18:52 ` Ritesh Harjani @ 2024-11-22 16:04 ` Darrick J. Wong 2024-11-22 18:07 ` Nirjhar Roy 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2024-11-22 16:04 UTC (permalink / raw) To: Ritesh Harjani Cc: Nirjhar Roy, fstests, linux-ext4, linux-xfs, ojaswin, zlang On Fri, Nov 22, 2024 at 12:22:41AM +0530, Ritesh Harjani wrote: > Nirjhar Roy <nirjhar@linux.ibm.com> writes: > > > On 11/21/24 13:23, Ritesh Harjani (IBM) wrote: > >> Nirjhar Roy <nirjhar@linux.ibm.com> writes: > >> > >>> _require_scratch_extsize helper function will be used in the > >>> the next patch to make the test run only on filesystems with > >>> extsize support. > >>> > >>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > >>> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> > >>> --- > >>> common/rc | 17 +++++++++++++++++ > >>> 1 file changed, 17 insertions(+) > >>> > >>> diff --git a/common/rc b/common/rc > >>> index cccc98f5..995979e9 100644 > >>> --- a/common/rc > >>> +++ b/common/rc > >>> @@ -48,6 +48,23 @@ _test_fsxattr_xflag() > >>> grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") > >>> } > >>> > >>> +# This test requires extsize support on the filesystem > >>> +_require_scratch_extsize() > >>> +{ > >>> + _require_scratch > >> _require_xfs_io_command "extsize" > >> > >> ^^^ Don't we need this too? > > Yes, good point. I will add this in the next revision. > >> > >>> + _scratch_mkfs > /dev/null > >>> + _scratch_mount > >>> + local filename=$SCRATCH_MNT/$RANDOM > >>> + local blksz=$(_get_block_size $SCRATCH_MNT) > >>> + local extsz=$(( blksz*2 )) > >>> + local res=$($XFS_IO_PROG -c "open -f $filename" -c "extsize $extsz" \ > >>> + -c "extsize") > >>> + _scratch_unmount > >>> + grep -q "\[$extsz\] $filename" <(echo $res) || \ > >>> + _notrun "this test requires extsize support on the filesystem" > >> Why grep when we can simply just check the return value of previous xfs_io command? > > No, I don't think we can rely on the return value of xfs_io. For ex, > > let's look at the following set of commands which are ran on an ext4 system: > > > > root@AMARPC: /mnt1/test$ xfs_io -V > > xfs_io version 5.13.0 > > root@AMARPC: /mnt1/test$ touch new > > root@AMARPC: /mnt1/test$ xfs_io -c "extsize 8k" new > > foreign file active, extsize command is for XFS filesystems only > > root@AMARPC: /mnt1/test$ echo "$?" > > 0 > > This incorrect return value might have been fixed in some later versions > > of xfs_io but there are still versions where we can't solely rely on the > > return value. > > Ok. That's bad, we then have to rely on grep. > Sure, thanks for checking and confirming that. You all should add CMD_FOREIGN_OK to the extsize command in xfs_io, assuming that you've not already done that in your dev workspace. --D > -ritesh > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function 2024-11-22 16:04 ` Darrick J. Wong @ 2024-11-22 18:07 ` Nirjhar Roy 2024-11-22 18:44 ` Darrick J. Wong 0 siblings, 1 reply; 15+ messages in thread From: Nirjhar Roy @ 2024-11-22 18:07 UTC (permalink / raw) To: Darrick J. Wong, Ritesh Harjani Cc: fstests, linux-ext4, linux-xfs, ojaswin, zlang On 11/22/24 21:34, Darrick J. Wong wrote: > On Fri, Nov 22, 2024 at 12:22:41AM +0530, Ritesh Harjani wrote: >> Nirjhar Roy <nirjhar@linux.ibm.com> writes: >> >>> On 11/21/24 13:23, Ritesh Harjani (IBM) wrote: >>>> Nirjhar Roy <nirjhar@linux.ibm.com> writes: >>>> >>>>> _require_scratch_extsize helper function will be used in the >>>>> the next patch to make the test run only on filesystems with >>>>> extsize support. >>>>> >>>>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >>>>> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> >>>>> --- >>>>> common/rc | 17 +++++++++++++++++ >>>>> 1 file changed, 17 insertions(+) >>>>> >>>>> diff --git a/common/rc b/common/rc >>>>> index cccc98f5..995979e9 100644 >>>>> --- a/common/rc >>>>> +++ b/common/rc >>>>> @@ -48,6 +48,23 @@ _test_fsxattr_xflag() >>>>> grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") >>>>> } >>>>> >>>>> +# This test requires extsize support on the filesystem >>>>> +_require_scratch_extsize() >>>>> +{ >>>>> + _require_scratch >>>> _require_xfs_io_command "extsize" >>>> >>>> ^^^ Don't we need this too? >>> Yes, good point. I will add this in the next revision. >>>>> + _scratch_mkfs > /dev/null >>>>> + _scratch_mount >>>>> + local filename=$SCRATCH_MNT/$RANDOM >>>>> + local blksz=$(_get_block_size $SCRATCH_MNT) >>>>> + local extsz=$(( blksz*2 )) >>>>> + local res=$($XFS_IO_PROG -c "open -f $filename" -c "extsize $extsz" \ >>>>> + -c "extsize") >>>>> + _scratch_unmount >>>>> + grep -q "\[$extsz\] $filename" <(echo $res) || \ >>>>> + _notrun "this test requires extsize support on the filesystem" >>>> Why grep when we can simply just check the return value of previous xfs_io command? >>> No, I don't think we can rely on the return value of xfs_io. For ex, >>> let's look at the following set of commands which are ran on an ext4 system: >>> >>> root@AMARPC: /mnt1/test$ xfs_io -V >>> xfs_io version 5.13.0 >>> root@AMARPC: /mnt1/test$ touch new >>> root@AMARPC: /mnt1/test$ xfs_io -c "extsize 8k" new >>> foreign file active, extsize command is for XFS filesystems only >>> root@AMARPC: /mnt1/test$ echo "$?" >>> 0 >>> This incorrect return value might have been fixed in some later versions >>> of xfs_io but there are still versions where we can't solely rely on the >>> return value. >> Ok. That's bad, we then have to rely on grep. >> Sure, thanks for checking and confirming that. > You all should add CMD_FOREIGN_OK to the extsize command in xfs_io, > assuming that you've not already done that in your dev workspace. > > --D Yes, I have tested with that as well. I have applied the following patch to xfsprogs and tested with the modified xfs_io. diff --git a/io/open.c b/io/open.c index 15850b55..6407b7e8 100644 --- a/io/open.c +++ b/io/open.c @@ -980,7 +980,7 @@ open_init(void) extsize_cmd.args = _("[-D | -R] [extsize]"); extsize_cmd.argmin = 0; extsize_cmd.argmax = -1; - extsize_cmd.flags = CMD_NOMAP_OK; + extsize_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK; extsize_cmd.oneline = _("get/set preferred extent size (in bytes) for the open file"); extsize_cmd.help = extsize_help; The return values are similar. root@AMARPC: /mnt1/scratch$ touch new root@AMARPC: /mnt1/scratch$ /home/ubuntu/xfsprogs-dev/io/xfs_io -c "extsize 8k" new root@AMARPC: /mnt1/scratch$ echo "$?" 0 root@AMARPC: /mnt1/scratch$ /home/ubuntu/xfsprogs-dev/io/xfs_io -c "extsize" new [0] new This is the reason I am not relying on the return value, instead I am checking if only the extsize gets changed, we will assume that the extsize support is there, else the test will _notrun. Also, root@AMARPC: /mnt1/scratch$ strace -f /mnt1/scratch$ /home/ubuntu/xfsprogs-dev/io/xfs_io -c "extsize 16k" new ... ... ioctl(3, FS_IOC_FSGETXATTR, {fsx_xflags=0, fsx_extsize=0, fsx_nextents=0, fsx_projid=0, fsx_cowextsize=0}) = 0 ioctl(3, FS_IOC_FSSETXATTR, {fsx_xflags=FS_XFLAG_EXTSIZE, fsx_extsize=16384, fsx_projid=0, fsx_cowextsize=0}) = 0 exit_group(0) Looking at the existing code for ext4_fileattr_set(), We validate the flags but I think we silently don't validate(and ignore) the xflags. Like, we have int err = -EOPNOTSUPP; if (flags & ~EXT4_FL_USER_VISIBLE) goto out; BUT we do NOT have something like int err = -EOPNOTSUPP; if (fa->fsx_flags & ~EXT4_VALID_XFLAGS) // where EXT4_VALID_XFLAGS should be an || of all the supported xflags in ext4. goto out; I am not sure what other filesystems do, but if we check whether the extsize got changed, then we can correctly determine extsize support. Does that make sense? --NR > >> -ritesh >> -- --- Nirjhar Roy Linux Kernel Developer IBM, Bangalore ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function 2024-11-22 18:07 ` Nirjhar Roy @ 2024-11-22 18:44 ` Darrick J. Wong 2024-11-22 19:06 ` Nirjhar Roy 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2024-11-22 18:44 UTC (permalink / raw) To: Nirjhar Roy Cc: Ritesh Harjani, fstests, linux-ext4, linux-xfs, ojaswin, zlang On Fri, Nov 22, 2024 at 11:37:17PM +0530, Nirjhar Roy wrote: > > On 11/22/24 21:34, Darrick J. Wong wrote: > > On Fri, Nov 22, 2024 at 12:22:41AM +0530, Ritesh Harjani wrote: > > > Nirjhar Roy <nirjhar@linux.ibm.com> writes: > > > > > > > On 11/21/24 13:23, Ritesh Harjani (IBM) wrote: > > > > > Nirjhar Roy <nirjhar@linux.ibm.com> writes: > > > > > > > > > > > _require_scratch_extsize helper function will be used in the > > > > > > the next patch to make the test run only on filesystems with > > > > > > extsize support. > > > > > > > > > > > > Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > > > > > > Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> > > > > > > --- > > > > > > common/rc | 17 +++++++++++++++++ > > > > > > 1 file changed, 17 insertions(+) > > > > > > > > > > > > diff --git a/common/rc b/common/rc > > > > > > index cccc98f5..995979e9 100644 > > > > > > --- a/common/rc > > > > > > +++ b/common/rc > > > > > > @@ -48,6 +48,23 @@ _test_fsxattr_xflag() > > > > > > grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") > > > > > > } > > > > > > +# This test requires extsize support on the filesystem > > > > > > +_require_scratch_extsize() > > > > > > +{ > > > > > > + _require_scratch > > > > > _require_xfs_io_command "extsize" > > > > > > > > > > ^^^ Don't we need this too? > > > > Yes, good point. I will add this in the next revision. > > > > > > + _scratch_mkfs > /dev/null > > > > > > + _scratch_mount > > > > > > + local filename=$SCRATCH_MNT/$RANDOM > > > > > > + local blksz=$(_get_block_size $SCRATCH_MNT) > > > > > > + local extsz=$(( blksz*2 )) > > > > > > + local res=$($XFS_IO_PROG -c "open -f $filename" -c "extsize $extsz" \ > > > > > > + -c "extsize") > > > > > > + _scratch_unmount > > > > > > + grep -q "\[$extsz\] $filename" <(echo $res) || \ > > > > > > + _notrun "this test requires extsize support on the filesystem" > > > > > Why grep when we can simply just check the return value of previous xfs_io command? > > > > No, I don't think we can rely on the return value of xfs_io. For ex, > > > > let's look at the following set of commands which are ran on an ext4 system: > > > > > > > > root@AMARPC: /mnt1/test$ xfs_io -V > > > > xfs_io version 5.13.0 > > > > root@AMARPC: /mnt1/test$ touch new > > > > root@AMARPC: /mnt1/test$ xfs_io -c "extsize 8k" new > > > > foreign file active, extsize command is for XFS filesystems only > > > > root@AMARPC: /mnt1/test$ echo "$?" > > > > 0 > > > > This incorrect return value might have been fixed in some later versions > > > > of xfs_io but there are still versions where we can't solely rely on the > > > > return value. > > > Ok. That's bad, we then have to rely on grep. > > > Sure, thanks for checking and confirming that. > > You all should add CMD_FOREIGN_OK to the extsize command in xfs_io, > > assuming that you've not already done that in your dev workspace. > > > > --D > > Yes, I have tested with that as well. I have applied the following patch to > xfsprogs and tested with the modified xfs_io. > > diff --git a/io/open.c b/io/open.c > index 15850b55..6407b7e8 100644 > --- a/io/open.c > +++ b/io/open.c > @@ -980,7 +980,7 @@ open_init(void) > extsize_cmd.args = _("[-D | -R] [extsize]"); > extsize_cmd.argmin = 0; > extsize_cmd.argmax = -1; > - extsize_cmd.flags = CMD_NOMAP_OK; > + extsize_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK; > extsize_cmd.oneline = > _("get/set preferred extent size (in bytes) for the open > file"); > extsize_cmd.help = extsize_help; > > The return values are similar. > > root@AMARPC: /mnt1/scratch$ touch new > root@AMARPC: /mnt1/scratch$ /home/ubuntu/xfsprogs-dev/io/xfs_io -c "extsize > 8k" new > root@AMARPC: /mnt1/scratch$ echo "$?" > 0 > root@AMARPC: /mnt1/scratch$ /home/ubuntu/xfsprogs-dev/io/xfs_io -c "extsize" > new > [0] new > > This is the reason I am not relying on the return value, instead I am > checking if only the extsize gets changed, we will assume that the extsize > support is there, else the test will _notrun. > > Also, > > root@AMARPC: /mnt1/scratch$ strace -f /mnt1/scratch$ > /home/ubuntu/xfsprogs-dev/io/xfs_io -c "extsize 16k" new > > ... > > ... > > ioctl(3, FS_IOC_FSGETXATTR, {fsx_xflags=0, fsx_extsize=0, fsx_nextents=0, > fsx_projid=0, fsx_cowextsize=0}) = 0 > ioctl(3, FS_IOC_FSSETXATTR, {fsx_xflags=FS_XFLAG_EXTSIZE, fsx_extsize=16384, > fsx_projid=0, fsx_cowextsize=0}) = 0 > exit_group(0) > > Looking at the existing code for ext4_fileattr_set(), We validate the flags > but I think we silently don't validate(and ignore) the xflags. Like, we have > > int err = -EOPNOTSUPP; > if (flags & ~EXT4_FL_USER_VISIBLE) > goto out; > > BUT we do NOT have something like > > int err = -EOPNOTSUPP; > if (fa->fsx_flags & ~EXT4_VALID_XFLAGS) // where EXT4_VALID_XFLAGS should be > an || of all the supported xflags in ext4. > goto out; > > I am not sure what other filesystems do, but if we check whether the extsize > got changed, then we can correctly determine extsize support. > > Does that make sense? You don't have to check fsx_flags if you don't use fileattr_fill_xflags. ext4 doesn't use that. --D > --NR > > > > > > > > -ritesh > > > > -- > --- > Nirjhar Roy > Linux Kernel Developer > IBM, Bangalore > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function 2024-11-22 18:44 ` Darrick J. Wong @ 2024-11-22 19:06 ` Nirjhar Roy 0 siblings, 0 replies; 15+ messages in thread From: Nirjhar Roy @ 2024-11-22 19:06 UTC (permalink / raw) To: Darrick J. Wong Cc: Ritesh Harjani, fstests, linux-ext4, linux-xfs, ojaswin, zlang On 11/23/24 00:14, Darrick J. Wong wrote: > On Fri, Nov 22, 2024 at 11:37:17PM +0530, Nirjhar Roy wrote: >> On 11/22/24 21:34, Darrick J. Wong wrote: >>> On Fri, Nov 22, 2024 at 12:22:41AM +0530, Ritesh Harjani wrote: >>>> Nirjhar Roy <nirjhar@linux.ibm.com> writes: >>>> >>>>> On 11/21/24 13:23, Ritesh Harjani (IBM) wrote: >>>>>> Nirjhar Roy <nirjhar@linux.ibm.com> writes: >>>>>> >>>>>>> _require_scratch_extsize helper function will be used in the >>>>>>> the next patch to make the test run only on filesystems with >>>>>>> extsize support. >>>>>>> >>>>>>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >>>>>>> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> >>>>>>> --- >>>>>>> common/rc | 17 +++++++++++++++++ >>>>>>> 1 file changed, 17 insertions(+) >>>>>>> >>>>>>> diff --git a/common/rc b/common/rc >>>>>>> index cccc98f5..995979e9 100644 >>>>>>> --- a/common/rc >>>>>>> +++ b/common/rc >>>>>>> @@ -48,6 +48,23 @@ _test_fsxattr_xflag() >>>>>>> grep -q "fsxattr.xflags.*\[.*$2.*\]" <($XFS_IO_PROG -c "stat -v" "$1") >>>>>>> } >>>>>>> +# This test requires extsize support on the filesystem >>>>>>> +_require_scratch_extsize() >>>>>>> +{ >>>>>>> + _require_scratch >>>>>> _require_xfs_io_command "extsize" >>>>>> >>>>>> ^^^ Don't we need this too? >>>>> Yes, good point. I will add this in the next revision. >>>>>>> + _scratch_mkfs > /dev/null >>>>>>> + _scratch_mount >>>>>>> + local filename=$SCRATCH_MNT/$RANDOM >>>>>>> + local blksz=$(_get_block_size $SCRATCH_MNT) >>>>>>> + local extsz=$(( blksz*2 )) >>>>>>> + local res=$($XFS_IO_PROG -c "open -f $filename" -c "extsize $extsz" \ >>>>>>> + -c "extsize") >>>>>>> + _scratch_unmount >>>>>>> + grep -q "\[$extsz\] $filename" <(echo $res) || \ >>>>>>> + _notrun "this test requires extsize support on the filesystem" >>>>>> Why grep when we can simply just check the return value of previous xfs_io command? >>>>> No, I don't think we can rely on the return value of xfs_io. For ex, >>>>> let's look at the following set of commands which are ran on an ext4 system: >>>>> >>>>> root@AMARPC: /mnt1/test$ xfs_io -V >>>>> xfs_io version 5.13.0 >>>>> root@AMARPC: /mnt1/test$ touch new >>>>> root@AMARPC: /mnt1/test$ xfs_io -c "extsize 8k" new >>>>> foreign file active, extsize command is for XFS filesystems only >>>>> root@AMARPC: /mnt1/test$ echo "$?" >>>>> 0 >>>>> This incorrect return value might have been fixed in some later versions >>>>> of xfs_io but there are still versions where we can't solely rely on the >>>>> return value. >>>> Ok. That's bad, we then have to rely on grep. >>>> Sure, thanks for checking and confirming that. >>> You all should add CMD_FOREIGN_OK to the extsize command in xfs_io, >>> assuming that you've not already done that in your dev workspace. >>> >>> --D >> Yes, I have tested with that as well. I have applied the following patch to >> xfsprogs and tested with the modified xfs_io. >> >> diff --git a/io/open.c b/io/open.c >> index 15850b55..6407b7e8 100644 >> --- a/io/open.c >> +++ b/io/open.c >> @@ -980,7 +980,7 @@ open_init(void) >> extsize_cmd.args = _("[-D | -R] [extsize]"); >> extsize_cmd.argmin = 0; >> extsize_cmd.argmax = -1; >> - extsize_cmd.flags = CMD_NOMAP_OK; >> + extsize_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK; >> extsize_cmd.oneline = >> _("get/set preferred extent size (in bytes) for the open >> file"); >> extsize_cmd.help = extsize_help; >> >> The return values are similar. >> >> root@AMARPC: /mnt1/scratch$ touch new >> root@AMARPC: /mnt1/scratch$ /home/ubuntu/xfsprogs-dev/io/xfs_io -c "extsize >> 8k" new >> root@AMARPC: /mnt1/scratch$ echo "$?" >> 0 >> root@AMARPC: /mnt1/scratch$ /home/ubuntu/xfsprogs-dev/io/xfs_io -c "extsize" >> new >> [0] new >> >> This is the reason I am not relying on the return value, instead I am >> checking if only the extsize gets changed, we will assume that the extsize >> support is there, else the test will _notrun. >> >> Also, >> >> root@AMARPC: /mnt1/scratch$ strace -f /mnt1/scratch$ >> /home/ubuntu/xfsprogs-dev/io/xfs_io -c "extsize 16k" new >> >> ... >> >> ... >> >> ioctl(3, FS_IOC_FSGETXATTR, {fsx_xflags=0, fsx_extsize=0, fsx_nextents=0, >> fsx_projid=0, fsx_cowextsize=0}) = 0 >> ioctl(3, FS_IOC_FSSETXATTR, {fsx_xflags=FS_XFLAG_EXTSIZE, fsx_extsize=16384, >> fsx_projid=0, fsx_cowextsize=0}) = 0 >> exit_group(0) >> >> Looking at the existing code for ext4_fileattr_set(), We validate the flags >> but I think we silently don't validate(and ignore) the xflags. Like, we have >> >> int err = -EOPNOTSUPP; >> if (flags & ~EXT4_FL_USER_VISIBLE) >> goto out; >> >> BUT we do NOT have something like >> >> int err = -EOPNOTSUPP; >> if (fa->fsx_flags & ~EXT4_VALID_XFLAGS) // where EXT4_VALID_XFLAGS should be >> an || of all the supported xflags in ext4. >> goto out; >> >> I am not sure what other filesystems do, but if we check whether the extsize >> got changed, then we can correctly determine extsize support. >> >> Does that make sense? > You don't have to check fsx_flags if you don't use fileattr_fill_xflags. > ext4 doesn't use that. > > --D Okay got it. Thank you. How does the overall logic of the _require_scratch_extsize() look? > >> --NR >> >> >> >>>> -ritesh >>>> >> -- >> --- >> Nirjhar Roy >> Linux Kernel Developer >> IBM, Bangalore >> -- --- Nirjhar Roy Linux Kernel Developer IBM, Bangalore ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 3/3] generic: Addition of new tests for extsize hints 2024-11-21 5:09 [PATCH v3 0/3] Addition of new tests for extsize hints Nirjhar Roy 2024-11-21 5:09 ` [PATCH v3 1/3] common/rc,xfs/207: Add a common helper function to check xflag bits Nirjhar Roy 2024-11-21 5:09 ` [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function Nirjhar Roy @ 2024-11-21 5:09 ` Nirjhar Roy 2024-11-21 17:27 ` Darrick J. Wong 2 siblings, 1 reply; 15+ messages in thread From: Nirjhar Roy @ 2024-11-21 5:09 UTC (permalink / raw) To: fstests; +Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, nirjhar This commit adds new tests that checks the behaviour of xfs/ext4 filesystems when extsize hint is set on file with inode size as 0, non-empty files with allocated and delalloc extents and so on. Although currently this test is placed under tests/generic, it only runs on xfs and there is an ongoing patch series[1] to enable extsize hints for ext4 as well. [1] https://lore.kernel.org/linux-ext4/cover.1726034272.git.ojaswin@linux.ibm.com/ Reviewed-by Ritesh Harjani (IBM) <ritesh.list@gmail.com> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> Suggested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> --- tests/generic/366 | 175 ++++++++++++++++++++++++++++++++++++++++++ tests/generic/366.out | 26 +++++++ 2 files changed, 201 insertions(+) create mode 100755 tests/generic/366 create mode 100644 tests/generic/366.out diff --git a/tests/generic/366 b/tests/generic/366 new file mode 100755 index 00000000..25d23f42 --- /dev/null +++ b/tests/generic/366 @@ -0,0 +1,175 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Nirjhar Roy (nirjhar@linux.ibm.com). All Rights Reserved. +# +# FS QA Test 366 +# +# This test verifies that extent allocation hint setting works correctly on +# files with no extents allocated and non-empty files which are truncated. +# It also checks that the extent hints setting fails with non-empty file +# i.e, with any file with allocated extents or delayed allocation. We also +# check if the extsize value and the xflag bit actually got reflected after +# setting/re-setting the extsize value. + +. ./common/config +. ./common/filter +. ./common/preamble + +_begin_fstest ioctl quick + +_fixed_by_kernel_commit "2a492ff66673 \ + xfs: Check for delayed allocations before setting \ + extsize" + +_require_scratch_extsize + +FILE_DATA_SIZE=1M + +get_default_extsize() +{ + if [ -z $1 ] || [ ! -d $1 ]; then + echo "Missing mount point argument for get_default_extsize" + exit 1 + fi + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/' +} + +filter_extsz() +{ + sed "s/\[$1\]/\[EXTSIZE\]/g" +} + +setup() +{ + _scratch_mkfs >> "$seqres.full" 2>&1 + _scratch_mount >> "$seqres.full" 2>&1 + BLKSZ=`_get_block_size $SCRATCH_MNT` + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` + EXTSIZE=$(( BLKSZ*2 )) + # Make sure the new extsize is not the same as the default + # extsize so that we can observe it changing + [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) +} + +read_file_extsize() +{ + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 +} + +check_extsz_and_xflag() +{ + local filename=$1 + local extsize=$2 + read_file_extsize $filename $extsize + _test_fsxattr_xflag $filename "extsize" && echo "e flag set" || \ + echo "e flag unset" +} + +check_extsz_xflag_across_remount() +{ + local filename=$1 + local extsize=$2 + _scratch_cycle_mount + check_extsz_and_xflag $filename $extsize +} + +# Extsize flag should be cleared when extsize is reset, so this function +# checks that this behavior is followed. +reset_extsz_and_recheck_extsz_xflag() +{ + local filename=$1 + echo "Re-setting extsize hint to 0" + $XFS_IO_PROG -c "extsize 0" $filename + check_extsz_xflag_across_remount $filename "0" +} + +check_extsz_xflag_before_and_after_reset() +{ + local filename=$1 + local extsize=$2 + check_extsz_xflag_across_remount $filename $extsize + reset_extsz_and_recheck_extsz_xflag $filename +} + +test_empty_file() +{ + echo "TEST: Set extsize on empty file" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE + echo +} + +test_data_delayed() +{ + echo "TEST: Set extsize on non-empty file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_data_allocated() +{ + echo "TEST: Set extsize on non-empty file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "extsize $EXTSIZE" | _filter_scratch + + echo "test for default extsize setting if any" + read_file_extsize $filename $DEFAULT_EXTSIZE + echo +} + +test_truncate_allocated() +{ + echo "TEST: Set extsize after truncating a file with allocated extents" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +test_truncate_delayed() +{ + echo "TEST: Set extsize after truncating a file with delayed allocation" + local filename=$1 + $XFS_IO_PROG \ + -c "open -f $filename" \ + -c "pwrite -q 0 $FILE_DATA_SIZE" \ + -c "truncate 0" \ + -c "extsize $EXTSIZE" \ + + check_extsz_xflag_across_remount $filename $EXTSIZE + echo +} + +setup +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE \ + BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" + +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- + +test_empty_file "$NEW_FILE_NAME_PREFIX"00 +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 + +status=0 +exit diff --git a/tests/generic/366.out b/tests/generic/366.out new file mode 100644 index 00000000..cdd2f5fa --- /dev/null +++ b/tests/generic/366.out @@ -0,0 +1,26 @@ +QA output created by 366 +TEST: Set extsize on empty file +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag set +Re-setting extsize hint to 0 +[EXTSIZE] SCRATCH_MNT/new-file-00 +e flag unset + +TEST: Set extsize on non-empty file with delayed allocation +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-01 + +TEST: Set extsize on non-empty file with allocated extents +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument +test for default extsize setting if any +[EXTSIZE] SCRATCH_MNT/new-file-02 + +TEST: Set extsize after truncating a file with allocated extents +[EXTSIZE] SCRATCH_MNT/new-file-03 +e flag set + +TEST: Set extsize after truncating a file with delayed allocation +[EXTSIZE] SCRATCH_MNT/new-file-04 +e flag set + -- 2.43.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] generic: Addition of new tests for extsize hints 2024-11-21 5:09 ` [PATCH v3 3/3] generic: Addition of new tests for extsize hints Nirjhar Roy @ 2024-11-21 17:27 ` Darrick J. Wong 2024-11-21 18:34 ` Nirjhar Roy 0 siblings, 1 reply; 15+ messages in thread From: Darrick J. Wong @ 2024-11-21 17:27 UTC (permalink / raw) To: Nirjhar Roy; +Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang On Thu, Nov 21, 2024 at 10:39:12AM +0530, Nirjhar Roy wrote: > This commit adds new tests that checks the behaviour of xfs/ext4 > filesystems when extsize hint is set on file with inode size as 0, > non-empty files with allocated and delalloc extents and so on. > Although currently this test is placed under tests/generic, it > only runs on xfs and there is an ongoing patch series[1] to > enable extsize hints for ext4 as well. > > [1] https://lore.kernel.org/linux-ext4/cover.1726034272.git.ojaswin@linux.ibm.com/ > > Reviewed-by Ritesh Harjani (IBM) <ritesh.list@gmail.com> > Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > Suggested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> > Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> Looks ok, though you might want to change generic/366 to a higher number because that's now taken. Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > --- > tests/generic/366 | 175 ++++++++++++++++++++++++++++++++++++++++++ > tests/generic/366.out | 26 +++++++ > 2 files changed, 201 insertions(+) > create mode 100755 tests/generic/366 > create mode 100644 tests/generic/366.out > > diff --git a/tests/generic/366 b/tests/generic/366 > new file mode 100755 > index 00000000..25d23f42 > --- /dev/null > +++ b/tests/generic/366 > @@ -0,0 +1,175 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2024 Nirjhar Roy (nirjhar@linux.ibm.com). All Rights Reserved. > +# > +# FS QA Test 366 > +# > +# This test verifies that extent allocation hint setting works correctly on > +# files with no extents allocated and non-empty files which are truncated. > +# It also checks that the extent hints setting fails with non-empty file > +# i.e, with any file with allocated extents or delayed allocation. We also > +# check if the extsize value and the xflag bit actually got reflected after > +# setting/re-setting the extsize value. > + > +. ./common/config > +. ./common/filter > +. ./common/preamble > + > +_begin_fstest ioctl quick > + > +_fixed_by_kernel_commit "2a492ff66673 \ > + xfs: Check for delayed allocations before setting \ > + extsize" > + > +_require_scratch_extsize > + > +FILE_DATA_SIZE=1M > + > +get_default_extsize() > +{ > + if [ -z $1 ] || [ ! -d $1 ]; then > + echo "Missing mount point argument for get_default_extsize" > + exit 1 > + fi > + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/' > +} > + > +filter_extsz() > +{ > + sed "s/\[$1\]/\[EXTSIZE\]/g" > +} > + > +setup() > +{ > + _scratch_mkfs >> "$seqres.full" 2>&1 > + _scratch_mount >> "$seqres.full" 2>&1 > + BLKSZ=`_get_block_size $SCRATCH_MNT` > + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` > + EXTSIZE=$(( BLKSZ*2 )) > + # Make sure the new extsize is not the same as the default > + # extsize so that we can observe it changing > + [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) > +} > + > +read_file_extsize() > +{ > + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 > +} > + > +check_extsz_and_xflag() > +{ > + local filename=$1 > + local extsize=$2 > + read_file_extsize $filename $extsize > + _test_fsxattr_xflag $filename "extsize" && echo "e flag set" || \ > + echo "e flag unset" > +} > + > +check_extsz_xflag_across_remount() > +{ > + local filename=$1 > + local extsize=$2 > + _scratch_cycle_mount > + check_extsz_and_xflag $filename $extsize > +} > + > +# Extsize flag should be cleared when extsize is reset, so this function > +# checks that this behavior is followed. > +reset_extsz_and_recheck_extsz_xflag() > +{ > + local filename=$1 > + echo "Re-setting extsize hint to 0" > + $XFS_IO_PROG -c "extsize 0" $filename > + check_extsz_xflag_across_remount $filename "0" > +} > + > +check_extsz_xflag_before_and_after_reset() > +{ > + local filename=$1 > + local extsize=$2 > + check_extsz_xflag_across_remount $filename $extsize > + reset_extsz_and_recheck_extsz_xflag $filename > +} > + > +test_empty_file() > +{ > + echo "TEST: Set extsize on empty file" > + local filename=$1 > + $XFS_IO_PROG \ > + -c "open -f $filename" \ > + -c "extsize $EXTSIZE" \ > + > + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE > + echo > +} > + > +test_data_delayed() > +{ > + echo "TEST: Set extsize on non-empty file with delayed allocation" > + local filename=$1 > + $XFS_IO_PROG \ > + -c "open -f $filename" \ > + -c "pwrite -q 0 $FILE_DATA_SIZE" \ > + -c "extsize $EXTSIZE" | _filter_scratch > + > + echo "test for default extsize setting if any" > + read_file_extsize $filename $DEFAULT_EXTSIZE > + echo > +} > + > +test_data_allocated() > +{ > + echo "TEST: Set extsize on non-empty file with allocated extents" > + local filename=$1 > + $XFS_IO_PROG \ > + -c "open -f $filename" \ > + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ > + -c "extsize $EXTSIZE" | _filter_scratch > + > + echo "test for default extsize setting if any" > + read_file_extsize $filename $DEFAULT_EXTSIZE > + echo > +} > + > +test_truncate_allocated() > +{ > + echo "TEST: Set extsize after truncating a file with allocated extents" > + local filename=$1 > + $XFS_IO_PROG \ > + -c "open -f $filename" \ > + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ > + -c "truncate 0" \ > + -c "extsize $EXTSIZE" \ > + > + check_extsz_xflag_across_remount $filename $EXTSIZE > + echo > +} > + > +test_truncate_delayed() > +{ > + echo "TEST: Set extsize after truncating a file with delayed allocation" > + local filename=$1 > + $XFS_IO_PROG \ > + -c "open -f $filename" \ > + -c "pwrite -q 0 $FILE_DATA_SIZE" \ > + -c "truncate 0" \ > + -c "extsize $EXTSIZE" \ > + > + check_extsz_xflag_across_remount $filename $EXTSIZE > + echo > +} > + > +setup > +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE \ > + BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" > + > +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- > + > +test_empty_file "$NEW_FILE_NAME_PREFIX"00 > +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 > +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 > +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 > +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 > + > +status=0 > +exit > diff --git a/tests/generic/366.out b/tests/generic/366.out > new file mode 100644 > index 00000000..cdd2f5fa > --- /dev/null > +++ b/tests/generic/366.out > @@ -0,0 +1,26 @@ > +QA output created by 366 > +TEST: Set extsize on empty file > +[EXTSIZE] SCRATCH_MNT/new-file-00 > +e flag set > +Re-setting extsize hint to 0 > +[EXTSIZE] SCRATCH_MNT/new-file-00 > +e flag unset > + > +TEST: Set extsize on non-empty file with delayed allocation > +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument > +test for default extsize setting if any > +[EXTSIZE] SCRATCH_MNT/new-file-01 > + > +TEST: Set extsize on non-empty file with allocated extents > +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument > +test for default extsize setting if any > +[EXTSIZE] SCRATCH_MNT/new-file-02 > + > +TEST: Set extsize after truncating a file with allocated extents > +[EXTSIZE] SCRATCH_MNT/new-file-03 > +e flag set > + > +TEST: Set extsize after truncating a file with delayed allocation > +[EXTSIZE] SCRATCH_MNT/new-file-04 > +e flag set > + > -- > 2.43.5 > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] generic: Addition of new tests for extsize hints 2024-11-21 17:27 ` Darrick J. Wong @ 2024-11-21 18:34 ` Nirjhar Roy 0 siblings, 0 replies; 15+ messages in thread From: Nirjhar Roy @ 2024-11-21 18:34 UTC (permalink / raw) To: Darrick J. Wong Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang On 11/21/24 22:57, Darrick J. Wong wrote: > On Thu, Nov 21, 2024 at 10:39:12AM +0530, Nirjhar Roy wrote: >> This commit adds new tests that checks the behaviour of xfs/ext4 >> filesystems when extsize hint is set on file with inode size as 0, >> non-empty files with allocated and delalloc extents and so on. >> Although currently this test is placed under tests/generic, it >> only runs on xfs and there is an ongoing patch series[1] to >> enable extsize hints for ext4 as well. >> >> [1] https://lore.kernel.org/linux-ext4/cover.1726034272.git.ojaswin@linux.ibm.com/ >> >> Reviewed-by Ritesh Harjani (IBM) <ritesh.list@gmail.com> >> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >> Suggested-by: Ojaswin Mujoo <ojaswin@linux.ibm.com> >> Signed-off-by: Nirjhar Roy <nirjhar@linux.ibm.com> > Looks ok, though you might want to change generic/366 to a higher number > because that's now taken. > > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> > > --D Okay. I will update the test sequence number in the next revision. --NR >> --- >> tests/generic/366 | 175 ++++++++++++++++++++++++++++++++++++++++++ >> tests/generic/366.out | 26 +++++++ >> 2 files changed, 201 insertions(+) >> create mode 100755 tests/generic/366 >> create mode 100644 tests/generic/366.out >> >> diff --git a/tests/generic/366 b/tests/generic/366 >> new file mode 100755 >> index 00000000..25d23f42 >> --- /dev/null >> +++ b/tests/generic/366 >> @@ -0,0 +1,175 @@ >> +#! /bin/bash >> +# SPDX-License-Identifier: GPL-2.0 >> +# Copyright (c) 2024 Nirjhar Roy (nirjhar@linux.ibm.com). All Rights Reserved. >> +# >> +# FS QA Test 366 >> +# >> +# This test verifies that extent allocation hint setting works correctly on >> +# files with no extents allocated and non-empty files which are truncated. >> +# It also checks that the extent hints setting fails with non-empty file >> +# i.e, with any file with allocated extents or delayed allocation. We also >> +# check if the extsize value and the xflag bit actually got reflected after >> +# setting/re-setting the extsize value. >> + >> +. ./common/config >> +. ./common/filter >> +. ./common/preamble >> + >> +_begin_fstest ioctl quick >> + >> +_fixed_by_kernel_commit "2a492ff66673 \ >> + xfs: Check for delayed allocations before setting \ >> + extsize" >> + >> +_require_scratch_extsize >> + >> +FILE_DATA_SIZE=1M >> + >> +get_default_extsize() >> +{ >> + if [ -z $1 ] || [ ! -d $1 ]; then >> + echo "Missing mount point argument for get_default_extsize" >> + exit 1 >> + fi >> + $XFS_IO_PROG -c "extsize" "$1" | sed 's/^\[\([0-9]\+\)\].*/\1/' >> +} >> + >> +filter_extsz() >> +{ >> + sed "s/\[$1\]/\[EXTSIZE\]/g" >> +} >> + >> +setup() >> +{ >> + _scratch_mkfs >> "$seqres.full" 2>&1 >> + _scratch_mount >> "$seqres.full" 2>&1 >> + BLKSZ=`_get_block_size $SCRATCH_MNT` >> + DEFAULT_EXTSIZE=`get_default_extsize $SCRATCH_MNT` >> + EXTSIZE=$(( BLKSZ*2 )) >> + # Make sure the new extsize is not the same as the default >> + # extsize so that we can observe it changing >> + [[ "$DEFAULT_EXTSIZE" -eq "$EXTSIZE" ]] && EXTSIZE=$(( BLKSZ*4 )) >> +} >> + >> +read_file_extsize() >> +{ >> + $XFS_IO_PROG -c "extsize" $1 | _filter_scratch | filter_extsz $2 >> +} >> + >> +check_extsz_and_xflag() >> +{ >> + local filename=$1 >> + local extsize=$2 >> + read_file_extsize $filename $extsize >> + _test_fsxattr_xflag $filename "extsize" && echo "e flag set" || \ >> + echo "e flag unset" >> +} >> + >> +check_extsz_xflag_across_remount() >> +{ >> + local filename=$1 >> + local extsize=$2 >> + _scratch_cycle_mount >> + check_extsz_and_xflag $filename $extsize >> +} >> + >> +# Extsize flag should be cleared when extsize is reset, so this function >> +# checks that this behavior is followed. >> +reset_extsz_and_recheck_extsz_xflag() >> +{ >> + local filename=$1 >> + echo "Re-setting extsize hint to 0" >> + $XFS_IO_PROG -c "extsize 0" $filename >> + check_extsz_xflag_across_remount $filename "0" >> +} >> + >> +check_extsz_xflag_before_and_after_reset() >> +{ >> + local filename=$1 >> + local extsize=$2 >> + check_extsz_xflag_across_remount $filename $extsize >> + reset_extsz_and_recheck_extsz_xflag $filename >> +} >> + >> +test_empty_file() >> +{ >> + echo "TEST: Set extsize on empty file" >> + local filename=$1 >> + $XFS_IO_PROG \ >> + -c "open -f $filename" \ >> + -c "extsize $EXTSIZE" \ >> + >> + check_extsz_xflag_before_and_after_reset $filename $EXTSIZE >> + echo >> +} >> + >> +test_data_delayed() >> +{ >> + echo "TEST: Set extsize on non-empty file with delayed allocation" >> + local filename=$1 >> + $XFS_IO_PROG \ >> + -c "open -f $filename" \ >> + -c "pwrite -q 0 $FILE_DATA_SIZE" \ >> + -c "extsize $EXTSIZE" | _filter_scratch >> + >> + echo "test for default extsize setting if any" >> + read_file_extsize $filename $DEFAULT_EXTSIZE >> + echo >> +} >> + >> +test_data_allocated() >> +{ >> + echo "TEST: Set extsize on non-empty file with allocated extents" >> + local filename=$1 >> + $XFS_IO_PROG \ >> + -c "open -f $filename" \ >> + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ >> + -c "extsize $EXTSIZE" | _filter_scratch >> + >> + echo "test for default extsize setting if any" >> + read_file_extsize $filename $DEFAULT_EXTSIZE >> + echo >> +} >> + >> +test_truncate_allocated() >> +{ >> + echo "TEST: Set extsize after truncating a file with allocated extents" >> + local filename=$1 >> + $XFS_IO_PROG \ >> + -c "open -f $filename" \ >> + -c "pwrite -qW 0 $FILE_DATA_SIZE" \ >> + -c "truncate 0" \ >> + -c "extsize $EXTSIZE" \ >> + >> + check_extsz_xflag_across_remount $filename $EXTSIZE >> + echo >> +} >> + >> +test_truncate_delayed() >> +{ >> + echo "TEST: Set extsize after truncating a file with delayed allocation" >> + local filename=$1 >> + $XFS_IO_PROG \ >> + -c "open -f $filename" \ >> + -c "pwrite -q 0 $FILE_DATA_SIZE" \ >> + -c "truncate 0" \ >> + -c "extsize $EXTSIZE" \ >> + >> + check_extsz_xflag_across_remount $filename $EXTSIZE >> + echo >> +} >> + >> +setup >> +echo -e "EXTSIZE = $EXTSIZE DEFAULT_EXTSIZE = $DEFAULT_EXTSIZE \ >> + BLOCKSIZE = $BLKSZ\n" >> "$seqres.full" >> + >> +NEW_FILE_NAME_PREFIX=$SCRATCH_MNT/new-file- >> + >> +test_empty_file "$NEW_FILE_NAME_PREFIX"00 >> +test_data_delayed "$NEW_FILE_NAME_PREFIX"01 >> +test_data_allocated "$NEW_FILE_NAME_PREFIX"02 >> +test_truncate_allocated "$NEW_FILE_NAME_PREFIX"03 >> +test_truncate_delayed "$NEW_FILE_NAME_PREFIX"04 >> + >> +status=0 >> +exit >> diff --git a/tests/generic/366.out b/tests/generic/366.out >> new file mode 100644 >> index 00000000..cdd2f5fa >> --- /dev/null >> +++ b/tests/generic/366.out >> @@ -0,0 +1,26 @@ >> +QA output created by 366 >> +TEST: Set extsize on empty file >> +[EXTSIZE] SCRATCH_MNT/new-file-00 >> +e flag set >> +Re-setting extsize hint to 0 >> +[EXTSIZE] SCRATCH_MNT/new-file-00 >> +e flag unset >> + >> +TEST: Set extsize on non-empty file with delayed allocation >> +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-01: Invalid argument >> +test for default extsize setting if any >> +[EXTSIZE] SCRATCH_MNT/new-file-01 >> + >> +TEST: Set extsize on non-empty file with allocated extents >> +xfs_io: FS_IOC_FSSETXATTR SCRATCH_MNT/new-file-02: Invalid argument >> +test for default extsize setting if any >> +[EXTSIZE] SCRATCH_MNT/new-file-02 >> + >> +TEST: Set extsize after truncating a file with allocated extents >> +[EXTSIZE] SCRATCH_MNT/new-file-03 >> +e flag set >> + >> +TEST: Set extsize after truncating a file with delayed allocation >> +[EXTSIZE] SCRATCH_MNT/new-file-04 >> +e flag set >> + >> -- >> 2.43.5 >> >> -- --- Nirjhar Roy Linux Kernel Developer IBM, Bangalore ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-11-22 19:06 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-11-21 5:09 [PATCH v3 0/3] Addition of new tests for extsize hints Nirjhar Roy 2024-11-21 5:09 ` [PATCH v3 1/3] common/rc,xfs/207: Add a common helper function to check xflag bits Nirjhar Roy 2024-11-21 17:20 ` Darrick J. Wong 2024-11-21 18:30 ` Nirjhar Roy 2024-11-21 5:09 ` [PATCH v3 2/3] common/rc: Add a new _require_scratch_extsize helper function Nirjhar Roy 2024-11-21 7:53 ` Ritesh Harjani 2024-11-21 18:33 ` Nirjhar Roy 2024-11-21 18:52 ` Ritesh Harjani 2024-11-22 16:04 ` Darrick J. Wong 2024-11-22 18:07 ` Nirjhar Roy 2024-11-22 18:44 ` Darrick J. Wong 2024-11-22 19:06 ` Nirjhar Roy 2024-11-21 5:09 ` [PATCH v3 3/3] generic: Addition of new tests for extsize hints Nirjhar Roy 2024-11-21 17:27 ` Darrick J. Wong 2024-11-21 18:34 ` Nirjhar Roy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).