On Aug 7, 2016 8:12 AM, "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
>
> Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> writes:
>
> > On Sat, Aug 06, 2016 at 11:09:54PM +0200, Miklos Szeredi wrote:
> >> On Sat, Aug 6, 2016 at 3:52 AM, Seth Forshee <
seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org> wrote:
> >> > On Fri, Aug 05, 2016 at 06:07:44PM -0500, Eric W. Biederman wrote:
> >> > What I'm not convinced of is that the userspace visible changes in
> >> > behavior won't break someone's software, even if they aren't really
> >> > getting acl enforcement.
> >>
> >> That's a key point.  Backward compatibility is important, and not even
> >> hard to do because fuse can negotiate supported features with the
> >> userspace filesystem.
> >>
> >> So we can have a new FUSE_POSIX_ACL feature flag in INIT, sent if
> >> "default_permissions" is on.
> >>
> >> If not set in INIT reply just pass all xattrs through to the
> >> filesystem.  Caching should not be done. Don't think about whether
> >> it's logical or not, or if anyone could use it for anything sane.
> >> Just do what we are doing currently.  Translating uids still makes
> >> sense, but that's another story.
> >
> > Translating uids is actually central to the differing positions that you
> > and Eric have. What Eric wants is for the only path for supporting posix
> > acls to be via the helpers, that way all concerns about translating uids
> > can be addressed there. If fuse is to allow the xattrs to be passed
> > directly through to the filesystem then there has to be a second
> > mechanism which translates the uids for that case.
>
> I think I will agree with Miklos.  Translating uids is another story and
> worst case we can work on that in September when I get back from
> vacation.  Nothing of what you are talking about will make things worse
> for translating uids, so it is perfectly fine to merge posix acl support
> into fuse and then handle uid translation.
>
> Until we set FS_USER_NS fuse it is fine to not get the uid translation.
>
> Which yields a very simple implementation:
>
> In fuse_fill_super:
>         if (!fc->posix_acl) {
>                 sb->s_xattr = fuse_xattr_handlers;
>         } else {
>                 sb->s_xattr = fuse_acl_xattr_handlers;
>         }
>
> Where fuse_acl_xattr_handlers are a different array that
> includes the posix acl interrcept (aka the array in patch 1 or the array
> in patch 2).
>
> Then fuse_get_acl and fuse_set_acl can just test fc->posix_acl
> and fail if that is not set.
>
> I think that is all you need to do, and we can worry about the other
> details after posix acl support has landed in fuse.

Ok.

> >> If the flag is set in INIT reply, then that means userspace filesystem
> >> wants handling of posix acl permission checking in kernel.  It would
> >> also mean that caching of posix acl are allowed (lifetime linked to
> >> attribute lifetime).
> >>
> >> If filesystem wants to explicitly disable posix acl support, then it
> >> can reply EOPNOTSUPP to getxattr and setxattr on "system.posix_acl_*".
> >>   Alternatively we can add a FUSE_NO_POSIX_ACL feature flag, that
> >> filesystem can return in reply to FUSE_POSIX_ACL.
> >>
> >> I agree that adding CONFIG_FUSE_FS_POSIX_ACL is probably not worth it,
> >> just make any such code dependent on CONFIG_FS_POSIX_ACL.
> >
> > But CONFIG_FS_POSIX_ACL doesn't have an input prompt and thus isn't
> > displayed in menuconfig, etc. If that's what you want, fine, but it
> > seems like an unusual situation.
>
> Then in Kconfig I would have FUSE_FS "select FS_POSIX_ACL".  That
> simplifies the problem.  Unless Miklos wants something more granular.

Which is the the alternative I offered a couple of emails ago ;-)

Thanks,
Seth