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

[Adrian Chadd]

"Solution dictated on account of personal life" is totally legit!


-a


On 21 June 2016 at 02:36, Edward Tomasz Napierała <trasz at freebsd.org> wrote:
> 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.
>
>