On Mon, Nov 07, 2022 at 08:42:17AM +0000, Filipe Manana wrote:
> On Mon, Nov 7, 2022 at 3:36 AM Zorro Lang <zlang@xxxxxxxxxx> wrote:
> >
> > On Mon, Nov 07, 2022 at 07:53:48AM +0800, Qu Wenruo wrote:
> > > There is a regression in v6.1-rc kernel, which will prevent btrfs scrub
> > > from detecting corruption (thus no repair either).
> > >
> > > The regression is caused by commit 786672e9e1a3 ("btrfs: scrub: use
> > > larger block size for data extent scrub").
> > >
> > > The new test case will:
> > >
> > > - Create a data extent with 2 sectors
> > > - Corrupt the second sector of above data extent
> > > - Scrub to make sure we detect the corruption
> > >
> > > Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> > > ---
> > > tests/btrfs/278 | 66 +++++++++++++++++++++++++++++++++++++++++++++
> > > tests/btrfs/278.out | 2 ++
> > > 2 files changed, 68 insertions(+)
> > > create mode 100755 tests/btrfs/278
> > > create mode 100644 tests/btrfs/278.out
> > >
> > > diff --git a/tests/btrfs/278 b/tests/btrfs/278
> > > new file mode 100755
> > > index 00000000..ebbf207a
> > > --- /dev/null
> > > +++ b/tests/btrfs/278
> > > @@ -0,0 +1,66 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (C) 2022 SUSE Linux Products GmbH. All Rights Reserved.
> > > +#
> > > +# FS QA Test 278
> > > +#
> > > +# A regression test for offending commit 786672e9e1a3 ("btrfs: scrub: use
> > > +# larger block size for data extent scrub"), which makes btrfs scrub unable
> > > +# to detect corruption if it's not the first sector of an data extent.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick scrub
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +. ./common/btrfs
> > > +
> > > +# real QA test starts here
> > > +
> > > +# Modify as appropriate.
> > > +_supported_fs btrfs
> > > +
> > > +# Need to use 4K as sector size
> > > +_require_btrfs_support_sectorsize 4096
> > > +_require_scratch
> >
> > Hi Darrick,
> >
> > I noticed that you created some scrub helpers in common/fuzzy:
> > # Do we have an online scrub program?
> > _require_scrub() {
> > case "${FSTYP}" in
> > "xfs")
> > test -x "$XFS_SCRUB_PROG" || _notrun "xfs_scrub not found"
> > ;;
> > *)
> > _notrun "No online scrub program for ${FSTYP}."
> > ;;
> > esac
> > }
> >
> > # Scrub the scratch filesystem metadata (online)
> > _scratch_scrub() {
> > case "${FSTYP}" in
> > "xfs")
> > $XFS_SCRUB_PROG -d -T -v "$@" $SCRATCH_MNT
> > ;;
> > *)
> > _fail "No online scrub program for ${FSTYP}."
> > ;;
> > esac
> > }
> >
> > and common/xfs:
> > _supports_xfs_scrub()
> >
> > (PS: How about change _require_scrub to _require_scrub_command, and then calls
> > _supports_xfs_scrub in _require_scrub to check kernel part? Or combine kernel
> > and userspace checking all into _require_scrub? )
> >
> > From the code logic, they're only support xfs now. But we can see btrfs support
> > scrub too. Did you plan to make them to be common helpers, or just for xfs fuzzy
> > test inside?
> >
> > Hi btrfs folks, do you think if btrfs need _require_scrub and _scratch_scrub?
>
> I don't think that provides any value for btrfs.
>
> The scrub feature has existed for well over a decade, and I don't
> think anyone is running fstests with kernels and btrfs-progs versions
> that don't have scrub.
> Even SLE11-SP2+ kernels (first SUSE enterprise distros supporting
> btrfs) have scrub...
>
> Plus I don't recall ever anyone complaining that fstests failed
> because the underlying kernel or btrfs-progs had no support for scrub.
Sure, thanks for your feedback! I'll think/talk about if we should change
that _require_scrub and _scratch_scrub to be more common helpers for fs which
has supported or will support scrub feature. I'll cc btrfs-list if I touch
btrfs cases :)
Thanks,
Zorro
>
>
> >
> > Thanks,
> > Zorro
> >
> > > +
> > > +_scratch_mkfs >> $seqres.full
> > > +_scratch_mount
> > > +
> > > +# Create a data extent with 2 sectors
> > > +$XFS_IO_PROG -fc "pwrite -S 0xff 0 8k" $SCRATCH_MNT/foobar >> $seqres.full
> > > +sync
> > > +
> > > +first_logical=$(_btrfs_get_first_logical $SCRATCH_MNT/foobar)
> > > +echo "logical of the first sector: $first_logical" >> $seqres.full
> > > +
> > > +second_logical=$(( $first_logical + 4096 ))
> > > +echo "logical of the second sector: $second_logical" >> $seqres.full
> > > +
> > > +second_physical=$(_btrfs_get_physical $second_logical 1)
> > > +echo "physical of the second sector: $second_physical" >> $seqres.full
> > > +
> > > +second_dev=$(_btrfs_get_device_path $second_logical 1)
> > > +echo "device of the second sector: $second_dev" >> $seqres.full
> > > +
> > > +_scratch_unmount
> > > +
> > > +# Corrupt the second sector of the data extent.
> > > +$XFS_IO_PROG -c "pwrite -S 0x00 $second_physical 4k" $second_dev >> $seqres.full
> > > +_scratch_mount
> > > +
> > > +# Redirect stderr and stdout, as if btrfs detected the unrepairable corruption,
> > > +# it will output an error message.
> > > +$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT &> $tmp.output
> > > +cat $tmp.output >> $seqres.full
> > > +_scratch_unmount
> > > +
> > > +if ! grep -q "csum=1" $tmp.output; then
> > > + echo "Scrub failed to detect corruption"
> > > +fi
> > > +
> > > +echo "Silence is golden"
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/btrfs/278.out b/tests/btrfs/278.out
> > > new file mode 100644
> > > index 00000000..b4c4a95d
> > > --- /dev/null
> > > +++ b/tests/btrfs/278.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 278
> > > +Silence is golden
> > > --
> > > 2.38.0
> > >
> >
>
