Skip to contentSkip to conversation

generic/062: more restrictive namespace for getfattr on cephfs

PATCH In review Jeff Layton 3 participants 4 messages Jul 24, 2019

lore
Share & tools
lore
#0 lore raw

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

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.

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

tests/generic/062 +13 -6
index 3fef02e8fc28..4aab20bc3135 100755
@@ -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
Signature · 1 line
2.21.0
#1 lore raw
Quoted 3 lines · 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.

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

#2 lore raw
In reply to Jeff Layton
Quoted 5 lines · 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.

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.

#3 lore raw
Quoted 6 lines · Amir Goldstein
Quoted 4 lines · 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.

Quoted 9 lines

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,

Signature · 1 line
Jeff Layton <[email protected]>