public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Alex Elder <aelder@sgi.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 2/4] xfstests: add _require_attrs
Date: Wed, 20 Oct 2010 13:03:11 -0500	[thread overview]
Message-ID: <1287597791.2284.99.camel@doink> (raw)
In-Reply-To: <20101015222902.GB3781@infradead.org>

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@lst.de>

. . .

> 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.

. . .



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2010-10-20 18:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-15 22:28 [PATCH 0/4] xfstests fixes for hfsplus Christoph Hellwig
2010-10-15 22:28 ` [PATCH 1/4] xfstests: fix _require_acl Christoph Hellwig
2010-10-20 17:35   ` Alex Elder
2010-10-15 22:29 ` [PATCH 2/4] xfstests: add _require_attrs Christoph Hellwig
2010-10-20 18:03   ` Alex Elder [this message]
2010-10-21  5:02     ` Christoph Hellwig
2010-10-21 12:49       ` Alex Elder
2010-10-15 22:29 ` [PATCH 3/4] xfstests: fix quota detection Christoph Hellwig
2010-10-20 17:36   ` Alex Elder
2010-10-15 22:29 ` [PATCH 4/4] xfstests: handle filesystems without FIEMAP support Christoph Hellwig
2010-10-20 17:37   ` Alex Elder
2010-10-21  5:05     ` Christoph Hellwig
2010-10-21  5:35       ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1287597791.2284.99.camel@doink \
    --to=aelder@sgi.com \
    --cc=hch@infradead.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox