[PATCH] xfstests: update xfs/096 for new behaviour

[PATCH] xfstests: update xfs/096 for new behaviour

[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

Re: [PATCH] xfstests: update xfs/096 for new behaviour

[Eryu Guan]

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

Fwd: [PATCH] xfstests: update xfs/096 for new behaviour

[Jan Tulak]

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

Re: [PATCH] xfstests: update xfs/096 for new behaviour

[Jan Tulak]

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

Re: [PATCH] xfstests: update xfs/096 for new behaviour

[Jan Tulak]

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

[PATCH v2] xfstests: update xfs/096 for new behaviour

[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

Re: [PATCH v2] xfstests: update xfs/096 for new behaviour

[Eryu Guan]

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
> 

Re: [PATCH v2] xfstests: update xfs/096 for new behaviour

[Dave Chinner]

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

Re: [PATCH v2] xfstests: update xfs/096 for new behaviour

[Eryu Guan]

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

Re: [PATCH v2] xfstests: update xfs/096 for new behaviour

[Jan Tulak]

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

[PATCH v3] xfstests: update xfs/096 for new behaviour

[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

Re: [PATCH v3] xfstests: update xfs/096 for new behaviour

[Eryu Guan]

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

Re: [PATCH v3] xfstests: update xfs/096 for new behaviour

[Jan Tulak]

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