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

[Zorro Lang <zlang@xxxxxxxxxx>]

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?

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
>