Re: [PATCH v2] fstests: btrfs: test setting compression via xattr on nodatacow files

[Filipe Manana <fdmanana@kernel.org>]

On Mon, Apr 25, 2022 at 12:22:26PM +0800, Chung-Chiang Cheng wrote:
> Compression and nodatacow are mutually exclusive. Besides ioctl, there
> is another way to setting compression via xattrs, and shouldn't produce
> invalid combinations.
> 
> Signed-off-by: Chung-Chiang Cheng <cccheng@synology.com>
> ---
>  tests/btrfs/264     | 90 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/264.out | 11 ++++++
>  2 files changed, 101 insertions(+)
>  create mode 100755 tests/btrfs/264
>  create mode 100644 tests/btrfs/264.out
> 
> diff --git a/tests/btrfs/264 b/tests/btrfs/264
> new file mode 100755
> index 00000000..bb11116c
> --- /dev/null
> +++ b/tests/btrfs/264
> @@ -0,0 +1,90 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 Synology Inc. All Rights Reserved.
> +#
> +# FS QA Test No. 264
> +#
> +# Compression and nodatacow are mutually exclusive. Besides ioctl, there
> +# is another way to setting compression via xattrs, and shouldn't produce
> +# invalid combinations.
> +#
> +# To prevent mix any compression-related options with nodatacow, FS_NOCOMP_FL
> +# is also rejected by ioctl as well as FS_COMPR_FL on nodatacow files. To
> +# align with it, no and none are also unacceptable in this test.
> +#
> +# The regression is fixed by a patch with the following subject:
> +#   btrfs: do not allow compression on nodatacow files
> +#
> +. ./common/preamble
> +_begin_fstest auto quick compress attr
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/attr
> +
> +# real QA test starts here
> +
> +_supported_fs btrfs
> +_require_scratch
> +_require_attrs

_require_chattr C

Is still needed, during the previous review I meant to remove only
the "_require_chattr c".

Otherwise the test looks good.

Probably the _require_chattr C can be added by the maintainer when the
patch is picked, so no need to send a v3 just for that.

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks for doing this.

> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +
> +set_compression() # $1: filename, $2: alg
> +{
> +	[ -f "$1" ] || return
> +	$SETFATTR_PROG -n "btrfs.compression" -v "$2" "$1" |& _filter_scratch
> +}
> +
> +# FS_NOCOMP_FL bit isn't recognized by chattr/lsattr before e2fsprogs 1.46.2
> +# In order to make this test available with an older version, we wrap the output
> +# of lsattr to distinguish FS_COMP_FL and FS_NOCOMP_FL
> +check_compression() # $1: filename
> +{
> +	$LSATTR_PROG -l "$1" | grep -q "Compression_Requested"
> +
> +	if [ $? -eq 0 ]; then
> +		echo "$1: Compression is set" | _filter_scratch
> +	else
> +		echo "$1: Compression is not set" | _filter_scratch
> +	fi
> +}
> +
> +#
> +# DATACOW
> +#
> +test_file="$SCRATCH_MNT/foo"
> +touch "$test_file"
> +$CHATTR_PROG -C "$test_file"
> +
> +set_compression "$test_file" zlib
> +check_compression "$test_file"
> +set_compression "$test_file" no
> +check_compression "$test_file"
> +set_compression "$test_file" lzo
> +check_compression "$test_file"
> +set_compression "$test_file" none
> +check_compression "$test_file"
> +set_compression "$test_file" zstd
> +check_compression "$test_file"
> +
> +#
> +# NODATACOW
> +#
> +test_file="$SCRATCH_MNT/bar"
> +touch "$test_file"
> +$CHATTR_PROG +C "$test_file"
> +
> +# all valid compression type are not allowed on nodatacow files
> +set_compression "$test_file" zlib
> +set_compression "$test_file" lzo
> +set_compression "$test_file" zstd
> +
> +# no/none are also not allowed on nodatacow files
> +set_compression "$test_file" no
> +set_compression "$test_file" none
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/264.out b/tests/btrfs/264.out
> new file mode 100644
> index 00000000..7dd36054
> --- /dev/null
> +++ b/tests/btrfs/264.out
> @@ -0,0 +1,11 @@
> +QA output created by 264
> +SCRATCH_MNT/foo: Compression is set
> +SCRATCH_MNT/foo: Compression is not set
> +SCRATCH_MNT/foo: Compression is set
> +SCRATCH_MNT/foo: Compression is not set
> +SCRATCH_MNT/foo: Compression is set
> +setfattr: SCRATCH_MNT/bar: Invalid argument
> +setfattr: SCRATCH_MNT/bar: Invalid argument
> +setfattr: SCRATCH_MNT/bar: Invalid argument
> +setfattr: SCRATCH_MNT/bar: Invalid argument
> +setfattr: SCRATCH_MNT/bar: Invalid argument
> -- 
> 2.25.1
>