Re: [PATCH 2/4] xfs: SGI ACLs: Fix caching and mode setting

[Brian Foster <bfoster@redhat.com>]

On Mon, Oct 26, 2015 at 04:39:20PM +0100, Andreas Gruenbacher wrote:
> On Mon, Oct 26, 2015 at 3:02 PM, Brian Foster <bfoster@redhat.com> wrote:
> > Finally, another random thought... another way to approach this whole
> > thing might be just to redirect the SGI_FILE_* xattr calls to the
> > system.posix_acl_* calls.
> 
> You mean silently changing the binary format of those attributes?
> That's the worst thing we could possibly do.
> 

It would change an implicit error/failure to an explicit one.
Regardless, I realized this particular approach is kind of pointless
anyways (as noted in my other mail).

Note that I'm not necessarily against what we're doing here, it might
very well be required. I'm just trying to step back and understand the
big picture before we go and put a band-aid on the immediate problem and
call it solved. To me, the ideal overall situation is that this
attribute is hidden and anybody that cares about ACLs can get/set them
through the appropriate ACL interface (the system.posix_acl_* xattrs).
The question is whether we can get anywhere close to that without
breaking things.

> > The SGI_FILE_* xattr data changes along with the required conversions
> > and whatnot, but then at least we expose data in a more generic format.
> 
> I really doubt that we can get rid of those attributes.
> 

The problem seems to boil down to the fact that the by-handle operations
(ATTRLIST_BY_HANDLE) don't include the system.posix_acl_* attributes,
and thus these aren't tracked by applications that use that interface
(such as xfsdump). I was hoping that whatever might have looked at the
SGI_ACL_* attrs would also have seen the posix_acl_* attr and thus we
could potentially get away with something like ignoring SGI_ACL_
operations (rather than failing them) without losing any information,
but that does not appear to be the case.

It still might be worth considering drawing a line in the sand (for
example, v5 filesystems or later) to deprecate the SGI_ACL_* xattrs. For
example:

- Fix the by-handle ATTR mechanisms to include the system.posix_acl_*
  xattrs.
- Hide the SGI_ACL_* xattrs from xattr lists.
- Add the SGI_ACL_* setxattr() validation as we're doing here for
  backwards compatibility, but include a deprecation log message warning
  in favor of using the above.
- At some point in the future, disallow the ability to get/set the
  SGI_ACL_* xattrs.

This also assumes there isn't some other reason we have SGI_ACL_*
exposed that I'm not aware of, which could certainly be the case.

Brian

> The patches in this queue don't "change" the format of those
> attributes, they only fix them for use in UID/GID  namespaces where
> they are currently broken. Validation when setting those xattrs could
> be removed, allowing sysadmins to set acls which the kernel and
> xfs_repair would reject; it seems rather pointless though.
> 
> Thanks,
> Andreas

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs