[PATCH] generic/062: more restrictive namespace for getfattr on cephfs

[PATCH] generic/062: more restrictive namespace for getfattr on cephfs

[Jeff Layton]

cephfs exposes some information via virtual xattrs and their presence
causes this test to fail. When running on cephfs, only search for
specific xattr classes to ensure that we skip the ceph.* ones.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 tests/generic/062 | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

I'm not thrilled with this patch, but it does seem to fix the problem
I was seeing running this test on cephfs. If there are better ways to
fix this, then I'd be open to them

It would be nice if we could use a negative lookahead for this, but
getfattr doesn't seem to accept those in its regexes.

diff --git a/tests/generic/062 b/tests/generic/062
index 3fef02e8fc28..4aab20bc3135 100755
--- a/tests/generic/062
+++ b/tests/generic/062
@@ -78,6 +78,13 @@ if [ "$USE_ATTR_SECURE" = yes ]; then
 else
     ATTR_MODES="user trusted"
 fi
+
+if [ "$FSTYP" = ceph ]; then
+	GLOBAL_NSP_FILTER='^(security|system|trusted|user)\.'
+else
+	GLOBAL_NSP_FILTER='.'
+fi
+
 for nsp in $ATTR_MODES; do
 	for inode in reg dir lnk dev/b dev/c dev/p; do
 
@@ -119,7 +126,7 @@ for nsp in $ATTR_MODES; do
 		getfattr -m $nsp -e hex -n $nsp.name2 $SCRATCH_MNT/$inode 2>&1 | invalid_attribute_filter
 
 		echo "*** final list (strings, type=$inode, nsp=$nsp)"
-		getfattr -m '.' -e hex $SCRATCH_MNT/$inode
+		getfattr -m $GLOBAL_NSP_FILTER -e hex $SCRATCH_MNT/$inode
 	
 	done
 done
@@ -149,11 +156,11 @@ _extend_test_bed
 
 echo
 echo "*** directory descent with us following symlinks"
-getfattr -h -L -R -m '.' -e hex $SCRATCH_MNT | _sort_getfattr_output
+getfattr -h -L -R -m $GLOBAL_NSP_FILTER -e hex $SCRATCH_MNT | _sort_getfattr_output
 
 echo
 echo "*** directory descent without following symlinks"
-getfattr -h -P -R -m '.' -e hex $SCRATCH_MNT | _sort_getfattr_output
+getfattr -h -P -R -m $GLOBAL_NSP_FILTER -e hex $SCRATCH_MNT | _sort_getfattr_output
 
 # 
 # Test the backup/restore code
@@ -166,7 +173,7 @@ _backup()
 	# we *do* sort the output by path, since it otherwise would depend on
 	# readdir order, which on some filesystems may change after re-creating
 	# the files.
-	_getfattr --absolute-names -dh -R -m '.' $SCRATCH_MNT | _sort_getfattr_output >$1
+	_getfattr --absolute-names -dh -R -m $GLOBAL_NSP_FILTER $SCRATCH_MNT | _sort_getfattr_output >$1
 	echo BACKUP $1 >>$seqres.full
 	cat $1 >> $seqres.full
 	[ ! -s $1 ] && echo "warning: $1 (backup file) is empty"
@@ -178,7 +185,7 @@ _backup $tmp.backup1
 echo "*** clear out the scratch device"
 rm -rf $(find $SCRATCH_MNT/* | grep -v "lost+found")
 echo "AFTER REMOVE" >>$seqres.full
-getfattr -L -R -m '.' $SCRATCH_MNT >>$seqres.full
+getfattr -L -R -m $GLOBAL_NSP_FILTER $SCRATCH_MNT >>$seqres.full
 
 echo "*** reset test bed with no extended attributes"
 _create_test_bed
@@ -189,7 +196,7 @@ setfattr -h --restore=$tmp.backup1
 _backup $tmp.backup2
 
 echo "AFTER RESTORE" >>$seqres.full
-getfattr -L -R -m '.' $SCRATCH_MNT >>$seqres.full
+getfattr -L -R -m $GLOBAL_NSP_FILTER $SCRATCH_MNT >>$seqres.full
 
 echo "*** compare before and after backups"
 diff $tmp.backup1 $tmp.backup2
-- 
2.21.0

Re: [PATCH] generic/062: more restrictive namespace for getfattr on cephfs

[Christoph Hellwig]

On Wed, Jul 24, 2019 at 10:55:11AM -0400, Jeff Layton wrote:
> cephfs exposes some information via virtual xattrs and their presence
> causes this test to fail. When running on cephfs, only search for
> specific xattr classes to ensure that we skip the ceph.* ones.

The test should fail.  Filesystems should not make up random crap in
their own namespaces.

Re: [PATCH] generic/062: more restrictive namespace for getfattr on cephfs

[Amir Goldstein]

On Wed, Jul 24, 2019 at 5:55 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> cephfs exposes some information via virtual xattrs and their presence
> causes this test to fail. When running on cephfs, only search for
> specific xattr classes to ensure that we skip the ceph.* ones.
>

Probably too late to ask this question now anyway, but does ceph have
a particular good reason to expose those virt xattr in listxattr?

I means its not xattrs that copy tools should try to copy and tools
should know about them and can try to getxattr without the need
to see them in listxattr.

I noticed this is the behavior of cifs w.r.t cifs. and smb3. virt xattr.

Thanks,
Amir.

Re: [PATCH] generic/062: more restrictive namespace for getfattr on cephfs

[Jeff Layton]

On Wed, 2019-07-24 at 18:49 +0300, Amir Goldstein wrote:
> On Wed, Jul 24, 2019 at 5:55 PM Jeff Layton <jlayton@kernel.org> wrote:
> > cephfs exposes some information via virtual xattrs and their presence
> > causes this test to fail. When running on cephfs, only search for
> > specific xattr classes to ensure that we skip the ceph.* ones.
> > 
> 
> Probably too late to ask this question now anyway, but does ceph have
> a particular good reason to expose those virt xattr in listxattr?
> 
> I means its not xattrs that copy tools should try to copy and tools
> should know about them and can try to getxattr without the need
> to see them in listxattr.
> 
> I noticed this is the behavior of cifs w.r.t cifs. and smb3. virt xattr.
> 

Good question. I don't see the need for us to present these in listxattr
and stopping that may be the best solution here. Let me investigate that
approach.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>