Re: [PATCH 2/3] btrfs/053: fix test failure when running with btrfs-progs v6.0+

From: Filipe Manana <fdmanana@kernel.org>
To: Zorro Lang <zlang@redhat.com>
Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 2/3] btrfs/053: fix test failure when running with btrfs-progs v6.0+
Date: Wed, 9 Nov 2022 16:43:57 +0000	[thread overview]
Message-ID: <CAL3q7H6W_TFq+9PUd62+EUVJortdpAL8VuEtLEfhM91pRcb41Q@mail.gmail.com> (raw)
In-Reply-To: <20221109155505.yki6xwqk4p4mhky6@zlang-mailbox>

On Wed, Nov 9, 2022 at 3:55 PM Zorro Lang <zlang@redhat.com> wrote:
>
> On Wed, Nov 09, 2022 at 11:43:35AM +0000, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > In btrfs-progs v6.0 the --leafsize (-l) command line option was removed,
> > so btrfs/053 always fails with v6.0+.
> >
> > The change was introduced by the following btrfs-progs commit:
> >
> >   f7a768d62498 ("btrfs-progs: mkfs: remove support for option --leafsize")
> >
> > Change the test to use --nodesize (-n) instead, since it exists in both
> > old and new btrfs-progs versions.
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >  tests/btrfs/053 | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/btrfs/053 b/tests/btrfs/053
> > index fbd2e7d9..c0446257 100755
> > --- a/tests/btrfs/053
> > +++ b/tests/btrfs/053
> > @@ -44,7 +44,7 @@ send_files_dir=$TEST_DIR/btrfs-test-$seq
> >  rm -fr $send_files_dir
> >  mkdir $send_files_dir
> >
> > -_scratch_mkfs "-l $leaf_size" >/dev/null 2>&1
> > +_scratch_mkfs "--nodesize $leaf_size" >/dev/null 2>&1
>
> We you said this case starts to fail on btrfs-progs v6.0, I'm wondering how it
> fail (I don't doubt it fails), at least not fails at here right?

It fails during mount in case no previous mkfs on the scratch device happened.
First time after booting a vm and without running any other test case.

>
> Actually I recommend mkfs output to .full file at here, especially when you use
> specified mkfs options to _scratch_mkfs helper. That really might fail, and
> the case might keep running (with old fs on SCRATCH_DEV), and we hard to notice
> that if no message output and no return value checking.
>
> The _scratch_mkfs doesn't _fail if it fails, it just return nonzero and output
> error message. So generally I recommend writting likes this (or other proper way
> which can detect mkfs failure):
>
>   _scratch_mkfs "--nodesize $leaf_size" >>$seqres.full 2>&1 || _fail "mkfs failed"

Yes, I remember some years ago a thread discussing silent mkfs failures being
ignored and getting unexpected results or tests passing when they shouldn't.

I'll add that, thanks.

>
> especially if there's specified mkfs options.
>
> Thanks,
> Zorro
>
> >  _scratch_mount
> >
> >  echo "hello world" > $SCRATCH_MNT/foobar
> > @@ -72,7 +72,7 @@ _run_btrfs_util_prog send -p $SCRATCH_MNT/mysnap1 -f $send_files_dir/2.snap \
> >  _scratch_unmount
> >  _check_scratch_fs
> >
> > -_scratch_mkfs "-l $leaf_size" >/dev/null 2>&1
> > +_scratch_mkfs "--nodesize $leaf_size" >/dev/null 2>&1
> >  _scratch_mount
> >
> >  _run_btrfs_util_prog receive -f $send_files_dir/1.snap $SCRATCH_MNT
> > --
> > 2.35.1
> >
>