Re: [PATCH v2] fstests: btrfs/048: amend property validation cases

[Anand Jain <anand.jain@oracle.com>]

On 6/4/19 8:02 pm, Eryu Guan wrote:
> On Fri, Apr 05, 2019 at 04:21:10PM +0300, Nikolay Borisov wrote:
>>
>>
>> On 3.04.19 г. 20:04 ч., Anand Jain wrote:
>>> Add more property validation cases which are fixed by the patches [1]
>>>   [1]
>>>    btrfs: fix vanished compression property after failed set
>>>    btrfs: fix zstd compression parameter
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>
>> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
> Thanks for the test and the review!
> 
> But this looks like a targeted regression test that may fail an existing
> test. It's better to write a new test for this.

Regression is only when fstests is upgraded. This
test case mentions the prerequisite kernel patches [1].
So that should suffice the concern?

Thanks, Anand

> 
> Thanks,
> Eryu
> 
>>
>>> ---
>>> v2: correct kernel patch titles in the test header and change log
>>>
>>>   tests/btrfs/048     | 23 +++++++++++++++++++++++
>>>   tests/btrfs/048.out | 16 ++++++++++++++++
>>>   2 files changed, 39 insertions(+)
>>>
>>> diff --git a/tests/btrfs/048 b/tests/btrfs/048
>>> index 588219579cc6..f6de0b8ca8b1 100755
>>> --- a/tests/btrfs/048
>>> +++ b/tests/btrfs/048
>>> @@ -6,6 +6,9 @@
>>>   #
>>>   # Btrfs properties test. The btrfs properties feature was introduced in the
>>>   # linux kernel 3.14.


[1]

>>> +# Fails without the kernel patches:
>>> +#  btrfs: fix vanished compression property after failed set
>>> +#  btrfs: fix zstd compression parameter
>>>   #
>>>   seq=`basename $0`
>>>   seqres=$RESULT_DIR/$seq
>>> @@ -34,6 +37,7 @@ _supported_os Linux
>>>   _require_test
>>>   _require_scratch
>>>   _require_btrfs_command "property"
>>> +_require_btrfs_command inspect-internal dump-super
>>>   
>>>   send_files_dir=$TEST_DIR/btrfs-test-$seq
>>>   
>>> @@ -203,5 +207,24 @@ $BTRFS_UTIL_PROG property get $SCRATCH_MNT/sv1 compression
>>>   touch $SCRATCH_MNT/sv1/file2
>>>   $BTRFS_UTIL_PROG property get $SCRATCH_MNT/sv1/file2 compression
>>>   
>>> +echo -e "\nTesting argument validation, should fail"
>>> +$BTRFS_UTIL_PROG property set $SCRATCH_MNT compression 'lz' | _filter_scratch
>>> +echo "***"
>>> +$BTRFS_UTIL_PROG property set $SCRATCH_MNT compression 'zli' | _filter_scratch
>>> +echo "***"
>>> +$BTRFS_UTIL_PROG property set $SCRATCH_MNT compression 'zst' | _filter_scratch
>>> +
>>> +echo -e "\nTesting if property is persistent across failed validation"
>>> +$BTRFS_UTIL_PROG property set $SCRATCH_MNT compression 'lzo'
>>> +$BTRFS_UTIL_PROG property set $SCRATCH_MNT compression 'zli' | _filter_scratch
>>> +$BTRFS_UTIL_PROG property get $SCRATCH_MNT compression
>>> +
>>> +echo -e "\nTesting generation is unchanged after failed validation"
>>> +$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
>>> +$BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV | grep '^generation'
>>> +$BTRFS_UTIL_PROG property set $SCRATCH_MNT compression 'lz' | _filter_scratch
>>> +$BTRFS_UTIL_PROG filesystem sync $SCRATCH_MNT
>>> +$BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV | grep '^generation'
>>> +
>>>   status=0
>>>   exit
>>> diff --git a/tests/btrfs/048.out b/tests/btrfs/048.out
>>> index 3e4e3d28950a..00f39bc01227 100644
>>> --- a/tests/btrfs/048.out
>>> +++ b/tests/btrfs/048.out
>>> @@ -76,3 +76,19 @@ compression=zlib
>>>   Testing subvolume property inheritance
>>>   compression=lzo
>>>   compression=lzo
>>> +
>>> +Testing argument validation, should fail
>>> +ERROR: failed to set compression for /mnt/scratch: Invalid argument
>>> +***
>>> +ERROR: failed to set compression for /mnt/scratch: Invalid argument
>>> +***
>>> +ERROR: failed to set compression for /mnt/scratch: Invalid argument
>>> +
>>> +Testing if property is persistent across failed validation
>>> +ERROR: failed to set compression for /mnt/scratch: Invalid argument
>>> +compression=lzo
>>> +
>>> +Testing generation is unchanged after failed validation
>>> +generation		7
>>> +ERROR: failed to set compression for /mnt/scratch: Invalid argument
>>> +generation		7
>>>