On 05/06/2014 03:00 AM, Pádraig Brady wrote:
>> I'll add a root only test later along the lines of the above.
> and the test...

Both, the change in copy.c and the test look good to me.
2 minor nits:

> --- a/tests/cp/cp-mv-enotsup-xattr.sh
> +++ b/tests/cp/cp-mv-enotsup-xattr.sh
> @@ -106,4 +106,24 @@ mv xattr/a noxattr/ 2>err || fail=1
>  test -s noxattr/a         || fail=1  # destination file must not be empty
>  test -s err               && fail=1  # there must be no stderr output
> 
> +# This should pass and copy xattrs of the symlink
> +# since they're not in the 'user.' namespace.
> +# Up to and including coreutils-8.22 xattrs of symlinks
> +# were not copied across file systems.
> +ln -s 'foo' xattr/symlink || framework_failure_
> +# Note 'user.' namespace is only supported on regular files/dirs
> +# so use the 'trusted.' namespace here
> +txattr='trusted.overlay.whiteout'
> +if setfattr -hn trusted.overlay.whiteout -v y xattr/symlink; then

s/trusted,overlay.whiteout/"$txattr"/

> +  # Note only root can read the 'trusted.' namespace
> +  if getfattr -h -m- -d xattr/symlink | grep -F "$txattr"; then
> +    mv xattr/symlink noxattr/ || fail=1
> +    getfattr -h -m- -d noxattr/symlink | grep -F "$txattr" || fail=1
> +  else
> +    echo "failed to get 'trusted.' xattr of symlink" >&2

This message looks like a failure. Maybe add something like
"skipping this part" to make it clear.

> +  fi
> +else
> +  echo "failed to set 'trusted.' xattr of symlink" >&2
> +fi
> +
>  Exit $fail

Thanks & have a nice day,
Berny



Reply via email to