generic/074: _cleanup potentially removes local FS files
PATCH 1/1 generic/074: _cleanup potentially removes local FS files
fstest_dir is set after _cleanup() is defined and _require_test is called. If _require_test fails (due to some unrelated bug in tested FS), _cleanup will attempt to remove $fstest_dir.*, which expands to .* This has the unfortunate effect of removing xfstests' .git and .gitignore.
Here is the 074.out.bad file for this case:
QA output created by 074
mount: permission denied
common/rc: retrying test device mount with external set
mount: permission denied
common/rc: could not mount /dev/pmem0 on /mnt
rm: refusing to remove '.' or '..' directory: skipping '.'
rm: refusing to remove '.' or '..' directory: skipping '..'
----
The only other test which could face the same problem is generic/285. However, that test defines _cleanup after the _require* calls, and so on _require_test failure, 285.out.bad contains the harmless:
./common/rc: line 1: _cleanup: command not found
--- tests/generic/074 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
tests/generic/074 +2 -1
@@ -33,7 +33,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _cleanup() { - rm -rf $fstest_dir.* $tmp.* + [ -n "$fstest_dir" ] && rm -rf $fstest_dir.* + rm -rf $tmp.* } # get standard environment, filters and checks
Signature · 1 line
2.9.3
Quoted 16 lines · Omer Zilberberg
fstest_diris set after_cleanup()is defined and_require_testis called. If_require_testfails (due to some unrelated bug in tested FS),_cleanupwill attempt to remove $fstest_dir.*, which expands to .* This has the unfortunate effect of removing xfstests' .git and .gitignore.Here is the 074.out.bad file for this case:
QA output created by 074
mount: permission denied
common/rc: retrying test device mount with external set
mount: permission denied
common/rc: could not mount/dev/pmem0on /mnt
rm: refusing to remove '.' or '..' directory: skipping '.'
rm: refusing to remove '.' or '..' directory: skipping '..'
----
Quoted 4 lines
The only other test which could face the same problem is generic/285. However, that test defines
_cleanupafter the _require* calls, and so on_require_testfailure, 285.out.bad contains the harmless:
./common/rc: line 1:_cleanup: command not found
How about defining fstest_dir before _cleanup function? Along with "here" and "tmp", which looks clearer and makes more sense to me.
Thanks,
Eryu
Quoted 27 lines
Signed-off-by Omer Zilberberg <[email protected]>--- tests/generic/074 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)tests/generic/074 +2
-1@@ -33,7 +33,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _cleanup() { - rm -rf $fstest_dir.* $tmp.* + [ -n "$fstest_dir" ] && rm -rf $fstest_dir.* + rm -rf $tmp.* } # get standard environment, filters and checksSignature · 6 lines
2.9.3 -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 17 lines · Eryu Guan
Quoted 16 lines · Omer Zilberberg
fstest_diris set after_cleanup()is defined and_require_testis called. If_require_testfails (due to some unrelated bug in tested FS),_cleanupwill attempt to remove $fstest_dir.*, which expands to .* This has the unfortunate effect of removing xfstests' .git and .gitignore.Here is the 074.out.bad file for this case:
QA output created by 074
mount: permission denied
common/rc: retrying test device mount with external set
mount: permission denied
common/rc: could not mount/dev/pmem0on /mnt
rm: refusing to remove '.' or '..' directory: skipping '.'
rm: refusing to remove '.' or '..' directory: skipping '..'
----
Quoted 7 lines
Quoted 4 lines
The only other test which could face the same problem is generic/285. However, that test defines
_cleanupafter the _require* calls, and so on_require_testfailure, 285.out.bad contains the harmless:
./common/rc: line 1:_cleanup: command not foundHow about defining
fstest_dirbefore_cleanupfunction? Along with "here" and "tmp", which looks clearer and makes more sense to me.
Sure, no problem. I'll send a new patch momentarily. Thanks for the review
Quoted 31 lines
Thanks,
EryuQuoted 27 lines
Signed-off-by Omer Zilberberg <[email protected]>--- tests/generic/074 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)tests/generic/074 +2
-1@@ -33,7 +33,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _cleanup() { - rm -rf $fstest_dir.* $tmp.* + [ -n "$fstest_dir" ] && rm -rf $fstest_dir.* + rm -rf $tmp.* } # get standard environment, filters and checksSignature · 6 lines
2.9.3 -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html