| Message ID | 20190403170437.14448-1-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] fstests: btrfs/048: amend property validation cases | expand |
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 >
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 > >
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 >>>
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 >>>> > >
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
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
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(+)