Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 19 Jun 2016 10:08:03 +0200
From:      Edward Tomasz =?utf-8?Q?Napiera=C5=82a?= <trasz@FreeBSD.org>
To:        Jan Beich <jbeich@vfemail.net>
Cc:        Alexander Motin <mav@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r299448 - in head/sys/cddl/contrib/opensolaris: common/acl uts/common/fs/zfs uts/common/sys
Message-ID:  <20160619080803.GA1638@brick>
In-Reply-To: <y468-hap1-wny@vfemail.net>
References:  <201605111343.u4BDhKhp076695@repo.freebsd.org> <y468-hap1-wny@vfemail.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On 0614T0232, Jan Beich wrote:
> Alexander Motin <mav@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@nexenta.com>
> >   Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
> >   Author: Kevin Crowe <kevin.crowe@nexenta.com>
> >   
> >   openzfs/openzfs@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.

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.

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.




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160619080803.GA1638>