* [PATCH] fstests: btrfs/048: amend property validation cases
2019-04-02 10:07 [PATCH 0/4 RESEND] btrfs: fix property bugs Anand Jain
@ 2019-04-02 10:07 ` Anand Jain
2019-04-02 12:51 ` Nikolay Borisov
0 siblings, 1 reply; 10+ messages in thread
From: Anand Jain @ 2019-04-02 10:07 UTC (permalink / raw)
To: linux-btrfs
Add more property validation cases which are fixed by the patches [1]
[1]
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
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
(As this fstest patch depends on the kernel patches,
this patch isn't sent to fstests mailing list yet).
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] 10+ messages in thread
* Re: [PATCH] fstests: btrfs/048: amend property validation cases
2019-04-02 10:07 ` [PATCH] fstests: btrfs/048: amend property validation cases Anand Jain
@ 2019-04-02 12:51 ` Nikolay Borisov
2019-04-03 0:32 ` Anand Jain
0 siblings, 1 reply; 10+ messages in thread
From: Nikolay Borisov @ 2019-04-02 12:51 UTC (permalink / raw)
To: Anand Jain, linux-btrfs
On 2.04.19 г. 13:07 ч., Anand Jain wrote:
> Add more property validation cases which are fixed by the patches [1]
> [1]
> 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
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> (As this fstest patch depends on the kernel patches,
> this patch isn't sent to fstests mailing list yet).
>
Doesn't matter, it's fine to have a test merged before respective kernel
patches have landed. In fact there is one btrfs test (can't remember
which one) authored by you which is failing, yet the respective kernel
patches have never been sent to the mailing list.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fstests: btrfs/048: amend property validation cases
2019-04-02 12:51 ` Nikolay Borisov
@ 2019-04-03 0:32 ` Anand Jain
0 siblings, 0 replies; 10+ messages in thread
From: Anand Jain @ 2019-04-03 0:32 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
On 2/4/19 8:51 PM, Nikolay Borisov wrote:
>
>
> On 2.04.19 г. 13:07 ч., Anand Jain wrote:
>> Add more property validation cases which are fixed by the patches [1]
>> [1]
>> 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
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> (As this fstest patch depends on the kernel patches,
>> this patch isn't sent to fstests mailing list yet).
>>
>
> Doesn't matter, it's fine to have a test merged before respective kernel
> patches have landed.
Ok.
> In fact there is one btrfs test (can't remember
> which one) authored by you which is failing, yet the respective kernel
> patches have never been sent to the mailing list.
fstests is also a good place to record known issues. Otherwise we can
create a group of known-issues with in fstests. What do you think?
Currently we have these two fstests failing because its fix is not
yet in the kernel.
btrfs/172:
Exposed bug: Punch hole fails at ENOSPC only for the misaligned offset.
Because punch hole has to zero a partial block in case of misaligned
offset. Do you know how to fix? Of late I started believing its a
limitation rather than bug. If we agree we can decompose and reuse
btrfs/172 for something else.
btrfs/154:
Exposed bug: Reappeared missing device fakes its assembly to the
volume.
Fix (as indicated in btrfs/154) V5 is in the mailing list, with
accepted review comments.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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
2019-04-07 11:54 ` Anand Jain
0 siblings, 1 reply; 10+ 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] 10+ 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
0 siblings, 1 reply; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread
end of thread, other threads:[~2019-04-14 2:47 UTC | newest]
Thread overview: 10+ 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-07 11:54 ` Anand Jain
2019-04-07 12:45 ` Nikolay Borisov
2019-04-14 2:47 ` Eryu Guan
-- strict thread matches above, loose matches on Subject: below --
2019-04-02 10:07 [PATCH 0/4 RESEND] btrfs: fix property bugs Anand Jain
2019-04-02 10:07 ` [PATCH] fstests: btrfs/048: amend property validation cases Anand Jain
2019-04-02 12:51 ` Nikolay Borisov
2019-04-03 0:32 ` Anand Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).