* [PATCH] xfstests: update xfs/096 for new behaviour @ 2016-06-02 9:10 Jan Tulak 2016-06-07 3:45 ` Eryu Guan ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Jan Tulak @ 2016-06-02 9:10 UTC (permalink / raw) To: fstests; +Cc: Jan Tulak Because we recently changed how mkfs behaves when it gets incorrect/invalid values, update the expected output to reflect the change. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- tests/xfs/096.out.internal | 60 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/tests/xfs/096.out.internal b/tests/xfs/096.out.internal index 80201d2..82c2043 100644 --- a/tests/xfs/096.out.internal +++ b/tests/xfs/096.out.internal @@ -2,18 +2,62 @@ QA output created by 096 # su too big but must be a multiple of fs block size too --- mkfs=-l version=2,su=262656 --- -log stripe unit (262656) must be a multiple of the block size (4096) +Illegal value 262656 for -l su option. value is too large +Usage: mkfs.xfs +/* blocksize */ [-b log=n|size=num] +/* data subvol */ [-d agcount=n,agsize=n,file,name=xxx,size=num, + (sunit=value,swidth=value|su=num,sw=num|noalign), + sectlog=n|sectsize=num +/* force overwrite */ [-f] +/* inode size */ [-i log=n|perblock=n|size=num,maxpct=n,attr=0|1|2, + projid32bit=0|1,sparse=0|1] +/* no discard */ [-K] +/* log subvol */ [-l agnum=n,internal,size=num,logdev=xxx,version=n + sunit=value|su=num,sectlog=n|sectsize=num, + lazy-count=0|1] +/* label */ [-L label (maximum 12 characters)] +/* naming */ [-n log=n|size=num,version=N|ci,ftype=0|1] +/* no-op info only */ [-N] +/* prototype file */ [-p fname] +/* quiet */ [-q] +/* realtime subvol */ [-r extsize=num,size=num,rtdev=xxx] +/* sectorsize */ [-s log=n|size=num] +/* version */ [-V] + devicename +<devicename> is required unless -d name=xxx is given. +<num> is xxx (bytes), xxxs (sectors), xxxb (fs blocks), xxxk (xxx KiB), + xxxm (xxx MiB), xxxg (xxx GiB), xxxt (xxx TiB) or xxxp (xxx PiB). +<value> is xxx (512 byte blocks). # test log stripe greater than LR size --- mkfs=-l version=2,su=266240 --- -meta-data=DEV isize=N agcount=N, agsize=N blks -data = bsize=4096 blocks=N, imaxpct=N - = sunit=0 swidth=0 blks, unwritten=1 -naming =version 2 bsize=4096 -log =LOG bsize=4096 blocks=N, version=N - = sunit=N blks -realtime =REALTIME extsz=N, blocks=N, rtextents=N +Illegal value 266240 for -l su option. value is too large +Usage: mkfs.xfs +/* blocksize */ [-b log=n|size=num] +/* data subvol */ [-d agcount=n,agsize=n,file,name=xxx,size=num, + (sunit=value,swidth=value|su=num,sw=num|noalign), + sectlog=n|sectsize=num +/* force overwrite */ [-f] +/* inode size */ [-i log=n|perblock=n|size=num,maxpct=n,attr=0|1|2, + projid32bit=0|1,sparse=0|1] +/* no discard */ [-K] +/* log subvol */ [-l agnum=n,internal,size=num,logdev=xxx,version=n + sunit=value|su=num,sectlog=n|sectsize=num, + lazy-count=0|1] +/* label */ [-L label (maximum 12 characters)] +/* naming */ [-n log=n|size=num,version=N|ci,ftype=0|1] +/* no-op info only */ [-N] +/* prototype file */ [-p fname] +/* quiet */ [-q] +/* realtime subvol */ [-r extsize=num,size=num,rtdev=xxx] +/* sectorsize */ [-s log=n|size=num] +/* version */ [-V] + devicename +<devicename> is required unless -d name=xxx is given. +<num> is xxx (bytes), xxxs (sectors), xxxb (fs blocks), xxxk (xxx KiB), + xxxm (xxx MiB), xxxg (xxx GiB), xxxt (xxx TiB) or xxxp (xxx PiB). +<value> is xxx (512 byte blocks). # same test but get log stripe from data stripe -- 2.5.5 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] xfstests: update xfs/096 for new behaviour 2016-06-02 9:10 [PATCH] xfstests: update xfs/096 for new behaviour Jan Tulak @ 2016-06-07 3:45 ` Eryu Guan [not found] ` <CACj3i733G7oz_tg4NANfz_=X5nYMp31k-FskEPXxHsaSxKtTEw@mail.gmail.com> 2016-06-29 10:18 ` [PATCH v2] " Jan Tulak 2016-07-01 16:14 ` [PATCH v3] " Jan Tulak 2 siblings, 1 reply; 13+ messages in thread From: Eryu Guan @ 2016-06-07 3:45 UTC (permalink / raw) To: Jan Tulak; +Cc: fstests On Thu, Jun 02, 2016 at 11:10:59AM +0200, Jan Tulak wrote: > Because we recently changed how mkfs behaves when it gets incorrect/invalid > values, update the expected output to reflect the change. This will break test with old-behavior xfsprogs. But I'm not sure what the best solution is.. And it seems that generic/054 and generic/055 are failing because of the same reason, if so, fix them together? Thanks, Eryu ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <CACj3i733G7oz_tg4NANfz_=X5nYMp31k-FskEPXxHsaSxKtTEw@mail.gmail.com>]
* Fwd: [PATCH] xfstests: update xfs/096 for new behaviour [not found] ` <CACj3i733G7oz_tg4NANfz_=X5nYMp31k-FskEPXxHsaSxKtTEw@mail.gmail.com> @ 2016-06-07 8:31 ` Jan Tulak 2016-06-23 11:22 ` Jan Tulak 1 sibling, 0 replies; 13+ messages in thread From: Jan Tulak @ 2016-06-07 8:31 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests Sorry for the spam, I'm sending it once more and this time it should pass vger.kernel.org spam filter. ---------- Forwarded message ---------- From: Jan Tulak <jtulak@redhat.com> Date: Tue, Jun 7, 2016 at 10:18 AM Subject: Re: [PATCH] xfstests: update xfs/096 for new behaviour To: Eryu Guan <eguan@redhat.com> Cc: fstests@vger.kernel.org On Tue, Jun 7, 2016 at 5:45 AM, Eryu Guan <eguan@redhat.com> wrote: > > On Thu, Jun 02, 2016 at 11:10:59AM +0200, Jan Tulak wrote: > > Because we recently changed how mkfs behaves when it gets incorrect/invalid > > values, update the expected output to reflect the change. > > This will break test with old-behavior xfsprogs. But I'm not sure what > the best solution is.. Hmm, well, an "if version > something" could work, together with testing the changed text directly in the test and making the correct output quiet. I can add something like that to xfstests and make it a function (has_mkfs_old_input_format) for easier use in the tests, but... It seems that only this single test is broken by the change, and I don't know if we want this backward compatibility in future tests. The only case when I see a usage would be finding a bug and then using the test to bisect the commit, while going over the change boundary. And will the persons doing this remember that there is a check for this? Or will they vaguely remember that there was some change and just look for the version and make their own "if version"? I would like a centralised solution, but I'm really afraid that it would be of no use. And moving the output text into the test is the only way I can think of for this specific test. > > > And it seems that generic/054 and generic/055 are failing because of the > same reason, if so, fix them together? These two tests should be fixed by the -l su minval patch, so it is just this one. Cheers, Jan -- Jan Tulak jtulak@redhat.com / jan@tulak.me ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] xfstests: update xfs/096 for new behaviour [not found] ` <CACj3i733G7oz_tg4NANfz_=X5nYMp31k-FskEPXxHsaSxKtTEw@mail.gmail.com> 2016-06-07 8:31 ` Fwd: " Jan Tulak @ 2016-06-23 11:22 ` Jan Tulak 2016-06-23 11:41 ` Jan Tulak 1 sibling, 1 reply; 13+ messages in thread From: Jan Tulak @ 2016-06-23 11:22 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests On Tue, Jun 7, 2016 at 10:18 AM, Jan Tulak <jtulak@redhat.com> wrote: > > > > On Tue, Jun 7, 2016 at 5:45 AM, Eryu Guan <eguan@redhat.com> wrote: >> >> On Thu, Jun 02, 2016 at 11:10:59AM +0200, Jan Tulak wrote: >> > Because we recently changed how mkfs behaves when it gets >> > incorrect/invalid >> > values, update the expected output to reflect the change. >> >> This will break test with old-behavior xfsprogs. But I'm not sure what >> the best solution is.. > > > Hmm, well, an "if version > something" could work, together with testing the > changed text directly in the test and making the correct output quiet. > > I can add something like that to xfstests and make it a function > (has_mkfs_old_input_format) for easier use in the tests, but... It seems > that only this single test is broken by the change, and I don't know if we > want this backward compatibility in future tests. > > The only case when I see a usage would be finding a bug and then using the > test to bisect the commit, while going over the change boundary. And will > the persons doing this remember that there is a check for this? Or will they > vaguely remember that there was some change and just look for the version > and make their own "if version"? > > I would like a centralised solution, but I'm really afraid that it would be > of no use. And moving the output text into the test is the only way I can > think of for this specific test. > > >> >> >> And it seems that generic/054 and generic/055 are failing because of the >> same reason, if so, fix them together? > > > These two tests should be fixed by the -l su minval patch, so it is just > this one. > Mmm, I spent some time on this but did not figure out any nice solution. Or... I found one, but I'm not sure how you will like it. Making the test to comply both versions is difficult because it is not just the error message that differs, but also that this run is now invalid: # test log stripe greater than LR size --- mkfs=-l version=2,su=266240 --- It differs also in what should fail. So rather than making some complicated logic, I got the idea to make a duplicity of this test. One will run with old version and skipped on the new, the other vice versa. Naming can utilize the text suffixes, so we would have xfs/096 and xfs/096-old-mkfs-inputs. It is not ideal, but looks better than some in-test filtering... What do you think? Cheers, Jan -- Jan Tulak jtulak@redhat.com / jan@tulak.me ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] xfstests: update xfs/096 for new behaviour 2016-06-23 11:22 ` Jan Tulak @ 2016-06-23 11:41 ` Jan Tulak 0 siblings, 0 replies; 13+ messages in thread From: Jan Tulak @ 2016-06-23 11:41 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests On Thu, Jun 23, 2016 at 1:22 PM, Jan Tulak <jtulak@redhat.com> wrote: > On Tue, Jun 7, 2016 at 10:18 AM, Jan Tulak <jtulak@redhat.com> wrote: >> >> >> >> On Tue, Jun 7, 2016 at 5:45 AM, Eryu Guan <eguan@redhat.com> wrote: >>> >>> On Thu, Jun 02, 2016 at 11:10:59AM +0200, Jan Tulak wrote: >>> > Because we recently changed how mkfs behaves when it gets >>> > incorrect/invalid >>> > values, update the expected output to reflect the change. >>> >>> This will break test with old-behavior xfsprogs. But I'm not sure what >>> the best solution is.. >> >> >> Hmm, well, an "if version > something" could work, together with testing the >> changed text directly in the test and making the correct output quiet. >> >> I can add something like that to xfstests and make it a function >> (has_mkfs_old_input_format) for easier use in the tests, but... It seems >> that only this single test is broken by the change, and I don't know if we >> want this backward compatibility in future tests. >> >> The only case when I see a usage would be finding a bug and then using the >> test to bisect the commit, while going over the change boundary. And will >> the persons doing this remember that there is a check for this? Or will they >> vaguely remember that there was some change and just look for the version >> and make their own "if version"? >> >> I would like a centralised solution, but I'm really afraid that it would be >> of no use. And moving the output text into the test is the only way I can >> think of for this specific test. >> >> >>> >>> >>> And it seems that generic/054 and generic/055 are failing because of the >>> same reason, if so, fix them together? >> >> >> These two tests should be fixed by the -l su minval patch, so it is just >> this one. >> > > Mmm, I spent some time on this but did not figure out any nice > solution. Or... I found one, but I'm not sure how you will like it. > > Making the test to comply both versions is difficult because it is not > just the error message that differs, but also that this run is now > invalid: > > # test log stripe greater than LR size > --- mkfs=-l version=2,su=266240 --- > > It differs also in what should fail. So rather than making some > complicated logic, I got the idea to make a duplicity of this test. > One will run with old version and skipped on the new, the other vice > versa. Naming can utilize the text suffixes, so we would have xfs/096 > and xfs/096-old-mkfs-inputs. > > It is not ideal, but looks better than some in-test filtering... What > do you think? > Just a correction, the new test name would be something like XXX-old-mkfs-inputs-096, the sequential number has to be unique. Thanks, Jan -- Jan Tulak jtulak@redhat.com / jan@tulak.me ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] xfstests: update xfs/096 for new behaviour 2016-06-02 9:10 [PATCH] xfstests: update xfs/096 for new behaviour Jan Tulak 2016-06-07 3:45 ` Eryu Guan @ 2016-06-29 10:18 ` Jan Tulak 2016-06-30 6:54 ` Eryu Guan 2016-07-01 16:14 ` [PATCH v3] " Jan Tulak 2 siblings, 1 reply; 13+ messages in thread From: Jan Tulak @ 2016-06-29 10:18 UTC (permalink / raw) To: fstests; +Cc: eguan, Jan Tulak Because we recently changed how mkfs behaves when it gets incorrect/invalid values, update the expected output to reflect the current status. However, keep also compatibility with the old version. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- CHANGE: added compatibility for the old xfsprogs. --- tests/xfs/096 | 26 ++++++++++++++--- tests/xfs/096.out.external.pre460 | 50 ++++++++++++++++++++++++++++++++ tests/xfs/096.out.internal | 60 +++++++++++++++++++++++++++++++++------ tests/xfs/096.out.internal.pre460 | 51 +++++++++++++++++++++++++++++++++ 4 files changed, 175 insertions(+), 12 deletions(-) create mode 100644 tests/xfs/096.out.external.pre460 create mode 100644 tests/xfs/096.out.internal.pre460 diff --git a/tests/xfs/096 b/tests/xfs/096 index f949e83..fe3d58c 100755 --- a/tests/xfs/096 +++ b/tests/xfs/096 @@ -39,6 +39,14 @@ _cleanup() rm -f $tmp.* } +# Get xfsprogs version as a number (4.7.0 => 470) +xfsprogs_ver() +{ + $MKFS_XFS_PROG -V | \ + grep -o "[0-9]\+\.[0-9]\+\.[0-9]\+" | \ + sed "s/\.//g" +} + # get standard environment, filters and checks . ./common/rc . ./common/filter @@ -106,12 +114,22 @@ _supported_os IRIX Linux _require_scratch _require_v2log -# choose .out file based on internal/external log +# Skip on old versions of xfsprogs +# and choose .out file based on internal/external log rm -f $seqfull.out -if [ "$USE_EXTERNAL" = yes ]; then - ln -s $seq.out.external $seqfull.out +version=$(xfsprogs_ver) +if [ $version -ge 460 ]; then + if [ "$USE_EXTERNAL" = yes ]; then + ln -s $seq.out.external $seqfull.out + else + ln -s $seq.out.internal $seqfull.out + fi else - ln -s $seq.out.internal $seqfull.out + if [ "$USE_EXTERNAL" = yes ]; then + ln -s $seq.out.external.pre460 $seqfull.out + else + ln -s $seq.out.internal.pre460 $seqfull.out + fi fi # maximum log record size diff --git a/tests/xfs/096.out.external.pre460 b/tests/xfs/096.out.external.pre460 new file mode 100644 index 0000000..3122330 --- /dev/null +++ b/tests/xfs/096.out.external.pre460 @@ -0,0 +1,50 @@ +QA output created by 096 + +# su too big but must be a multiple of fs block size too +--- mkfs=-l version=2,su=262656 --- +log stripe unit (262656) must be a multiple of the block size (4096) + + +# test log stripe greater than LR size +--- mkfs=-l version=2,su=266240 --- +meta-data=DEV isize=N agcount=N, agsize=N blks +data = bsize=4096 blocks=N, imaxpct=N + = sunit=0 swidth=0 blks, unwritten=1 +naming =version 2 bsize=4096 +log =LOG bsize=4096 blocks=N, version=N +realtime =REALTIME extsz=N, blocks=N, rtextents=N + + +# same test but get log stripe from data stripe +--- mkfs=-l version=2 -d su=266240,sw=1 --- +meta-data=DEV isize=N agcount=N, agsize=N blks +data = bsize=4096 blocks=N, imaxpct=N + = sunit=65 swidth=65 blks, unwritten=1 +naming =version 2 bsize=4096 +log =LOG bsize=4096 blocks=N, version=N + = sunit=N blks +realtime =REALTIME extsz=N, blocks=N, rtextents=N + + +# test out data stripe +--- mkfs=-m crc=0 -l version=1 -d su=266240,sw=1 --- +meta-data=DEV isize=N agcount=N, agsize=N blks +data = bsize=4096 blocks=N, imaxpct=N + = sunit=65 swidth=65 blks, unwritten=1 +naming =version 2 bsize=4096 +log =LOG bsize=4096 blocks=N, version=N + = sunit=N blks +realtime =REALTIME extsz=N, blocks=N, rtextents=N + + +# test out data stripe the same but using sunit & swidth +--- mkfs=-m crc=0 -l version=1 -d sunit=520,swidth=520 --- +meta-data=DEV isize=N agcount=N, agsize=N blks +data = bsize=4096 blocks=N, imaxpct=N + = sunit=65 swidth=65 blks, unwritten=1 +naming =version 2 bsize=4096 +log =LOG bsize=4096 blocks=N, version=N + = sunit=N blks +realtime =REALTIME extsz=N, blocks=N, rtextents=N + + diff --git a/tests/xfs/096.out.internal b/tests/xfs/096.out.internal index 80201d2..82c2043 100644 --- a/tests/xfs/096.out.internal +++ b/tests/xfs/096.out.internal @@ -2,18 +2,62 @@ QA output created by 096 # su too big but must be a multiple of fs block size too --- mkfs=-l version=2,su=262656 --- -log stripe unit (262656) must be a multiple of the block size (4096) +Illegal value 262656 for -l su option. value is too large +Usage: mkfs.xfs +/* blocksize */ [-b log=n|size=num] +/* data subvol */ [-d agcount=n,agsize=n,file,name=xxx,size=num, + (sunit=value,swidth=value|su=num,sw=num|noalign), + sectlog=n|sectsize=num +/* force overwrite */ [-f] +/* inode size */ [-i log=n|perblock=n|size=num,maxpct=n,attr=0|1|2, + projid32bit=0|1,sparse=0|1] +/* no discard */ [-K] +/* log subvol */ [-l agnum=n,internal,size=num,logdev=xxx,version=n + sunit=value|su=num,sectlog=n|sectsize=num, + lazy-count=0|1] +/* label */ [-L label (maximum 12 characters)] +/* naming */ [-n log=n|size=num,version=N|ci,ftype=0|1] +/* no-op info only */ [-N] +/* prototype file */ [-p fname] +/* quiet */ [-q] +/* realtime subvol */ [-r extsize=num,size=num,rtdev=xxx] +/* sectorsize */ [-s log=n|size=num] +/* version */ [-V] + devicename +<devicename> is required unless -d name=xxx is given. +<num> is xxx (bytes), xxxs (sectors), xxxb (fs blocks), xxxk (xxx KiB), + xxxm (xxx MiB), xxxg (xxx GiB), xxxt (xxx TiB) or xxxp (xxx PiB). +<value> is xxx (512 byte blocks). # test log stripe greater than LR size --- mkfs=-l version=2,su=266240 --- -meta-data=DEV isize=N agcount=N, agsize=N blks -data = bsize=4096 blocks=N, imaxpct=N - = sunit=0 swidth=0 blks, unwritten=1 -naming =version 2 bsize=4096 -log =LOG bsize=4096 blocks=N, version=N - = sunit=N blks -realtime =REALTIME extsz=N, blocks=N, rtextents=N +Illegal value 266240 for -l su option. value is too large +Usage: mkfs.xfs +/* blocksize */ [-b log=n|size=num] +/* data subvol */ [-d agcount=n,agsize=n,file,name=xxx,size=num, + (sunit=value,swidth=value|su=num,sw=num|noalign), + sectlog=n|sectsize=num +/* force overwrite */ [-f] +/* inode size */ [-i log=n|perblock=n|size=num,maxpct=n,attr=0|1|2, + projid32bit=0|1,sparse=0|1] +/* no discard */ [-K] +/* log subvol */ [-l agnum=n,internal,size=num,logdev=xxx,version=n + sunit=value|su=num,sectlog=n|sectsize=num, + lazy-count=0|1] +/* label */ [-L label (maximum 12 characters)] +/* naming */ [-n log=n|size=num,version=N|ci,ftype=0|1] +/* no-op info only */ [-N] +/* prototype file */ [-p fname] +/* quiet */ [-q] +/* realtime subvol */ [-r extsize=num,size=num,rtdev=xxx] +/* sectorsize */ [-s log=n|size=num] +/* version */ [-V] + devicename +<devicename> is required unless -d name=xxx is given. +<num> is xxx (bytes), xxxs (sectors), xxxb (fs blocks), xxxk (xxx KiB), + xxxm (xxx MiB), xxxg (xxx GiB), xxxt (xxx TiB) or xxxp (xxx PiB). +<value> is xxx (512 byte blocks). # same test but get log stripe from data stripe diff --git a/tests/xfs/096.out.internal.pre460 b/tests/xfs/096.out.internal.pre460 new file mode 100644 index 0000000..80201d2 --- /dev/null +++ b/tests/xfs/096.out.internal.pre460 @@ -0,0 +1,51 @@ +QA output created by 096 + +# su too big but must be a multiple of fs block size too +--- mkfs=-l version=2,su=262656 --- +log stripe unit (262656) must be a multiple of the block size (4096) + + +# test log stripe greater than LR size +--- mkfs=-l version=2,su=266240 --- +meta-data=DEV isize=N agcount=N, agsize=N blks +data = bsize=4096 blocks=N, imaxpct=N + = sunit=0 swidth=0 blks, unwritten=1 +naming =version 2 bsize=4096 +log =LOG bsize=4096 blocks=N, version=N + = sunit=N blks +realtime =REALTIME extsz=N, blocks=N, rtextents=N + + +# same test but get log stripe from data stripe +--- mkfs=-l version=2 -d su=266240,sw=1 --- +meta-data=DEV isize=N agcount=N, agsize=N blks +data = bsize=4096 blocks=N, imaxpct=N + = sunit=65 swidth=65 blks, unwritten=1 +naming =version 2 bsize=4096 +log =LOG bsize=4096 blocks=N, version=N + = sunit=N blks +realtime =REALTIME extsz=N, blocks=N, rtextents=N + + +# test out data stripe +--- mkfs=-m crc=0 -l version=1 -d su=266240,sw=1 --- +meta-data=DEV isize=N agcount=N, agsize=N blks +data = bsize=4096 blocks=N, imaxpct=N + = sunit=65 swidth=65 blks, unwritten=1 +naming =version 2 bsize=4096 +log =LOG bsize=4096 blocks=N, version=N + = sunit=N blks +realtime =REALTIME extsz=N, blocks=N, rtextents=N + + +# test out data stripe the same but using sunit & swidth +--- mkfs=-m crc=0 -l version=1 -d sunit=520,swidth=520 --- +meta-data=DEV isize=N agcount=N, agsize=N blks +data = bsize=4096 blocks=N, imaxpct=N + = sunit=65 swidth=65 blks, unwritten=1 +naming =version 2 bsize=4096 +log =LOG bsize=4096 blocks=N, version=N + = sunit=N blks +realtime =REALTIME extsz=N, blocks=N, rtextents=N + + -- 2.5.5 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] xfstests: update xfs/096 for new behaviour 2016-06-29 10:18 ` [PATCH v2] " Jan Tulak @ 2016-06-30 6:54 ` Eryu Guan 2016-07-01 0:37 ` Dave Chinner 0 siblings, 1 reply; 13+ messages in thread From: Eryu Guan @ 2016-06-30 6:54 UTC (permalink / raw) To: Jan Tulak; +Cc: fstests, Dave Chinner On Wed, Jun 29, 2016 at 12:18:55PM +0200, Jan Tulak wrote: > Because we recently changed how mkfs behaves when it gets incorrect/invalid > values, update the expected output to reflect the current status. > However, keep also compatibility with the old version. > > Signed-off-by: Jan Tulak <jtulak@redhat.com> > --- > CHANGE: added compatibility for the old xfsprogs. Sorry for the late response, because I was lost on this :) Hi Dave - what's the rule/policy of maintaining the backword compatibility in fstests? I know that efforts have been made to make sure new changes don't break old binaries, but is that a must or a best-to-have? And what do you think about the xfsprogs version comparing? (I'm OK with it :-)) Thanks! Eryu > > --- > tests/xfs/096 | 26 ++++++++++++++--- > tests/xfs/096.out.external.pre460 | 50 ++++++++++++++++++++++++++++++++ > tests/xfs/096.out.internal | 60 +++++++++++++++++++++++++++++++++------ > tests/xfs/096.out.internal.pre460 | 51 +++++++++++++++++++++++++++++++++ > 4 files changed, 175 insertions(+), 12 deletions(-) > create mode 100644 tests/xfs/096.out.external.pre460 > create mode 100644 tests/xfs/096.out.internal.pre460 > > diff --git a/tests/xfs/096 b/tests/xfs/096 > index f949e83..fe3d58c 100755 > --- a/tests/xfs/096 > +++ b/tests/xfs/096 > @@ -39,6 +39,14 @@ _cleanup() > rm -f $tmp.* > } > > +# Get xfsprogs version as a number (4.7.0 => 470) > +xfsprogs_ver() > +{ > + $MKFS_XFS_PROG -V | \ > + grep -o "[0-9]\+\.[0-9]\+\.[0-9]\+" | \ > + sed "s/\.//g" > +} > + > # get standard environment, filters and checks > . ./common/rc > . ./common/filter > @@ -106,12 +114,22 @@ _supported_os IRIX Linux > _require_scratch > _require_v2log > > -# choose .out file based on internal/external log > +# Skip on old versions of xfsprogs > +# and choose .out file based on internal/external log > rm -f $seqfull.out > -if [ "$USE_EXTERNAL" = yes ]; then > - ln -s $seq.out.external $seqfull.out > +version=$(xfsprogs_ver) > +if [ $version -ge 460 ]; then > + if [ "$USE_EXTERNAL" = yes ]; then > + ln -s $seq.out.external $seqfull.out > + else > + ln -s $seq.out.internal $seqfull.out > + fi > else > - ln -s $seq.out.internal $seqfull.out > + if [ "$USE_EXTERNAL" = yes ]; then > + ln -s $seq.out.external.pre460 $seqfull.out > + else > + ln -s $seq.out.internal.pre460 $seqfull.out > + fi > fi > > # maximum log record size > diff --git a/tests/xfs/096.out.external.pre460 b/tests/xfs/096.out.external.pre460 > new file mode 100644 > index 0000000..3122330 > --- /dev/null > +++ b/tests/xfs/096.out.external.pre460 > @@ -0,0 +1,50 @@ > +QA output created by 096 > + > +# su too big but must be a multiple of fs block size too > +--- mkfs=-l version=2,su=262656 --- > +log stripe unit (262656) must be a multiple of the block size (4096) > + > + > +# test log stripe greater than LR size > +--- mkfs=-l version=2,su=266240 --- > +meta-data=DEV isize=N agcount=N, agsize=N blks > +data = bsize=4096 blocks=N, imaxpct=N > + = sunit=0 swidth=0 blks, unwritten=1 > +naming =version 2 bsize=4096 > +log =LOG bsize=4096 blocks=N, version=N > +realtime =REALTIME extsz=N, blocks=N, rtextents=N > + > + > +# same test but get log stripe from data stripe > +--- mkfs=-l version=2 -d su=266240,sw=1 --- > +meta-data=DEV isize=N agcount=N, agsize=N blks > +data = bsize=4096 blocks=N, imaxpct=N > + = sunit=65 swidth=65 blks, unwritten=1 > +naming =version 2 bsize=4096 > +log =LOG bsize=4096 blocks=N, version=N > + = sunit=N blks > +realtime =REALTIME extsz=N, blocks=N, rtextents=N > + > + > +# test out data stripe > +--- mkfs=-m crc=0 -l version=1 -d su=266240,sw=1 --- > +meta-data=DEV isize=N agcount=N, agsize=N blks > +data = bsize=4096 blocks=N, imaxpct=N > + = sunit=65 swidth=65 blks, unwritten=1 > +naming =version 2 bsize=4096 > +log =LOG bsize=4096 blocks=N, version=N > + = sunit=N blks > +realtime =REALTIME extsz=N, blocks=N, rtextents=N > + > + > +# test out data stripe the same but using sunit & swidth > +--- mkfs=-m crc=0 -l version=1 -d sunit=520,swidth=520 --- > +meta-data=DEV isize=N agcount=N, agsize=N blks > +data = bsize=4096 blocks=N, imaxpct=N > + = sunit=65 swidth=65 blks, unwritten=1 > +naming =version 2 bsize=4096 > +log =LOG bsize=4096 blocks=N, version=N > + = sunit=N blks > +realtime =REALTIME extsz=N, blocks=N, rtextents=N > + > + > diff --git a/tests/xfs/096.out.internal b/tests/xfs/096.out.internal > index 80201d2..82c2043 100644 > --- a/tests/xfs/096.out.internal > +++ b/tests/xfs/096.out.internal > @@ -2,18 +2,62 @@ QA output created by 096 > > # su too big but must be a multiple of fs block size too > --- mkfs=-l version=2,su=262656 --- > -log stripe unit (262656) must be a multiple of the block size (4096) > +Illegal value 262656 for -l su option. value is too large > +Usage: mkfs.xfs > +/* blocksize */ [-b log=n|size=num] > +/* data subvol */ [-d agcount=n,agsize=n,file,name=xxx,size=num, > + (sunit=value,swidth=value|su=num,sw=num|noalign), > + sectlog=n|sectsize=num > +/* force overwrite */ [-f] > +/* inode size */ [-i log=n|perblock=n|size=num,maxpct=n,attr=0|1|2, > + projid32bit=0|1,sparse=0|1] > +/* no discard */ [-K] > +/* log subvol */ [-l agnum=n,internal,size=num,logdev=xxx,version=n > + sunit=value|su=num,sectlog=n|sectsize=num, > + lazy-count=0|1] > +/* label */ [-L label (maximum 12 characters)] > +/* naming */ [-n log=n|size=num,version=N|ci,ftype=0|1] > +/* no-op info only */ [-N] > +/* prototype file */ [-p fname] > +/* quiet */ [-q] > +/* realtime subvol */ [-r extsize=num,size=num,rtdev=xxx] > +/* sectorsize */ [-s log=n|size=num] > +/* version */ [-V] > + devicename > +<devicename> is required unless -d name=xxx is given. > +<num> is xxx (bytes), xxxs (sectors), xxxb (fs blocks), xxxk (xxx KiB), > + xxxm (xxx MiB), xxxg (xxx GiB), xxxt (xxx TiB) or xxxp (xxx PiB). > +<value> is xxx (512 byte blocks). > > > # test log stripe greater than LR size > --- mkfs=-l version=2,su=266240 --- > -meta-data=DEV isize=N agcount=N, agsize=N blks > -data = bsize=4096 blocks=N, imaxpct=N > - = sunit=0 swidth=0 blks, unwritten=1 > -naming =version 2 bsize=4096 > -log =LOG bsize=4096 blocks=N, version=N > - = sunit=N blks > -realtime =REALTIME extsz=N, blocks=N, rtextents=N > +Illegal value 266240 for -l su option. value is too large > +Usage: mkfs.xfs > +/* blocksize */ [-b log=n|size=num] > +/* data subvol */ [-d agcount=n,agsize=n,file,name=xxx,size=num, > + (sunit=value,swidth=value|su=num,sw=num|noalign), > + sectlog=n|sectsize=num > +/* force overwrite */ [-f] > +/* inode size */ [-i log=n|perblock=n|size=num,maxpct=n,attr=0|1|2, > + projid32bit=0|1,sparse=0|1] > +/* no discard */ [-K] > +/* log subvol */ [-l agnum=n,internal,size=num,logdev=xxx,version=n > + sunit=value|su=num,sectlog=n|sectsize=num, > + lazy-count=0|1] > +/* label */ [-L label (maximum 12 characters)] > +/* naming */ [-n log=n|size=num,version=N|ci,ftype=0|1] > +/* no-op info only */ [-N] > +/* prototype file */ [-p fname] > +/* quiet */ [-q] > +/* realtime subvol */ [-r extsize=num,size=num,rtdev=xxx] > +/* sectorsize */ [-s log=n|size=num] > +/* version */ [-V] > + devicename > +<devicename> is required unless -d name=xxx is given. > +<num> is xxx (bytes), xxxs (sectors), xxxb (fs blocks), xxxk (xxx KiB), > + xxxm (xxx MiB), xxxg (xxx GiB), xxxt (xxx TiB) or xxxp (xxx PiB). > +<value> is xxx (512 byte blocks). > > > # same test but get log stripe from data stripe > diff --git a/tests/xfs/096.out.internal.pre460 b/tests/xfs/096.out.internal.pre460 > new file mode 100644 > index 0000000..80201d2 > --- /dev/null > +++ b/tests/xfs/096.out.internal.pre460 > @@ -0,0 +1,51 @@ > +QA output created by 096 > + > +# su too big but must be a multiple of fs block size too > +--- mkfs=-l version=2,su=262656 --- > +log stripe unit (262656) must be a multiple of the block size (4096) > + > + > +# test log stripe greater than LR size > +--- mkfs=-l version=2,su=266240 --- > +meta-data=DEV isize=N agcount=N, agsize=N blks > +data = bsize=4096 blocks=N, imaxpct=N > + = sunit=0 swidth=0 blks, unwritten=1 > +naming =version 2 bsize=4096 > +log =LOG bsize=4096 blocks=N, version=N > + = sunit=N blks > +realtime =REALTIME extsz=N, blocks=N, rtextents=N > + > + > +# same test but get log stripe from data stripe > +--- mkfs=-l version=2 -d su=266240,sw=1 --- > +meta-data=DEV isize=N agcount=N, agsize=N blks > +data = bsize=4096 blocks=N, imaxpct=N > + = sunit=65 swidth=65 blks, unwritten=1 > +naming =version 2 bsize=4096 > +log =LOG bsize=4096 blocks=N, version=N > + = sunit=N blks > +realtime =REALTIME extsz=N, blocks=N, rtextents=N > + > + > +# test out data stripe > +--- mkfs=-m crc=0 -l version=1 -d su=266240,sw=1 --- > +meta-data=DEV isize=N agcount=N, agsize=N blks > +data = bsize=4096 blocks=N, imaxpct=N > + = sunit=65 swidth=65 blks, unwritten=1 > +naming =version 2 bsize=4096 > +log =LOG bsize=4096 blocks=N, version=N > + = sunit=N blks > +realtime =REALTIME extsz=N, blocks=N, rtextents=N > + > + > +# test out data stripe the same but using sunit & swidth > +--- mkfs=-m crc=0 -l version=1 -d sunit=520,swidth=520 --- > +meta-data=DEV isize=N agcount=N, agsize=N blks > +data = bsize=4096 blocks=N, imaxpct=N > + = sunit=65 swidth=65 blks, unwritten=1 > +naming =version 2 bsize=4096 > +log =LOG bsize=4096 blocks=N, version=N > + = sunit=N blks > +realtime =REALTIME extsz=N, blocks=N, rtextents=N > + > + > -- > 2.5.5 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] xfstests: update xfs/096 for new behaviour 2016-06-30 6:54 ` Eryu Guan @ 2016-07-01 0:37 ` Dave Chinner 2016-07-01 3:05 ` Eryu Guan 0 siblings, 1 reply; 13+ messages in thread From: Dave Chinner @ 2016-07-01 0:37 UTC (permalink / raw) To: Eryu Guan; +Cc: Jan Tulak, fstests On Thu, Jun 30, 2016 at 02:54:59PM +0800, Eryu Guan wrote: > On Wed, Jun 29, 2016 at 12:18:55PM +0200, Jan Tulak wrote: > > Because we recently changed how mkfs behaves when it gets incorrect/invalid > > values, update the expected output to reflect the current status. > > However, keep also compatibility with the old version. > > > > Signed-off-by: Jan Tulak <jtulak@redhat.com> > > --- > > CHANGE: added compatibility for the old xfsprogs. > > Sorry for the late response, because I was lost on this :) > > Hi Dave - what's the rule/policy of maintaining the backword > compatibility in fstests? We try to ensure that tests that work/pass on old versions of utilities continue to do so, even as the newer code changes. If the new code changes too much, then we can either stop running the test on older code, or we fork the test for the new code.... > I know that efforts have been made to make > sure new changes don't break old binaries, but is that a must or a > best-to-have? And what do you think about the xfsprogs version > comparing? (I'm OK with it :-)) We've tried to avoid using version numbers for comparisons, because that becomes a downward spiral into a mess. Instead, we have gone down the path of testing for supported features in binaries and filesystems, not checking version numbers. i.e. we don't care about the version number - we care about the feature that the binary provides. Those checks are self documenting - the test tells use what it requires which something that version number checks do not explain at all. In this case, we have a change in a binary that turns warnings into errors or issues errors rather than silently ignores what the user asked for and uses defaults. We already filter out anything relevant from the result to support all the changes in binary output since the test was introduced, so we really can't tell if the value substitution behaviour has changed anymore. IOWs, this test really isn't serving much purpose as a regression test anymore. >From that perspective, I'd say we either remove it or we stop trying to update it further by adding a new requires check for an old mkfs binary that silently accepts invalid log stripe unit sizes. i.e. don't add version number checks, add a feature check so that it only runs on old mkfs binaries but not new ones. e.g. _require_mkfs_accept_invalid_log_sunit() Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] xfstests: update xfs/096 for new behaviour 2016-07-01 0:37 ` Dave Chinner @ 2016-07-01 3:05 ` Eryu Guan 2016-07-01 12:12 ` Jan Tulak 0 siblings, 1 reply; 13+ messages in thread From: Eryu Guan @ 2016-07-01 3:05 UTC (permalink / raw) To: Dave Chinner, Jan Tulak; +Cc: fstests On Fri, Jul 01, 2016 at 10:37:00AM +1000, Dave Chinner wrote: > On Thu, Jun 30, 2016 at 02:54:59PM +0800, Eryu Guan wrote: > > On Wed, Jun 29, 2016 at 12:18:55PM +0200, Jan Tulak wrote: > > > Because we recently changed how mkfs behaves when it gets incorrect/invalid > > > values, update the expected output to reflect the current status. > > > However, keep also compatibility with the old version. > > > > > > Signed-off-by: Jan Tulak <jtulak@redhat.com> > > > --- > > > CHANGE: added compatibility for the old xfsprogs. > > > > Sorry for the late response, because I was lost on this :) > > > > Hi Dave - what's the rule/policy of maintaining the backword > > compatibility in fstests? > > We try to ensure that tests that work/pass on old versions of > utilities continue to do so, even as the newer code changes. If the > new code changes too much, then we can either stop running the test > on older code, or we fork the test for the new code.... Thanks Dave for the clarification! > > > I know that efforts have been made to make > > sure new changes don't break old binaries, but is that a must or a > > best-to-have? And what do you think about the xfsprogs version > > comparing? (I'm OK with it :-)) > > We've tried to avoid using version numbers for comparisons, because > that becomes a downward spiral into a mess. Instead, we have > gone down the path of testing for supported features in binaries and > filesystems, not checking version numbers. i.e. we don't care about > the version number - we care about the feature that the binary > provides. Those checks are self documenting - the test tells use > what it requires which something that version number checks do not > explain at all. Makes sense. > > In this case, we have a change in a binary that turns warnings into > errors or issues errors rather than silently ignores what the user > asked for and uses defaults. We already filter out anything relevant > from the result to support all the changes in binary output since > the test was introduced, so we really can't tell if the value > substitution behaviour has changed anymore. IOWs, this test really > isn't serving much purpose as a regression test anymore. > > From that perspective, I'd say we either remove it or we stop trying > to update it further by adding a new requires check for an old mkfs > binary that silently accepts invalid log stripe unit sizes. i.e. > don't add version number checks, add a feature check so that it only > runs on old mkfs binaries but not new ones. e.g. > _require_mkfs_accept_invalid_log_sunit() This looks good to me. Hi Jan - Can you please send an updated version as Dave suggested above? And I think the input-validation test could be updated as well to make it only run on newer mkfs. Thanks a lot! Eryu ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] xfstests: update xfs/096 for new behaviour 2016-07-01 3:05 ` Eryu Guan @ 2016-07-01 12:12 ` Jan Tulak 0 siblings, 0 replies; 13+ messages in thread From: Jan Tulak @ 2016-07-01 12:12 UTC (permalink / raw) To: Eryu Guan; +Cc: Dave Chinner, fstests On Fri, Jul 1, 2016 at 5:05 AM, Eryu Guan <eguan@redhat.com> wrote: > On Fri, Jul 01, 2016 at 10:37:00AM +1000, Dave Chinner wrote: >> On Thu, Jun 30, 2016 at 02:54:59PM +0800, Eryu Guan wrote: >> > On Wed, Jun 29, 2016 at 12:18:55PM +0200, Jan Tulak wrote: >> > > Because we recently changed how mkfs behaves when it gets incorrect/invalid >> > > values, update the expected output to reflect the current status. >> > > However, keep also compatibility with the old version. >> > > >> > > Signed-off-by: Jan Tulak <jtulak@redhat.com> >> > > --- >> > > CHANGE: added compatibility for the old xfsprogs. >> > >> > Sorry for the late response, because I was lost on this :) >> > >> > Hi Dave - what's the rule/policy of maintaining the backword >> > compatibility in fstests? >> >> We try to ensure that tests that work/pass on old versions of >> utilities continue to do so, even as the newer code changes. If the >> new code changes too much, then we can either stop running the test >> on older code, or we fork the test for the new code.... > > Thanks Dave for the clarification! > >> >> > I know that efforts have been made to make >> > sure new changes don't break old binaries, but is that a must or a >> > best-to-have? And what do you think about the xfsprogs version >> > comparing? (I'm OK with it :-)) >> >> We've tried to avoid using version numbers for comparisons, because >> that becomes a downward spiral into a mess. Instead, we have >> gone down the path of testing for supported features in binaries and >> filesystems, not checking version numbers. i.e. we don't care about >> the version number - we care about the feature that the binary >> provides. Those checks are self documenting - the test tells use >> what it requires which something that version number checks do not >> explain at all. > OK, thanks for explaining it. > >> >> In this case, we have a change in a binary that turns warnings into >> errors or issues errors rather than silently ignores what the user >> asked for and uses defaults. We already filter out anything relevant >> from the result to support all the changes in binary output since >> the test was introduced, so we really can't tell if the value >> substitution behaviour has changed anymore. IOWs, this test really >> isn't serving much purpose as a regression test anymore. >> >> From that perspective, I'd say we either remove it or we stop trying >> to update it further by adding a new requires check for an old mkfs >> binary that silently accepts invalid log stripe unit sizes. i.e. >> don't add version number checks, add a feature check so that it only >> runs on old mkfs binaries but not new ones. e.g. >> _require_mkfs_accept_invalid_log_sunit() All right. I think that the feature check should remain in the test, at least until there are more tests where this is needed and there is a clear idea what features are useful to have a check for. But other than that, I agree. > > This looks good to me. > > Hi Jan - Can you please send an updated version as Dave suggested above? > And I think the input-validation test could be updated as well to make > it only run on newer mkfs. > Yes, I will send both. Cheers, Jan -- Jan Tulak jtulak@redhat.com / jan@tulak.me ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] xfstests: update xfs/096 for new behaviour 2016-06-02 9:10 [PATCH] xfstests: update xfs/096 for new behaviour Jan Tulak 2016-06-07 3:45 ` Eryu Guan 2016-06-29 10:18 ` [PATCH v2] " Jan Tulak @ 2016-07-01 16:14 ` Jan Tulak 2016-07-13 10:38 ` Eryu Guan 2 siblings, 1 reply; 13+ messages in thread From: Jan Tulak @ 2016-07-01 16:14 UTC (permalink / raw) To: fstests; +Cc: david, eguan, Jan Tulak Because we recently changed how mkfs behaves when it gets incorrect/invalid values, add a feature check to run this test only on older binaries, which accepts invalid sunit values. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- UPDATE: Change it to _notrun on newer binaries. Commit message updated respectivvely. --- tests/xfs/096 | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/tests/xfs/096 b/tests/xfs/096 index f949e83..803b49d 100755 --- a/tests/xfs/096 +++ b/tests/xfs/096 @@ -39,6 +39,20 @@ _cleanup() rm -f $tmp.* } +# maximum log record size +max_lr_size=`expr 256 \* 1024` +big_su=`expr $max_lr_size + 4096` + +requires_mkfs_accept_invalid_log_sunit() +{ + accepts=`mkfs.xfs -N -l version=2,su=$big_su 2>&1 | \ + grep -ci "No device name"` + if [ "$accepts" -eq 0 ];then + _notrun "Runs only on older xfsprogs accepting invalid log sunit" + fi + return 1 +} + # get standard environment, filters and checks . ./common/rc . ./common/filter @@ -97,6 +111,9 @@ mkfs_filter() | grep -v parent } +# skip on newer versions +requires_mkfs_accept_invalid_log_sunit + # real QA test starts here rm -f $seqres.full @@ -108,16 +125,13 @@ _require_v2log # choose .out file based on internal/external log rm -f $seqfull.out + if [ "$USE_EXTERNAL" = yes ]; then ln -s $seq.out.external $seqfull.out else ln -s $seq.out.internal $seqfull.out fi -# maximum log record size -max_lr_size=`expr 256 \* 1024` - -big_su=`expr $max_lr_size + 4096` # # Test out various mkfs param combinations -- 2.5.5 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3] xfstests: update xfs/096 for new behaviour 2016-07-01 16:14 ` [PATCH v3] " Jan Tulak @ 2016-07-13 10:38 ` Eryu Guan 2016-07-14 10:25 ` Jan Tulak 0 siblings, 1 reply; 13+ messages in thread From: Eryu Guan @ 2016-07-13 10:38 UTC (permalink / raw) To: Jan Tulak; +Cc: fstests, david On Fri, Jul 01, 2016 at 06:14:34PM +0200, Jan Tulak wrote: > Because we recently changed how mkfs behaves when it gets incorrect/invalid > values, add a feature check to run this test only on older binaries, which > accepts invalid sunit values. > > Signed-off-by: Jan Tulak <jtulak@redhat.com> > --- > UPDATE: > Change it to _notrun on newer binaries. Commit message updated respectivvely. > --- > tests/xfs/096 | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/tests/xfs/096 b/tests/xfs/096 > index f949e83..803b49d 100755 > --- a/tests/xfs/096 > +++ b/tests/xfs/096 > @@ -39,6 +39,20 @@ _cleanup() > rm -f $tmp.* > } > > +# maximum log record size > +max_lr_size=`expr 256 \* 1024` > +big_su=`expr $max_lr_size + 4096` > + > +requires_mkfs_accept_invalid_log_sunit() > +{ > + accepts=`mkfs.xfs -N -l version=2,su=$big_su 2>&1 | \ > + grep -ci "No device name"` > + if [ "$accepts" -eq 0 ];then > + _notrun "Runs only on older xfsprogs accepting invalid log sunit" > + fi You're counting the number of "No device name" string, which doesn't seem reliable to me. I'd prefer to rely on whether mkfs.xfs really accepts invalid log sunit, so how about: require_mkfs_accept_invalid_log_sunit() { local fsimg=$tmp.img touch $fsimg $MKFS_XFS_PROG -N -l version=2,su=$big_su -d name=$fsimg,size=1g >/dev/null 2>&1 if [ $? -ne 0 ]; then _notrun "This test requires v4.5 or older xfsprogs" fi } (These mkfs argument checks went to v4.7-rc1, and v4.5 is the latest "older" xfsprogs, right?) Thanks, Eryu ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] xfstests: update xfs/096 for new behaviour 2016-07-13 10:38 ` Eryu Guan @ 2016-07-14 10:25 ` Jan Tulak 0 siblings, 0 replies; 13+ messages in thread From: Jan Tulak @ 2016-07-14 10:25 UTC (permalink / raw) To: Eryu Guan; +Cc: fstests, Chinner, Dave On Wed, Jul 13, 2016 at 12:38 PM, Eryu Guan <eguan@redhat.com> wrote: > On Fri, Jul 01, 2016 at 06:14:34PM +0200, Jan Tulak wrote: >> Because we recently changed how mkfs behaves when it gets incorrect/invalid >> values, add a feature check to run this test only on older binaries, which >> accepts invalid sunit values. >> >> Signed-off-by: Jan Tulak <jtulak@redhat.com> >> --- >> UPDATE: >> Change it to _notrun on newer binaries. Commit message updated respectivvely. >> --- >> tests/xfs/096 | 22 ++++++++++++++++++---- >> 1 file changed, 18 insertions(+), 4 deletions(-) >> >> diff --git a/tests/xfs/096 b/tests/xfs/096 >> index f949e83..803b49d 100755 >> --- a/tests/xfs/096 >> +++ b/tests/xfs/096 >> @@ -39,6 +39,20 @@ _cleanup() >> rm -f $tmp.* >> } >> >> +# maximum log record size >> +max_lr_size=`expr 256 \* 1024` >> +big_su=`expr $max_lr_size + 4096` >> + >> +requires_mkfs_accept_invalid_log_sunit() >> +{ >> + accepts=`mkfs.xfs -N -l version=2,su=$big_su 2>&1 | \ >> + grep -ci "No device name"` >> + if [ "$accepts" -eq 0 ];then >> + _notrun "Runs only on older xfsprogs accepting invalid log sunit" >> + fi > > You're counting the number of "No device name" string, which doesn't > seem reliable to me. I'd prefer to rely on whether mkfs.xfs really > accepts invalid log sunit, so how about: > > require_mkfs_accept_invalid_log_sunit() > { > local fsimg=$tmp.img > touch $fsimg > > $MKFS_XFS_PROG -N -l version=2,su=$big_su -d name=$fsimg,size=1g >/dev/null 2>&1 > if [ $? -ne 0 ]; then > _notrun "This test requires v4.5 or older xfsprogs" > fi > } This looks better, but I'm ditching this detection entirely. I will go for the common/rc solution and use the code from xfs/400, only inverted. That seems to me like a better way in long term, and it makes the new common/rc _requires a bit more useful. > > (These mkfs argument checks went to v4.7-rc1, and v4.5 is the latest > "older" xfsprogs, right?) Yes, according to VERSION file and tags in the repo. Thanks, Jan -- Jan Tulak jtulak@redhat.com / jan@tulak.me ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-07-14 10:25 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-02 9:10 [PATCH] xfstests: update xfs/096 for new behaviour Jan Tulak 2016-06-07 3:45 ` Eryu Guan [not found] ` <CACj3i733G7oz_tg4NANfz_=X5nYMp31k-FskEPXxHsaSxKtTEw@mail.gmail.com> 2016-06-07 8:31 ` Fwd: " Jan Tulak 2016-06-23 11:22 ` Jan Tulak 2016-06-23 11:41 ` Jan Tulak 2016-06-29 10:18 ` [PATCH v2] " Jan Tulak 2016-06-30 6:54 ` Eryu Guan 2016-07-01 0:37 ` Dave Chinner 2016-07-01 3:05 ` Eryu Guan 2016-07-01 12:12 ` Jan Tulak 2016-07-01 16:14 ` [PATCH v3] " Jan Tulak 2016-07-13 10:38 ` Eryu Guan 2016-07-14 10:25 ` Jan Tulak
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.