From: Zorro Lang <zlang@redhat.com>
To: Andrey Albershteyn <aalbersh@redhat.com>
Cc: "Darrick J. Wong" <djwong@kernel.org>,
fstests@vger.kernel.org, fsverity@lists.linux.dev
Subject: Re: [PATCH] common/rc: make _get_max_file_size find file size on mount point
Date: Wed, 13 Sep 2023 21:57:19 +0800 [thread overview]
Message-ID: <20230913135719.yrv7kzulfnom7ahw@zlang-mailbox> (raw)
In-Reply-To: <ihs7k3gvnoa5bii5ugng6k3xlr3gerdtrkmkqkxq7yekmnydhf@7v53tjceewuh>
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@redhat.com>
> > > > ---
> > > > 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@kernel.org>
> >
>
> 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
>
prev parent reply other threads:[~2023-09-13 13:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-11 20:06 [PATCH] common/rc: make _get_max_file_size find file size on mount point Andrey Albershteyn
2023-09-12 12:42 ` Zorro Lang
2023-09-12 14:56 ` Darrick J. Wong
2023-09-13 8:44 ` Andrey Albershteyn
2023-09-13 13:57 ` Zorro Lang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230913135719.yrv7kzulfnom7ahw@zlang-mailbox \
--to=zlang@redhat.com \
--cc=aalbersh@redhat.com \
--cc=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=fsverity@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).