Hi Pádraig,

> > 2024-07-15  Bruno Haible  <br...@clisp.org>
> > 
> >     qcopy-acl: Fix copying of ACLs on CentOS 7 (regression 2023-01-12).
> >     * lib/qcopy-acl.c: Include <string.h>, <linux/xattr.h>.
> >     (XATTR_NAME_NFSV4_ACL, XATTR_NAME_POSIX_ACL_ACCESS,
> >     XATTR_NAME_POSIX_ACL_DEFAULT): New macros, from file-has-acl.c.
> >     (is_attr_permissions): Test for these names explicitly.
> >     * m4/acl.m4 (gl_QCOPY_ACL): New macro.
> >     * modules/qcopy-acl (Files): Add m4/acl.m4.
> >     (configure.ac): Invoke gl_QCOPY_ACL.
> > 
> > diff --git a/lib/qcopy-acl.c b/lib/qcopy-acl.c
> > index dfc39cead0..877f42588b 100644
> > --- a/lib/qcopy-acl.c
> > +++ b/lib/qcopy-acl.c
> > @@ -26,6 +26,20 @@
> >   #if USE_XATTR
> >   
> >   # include <attr/libattr.h>
> > +# include <string.h>
> > +
> > +# if HAVE_LINUX_XATTR_H
> > +#  include <linux/xattr.h>
> > +# endif
> > +# ifndef XATTR_NAME_NFSV4_ACL
> > +#  define XATTR_NAME_NFSV4_ACL "system.nfs4_acl"
> > +# endif
> > +# ifndef XATTR_NAME_POSIX_ACL_ACCESS
> > +#  define XATTR_NAME_POSIX_ACL_ACCESS "system.posix_acl_access"
> > +# endif
> > +# ifndef XATTR_NAME_POSIX_ACL_DEFAULT
> > +#  define XATTR_NAME_POSIX_ACL_DEFAULT "system.posix_acl_default"
> > +# endif
> >   
> >   /* Returns 1 if NAME is the name of an extended attribute that is related
> >      to permissions, i.e. ACLs.  Returns 0 otherwise.  */
> > @@ -33,7 +47,12 @@
> >   static int
> >   is_attr_permissions (const char *name, struct error_context *ctx)
> >   {
> > -  return attr_copy_action (name, ctx) == ATTR_ACTION_PERMISSIONS;
> > +  /* We need to explicitly test for the known extended attribute names,
> > +     because at least on CentOS 7, attr_copy_action does not do it.  */
> > +  return strcmp (name, XATTR_NAME_POSIX_ACL_ACCESS) == 0
> > +         || strcmp (name, XATTR_NAME_POSIX_ACL_DEFAULT) == 0
> > +         || strcmp (name, XATTR_NAME_NFSV4_ACL) == 0
> > +         || attr_copy_action (name, ctx) == ATTR_ACTION_PERMISSIONS;
> >   }
> >   
> 
> I was wondering a little about the generality of this patch,
> and how it somwehat overrides /etc/xattr.conf which gives configurability
> over how these xattrs are treated.
> 
> I checked one centos 7 system, and it didn't have a /etc/xattr.conf file
> which might explain the behavior noticed above.
> 
> Then on centos 8 we have:
> system.nfs4_acl                       permissions
> system.nfs4acl                        permissions
> system.posix_acl_access               permissions
> system.posix_acl_default      permissions
> 
> While on Fedora 42 we have:
> system.posix_acl_access               permissions
> system.posix_acl_default      permissions

And what do you suggest?

  - Do you suggest that on CentOS 7, the lack of a /etc/xattr.conf file
    should be considered like an intention to NOT copy any ACL xattrs,
    and that therefore the aforementioned unit tests SHOULD fail?

  - Do you suggest that on Fedora 42, the lack of the system.nfs4*acl in
    /etc/xattr.conf should be considered like an intention to NOT copy
    NFSv4 ACLs, and that therefore copying files with such ACLs SHOULD
    produce errors since they are not supported?

> I'm not sure why nfs4 acls were only considered for a few Red Hat releases,
> but it might explain the (already resolved) issue with nfs4 acls at
> https://bugzilla.redhat.com/2363149

The major part of Paul's patch there was to optimize the number of system
calls in the case of NFSv4 ACLs. So, even if this patch might not have been
essential on Fedora 42 (because NFSv4 ACLs are intentionally unsupported
there), it is useful for CentOS 8. Right?

Bruno




Reply via email to