Subject: Re: [vserver] testfs.sh-0.20 patches
From: Corey Wright <undefined@pobox.com>
Date: Fri, 21 Aug 2009 17:06:55 -0500
Fri, 21 Aug 2009 17:06:55 -0500
On Fri, 21 Aug 2009 20:41:53 +0200
Herbert Poetzl <herbert@13thfloor.at> wrote:

> On Fri, Aug 21, 2009 at 08:48:46AM -0400, Mark Little wrote:
> > On Fri, 21 Aug 2009 10:03:07 +0200, Herbert Poetzl
> > <herbert@13thfloor.at> wrote:
> > > On Fri, Aug 21, 2009 at 12:07:39AM -0500, Corey Wright wrote:
> > >> 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?
> > > 
> > > close, but not perfect, we should at least (explicitly or
> > > implicitly) ensure that chattr _exists_ and was executed,
> > > because otherwise checking for changes doesn't make much
> > > sense (i.e. will give a false positive)
> > > 
> > > if that is done somehow, we can forget about the chattr
> > > return code completely ...
> > 
> > Could you first do a test of creating something in /tmp (on the host,
> > not in a guest context) and then chattr and verify that the changes
> > DID happen.. If so we assume chattr is working fine and then can trust
> > the results of it within the contexts?
> 
> well, the problem is not that chattr doesn't work, the
> problem is that some versions report success, even when
> they could not possibly have succeeded (e.g. no file 
> was found or similar)
> 
> but yes, we might do a chattr test where it is supposed
> to work (i.e. change something) first and check that
> and if it fails, further tests utilizing chattr can be
> considered invalid 

and that's what this patch does (plus a little extra):
1. test initial state of directory
2. use chattr on the directory
3. verify chattr actions on the directory
3. repeat chattr test after setting directory as barrier

overview within patch and modified lines commented in-line (though feel to
remove all comments, as the original code had no comments, or only remove
obvious comments).

corey
-- 
undefined@pobox.com


instead of assumming the directory's initial state (xattrs & mutability), test
it.

instead of trusting chattr works and gives the exit code, verify chattr's
actions on the directory by examining its xattrs, and do this both before and
after setting it as a barrier.

--- testfs.sh-0.20b	2009-08-20 19:45:48.000000000 -0500
+++ testfs.sh-0.20c	2009-08-21 16:45:44.000000000 -0500
@@ -479,6 +479,8 @@ function do_test () {
 	    local lsattr_U="-+(-)-i-+(-)-t*(-)"
 	    ;;
 	  *)
+	    # non-barrier, immutable directory (used in testing chattr)
+	    local attr_I="-+(-)-buI-"
 	    local attr_B="-+(-)-Bui-" attr_b="-+(-)-bui-"
 	    local attr_U="-+(-)--UI-" attr_u="-+(-)--ui-"
 	    local lsattr_B="-+(-)?(A)+(-)"
@@ -491,10 +493,25 @@ function do_test () {
 
 	eexec 101  	0 "mount -t $fs -o $mopt $DEV $MNT 3>&2" || return
 	eeval		  "mkdir -p $dpath"
+	# verify initial xattrs
+	eexec ???	0 "do_xattr_verify $MNT showattr $dpath $attr_b"
+	# verify initial mutability
+	eexec ???	0 "do_xattr_barrier $MNT $dpath ..."
+	# exercise chattr
+	eeval		  "go_xid 2 chattr =i $dpath"
+	# verify that chattr succeeded
+	eexec ???	0 "do_xattr_verify $MNT showattr $dpath $attr_I"
+	# revert chattr's actions
+	eeval		  "go_xid 2 chattr -i $dpath"
+	# verify xattrs back to initial state
+	eexec ???	0 "do_xattr_verify $MNT showattr $dpath $attr_b"
 	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"
+	# test immutability of barrier from a non-privileged context
+	eeval		  "go_xid 2 chattr =i $dpath"
+	# verify that chattr failed
+	eexec 105	0 "do_xattr_verify $MNT showattr $dpath $attr_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"