[PATCH][WIP] Make vfs_acl_xattr use hash of the posix ACL

Christian Ambach ambi at samba.org
Tue Oct 16 04:06:14 MDT 2012


Hi Andrew,

On 10/16/2012 10:16 AM, Andrew Bartlett wrote:

> The approach was accepted then (the ACL-blob design was proposed),
> and again just before RC1 when I was able to get the VFS layer
> changes merged.

And with that you broke vfs_gpfs.c the first time.

> Despite this, and probably due to the issues on the gpfs side, I
> seem to have annoyed you significantly with this patch series, and
> I'm sorry about that.

The mistakes made in the second patch were quite obvious and this is 
what somehow annoyed me.

> In all the work I do on Samba, like others I have to make small and
> large changes in preparation for the actual changes I desire to
> implement, and generally implement these (with review where
> appropriate) in advance of the actual change I want.  Typically (but
> not in this particular case) these changes provide some incremental
> benefit on their own.

To me, these preparations seem to have been without proper care when
touching code that you cannot compile-check on your own.

What you are after are large changes to the ACL code and there are still
lots of discussions if your approach is correct or not (that were
initiated by the initial prep checkins that obviously did not provide
enough of the big picture).
Bad thing is that such threads can start to clutter information over
lots of places. Would be really good to see all necessary information
(the why and how to be put into a single place, like a design document
that is used in regular company software development organizations).
Once the "why" has been verified, dealing with the "how" will become
much easier.

In my opinion, any preparations needed for work items of that scale
should be kept back until the change that requires them has been ironed
out. But I do not have explicit authority and you can still choose to
ignore me.
I am not especially nit-picking on you, there have been others in the
team doing the same.

Until you broke the code a second time, I didn't even look at what you
were doing, but now you have sparked my interest and I am not sure I can 
agree with some of the assumptions you made (e.g. that NFSv4 ACLs are 
near enough to Windows SDs... that's a common misunderstanding).

> Occasionally these ideas don't work out, and I've been involved in
> removing some failed experiments that didn't develop into useful
> features.

And so it potentially requires all preparatory work to be taken out
again. This potentially creates unnecessary grief and fuzz.

> That said, I'm really not sure what more you want me to do from
> here.

Independent of the result of the review-policy vote:
1. run all patches that changes files you cannot compile on your system
(because of missing headers or different OS) by somebody who can at
least verify that the files compile fine and if there are platform
specific things you need to consider (as the GPFS ACL blob). That's
something the mandatory review would cover on (if applied correctly).

2. and to the topic of this thread: provide a write-up of the "why's"
that lists some necessary information like problem statements and
proposed solution on a high-level.

Sorry for being so picky here, but I have seen enough issues with ACLs
at our customers and I also would like to put an end to them, but done
right.

Cheers,
Christian



More information about the samba-technical mailing list