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