[PATCH 1/2] common/rc: Check 'tPnE' flags on a directory instead of a regilar file

[PATCH 1/2] common/rc: Check 'tPnE' flags on a directory instead of a regilar file

[Xiao Yang]

'tPnE' flags are only valid for a directory so check them on a directory.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 common/rc | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index aa5a7409..cf31eebc 100644
--- a/common/rc
+++ b/common/rc
@@ -2168,8 +2168,14 @@ _require_xfs_io_command()
 		fi
 		# Test xfs_io chattr support AND
 		# filesystem FS_IOC_FSSETXATTR support
-		testio=`$XFS_IO_PROG -F -f -c "chattr +$param" $testfile 2>&1`
-		$XFS_IO_PROG -F -f -r -c "chattr -$param" $testfile 2>&1
+		# 'tPnE' flags are only valid for a directory so check them on a directory.
+		if echo "$param" | egrep -q 't|P|n|E'; then
+			testio=`$XFS_IO_PROG -F -c "chattr +$param" $TEST_DIR 2>&1`
+			$XFS_IO_PROG -F -r -c "chattr -$param" $TEST_DIR 2>&1
+		else
+			testio=`$XFS_IO_PROG -F -f -c "chattr +$param" $testfile 2>&1`
+			$XFS_IO_PROG -F -r -c "chattr -$param" $testfile 2>&1
+		fi
 		param_checked="+$param"
 		;;
 	"chproj")
-- 
2.21.0

[PATCH 2/2] common/rc: Add extra check for xfs_io -c "chattr" on XFS

[Xiao Yang]

On XFS, ioctl(FSSETXATTR)(called by xfs_io -c "chattr") maskes off unsupported
or invalid flags silently.  For example,
1) With kernel v4.4 which doesn't support dax flag, try to set dax flag on a
file by the lastest xfs_io -c "chattr" command:
--------------------------------------------
# xfs_io -f -c "chattr +x" testfile;echo $?
0
# xfs_io -c "lsattr" testfile
----------------X testfile
--------------------------------------------
2) Realtime inheritance flag is only valid for a directory and try to set
realtime inheritance flag on a file:
--------------------------------------------
# xfs_io -f -c "chattr +t" testfile;echo $?
0
# xfs_io -c "lsattr" testfile
----------------X testfile
--------------------------------------------

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 common/rc | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/common/rc b/common/rc
index cf31eebc..e40f0a8a 100644
--- a/common/rc
+++ b/common/rc
@@ -2158,6 +2158,7 @@ _require_xfs_io_command()
 	local param="$*"
 	local param_checked=""
 	local opts=""
+	local attr_info=""
 
 	local testfile=$TEST_DIR/$$.xfs_io
 	local testio
@@ -2171,9 +2172,11 @@ _require_xfs_io_command()
 		# 'tPnE' flags are only valid for a directory so check them on a directory.
 		if echo "$param" | egrep -q 't|P|n|E'; then
 			testio=`$XFS_IO_PROG -F -c "chattr +$param" $TEST_DIR 2>&1`
+			attr_info=`$XFS_IO_PROG -F -r -c "lsattr" $TEST_DIR | awk '{print $1}'`
 			$XFS_IO_PROG -F -r -c "chattr -$param" $TEST_DIR 2>&1
 		else
 			testio=`$XFS_IO_PROG -F -f -c "chattr +$param" $testfile 2>&1`
+			attr_info=`$XFS_IO_PROG -F -r -c "lsattr" $testfile | awk '{print $1}'`
 			$XFS_IO_PROG -F -r -c "chattr -$param" $testfile 2>&1
 		fi
 		param_checked="+$param"
@@ -2297,6 +2300,19 @@ _require_xfs_io_command()
 		echo $testio | grep -q "\(invalid option\|not supported\)" && \
 			_notrun "xfs_io $command doesn't support $param"
 	fi
+
+	# On XFS, ioctl(FSSETXATTR)(called by xfs_io -c "chattr") maskes off unsupported
+	# or invalid flags silently so need to check these flags by extra ioctl(FSGETXATTR)
+	# (called by xfs_io -c "lsattr").
+	# The following URL explains why we don't change the behavior of XFS.
+	# https://www.spinics.net/lists/linux-xfs/msg44725.html
+	if [ -n "$attr_info" -a "$FSTYP" = "xfs" ]; then
+		local num=${#param}
+		for i in $(seq 0 $((num-1))); do
+			echo $attr_info | grep -q "${param:$i:1}" || \
+				_notrun "xfs_io $command +${param:$i:1} support is missing (unknown flag in kernel)"
+		done
+	fi
 }
 
 # check that kernel and filesystem support direct I/O
-- 
2.21.0

Re: [PATCH 1/2] common/rc: Check 'tPnE' flags on a directory instead of a regilar file

[Eryu Guan]

On Tue, Sep 08, 2020 at 09:15:22PM +0800, Xiao Yang wrote:
> 'tPnE' flags are only valid for a directory so check them on a directory.
> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  common/rc | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index aa5a7409..cf31eebc 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2168,8 +2168,14 @@ _require_xfs_io_command()
>  		fi
>  		# Test xfs_io chattr support AND
>  		# filesystem FS_IOC_FSSETXATTR support
> -		testio=`$XFS_IO_PROG -F -f -c "chattr +$param" $testfile 2>&1`
> -		$XFS_IO_PROG -F -f -r -c "chattr -$param" $testfile 2>&1
> +		# 'tPnE' flags are only valid for a directory so check them on a directory.
> +		if echo "$param" | egrep -q 't|P|n|E'; then
> +			testio=`$XFS_IO_PROG -F -c "chattr +$param" $TEST_DIR 2>&1`
> +			$XFS_IO_PROG -F -r -c "chattr -$param" $TEST_DIR 2>&1

I don't think it's a good idea to try chattr on $TEST_DIR, it may change
the behavior of all sub-dirs and files in it and cause unexpected
failures. (I know we do "chattr -$param" right away, but it still looks
dangerous to me.) It's better to create a new $testdir like $testfile to
try on, and remove it after test.

Thanks,
Eryu

> +		else
> +			testio=`$XFS_IO_PROG -F -f -c "chattr +$param" $testfile 2>&1`
> +			$XFS_IO_PROG -F -r -c "chattr -$param" $testfile 2>&1
> +		fi
>  		param_checked="+$param"
>  		;;
>  	"chproj")
> -- 
> 2.21.0
> 
>