On Fri, Aug 21, 2009 at 12:07:39AM -0500, Corey Wright wrote: > 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? 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 ... best, Herbert > (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"