abee79f vfs: Use posix_sys_acl_blob_get_file in vfs_gpfs for posix ACLs

simo idra at samba.org
Thu Oct 11 10:32:12 MDT 2012


On Thu, 2012-10-11 at 09:06 -0700, Jeremy Allison wrote:
> On Thu, Oct 11, 2012 at 01:32:27PM +0200, Volker Lendecke wrote:
> > On Thu, Oct 11, 2012 at 10:16:04PM +1100, Andrew Bartlett wrote:
> > > On Thu, 2012-10-11 at 13:02 +0200, Christian Ambach wrote:
> > > > Andrew,
> > > 
> > > > I do not really understand what the 
> > > > expected behavior of of gpfsacl_sys_acl_blob_get_fd() and 
> > > > gpfsacl_sys_acl_blob_get_fd() should be. GPFS supports both the posix 
> > > > and NFSv4 ACL model, so I guess it will take special care that both 
> > > > cases are treated correctly.
> > > 
> > > The idea is simply:
> > > 
> > > If there is an NFSv4 ACL, then we give an error.  The layers above this
> > > cope with errors, and when the patch to vfs_acl_common is applied
> > > (currently there are no callers) it will simply fall back to the hash of
> > > the NT ACL.  (And given the simple mapping, this is less important to
> > > implement). 
> > > 
> > > If it is a posix ACL, then call the posix helper function that
> > > linearlises the posix ACL into a blob, along with the owner, group and
> > > mode.
> > > 
> > > What was the error BTW?
> > 
> > You DID notice that you broke code, right? AGAIN?
> 
> Ok. I think it's time to revive Volker's original idea of reviewer-must-push
> for *all* changes, even for master.
> 
> The fault is not entirely Andrew's, both Simo and I missed this
> problem (along with the subsequent crash bug) in the review, so
> this means we really didn't do a good enough job on this review.

Well mine was just a first order review,
I asked for the style changes and break up to make the changes readable
*exactly* because the first version was a gargantuan patch with a lot of
repetitions and long lines.

My intention was to do a second review of the single patches once those
changes were made.
Apparently though Andrew decided my request were unreasonable, applied a
very annoying 'indenting patch' I also asked not to push in that form on
IRC. 
And then just merely pushed everything to master without waiting for a
second review.
I find this behavior disrespectful of the time and resources the other
team members put when they make patches and then seek feedback and fix
their patches based on that feedback.

> Enforcing reviewer-must-push will concentrate the mind wonderfully
> when doing reviews, as you'll have to add a signed-off-by line
> in the push to master :-).
> 
> What do Team members think ? Do I have anyone to second this ?

Given the above 'incident' is not an isolated episode but happens
frequently, I say +1
I already generally get a sign-off from reviewers and let them push
anyway.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>



More information about the samba-technical mailing list