bug#33644: [PATCH] cp --preserve=xattr: preserve NFSv4 ACL extended attributes

[Pádraig Brady]

On 06/12/18 05:08, Kamil Dudka wrote:
> ... which cannot be preserved by other means
> 
> Bug: https://bugzilla.redhat.com/1031423#c4
> ---
>  src/copy.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/src/copy.c b/src/copy.c
> index 3221b9997..754c5e1aa 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -640,6 +640,17 @@ copy_attr_free (struct error_context *ctx _GL_UNUSED,
>  {
>  }
>  
> +/* Include NFSv4 ACL extended attributes, which cannot be preserved by
> +   other means.  Otherwise honor attributes configured for exclusion
> +   in /etc/xattr.conf.  Return zero to skip.  */
> +static int
> +check_not_nfs4_acl (const char *name, struct error_context *ctx)
> +{
> +  return attr_copy_check_permissions(name, ctx)
> +         || !STRNCMP_LIT (name, "system.nfs4_acl")
> +         || !STRNCMP_LIT (name, "system.nfs4acl");
> +}
> +
>  /* Exclude SELinux extended attributes that are otherwise handled,
>     and are problematic to copy again.  Also honor attributes
>     configured for exclusion in /etc/xattr.conf.
> @@ -649,7 +660,7 @@ static int
>  check_selinux_attr (const char *name, struct error_context *ctx)
>  {
>    return STRNCMP_LIT (name, "security.selinux")
> -         && attr_copy_check_permissions (name, ctx);
> +         && check_not_nfs4_acl (name, ctx);
>  }
>  
>  /* If positive SRC_FD and DST_FD descriptors are passed,
> @@ -663,6 +674,9 @@ copy_attr (char const *src_path, int src_fd,
>    bool all_errors = (!x->data_copy_required || x->require_preserve_xattr);
>    bool some_errors = (!all_errors && !x->reduce_diagnostics);
>    bool selinux_done = (x->preserve_security_context || 
> x->set_security_context);
> +  int (*check) (const char *, struct error_context *) = (selinux_done)
> +    ? check_selinux_attr
> +    : check_not_nfs4_acl;
>    struct error_context ctx =
>    {
>      .error = all_errors ? copy_attr_allerror : copy_attr_error,
> @@ -670,12 +684,10 @@ copy_attr (char const *src_path, int src_fd,
>      .quote_free = copy_attr_free
>    };
>    if (0 <= src_fd && 0 <= dst_fd)
> -    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd,
> -                        selinux_done ? check_selinux_attr : NULL,
> +    ret = attr_copy_fd (src_path, src_fd, dst_path, dst_fd, check,
>                          (all_errors || some_errors ? &ctx : NULL));
>    else
> -    ret = attr_copy_file (src_path, dst_path,
> -                          selinux_done ? check_selinux_attr : NULL,
> +    ret = attr_copy_file (src_path, dst_path, check,
>                            (all_errors || some_errors ? &ctx : NULL));
>  
>    return ret == 0;
> 

This patch is confusing to read, though looks functional.
It's clearer of you rename check_not_nfs4_acl() to check_but_allow_nfs4_acl().

So in summary, any xattr in /etc/xattr.conf is _not_ copied.
You want to essentially ignore the nfs4 entries in that config file.
So why not just remove the entries from that file?
Is that something that could be done in attr.git?
Why would one want to treat nfs4 attrs differently to the posix_acl_access 
attrs?

thanks,
Pádraig.