xfstests: change directory to / before _cleanup_testdir in test 135

xfstests: change directory to / before _cleanup_testdir in test 135

[Boris Ranto]

Nfs tries to umount $testdir in _cleanup_testdir function. The test 135 calls the function from directory $SCRATCH_MNT that is equal to $testdir (at least for nfs). The umount will therefore fail causing the test to fail due to the output mismatch.

This simple patch fixes the issue for me.

Signed-off-by: Boris Ranto <branto@redhat.com>

diff -urpN a/xfstests/135 b/xfstests/135
--- a/xfstests/135	2011-03-07 14:54:15.855172101 +0100
+++ b/xfstests/135	2011-03-07 14:54:29.895048375 +0100
@@ -34,6 +34,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 1
 
 _cleanup()
 {
+    cd /
     _cleanup_testdir
 }


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: xfstests: change directory to / before _cleanup_testdir in test 135

[Alex Elder]

On Mon, 2011-03-07 at 16:25 +0100, Boris Ranto wrote:
> Nfs tries to umount $testdir in _cleanup_testdir function. The test 135 calls the function from directory $SCRATCH_MNT that is equal to $testdir (at least for nfs). The umount will therefore fail causing the test to fail due to the output mismatch.
> 
> This simple patch fixes the issue for me.
> 
> Signed-off-by: Boris Ranto <branto@redhat.com>

This looks OK to me.  Most other tests do this chdir
in their cleanup function.

I did a quick scan and found that test 126 may suffer the
same problem.  Can you check this?  We could include the
fix for both tests in the same commit.


It also looks to me like tests 069, 089 might have a
similar issue if they get interrupted.

					-Alex

> diff -urpN a/xfstests/135 b/xfstests/135
> --- a/xfstests/135	2011-03-07 14:54:15.855172101 +0100
> +++ b/xfstests/135	2011-03-07 14:54:29.895048375 +0100
> @@ -34,6 +34,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 1
>  
>  _cleanup()
>  {
> +    cd /
>      _cleanup_testdir
>  }
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: xfstests: change directory to / before _cleanup_testdir in test 135

[Boris Ranto]

On Mon, 2011-03-07 at 14:12 -0600, Alex Elder wrote:
> On Mon, 2011-03-07 at 16:25 +0100, Boris Ranto wrote:
> > Nfs tries to umount $testdir in _cleanup_testdir function. The test 135 calls the function from directory $SCRATCH_MNT that is equal to $testdir (at least for nfs). The umount will therefore fail causing the test to fail due to the output mismatch.
> > 
> > This simple patch fixes the issue for me.
> > 
> > Signed-off-by: Boris Ranto <branto@redhat.com>
> 
> This looks OK to me.  Most other tests do this chdir
> in their cleanup function.
> 
> I did a quick scan and found that test 126 may suffer the
> same problem.  Can you check this?  We could include the
> fix for both tests in the same commit.
> 

Yes, the test needs cd /, too. Actually the test 126 also does double
umount thanks to the _cleanup before exit and the trap command. So the
removal of the call of the _cleanup function before exit is necessary,
too.

> 
> It also looks to me like tests 069, 089 might have a
> similar issue if they get interrupted.

Yes, that's also true, if the tests are interrupted and then immediately
run again the umount will fail due to the processes in background but
I'm not sure whether it is worth fixing (and if there is a good way to
fix it).
I've had bigger problems with double mount/umount in several tests
(namely: double umount - 124, 128, double mount - 129, 130). The double
umounts occur due to the use of trapped _cleanup and umount $SCRATCH_MNT
in the tests. The double mounts occur due to the use of _setup_testdir
and _scratch_mount (both of them mount $SCRATCH_MNT in the nfs case).
The problem is that I'm not sure how to fix these without any change in
the behaviour.

> 
> 					-Alex
> 
> > diff -urpN a/xfstests/135 b/xfstests/135
> > --- a/xfstests/135	2011-03-07 14:54:15.855172101 +0100
> > +++ b/xfstests/135	2011-03-07 14:54:29.895048375 +0100
> > @@ -34,6 +34,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 1
> >  
> >  _cleanup()
> >  {
> > +    cd /
> >      _cleanup_testdir
> >  }
> > 
> > 
> > _______________________________________________
> > xfs mailing list
> > xfs@oss.sgi.com
> > http://oss.sgi.com/mailman/listinfo/xfs
> 
> 
> 


I suppose that at least this patch could be committed, now:

diff -urpN a/xfstests/126 b/xfstests/126
--- a/xfstests/126	2011-03-07 14:52:09.038172203 +0100
+++ b/xfstests/126	2011-03-08 14:18:28.754172294 +0100
@@ -34,6 +34,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 1
 
 _cleanup()
 {
+    cd /
     _cleanup_testdir
 }
 
@@ -73,5 +74,4 @@ $QA_FS_PERMS 040 99 99 99 500 r 1
 $QA_FS_PERMS 400 99 99 200 99 r 1
 
 status=0
-_cleanup
 exit
diff -urpN a/xfstests/135 b/xfstests/135
--- a/xfstests/135	2011-03-07 14:54:15.855172101 +0100
+++ b/xfstests/135	2011-03-07 14:54:29.895048375 +0100
@@ -34,6 +34,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 1
 
 _cleanup()
 {
+    cd /
     _cleanup_testdir
 }

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: xfstests: change directory to / before _cleanup_testdir in test 135

[Alex Elder]

On Tue, 2011-03-08 at 14:29 +0100, Boris Ranto wrote:
> On Mon, 2011-03-07 at 14:12 -0600, Alex Elder wrote:
> > On Mon, 2011-03-07 at 16:25 +0100, Boris Ranto wrote:
> > > Nfs tries to umount $testdir in _cleanup_testdir function. The test 135 calls the function from directory $SCRATCH_MNT that is equal to $testdir (at least for nfs). The umount will therefore fail causing the test to fail due to the output mismatch.
> > > 
> > > This simple patch fixes the issue for me.
> > > 
> > > Signed-off-by: Boris Ranto <branto@redhat.com>
> > 
> > This looks OK to me.  Most other tests do this chdir
> > in their cleanup function.
> > 
> > I did a quick scan and found that test 126 may suffer the
> > same problem.  Can you check this?  We could include the
> > fix for both tests in the same commit.
> > 
> 
> Yes, the test needs cd /, too. Actually the test 126 also does double
> umount thanks to the _cleanup before exit and the trap command. So the
> removal of the call of the _cleanup function before exit is necessary,
> too.

I will commit this patch, fixing the problems in 126 and 135.
I'll massage the commit message a bit to fit what it does,
and the your discussion here.

I'm not going to do anything (at the moment) about the
double mount/umount issues you mention.

Thanks for submitting the fixes.

					-Alex

> > 
> > It also looks to me like tests 069, 089 might have a
> > similar issue if they get interrupted.
> 
> Yes, that's also true, if the tests are interrupted and then immediately
> run again the umount will fail due to the processes in background but
> I'm not sure whether it is worth fixing (and if there is a good way to
> fix it).
> I've had bigger problems with double mount/umount in several tests
> (namely: double umount - 124, 128, double mount - 129, 130). The double
> umounts occur due to the use of trapped _cleanup and umount $SCRATCH_MNT
> in the tests. The double mounts occur due to the use of _setup_testdir
> and _scratch_mount (both of them mount $SCRATCH_MNT in the nfs case).
> The problem is that I'm not sure how to fix these without any change in
> the behaviour.
> 
> > 
> > 					-Alex
> > 
. . .

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs