| Message ID | 4412cece5c3f2175fa076a3b29fe6d0bb4c43a6e.1732126365.git.nirjhar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Addition of new tests for extsize hints | expand |
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
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 <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
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"