273: don't delete everything if $SCRATCH_MNT isn't set
PATCH 1/1 273: don't delete everything if $SCRATCH_MNT isn't set
The cleanup function is trapped before _require_scratch() is called, and then expands "rm -rf $SCRATCH_MNT/*" to "rm -rf /*"
--- 273 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
273 +5 -1
@@ -35,7 +35,11 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _cleanup() { cd / - rm -rf $SCRATCH_MNT/* $tmp.* + if [ ! -z $SCRATCH_DEV ] && [ ! -z $SCRATCH_MNT ]; + then + rm -rf $SCRATCH_MNT + fi + rm -rf $tmp.* _scratch_unmount }
Signature · 6 lines
1.7.10 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 2 lines · Bryan Schumaker
The cleanup function is trapped before
_require_scratch()is called, and then expands "rm -rf $SCRATCH_MNT/*" to "rm -rf /*"
It doesn't need to clean up the scratch device - tests need to mkfs it first before using it, so just remove the $tmp* files. The sae fix is needed for tests 274 and 275.
Also, it shouldn't be using "-rf" to do cleanup - "rm -f $tmp.*" is sufficient to remove all the tmp files...
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 10 lines · Dave Chinner
Quoted 2 lines · Bryan Schumaker
The cleanup function is trapped before
_require_scratch()is called, and then expands "rm -rf $SCRATCH_MNT/*" to "rm -rf /*"It doesn't need to clean up the scratch device - tests need to mkfs it first before using it, so just remove the $tmp* files. The sae fix is needed for tests 274 and 275.
Also, it shouldn't be using "-rf" to do cleanup - "rm -f $tmp.*" is sufficient to remove all the tmp files...
Thanks for looking! Maybe something like this? (copying what 272 does):
273 +13 -9
@@ -30,14 +30,7 @@ echo "QA output created by $seq" here=`pwd` tmp=/tmp/$$ status=0 # success is the default! -trap "_cleanup; exit \$status" 0 1 2 3 15 - -_cleanup() -{ - cd / - rm -rf $SCRATCH_MNT/* $tmp.* - _scratch_unmount -} +trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15 . ./common.rc . ./common.filter
@@ -138,7 +131,18 @@ umount $SCRATCH_DEV 2>/dev/null _scratch_mkfs_sized $((2 * 1024 * 1024 * 1024)) >>$seq.full 2>&1 _scratch_mount -_do_workload +if ! _do_workload; then + echo "workout failed" + _scratch_unmount + status=1 + exit +fi + +if ! _scratch_unmount; then + echo "failed to umount" + status=1 + exit +fi _check_scratch_fs status=$?
Signature · 3 lines
1.7.10 - Bryan
Quoted 4 lines
Cheers,
Dave.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 10 lines · Bryan Schumaker
Quoted 8 lines · Dave Chinner
Quoted 2 lines · Bryan Schumaker
The cleanup function is trapped before
_require_scratch()is called, and then expands "rm -rf $SCRATCH_MNT/*" to "rm -rf /*"It doesn't need to clean up the scratch device - tests need to mkfs it first before using it, so just remove the $tmp* files. The sae fix is needed for tests 274 and 275.
Quoted 5 lines
Quoted 2 lines
Also, it shouldn't be using "-rf" to do cleanup - "rm -f $tmp.*" is sufficient to remove all the tmp files...
Thanks for looking! Maybe something like this? (copying what 272 does):
This is actually not needed at all. Simply removing the 'rm -rf $SCRATCH_MNT/*' line should be good enough :).
There is no need to remove anything from SCRATCH_DEV on cleanup, as well as prior to actual testing.
And Dave is right, there are more tests like this which has to be fixed.
Quoted 42 lines
273 +13
-9@@ -30,14 +30,7 @@ echo "QA output created by $seq" here=`pwd` tmp=/tmp/$$ status=0 # success is the default! -trap "_cleanup; exit \$status" 0 1 2 3 15 - -_cleanup() -{ - cd / - rm -rf $SCRATCH_MNT/* $tmp.* - _scratch_unmount -} +trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15 . ./common.rc . ./common.filter@@ -138,7 +131,18 @@ umount $SCRATCH_DEV 2>/dev/null _scratch_mkfs_sized $((2 * 1024 * 1024 * 1024)) >>$seq.full 2>&1 _scratch_mount -_do_workload +if ! _do_workload; then + echo "workout failed" + _scratch_unmount + status=1 + exit +fi + +if ! _scratch_unmount; then + echo "failed to umount" + status=1 + exit +fi _check_scratch_fs status=$?
Signature · 4 lines
_______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Dave Chinner
Quoted 2 lines · Bryan Schumaker
The cleanup function is trapped before
_require_scratch()is called, and then expands "rm -rf $SCRATCH_MNT/*" to "rm -rf /*"It doesn't need to clean up the scratch device - tests need to mkfs it first before using it, so just remove the $tmp* files. The sae fix is needed for tests 274 and 275.
Given his address I assume Bryan might test on NFS, which has funky semantics for the scratch device.
Is that true Bryan? I really need to dive back into how we handle NFS in this respect to have a good judgement on this patch.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 9 lines · Christoph Hellwig
Quoted 7 lines · Dave Chinner
Quoted 2 lines · Bryan Schumaker
The cleanup function is trapped before
_require_scratch()is called, and then expands "rm -rf $SCRATCH_MNT/*" to "rm -rf /*"It doesn't need to clean up the scratch device - tests need to mkfs it first before using it, so just remove the $tmp* files. The sae fix is needed for tests 274 and 275.
Quoted 6 lines
Given his address I assume Bryan might test on NFS, which has funky semantics for the scratch device.
Is that true Bryan? I really need to dive back into how we handle NFS in this respect to have a good judgement on this patch.
Yeah, I'm testing on NFS (I guess the netapp address is a bit of a giveaway...). I don't think this test runs on NFS, but if the scratch variables are unset then it'll still delete the root directory of the NFS client.
- Bryan
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs