| Message ID | 1440757500.29614.26.camel@ari-macbook (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 28, 2015 at 01:25:00PM +0300, Ari Sundholm wrote: > From: Ari Sundholm <ari@tuxera.com> > > There are filesystems that do not support hard links that we want to run > xfstests on. Add a function for checking for support and use it wherever > hard links are used. As I've already pointed out, all of the filesystems that xfstests supports have working hardlinks, so this is a test that will never fail for anyone running xfstests on supported filesystems. Also, I'm left to wonder what kernel filesystem supports journaling but does not support hardlinks (as noted by the number of tests you change that have "_require_metadata_journaling"). Why aren't you also posting the patches to support the filesystem that requires this "don't use hardlinks" patch? And FWIW: > +_require_test_hardlinks() > +{ > + target=`mktemp -p $TEST_DIR` > + link=`mktemp -p $TEST_DIR -u` > + ln `basename $target` $link > + if [ "$?" -ne 0 ]; then > + rm -f $target > + _notrun "Require hardlinks support" > + fi > + rm -f $target $link > +} You check for support on the TEST_DEV, but ... > diff --git a/tests/generic/039 b/tests/generic/039 > index 4bbfc50..367a6d0 100755 > --- a/tests/generic/039 > +++ b/tests/generic/039 > @@ -55,6 +55,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > _supported_fs generic > _supported_os Linux > _need_to_be_root > +_require_test_hardlinks > _require_scratch > _require_dm_flakey > _require_metadata_journaling $SCRATCH_DEV All but two of the tests you've added this check to make hard links on the SCRATCH_DEV, which can have a different configuration to the TEST_DEV. Cheers, Dave.
On Mon, Aug 31, 2015 at 09:43:33AM +1000, Dave Chinner wrote: > On Fri, Aug 28, 2015 at 01:25:00PM +0300, Ari Sundholm wrote: > > From: Ari Sundholm <ari@tuxera.com> > > > > There are filesystems that do not support hard links that we want to run > > xfstests on. Add a function for checking for support and use it wherever > > hard links are used. > > As I've already pointed out, all of the filesystems that xfstests > supports have working hardlinks, so this is a test that will never > fail for anyone running xfstests on supported filesystems. > > Also, I'm left to wonder what kernel filesystem supports > journaling but does not support hardlinks (as noted by the number of > tests you change that have "_require_metadata_journaling"). Why > aren't you also posting the patches to support the filesystem that > requires this "don't use hardlinks" patch? Just to clarify this: I don't care if people are using xfstests to test proprietary filesystem modules. I'm happy to add proper support for just about any filesystem, but that support needs to be *in full* so *anyone* can run the tests on that filesystem. What I really don't like is people being evasive about the reason they want something changed. If there isn't a clear, full and convincing explanation of why a change should be made, then I will push back until such an explanation is given. Cheers, Dave.
diff --git a/common/rc b/common/rc index 780b7ed..d3aeddf 100644 --- a/common/rc +++ b/common/rc @@ -2714,6 +2714,18 @@ _require_test_symlinks() rm -f $target $link } +_require_test_hardlinks() +{ + target=`mktemp -p $TEST_DIR` + link=`mktemp -p $TEST_DIR -u` + ln `basename $target` $link + if [ "$?" -ne 0 ]; then + rm -f $target + _notrun "Require hardlinks support" + fi + rm -f $target $link +} + _require_test_fcntl_advisory_locks() { [ "$FSTYP" != "cifs" ] && return 0 diff --git a/tests/generic/002 b/tests/generic/002 index f63b208..0b96709 100755 --- a/tests/generic/002 +++ b/tests/generic/002 @@ -43,6 +43,7 @@ _cleanup() # real QA test starts here _supported_fs generic _supported_os IRIX Linux +_require_test_hardlinks _require_test echo "Silence is goodness ..." diff --git a/tests/generic/039 b/tests/generic/039 index 4bbfc50..367a6d0 100755 --- a/tests/generic/039 +++ b/tests/generic/039 @@ -55,6 +55,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fs generic _supported_os Linux _need_to_be_root +_require_test_hardlinks _require_scratch _require_dm_flakey _require_metadata_journaling $SCRATCH_DEV diff --git a/tests/generic/040 b/tests/generic/040 index c841fbc..1b8287f 100755 --- a/tests/generic/040 +++ b/tests/generic/040 @@ -60,6 +60,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fs generic _supported_os Linux _need_to_be_root +_require_test_hardlinks _require_scratch _require_dm_flakey _require_metadata_journaling $SCRATCH_DEV diff --git a/tests/generic/041 b/tests/generic/041 index f38b662..588d5ff 100755 --- a/tests/generic/041 +++ b/tests/generic/041 @@ -64,6 +64,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fs generic _supported_os Linux _need_to_be_root +_require_test_hardlinks _require_scratch _require_dm_flakey _require_metadata_journaling $SCRATCH_DEV diff --git a/tests/generic/056 b/tests/generic/056 index 8bb1522..e111bec 100755 --- a/tests/generic/056 +++ b/tests/generic/056 @@ -53,6 +53,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fs generic _supported_os Linux _need_to_be_root +_require_test_hardlinks _require_scratch _require_dm_flakey _require_metadata_journaling $SCRATCH_DEV diff --git a/tests/generic/057 b/tests/generic/057 index 3b9f89e..d3b1aa2 100755 --- a/tests/generic/057 +++ b/tests/generic/057 @@ -53,6 +53,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fs generic _supported_os Linux _need_to_be_root +_require_test_hardlinks _require_scratch _require_dm_flakey _require_metadata_journaling $SCRATCH_DEV diff --git a/tests/generic/065 b/tests/generic/065 index 739a4d5..5403c53 100755 --- a/tests/generic/065 +++ b/tests/generic/065 @@ -54,6 +54,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fs generic _supported_os Linux _need_to_be_root +_require_test_hardlinks _require_scratch _require_dm_flakey _require_metadata_journaling $SCRATCH_DEV diff --git a/tests/generic/066 b/tests/generic/066 index cb36506..51aa22a 100755 --- a/tests/generic/066 +++ b/tests/generic/066 @@ -58,6 +58,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fs generic _supported_os Linux _need_to_be_root +_require_test_hardlinks _require_scratch _require_dm_flakey _require_attrs diff --git a/tests/generic/084 b/tests/generic/084 index 3fec6c2..86c38db 100755 --- a/tests/generic/084 +++ b/tests/generic/084 @@ -46,6 +46,7 @@ _cleanup() # real QA test starts here _supported_fs generic _supported_os Linux +_require_test_hardlinks _require_scratch link_unlink_storm() diff --git a/tests/generic/090 b/tests/generic/090 index a1f2b89..c3eeb1a 100755 --- a/tests/generic/090 +++ b/tests/generic/090 @@ -52,6 +52,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fs generic _supported_os Linux _need_to_be_root +_require_test_hardlinks _require_scratch _require_dm_flakey _require_metadata_journaling $SCRATCH_DEV diff --git a/tests/generic/104 b/tests/generic/104 index eeb7363..9fed5f2 100755 --- a/tests/generic/104 +++ b/tests/generic/104 @@ -48,6 +48,7 @@ _cleanup() _need_to_be_root _supported_fs generic _supported_os Linux +_require_test_hardlinks _require_scratch _require_dm_flakey _require_metadata_journaling $SCRATCH_DEV diff --git a/tests/generic/106 b/tests/generic/106 index 0afee41..59efe7c 100755 --- a/tests/generic/106 +++ b/tests/generic/106 @@ -47,6 +47,7 @@ _cleanup() _need_to_be_root _supported_fs generic _supported_os Linux +_require_test_hardlinks _require_scratch _require_dm_flakey _require_metadata_journaling $SCRATCH_DEV diff --git a/tests/generic/236 b/tests/generic/236 index 12ea0bc..df0250f 100755 --- a/tests/generic/236 +++ b/tests/generic/236 @@ -43,6 +43,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fs generic # only Linux supports fallocate _supported_os Linux +_require_test_hardlinks _require_test rm -f $TEST_DIR/ouch* diff --git a/tests/generic/294 b/tests/generic/294 index 3fa6ba2..944b6a1 100755 --- a/tests/generic/294 +++ b/tests/generic/294 @@ -46,6 +46,7 @@ _cleanup() # Modify as appropriate. _supported_fs generic _supported_os Linux +_require_test_hardlinks _require_scratch rm -f $seqres.full