xfstests-bld: add exclude file for ext3 tests
PATCH 1/1 xfstests-bld: add exclude file for ext3 tests
Add an exclude file for the ext3 test case to prevent failure reports from tests that exercise unsupported online defrag functionality.
Two online defrag tests - ext4/307 and /308 - are not included because they contain explicit requirements for fallocate support that prevents them from running on an emulated ext3 file system.
It may be possible to modify the excluded tests at a future date so they will not run unless the test file system is extent mapped. For now, this patch is an expedient measure to reduce testing noise.
--- kvm-xfstests/test-appliance/files/root/conf/ext3.exclude | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 kvm-xfstests/test-appliance/files/root/conf/ext3.exclude
kvm-xfstests/test-appliance/files/root/conf/ext3.exclude new file +6 -0
@@ -0,0 +1,6 @@ +# ext4's ext3 emulation does not support on-line +# defrag, which requires extent-based files +ext4/301 +ext4/302 +ext4/303 +ext4/304
Signature · 1 line
2.1.4
Quoted 2 lines · Eric Whitney
Add an exclude file for the ext3 test case to prevent failure reports from tests that exercise unsupported online defrag functionality.
ext3 should not run these tests because of the _requires_defrag check in these tests results in a _notrun command being run when FSTYP=ext3.
Quoted 3 lines
Two online defrag tests - ext4/307 and /308 - are not included because they contain explicit requirements for fallocate support that prevents them from running on an emulated ext3 file system.
Same here.
Quoted 16 lines
It may be possible to modify the excluded tests at a future date so they will not run unless the test file system is extent mapped. For now, this patch is an expedient measure to reduce testing noise.
Signed-off-by Eric Whitney <[email protected]>--- kvm-xfstests/test-appliance/files/root/conf/ext3.exclude | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 kvm-xfstests/test-appliance/files/root/conf/ext3.excludekvm-xfstests/test-appliance/files/root/conf/ext3.exclude new file +0
-0
kvm-xfstests/test-appliance/files/root/conf/ext3.exclude line 6
Quoted 6 lines
kvm-xfstests/test-appliance/files/root/conf/ext3.exclude new file +6
-0@@ -0,0 +1,6 @@ +# ext4's ext3 emulation does not support on-line +# defrag, which requires extent-based files +ext4/301 +ext4/302 +ext4/303 +ext4/304
So what we really need to understand is why _require_defrag is not preventing these tests beign run on an ext3 filesystem?
Eric, what is the output of these tests when it is run on an ext3 filesytsem? What is the fstyp you are testing?
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 7 lines · Dave Chinner
Quoted 2 lines · Eric Whitney
Add an exclude file for the ext3 test case to prevent failure reports from tests that exercise unsupported online defrag functionality.
ext3 should not run these tests because of the
_requires_defragcheck in these tests results in a_notruncommand being run when FSTYP=ext3.
In this particular case, the test is being run with
FSTYP=ext4
MKFS_OPTIONS="-q -O ^extents,^flex_bg,^uninit_bg,^64bit,^metadata_csum,^huge_file,^dir_nlink,^extra_isize" EXT_MOUNT_OPTIONS="nodelalloc"
This is why _requires_defrag is passing and so ext4/307 and ext4/308 is allowed to run.
Historically, this test was set up in this fashion because I wanted to make sure the ext4 kernel code would be used, and not the code found in fs/ext3 (this was before fs/ext3 was removed from the kernel). Since we want to support testing on ancient kernels (either 3.10 android kernels or 2.6.34 RHEL6 kernels), using this strategy for testing ext4's support for file systems with ext3 features is something that still makes sense to do, and so I would want to keep running tests using this setup.
Quoted 5 lines
Quoted 3 lines
Two online defrag tests - ext4/307 and /308 - are not included because they contain explicit requirements for fallocate support that prevents them from running on an emulated ext3 file system.
Same here.
So probably the right answer here is to change _require_defrag so that for EXT4, to also add the assertion:
_require_xfs_io_command "falloc"
I'll note this is a not quite guaranteed to be correct because _require_xfs_io_command tests to see whether or not falloc works on TEST_DEV, and these tests are actually create a test file system on SCRATCH_DEV, and in theory TEST_DEV and SCRATCH_DEV could have different file system features. That's because xfstests never runs mkfs on TEST_DEV, but SCRATCH_DEV does get mkfs'ed and in theory the file system features set by MKFS_OPTIONS could be different from what exists on TEST_DEV.
I'm willing to consider this a test configuration error, and certainly kvm-xfstests doesn't ever set up such a arguably non-sensible test configuration.
- Ted
Quoted 5 lines · Theodore Ts'o
So probably the right answer here is to change
_require_defragso that for EXT4, to also add the assertion:
_require_xfs_io_command"falloc"
So like this... (BTW what's up with my other pending xfstests patches? Would you like me to resend a whole back with the reviewed-by lines merged in?)
- Ted
Quoted 1 line
From
a1cd89ca74d8e8cd345b88e62a5eabab70aed62eMon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sun, 21 Feb 2016 00:12:00 -0500
Subject: [PATCH] defrag: require falloc support for ext4 defrag
The e4defrag program requires the use of fallocate. Certain ext4 file system configurations which don't enable extents do not support fallocate, so we should skip the defrag tests in that case.
--- common/defrag | 1 + 1 file changed, 1 insertion(+)
common/defrag +1 -0
@@ -28,6 +28,7 @@ _require_defrag() ;; ext4|ext4dev) DEFRAG_PROG="$E4DEFRAG_PROG" + _require_xfs_io_command "falloc" ;; btrfs) DEFRAG_PROG="$BTRFS_UTIL_PROG filesystem defragment"
Signature · 1 line
2.5.0
Quoted 9 lines · Theodore Ts'o
Quoted 7 lines · Dave Chinner
Quoted 2 lines · Eric Whitney
Add an exclude file for the ext3 test case to prevent failure reports from tests that exercise unsupported online defrag functionality.
ext3 should not run these tests because of the
_requires_defragcheck in these tests results in a_notruncommand being run when FSTYP=ext3.
Quoted 8 lines
In this particular case, the test is being run with
FSTYP=ext4
MKFS_OPTIONS="-q -O ^extents,^flex_bg,^uninit_bg,^64bit,^metadata_csum,^huge_file,^dir_nlink,^extra_isize" EXT_MOUNT_OPTIONS="nodelalloc"This is why
_requires_defragis passing and so ext4/307 and ext4/308 is allowed to run.
Ok, I suspected that this was the reason for it occurring, but that's not really testing "ext3" in the true sense - it's a modified ext4 on-disk variant that happens to match the ext3 configuration.
Quoted 11 lines
Quoted 5 lines
Quoted 3 lines
Two online defrag tests - ext4/307 and /308 - are not included because they contain explicit requirements for fallocate support that prevents them from running on an emulated ext3 file system.
Same here.
So probably the right answer here is to change
_require_defragso that for EXT4, to also add the assertion:
_require_xfs_io_command"falloc"
Quoted 8 lines
I'll note this is a not quite guaranteed to be correct because
_require_xfs_io_commandtests to see whether or not falloc works onTEST_DEV, and these tests are actually create a test file system onSCRATCH_DEV, and in theoryTEST_DEVandSCRATCH_DEVcould have different file system features. That's because xfstests never runs mkfs onTEST_DEV, butSCRATCH_DEVdoes get mkfs'ed and in theory the file system features set byMKFS_OPTIONScould be different from what exists onTEST_DEV.
Right - that's normally why we have "_require_scratch_foo" so that it's specific which device requires that functionality. Seems easy enough to fix that problem.
back to the ext4-as-ext3, defrag requires extents to be enabled in the ext4 filesystem, right? So you could check whether than flag is set easily enough with something like this:
dumpe2fs -h $dev |grep features |grep -q extent
if [ $? -ne 0 ]; then
_notrun "Filesystem does not support defrag operations"
fi
That would work for everything with a fstyp of ext4, wouldn't it?
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Quoted 4 lines · Dave Chinner
Ok, I suspected that this was the reason for it occurring, but that's not really testing "ext3" in the true sense - it's a modified ext4 on-disk variant that happens to match the ext3 configuration.
Yes, although this is also how ext4 emulates fs/ext3 behavior -- either because fs/ext3 has been removed, as it has in newer kernels, or because (as most distributions have been shipping for a while) fs/ext3 has been disabled via Kconfig, and CONFIG_EXT4_USE_FOR_EXT23 is enabled. I'm not doing this any more, but for a while I had been using an ext3 file system for the root file system in the KVM images so I could do things with ext4 for testing purposes without interfering with the ability of the KVM image to boot. So in practice it is effectively the same as testing "ext3" using some white-box knowledge of how ext4 emulates ext3 under the covers.
Quoted 9 lines
Quoted 8 lines
I'll note this is a not quite guaranteed to be correct because
_require_xfs_io_commandtests to see whether or not falloc works onTEST_DEV, and these tests are actually create a test file system onSCRATCH_DEV, and in theoryTEST_DEVandSCRATCH_DEVcould have different file system features. That's because xfstests never runs mkfs onTEST_DEV, butSCRATCH_DEVdoes get mkfs'ed and in theory the file system features set byMKFS_OPTIONScould be different from what exists onTEST_DEV.
Quoted 3 lines
Right - that's normally why we have "
_require_scratch_foo" so that it's specific which device requires that functionality. Seems easy enough to fix that problem.
For what it's worth here are the list of generic tests that (a) have _require_scratch and (b) have _require_xfs_io_command "falloc":
032 038 042 071 094 103 223 250 252 274 299 300 312 324
So it probably does make sense to have a _require_scratch_falloc helper macro, although in practice I rather doubt folks have been tripped up by this until now. Still, it would be more correct.
Alternative, it might be interesting to consider would be a test at the beginning of the xfstests run where the feature set for the TEST_DEV is compared against SCRATCH_DEV when created using _scratch_mkfs and if they differ, to issue a warning. This would be an ext4-specific thing, and I'm not sure if you would consider this within scope of xfstests, or whether this is something that belongs in a test-runner framework such as gce-xfstests. I can see arguments for this either way, although I would lean towards putting such a sanity check in kvm-xfstests/gce-xfstests. What is your preference?
Quoted 10 lines
back to the ext4-as-ext3, defrag requires extents to be enabled in the ext4 filesystem, right? So you could check whether than flag is set easily enough with something like this:
dumpe2fs -h $dev |grep features |grep -q extent
if [ $? -ne 0 ]; then
_notrun"Filesystem does not support defrag operations"
fiThat would work for everything with a fstyp of ext4, wouldn't it?
Yes, or use something like a _require_scratch_falloc helper.
- Ted
Quoted 14 lines · Theodore Ts'o
Quoted 9 lines · Dave Chinner
Quoted 8 lines
I'll note this is a not quite guaranteed to be correct because
_require_xfs_io_commandtests to see whether or not falloc works onTEST_DEV, and these tests are actually create a test file system onSCRATCH_DEV, and in theoryTEST_DEVandSCRATCH_DEVcould have different file system features. That's because xfstests never runs mkfs onTEST_DEV, butSCRATCH_DEVdoes get mkfs'ed and in theory the file system features set byMKFS_OPTIONScould be different from what exists onTEST_DEV.Quoted 3 lines
Right - that's normally why we have "
_require_scratch_foo" so that it's specific which device requires that functionality. Seems easy enough to fix that problem.
Quoted 8 lines
For what it's worth here are the list of generic tests that (a) have
_require_scratchand (b) have_require_xfs_io_command"falloc":032 038 042 071 094 103 223 250 252 274 299 300 312 324
So it probably does make sense to have a
_require_scratch_fallochelper macro, although in practice I rather doubt folks have been tripped up by this until now. Still, it would be more correct.
Well, that's a different problem, unrealted to the fact that _require_defrag is giving the wrong answer to certain ext4 configurations.
As it is, I'm not sure that it's at all common that two filesystems of the same type on the same kernel will give different answers to fallocate. That seems like a corner case of ext4's whacky feature matrix, and so is is not something we typically need to care about, nor is it something anyone should have to remember when writing or reviewing a new test....
Quoted 9 lines
Alternative, it might be interesting to consider would be a test at the beginning of the xfstests run where the feature set for the
TEST_DEVis compared againstSCRATCH_DEVwhen created using_scratch_mkfsand if they differ, to issue a warning. This would be an ext4-specific thing, and I'm not sure if you would consider this within scope of xfstests, or whether this is something that belongs in a test-runner framework such as gce-xfstests. I can see arguments for this either way, although I would lean towards putting such a sanity check in kvm-xfstests/gce-xfstests. What is your preference?
I don't think it's particularly sane to make xfstests try to support ext3 configurations with FSTYP=ext4 when you can just use FSTYP=ext3 and everything just works.
Quoted 4 lines
Quoted 4 lines
back to the ext4-as-ext3, defrag requires extents to be enabled in the ext4 filesystem, right? So you could check whether than flag is set easily enough with something like this:
Quoted 8 lines
Quoted 6 lines
dumpe2fs -h $dev |grep features |grep -q extent
if [ $? -ne 0 ]; then
_notrun"Filesystem does not support defrag operations"
fiThat would work for everything with a fstyp of ext4, wouldn't it?
Yes, or use something like a
_require_scratch_fallochelper.
I'd much prefer that we fix the _require_defrag helper to correctly determine if an ext4 filesystem can support defrag rather than overload some other helper with undocumented behaviour that may be completely unrelated to the test at hand and rely on people to remember that.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]