Re: [PATCH] fstests: btrfs: add a regression test case to make sure scrub can detect errors

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



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.


>
> 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
> >
>



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux