Subject: Re: [vserver] testfs.sh-0.20 patches
From: Corey Wright <undefined@pobox.com>
Date: Fri, 21 Aug 2009 00:07:39 -0500
Fri, 21 Aug 2009 00:07:39 -0500
On Thu, 20 Aug 2009 15:18:51 +0200
Herbert Poetzl <herbert@13thfloor.at> wrote:

> On Wed, Aug 19, 2009 at 01:48:33PM -0500, Corey Wright wrote:
> > change expected result to fail (0 -> 1) as trying to execute chattr on a
> > barrier from context 2 is expected to fail.
> > 
> > this test is problematic because some versions of chattr (e2fsprogs
> > 1.40.8-2ubuntu2 in ubuntu hardy) return success even though the barrier
> > is not visible to chattr (ie "chattr: No such file or directory while
> > trying to stat <barrier>").  so even though chattr failed to act on the
> > barrier (as it could not see it), it still reports success (ie exit
> > code 0).
> > 
> > todo/suggestion: first stat the barrier and only try to chattr the
> > barrier if stat is successful (because if stat fails, then chattr will
> > definitely fail).
> 
> should probably be the other way round, i.e.
> chattr && stat 
> so that the chattr is executed in any case, and
> the effect on the directory can be checked in
> a second step ...

how about exec'ing chattr instead of eval'ing it (as chattr is too buggy to
test with) and instead test the xattrs (using showattr & lsattr as done
elsewhere in the code) after the attempted chattr to insure it didn't change
them?

(see comment/explanation at top of patch.)

corey
-- 
undefined@pobox.com


testing the return value of chattr is useless:
1. sometimes returns success when the barrier couldn't even be stat'ed
2. returns success when the barrier couldn't be chattr'ed (permission denied)

the only meaningful test (besides testing chattr's stderr) is to verify the
barrier's xattrs didn't change after running chattr.  this is also easiest to
implement as it is already done elsewhere in the code (cut & paste).

so, eeval chattr instead of eexec'ing it, and add two tests verifying the
xattrs (using both showattr & lsattr).  number the tests 105a & 105b, though
only "105" is printed for both tests.  only one test is probably needed
(preference to showattr as it dumps all xattrs), but just replicating what was
done in tests 102 & 103 (showattr & lsattr, respectively).

--- testfs.sh-0.20b	2009-08-20 19:45:48.000000000 -0500
+++ testfs.sh-0.20c	2009-08-20 20:36:02.000000000 -0500
@@ -494,7 +494,9 @@ function do_test () {
 	eexec 102	0 "setattr --barrier $dpath"
 	eexec 103	0 "do_xattr_verify $MNT showattr $dpath $attr_B"
 	eexec 104	0 "do_xattr_verify $MNT lsattr $dpath $lsattr_B"
-	eexec 105	1 "go_xid 2 chattr =i $dpath"
+	eeval		  "go_xid 2 chattr =i $dpath"
+	eexec 105a	0 "do_xattr_verify $MNT showattr $dpath $attr_B"
+	eexec 105b	0 "do_xattr_verify $MNT lsattr $dpath $lsattr_B"
 	eexec 106	0 "do_xattr_barrier $MNT $dpath ..^"
 	eexec 108	0 "setattr --~barrier $dpath"
 	eexec 109	0 "do_xattr_verify $MNT showattr $dpath $attr_b"