[PATCH] xfstests-bld: add exclude file for data_journal_noleak tests

[PATCH] xfstests-bld: add exclude file for data_journal_noleak tests

[Eric Whitney]

Duplicate the contents of the data_journal exclude file for the
data_journal_noleak test case.  This will prevent failure reports from
tests already known to exercise unsupported online defrag functionality.
Add an explanatory comment to both exclude files for future reference.

Signed-off-by: Eric Whitney <enwlinux@gmail.com>
---
 kvm-xfstests/test-appliance/files/root/conf/data_journal.exclude   | 1 +
 .../test-appliance/files/root/conf/data_journal_noleak.exclude     | 7 +++++++
 2 files changed, 8 insertions(+)
 create mode 100644 kvm-xfstests/test-appliance/files/root/conf/data_journal_noleak.exclude

diff --git a/kvm-xfstests/test-appliance/files/root/conf/data_journal.exclude b/kvm-xfstests/test-appliance/files/root/conf/data_journal.exclude
index b925016..7190c8c 100644
--- a/kvm-xfstests/test-appliance/files/root/conf/data_journal.exclude
+++ b/kvm-xfstests/test-appliance/files/root/conf/data_journal.exclude
@@ -1,3 +1,4 @@
+# data_journal does not support on-line defrag
 ext4/301
 ext4/302
 ext4/303
diff --git a/kvm-xfstests/test-appliance/files/root/conf/data_journal_noleak.exclude b/kvm-xfstests/test-appliance/files/root/conf/data_journal_noleak.exclude
new file mode 100644
index 0000000..7190c8c
--- /dev/null
+++ b/kvm-xfstests/test-appliance/files/root/conf/data_journal_noleak.exclude
@@ -0,0 +1,7 @@
+# data_journal does not support on-line defrag
+ext4/301
+ext4/302
+ext4/303
+ext4/304
+ext4/307
+ext4/308
-- 
2.1.4

Re: [PATCH] xfstests-bld: add exclude file for data_journal_noleak tests

[Dave Chinner]

On Tue, Jan 12, 2016 at 02:52:48PM -0500, Eric Whitney wrote:
> Duplicate the contents of the data_journal exclude file for the
> data_journal_noleak test case.  This will prevent failure reports from
> tests already known to exercise unsupported online defrag functionality.
> Add an explanatory comment to both exclude files for future reference.
> 
> Signed-off-by: Eric Whitney <enwlinux@gmail.com>

Can you please make these changes directly to xfstests rather than
keeping information about what tests run under what circumstances in
some external test wrapper?  i.e. this can be implementing by adding
a _requires_no_data_journal() check and adding it to each test.

That way every one who runs xfstests on ext4 benefit from this
exclusion, rather than just kvm-xfstests users.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] xfstests-bld: add exclude file for data_journal_noleak tests

[Theodore Ts'o]

On Wed, Jan 13, 2016 at 07:35:54AM +1100, Dave Chinner wrote:
> On Tue, Jan 12, 2016 at 02:52:48PM -0500, Eric Whitney wrote:
> > Duplicate the contents of the data_journal exclude file for the
> > data_journal_noleak test case.  This will prevent failure reports from
> > tests already known to exercise unsupported online defrag functionality.
> > Add an explanatory comment to both exclude files for future reference.
> > 
> > Signed-off-by: Eric Whitney <enwlinux@gmail.com>
> 
> Can you please make these changes directly to xfstests rather than
> keeping information about what tests run under what circumstances in
> some external test wrapper?  i.e. this can be implementing by adding
> a _requires_no_data_journal() check and adding it to each test.

We've got a number of these exclusions, and some of them require
making changes that have been rejected by you previously as adding too
much hair that is ext4 specific.  (For example, ways of restricting
the tests from using punch hole or collapse range.)

So I'm carrying a number of these patches out of tree since they've
been rejected by upstream, and they are all necessary to make it to
exclude certain tests based on the ext4 configuration.  I had assumed
you were philosophically against these sorts of exclusions except by
creating large numbers of extra xfstests groups, which is something
that I don't believe is maintainable from my perspective --- or at
least, it's easier for me to carry these patches out of tree than to
be constantly updating a groups file and then having to modulate which
groups to include or exclude on a fs config file.

The _require_data_journal_noleak annotation is certainly more
acceptable from my perspective, but I had assumed you were
philosophically against such changes, so I had stopped trying to get
these sorts of changes upstream.

Also, in this specific case, we may end up dropping this change
anyway, since if the bug fix has perculated all the way back to all of
the stable kernel trees, the need for the data_journal_noleak
configuration will go away, since at the moment I'm only interested in
supporting the kvm-xfstests for mainline plus the long term stable
kernel trees.

What would actually be cool is some way of expressing in a config file
tests that should be excluded given a certain file system
configuration and kernel version ranges, which would also serve as
documentation for when certain bugs were fixed/backported to the
stable kernels.  The question though is whether such a thing belongs
in core xfstests, or in the test runners.  I had assumed that
philosophically you were against pushing this sort of thing into
xfstests, but if this is something you would be interested in, perhaps
we can work on this and try to get something upstream to xfstests
which is a superset of the current exclusion rules we have in
kvm-xfstests.

Cheers,

						- Ted

Re: [PATCH] xfstests-bld: add exclude file for data_journal_noleak tests

[Dave Chinner]

On Wed, Jan 13, 2016 at 11:55:07AM -0500, Theodore Ts'o wrote:
> On Wed, Jan 13, 2016 at 07:35:54AM +1100, Dave Chinner wrote:
> > On Tue, Jan 12, 2016 at 02:52:48PM -0500, Eric Whitney wrote:
> > > Duplicate the contents of the data_journal exclude file for the
> > > data_journal_noleak test case.  This will prevent failure reports from
> > > tests already known to exercise unsupported online defrag functionality.
> > > Add an explanatory comment to both exclude files for future reference.
> > > 
> > > Signed-off-by: Eric Whitney <enwlinux@gmail.com>
> > 
> > Can you please make these changes directly to xfstests rather than
> > keeping information about what tests run under what circumstances in
> > some external test wrapper?  i.e. this can be implementing by adding
> > a _requires_no_data_journal() check and adding it to each test.
> 
> We've got a number of these exclusions, and some of them require
> making changes that have been rejected by you previously as adding too
> much hair that is ext4 specific.  (For example, ways of restricting
> the tests from using punch hole or collapse range.)
> 
> So I'm carrying a number of these patches out of tree since they've
> been rejected by upstream, and they are all necessary to make it to
> exclude certain tests based on the ext4 configuration.  I had assumed
> you were philosophically against these sorts of exclusions except by
> creating large numbers of extra xfstests groups,

No, like all patches that people propose for inclusion, I look at
them on a case by case basis and either take them or suggest
better/different ways to implement the desired functionality.

I'm always prepared to change my mind given sufficient
justification/evidence of the benefit of a change. For stuff like
test exclusions and config dependent test execution, the behaviour
of xfstests has always been in flux. The general rule of thumb we've
settled on recently is that if support/exclusion can be cleanly
implemented as a _requires rule, then that's the way it should be
done...

> What would actually be cool is some way of expressing in a config file
> tests that should be excluded given a certain file system
> configuration and kernel version ranges, which would also serve as
> documentation for when certain bugs were fixed/backported to the
> stable kernels. 

We already have expunge files for this. xfstests not in the business
of documenting what bug fixes are in what kernel - it's just a test
harness and a bunch of tests.

Fine grained execution environment constraints are the
responsibility of the user, not the test harness. i.e. The xfstests
harness can do coarse-grained "feature present" checks (i.e.
_requires rules), but information about known bugs in certain
envrionments needs to be provided to xfstests by the buggy
environment...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com