svn commit: r299448 - in head/sys/cddl/contrib/opensolaris: common/acl uts/common/fs/zfs uts/common/sys

[Edward Tomasz Napierała]

On 0619T1733, Alexander Motin wrote:
> On 19.06.16 17:28, Cy Schubert wrote:
> > In message <20160619080803.GA1638 at brick>, Edward Tomasz 
> > =?utf-8?Q?Napiera=C5=82
> > a?= writes:
> >> On 0614T0232, Jan Beich wrote:
> >>> Alexander Motin <mav at FreeBSD.org> writes:
> >>>
> >>>> Author: mav
> >>>> Date: Wed May 11 13:43:20 2016
> >>>> New Revision: 299448
> >>>> URL: https://svnweb.freebsd.org/changeset/base/299448
> >>>>
> >>>> Log:
> >>>>   MFV r299442: 6762 POSIX write should imply DELETE_CHILD on directories 
> >> - and
> >>>>   some additional considerations
> >>>>   
> >>>>   Reviewed by: Gordon Ross <gwr at nexenta.com>
> >>>>   Reviewed by: Yuri Pankov <yuri.pankov at nexenta.com>
> >>>>   Author: Kevin Crowe <kevin.crowe at nexenta.com>
> >>>>   
> >>>>   openzfs/openzfs at d316fffc9c361532a482208561bbb614dac7f916
> >>>
> >>> This commit confuses acl_is_trivial_np(3). Notice '+' in ls(1) and 'D'
> >>> in getfacl(1) outputs.
> >>
> >> It's not just that.
> >>
> >> Those changes:
> >>
> >> 1. Confuse acl_is_trivial_np(3), as you say.  It's hard to fix in libc,
> >>    because they make trivial ACLs different for files and directories,
> >>    and acl_is_trivial_np(3) has no way of telling which is which.
> >>
> >> 2. They make delete deny permission take precedence over the containing
> >>    directory write allow permission, which is rather different from what
> >>    people expect in unix systems, and is against the NFSv4 specification,
> >>    even though it might be a better fit for Windows.
> > 
> > This is Windows behavior and inconsistent with the rest of FreeBSD and any 
> > UNIX or Linux system.
> > 
> >>
> >> 3. They make umask apply to inherit_only permissions, and
> >>
> >> 4. I don't fully understand this one yet, but from the ACL regression
> >>    test suite (which lives in tests/sys/acl/, and I'd appreciate people
> >>    actually ran this before committing ACL-related changes) it looks
> >>    like it makes umask not apply to the stuff it should.
> >>
> >> The #1 could be fixed by making ZFS not setting delete_child on write,
> >> basically reverting to the previous behaviour in that aspect.  As for
> >> the others...  I'm not saying each one of those is wrong, but they
> >> certainly warrant further discussion, especially #2 and #4.
> > 
> > I think #2 is wrong behavior on any UNIX-like or POSIX system.
> > 
> >>
> >> Basically, what I'm trying to say is that we should consider backing
> >> this out for 11.0-RELEASE, reverting to the previous semantics, verified
> >> by passing the regression tests.
> > 
> > Agreed.
> > 
> > What in FreeBSD was this patch supposed to solve in the first place?
> 
> Growing divergence from OpenZFS upstream.  I am not advocating this
> patch, but it would be good, if possible, to not revert it completely,
> but block wrong behavior with some minimal ifdefs to make further ZFS
> merges easier.  Help would be appreciated. ;)

Our family just expanded, and thus I'm afraid I won't be able to help
for the next few weeks.  That's one of the reasons why I've suggested
the backout for 11.0 - not a permanent "let's ignore this piece of code
forever" backout, but a temporary one, for 11.0; we would then go back
to the topic after the release.