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"
Received on Fri Aug 21 09:03:18 2009