On Wed, Sep 13, 2023 at 10:44:50AM +0200, Andrey Albershteyn wrote:
> On 2023-09-12 07:56:25, Darrick J. Wong wrote:
> > On Tue, Sep 12, 2023 at 08:42:13PM +0800, Zorro Lang wrote:
> > > On Mon, Sep 11, 2023 at 10:06:17PM +0200, Andrey Albershteyn wrote:
> > > > Currently, _get_max_file_size finds max file size on $TEST_DIR.
> > > > The tests/generic/692 uses this function to detect file size and
> > > > then tries to create a file on $SCRATCH_MNT.
> > > >
> > > > This works fine when test and scratch filesystems have the same
> > > > block size. However, it will fail if they differ, for example, TEST
> > > > is 4k and SCRATCH is 1k (file will be too big).
> > >
> > > I thought we generally use same mkfs options on TEST_DEV and SCRATCH_DEV.
> > > If you'd like to test 1k blocksize ext4, shouldn't you make 1k blocksize
> > > ext4 on TEST_DEV before testing.
> >
> > Agreed, TEST_DEV (AFAIK) is supposed to have the same options as
> > SCRATCH_MNT.
>
> Make sense but I didn't know that this is required, would it make
> sense to add a check for at least block size? I was running -s
> ext4_1k -s ext4_4k without recreating the TEST_DEV and that wasn't
> obvious where the problem is as test is using SCRATCH.
I remembered there's a global parameter named "RECREATE_TEST_DEV", if it's
"true", the TEST_DEV will be recreated after a section testing done. You can
check if it works :)
>
> >
> > That said, I support adding the flexibility to supply a mount point,
> > since that would be generally useful given the number of tests that add
> > things to MKFS_OPTIONS...
> >
> > > Thanks,
> > > Zorro
> > >
> > > >
> > > > Found this by running generic/692 on ext4 with -b 1024.
> > > >
> > > > Make _get_max_file_size accept mount point on which to detect max
> > > > file size.
> > > >
> > > > Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> > > > ---
> > > > common/rc | 8 +++++++-
> > > > tests/generic/299 | 2 +-
> > > > tests/generic/485 | 2 +-
> > > > tests/generic/692 | 2 +-
> > > > 4 files changed, 10 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/common/rc b/common/rc
> > > > index 68d2ad04..d17ec73a 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -4809,7 +4809,13 @@ _require_scratch_feature()
> > > > # be UINT32_MAX * block_size, but other filesystems may allow up to LLONG_MAX.
> > > > _get_max_file_size()
> > > > {
> > > > - local testfile=$TEST_DIR/maxfilesize.$seq
> > > > + if [ -z $1 ] || [ ! -d $1 ]; then
> > > > + echo "Missing mount point argument for _get_max_file_size"
> > > > + exit 1
> > > > + fi
> > > > +
> > > > + local mnt=$1
> > > > + local testfile=$mnt/maxfilesize.$seq
> > > > local l=0
> > > > local r=9223372036854775807 # LLONG_MAX
> > > >
> > > > diff --git a/tests/generic/299 b/tests/generic/299
> > > > index d8ecff53..0cd12202 100755
> > > > --- a/tests/generic/299
> > > > +++ b/tests/generic/299
> > > > @@ -30,7 +30,7 @@ NUM_JOBS=$((4*LOAD_FACTOR))
> > > > BLK_DEV_SIZE=`blockdev --getsz $SCRATCH_DEV`
> > > > FILE_SIZE=$((BLK_DEV_SIZE * 512))
> > > >
> > > > -max_file_size=$(_get_max_file_size)
> > > > +max_file_size=$(_get_max_file_size $TEST_DIR)
> > > > if [ $max_file_size -lt $FILE_SIZE ]; then
> > > > FILE_SIZE=$max_file_size
> > > > fi
> > > > diff --git a/tests/generic/485 b/tests/generic/485
> > > > index 3f7749ff..8bab450b 100755
> > > > --- a/tests/generic/485
> > > > +++ b/tests/generic/485
> > > > @@ -30,7 +30,7 @@ _require_xfs_io_command "finsert"
> > > > _require_xfs_io_command "truncate"
> > > >
> > > > block_size=$(_get_file_block_size $TEST_DIR)
> > > > -max_file_size=$(_get_max_file_size)
> > > > +max_file_size=$(_get_max_file_size $TEST_DIR)
> > > > max_blocks=$((max_file_size / block_size))
> > > > testfile=$TEST_DIR/testfile.$seq
> > > >
> > > > diff --git a/tests/generic/692 b/tests/generic/692
> > > > index 95f6ec04..3fb8ac01 100755
> > > > --- a/tests/generic/692
> > > > +++ b/tests/generic/692
> > > > @@ -40,7 +40,7 @@ _scratch_mount
> > > >
> > > > fsv_file=$SCRATCH_MNT/file.fsv
> > > >
> > > > -max_sz=$(_get_max_file_size)
> > > > +max_sz=$(_get_max_file_size $SCRATCH_MNT)
> >
> > ...LGTM, so
> > Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
>
> Thanks!
>
> > --D
> >
> > > > _fsv_scratch_begin_subtest "way too big: fail on first merkle block"
> > > > truncate -s $max_sz $fsv_file
> > > > _fsv_enable $fsv_file |& _filter_scratch
> > > > --
> > > > 2.40.1
> > > >
> > >
> >
>
> --
> - Andrey
>
