* [PATCH] fstests: btrfs/048: amend property validation cases
@ 2019-04-03 16:54 Anand Jain
2019-04-03 17:04 ` [PATCH v2] " Anand Jain
0 siblings, 1 reply; 8+ messages in thread
From: Anand Jain @ 2019-04-03 16:54 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs
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>
---
tests/btrfs/048 | 25 +++++++++++++++++++++++++
tests/btrfs/048.out | 16 ++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/tests/btrfs/048 b/tests/btrfs/048
index 588219579cc6..657634f7cc3e 100755
--- a/tests/btrfs/048
+++ b/tests/btrfs/048
@@ -6,6 +6,11 @@
#
# Btrfs properties test. The btrfs properties feature was introduced in the
# linux kernel 3.14.
+# Fails without the kernel patches:
+# btrfs: fix property validate fail should not increment generation
+# btrfs: open code btrfs_set_prop in inherit_prop
+# btrfs: fix vanished compression property after failed set
+# btrfs: fix zstd compression parameter
#
seq=`basename $0`
seqres=$RESULT_DIR/$seq
@@ -34,6 +39,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 +209,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
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2] fstests: btrfs/048: amend property validation cases
2019-04-03 16:54 [PATCH] fstests: btrfs/048: amend property validation cases Anand Jain
@ 2019-04-03 17:04 ` Anand Jain
2019-04-05 13:21 ` Nikolay Borisov
0 siblings, 1 reply; 8+ messages in thread
From: Anand Jain @ 2019-04-03 17:04 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs
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>
---
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.
+# 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
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] fstests: btrfs/048: amend property validation cases
2019-04-03 17:04 ` [PATCH v2] " Anand Jain
@ 2019-04-05 13:21 ` Nikolay Borisov
2019-04-06 12:02 ` Eryu Guan
0 siblings, 1 reply; 8+ messages in thread
From: Nikolay Borisov @ 2019-04-05 13:21 UTC (permalink / raw)
To: Anand Jain, fstests; +Cc: linux-btrfs
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>
> ---
> 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.
> +# 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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] fstests: btrfs/048: amend property validation cases
2019-04-05 13:21 ` Nikolay Borisov
@ 2019-04-06 12:02 ` Eryu Guan
0 siblings, 0 replies; 8+ messages in thread
From: Eryu Guan @ 2019-04-06 12:02 UTC (permalink / raw)
To: Nikolay Borisov, Anand Jain; +Cc: Anand Jain, fstests, linux-btrfs
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.
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.
> > +# 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
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] fstests: btrfs/048: amend property validation cases
@ 2019-04-06 12:02 ` Eryu Guan
0 siblings, 0 replies; 8+ messages in thread
From: Eryu Guan @ 2019-04-06 12:02 UTC (permalink / raw)
To: Nikolay Borisov, Anand Jain; +Cc: fstests, linux-btrfs
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.
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.
> > +# 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
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] fstests: btrfs/048: amend property validation cases
2019-04-06 12:02 ` Eryu Guan
(?)
@ 2019-04-07 11:54 ` Anand Jain
2019-04-07 12:45 ` Nikolay Borisov
-1 siblings, 1 reply; 8+ messages in thread
From: Anand Jain @ 2019-04-07 11:54 UTC (permalink / raw)
To: Eryu Guan, Nikolay Borisov; +Cc: fstests, linux-btrfs
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
>>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] fstests: btrfs/048: amend property validation cases
2019-04-07 11:54 ` Anand Jain
@ 2019-04-07 12:45 ` Nikolay Borisov
2019-04-14 2:47 ` Eryu Guan
0 siblings, 1 reply; 8+ messages in thread
From: Nikolay Borisov @ 2019-04-07 12:45 UTC (permalink / raw)
To: Anand Jain, Eryu Guan; +Cc: fstests, linux-btrfs
On 7.04.19 г. 14:54 ч., Anand Jain wrote:
> 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?
I agree with Anand here, this is an extension to an existing test, which
covers specific feature. IMO it's not good to always introduce new tests
because every invocation of a test comes with an overhead of spawning
processes and whatnot. THis is not a problem for 10 tests, but currently
for btrfs we execute around 600 tests each one "wasting" some cycles to
spawn bash processes to execute the actual test.
>
> 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
>>>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] fstests: btrfs/048: amend property validation cases
2019-04-07 12:45 ` Nikolay Borisov
@ 2019-04-14 2:47 ` Eryu Guan
0 siblings, 0 replies; 8+ messages in thread
From: Eryu Guan @ 2019-04-14 2:47 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: Anand Jain, fstests, linux-btrfs
On Sun, Apr 07, 2019 at 03:45:36PM +0300, Nikolay Borisov wrote:
>
>
> On 7.04.19 г. 14:54 ч., Anand Jain wrote:
> > 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?
>
> I agree with Anand here, this is an extension to an existing test, which
> covers specific feature. IMO it's not good to always introduce new tests
> because every invocation of a test comes with an overhead of spawning
> processes and whatnot. THis is not a problem for 10 tests, but currently
> for btrfs we execute around 600 tests each one "wasting" some cycles to
> spawn bash processes to execute the actual test.
Fair enough. We're having more tests now, and we do consider "merging"
some tests into one case.
Thanks,
Eryu
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-04-14 2:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-03 16:54 [PATCH] fstests: btrfs/048: amend property validation cases Anand Jain
2019-04-03 17:04 ` [PATCH v2] " Anand Jain
2019-04-05 13:21 ` Nikolay Borisov
2019-04-06 12:02 ` Eryu Guan
2019-04-06 12:02 ` Eryu Guan
2019-04-07 11:54 ` Anand Jain
2019-04-07 12:45 ` Nikolay Borisov
2019-04-14 2:47 ` Eryu Guan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.