All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.