On Thu, Sep 10, 2020 at 07:43:53PM +0000, Frank van der Linden wrote:
> Filesystems may not support all xattr types. But, _require_attr assumes
> that being able to use "user" namespace xattrs means that all namespaces
> ("trusted", "system", etc) are supported. This breaks on NFS, that only
> supports the "user" namespace, and a few cases in the "system" namespace.
>
> Change _require_attrs to optionally take namespace arguments that specify
> the namespaces to check for. The default behavior (no arguments) is still
> to check for the "user" namespace only.
>
> Signed-off-by: Frank van der Linden <fllinden@xxxxxxxxxx>
This patchset looks great to me, thanks!
Some minor nits below (and I've fixed them on commit, so there's no need
to resend :)
> ---
> common/attr | 49 +++++++++++++++++++++++++++++++------------------
> 1 file changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/common/attr b/common/attr
> index 20049de0..c60cb6ed 100644
> --- a/common/attr
> +++ b/common/attr
> @@ -175,30 +175,43 @@ _list_acl()
>
> _require_attrs()
> {
> + local args
> + local nsp
> +
> + if [ $# -eq 0 ];
> + then
We prefer the following coding style in fstests
if [ $# -eq 0 ]; then
args="user"
else
args="$*"
fi
> + args="user"
> + else
> + args="$*"
> + fi
And you've almost re-written the whole _require_attrs(), it's better to
use tab as indention instead of 4 spaces (we're in the (very slow)
progress converting all 4-spaces indention to tab, except there're old
code using 4-spaces around).
> +
> [ -n "$ATTR_PROG" ] || _notrun "attr command not found"
> [ -n "$GETFATTR_PROG" ] || _notrun "getfattr command not found"
> [ -n "$SETFATTR_PROG" ] || _notrun "setfattr command not found"
>
> - #
> - # Test if chacl is able to write an attribute on the target filesystems.
> - # On really old kernels the system calls might not be implemented at all,
> - # but the more common case is that the tested filesystem simply doesn't
> - # support attributes. Note that we can't simply list attributes as
> - # various security modules generate synthetic attributes not actually
> - # stored on disk.
> - #
> - touch $TEST_DIR/syscalltest
> - attr -s "user.xfstests" -V "attr" $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1
> - cat $TEST_DIR/syscalltest.out >> $seqres.full
> + for nsp in $args
> + do
Same here, use the format below
for nsp in $args; do
<do things here>
done
Thanks,
Eryu
> + #
> + # Test if chacl is able to write an attribute on the target filesystems.
> + # On really old kernels the system calls might not be implemented at all,
> + # but the more common case is that the tested filesystem simply doesn't
> + # support attributes. Note that we can't simply list attributes as
> + # various security modules generate synthetic attributes not actually
> + # stored on disk.
> + #
> + touch $TEST_DIR/syscalltest
> + $SETFATTR_PROG -n "$nsp.xfstests" -v "attr" $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1
> + cat $TEST_DIR/syscalltest.out >> $seqres.full
>
> - if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then
> - _notrun "kernel does not support attrs"
> - fi
> - if grep -q 'Operation not supported' $TEST_DIR/syscalltest.out; then
> - _notrun "attrs not supported by this filesystem type: $FSTYP"
> - fi
> + if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then
> + _notrun "kernel does not support attrs"
> + fi
> + if grep -q 'Operation not supported' $TEST_DIR/syscalltest.out; then
> + _notrun "attr namespace $nsp not supported by this filesystem type: $FSTYP"
> + fi
>
> - rm -f $TEST_DIR/syscalltest.out
> + rm -f $TEST_DIR/syscalltest.out
> + done
> }
>
> _require_attr_v1()
> --
> 2.16.6
