[PATCH 2/3 v4] vfs_posixacl: expose acl_t <--> smb_acl_t converter functions

[Andrew Bartlett]

On Fri, 2013-05-24 at 10:41 -0700, Anand Avati wrote:
> On 5/24/13 1:41 AM, Andrew Bartlett wrote:
> > On Thu, 2013-05-23 at 22:29 -0400, Anand Avati wrote:
> >> These converters are used in vfs_glusterfs for storing posix acls
> >> as xattrs (where acl_set_file does not work)
> >
> > I was all keen to submit this to autobuild, but it doesn't even compile:
> >
> > [2693/3956] Compiling source3/lib/sysacls.c
> > In file included from ../source3/lib/sysacls.c:26:0:
> > ../source3/modules/vfs_posixacl.h:47:39: error: unknown type name
> > ‘acl_t’
> > ../source3/modules/vfs_posixacl.h:48:1: error: unknown type name ‘acl_t’
> > Waf: Leaving directory `/data/samba/git/samba-push/bin'
> > Build failed:  -> task failed (err #1):
> > 	{task: cc sysacls.c -> sysacls_92.o}
> > make: *** [all] Error 1
> >
> > I think we need to make glusterfs depend on posix ACLs (presumably the
> > converter function you use) and have these header definitions depend on
> > whatever provides acl_t.
> >
> > In general we don't like conditional stuff in our headers, which is why
> > there is the whole smb_acl stuff in the first place, but this is a bit
> > of a special case.
> >
> > Andrew Bartlett
> >
> 
> 
> Looks like a slip between my v3 and v4 patches again. In v3 I had 
> '#include <sys/acl.h>' in vfs_posixacl.h in the 3rd patch (vfs_glusterfs 
> patch). I was trying to move that line into the 2nd patch (which changes 
> vfs_posixacl.h) and somehow it has gotten missed! Will resubmit.

OK, so that's probably because you tried to only send in the 3rd path
last time (at least, I didn't see a new version of the others, so it
could be my mistake). 

Looking again at the code, I'm now not at all convinced that exposing
these functions from this file is the right thing to do.  This is a vfs
module, you shouldn't have one vfs module depend on another.  This is
only appearing to work because of what is currently set as a 'static'
module.

Nor should we be adding <sys/acl.h> directly to a header - we always go
via our system/filesys.h header, to ensure portability.  The same
applies to the glusterfs mdoule btw. 

That means that these helper functions need to move to a different
helper library, not just be exposed as-is.  Then that file has to be
built only if we have acl_t and and acl iterator functions (essentially
HAVE_POSIX_ACLS)

Finally, this does make me concerned:  How is the link between the OS
storage format for ACLs (exposed via acl_copy_int et al) and what
glusterfs uses defined?  That is, can glusterfs be used cross-platform,
in a situation where each host OS might have a different 'on-disc' ACL
format, or will glusterfs always be linux-only?

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org