xfs
[Top] [All Lists]

Re: [PATCH 2/4] xfstests: add _require_attrs

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/4] xfstests: add _require_attrs
From: Alex Elder <aelder@xxxxxxx>
Date: Wed, 20 Oct 2010 13:03:11 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20101015222902.GB3781@xxxxxxxxxxxxx>
References: <20101015222820.GA3655@xxxxxxxxxxxxx> <20101015222902.GB3781@xxxxxxxxxxxxx>
Reply-to: aelder@xxxxxxx
On Fri, 2010-10-15 at 18:29 -0400, Christoph Hellwig wrote:
> Add a new helper to check if extended attributes are supported.  It
> errors out if any of the attr tools are not found, or if a filesystem
> does not support setting attributes.
> 
> Remove the opencoded checks for the attr tools from various tests
> now that we do them in common code.

Generally this looks good.

I was going to just make a few suggestions and ask you
to fix them before committing, but  I think there are
enough that I think it would be good to re-submit this.

> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

. . .

> Index: xfstests-dev/021
> ===================================================================
> --- xfstests-dev.orig/021     2010-10-15 12:52:08.000000000 +0000
> +++ xfstests-dev/021  2010-10-15 14:11:10.000000000 +0000

This file calls "attr" and "getfattr" directly in one spot each,
rather than using "${ATTR_PROG}" and "${GETFATTR_PROG}".  This
conceivably means that the one used (or attempted) could disagree
with the one that _require_attrs checks for.

Can you fix these references?

. . .

> Index: xfstests-dev/062
> ===================================================================
> --- xfstests-dev.orig/062     2010-10-15 12:52:08.000000000 +0000
> +++ xfstests-dev/062  2010-10-15 14:11:22.000000000 +0000

Same problem in this file--it consistently calls setfattr
and getfattr directly rather than using their corresponding
variables.
 
. . .

> Index: xfstests-dev/093
> ===================================================================
> --- xfstests-dev.orig/093     2010-10-15 12:52:09.000000000 +0000
> +++ xfstests-dev/093  2010-10-15 12:52:59.000000000 +0000

Same problem here, with the use of attr directly.


It looks like tests 097 and 098 should call
_require_attr also.  And then change their direct
references to "attr" to use "${ATTR_PROG}" as well.

. . .

> Index: xfstests-dev/115
> ===================================================================
> --- xfstests-dev.orig/115     2010-10-15 12:52:09.000000000 +0000
> +++ xfstests-dev/115  2010-10-15 14:12:21.000000000 +0000

This file should not call attr directly.

. . .

> Index: xfstests-dev/136
> ===================================================================
> --- xfstests-dev.orig/136     2010-10-15 12:52:33.000000000 +0000
> +++ xfstests-dev/136  2010-10-15 14:12:35.000000000 +0000

This file should not call attr directly.

. . .



<Prev in Thread] Current Thread [Next in Thread>