[PATCH] Storing the ACL control flags with vfs gpfs.

[Alexander Werth]

On Wed, 2014-03-19 at 15:10 -0700, Christof Schmitt wrote:
> On Mon, Mar 17, 2014 at 03:02:10PM +0100, Alexander Werth wrote:
> > I forgot the signed-off-by and added some text to the commit message.
> > 
> > Please comment/push.
> 
> I think the overall approach is good, and avoiding the GPFS 3.5 defines
> does not make the code too ugly. Some comments:
> 
> 0001-vfs-Support-NFS-control-flags-in-nfs4_acls.c.patch
> 
> --- a/source3/modules/nfs4_acls.c
> +++ b/source3/modules/nfs4_acls.c
> @@ -49,6 +49,7 @@ typedef struct _SMB_ACE4_INT_T
>  typedef struct _SMB_ACL4_INT_T
>  {
>  	uint32	magic;
> +	uint16  controlflags;
>  	uint32	naces;
>  	SMB_ACE4_INT_T	*first;
>  	SMB_ACE4_INT_T	*last;
> 
> Should we use uint16_t throughout these patches to move the code towards
> the C99 types? It is different from the existing code in the file, but
> changing it has to start at some point.

Ok, changed. Opening the stage for clean-up patches.

> 0002-vfs-Store-ACL-control-flags-in-gpfs-vfs-module.patch
> 
> --- a/source3/modules/vfs_gpfs.c
> +++ b/source3/modules/vfs_gpfs.c
> ...
> @@ -207,6 +225,34 @@ static int vfs_gpfs_get_real_filename(struct
> vfs_handle_struct *handle,
>  	return 0;
>  }
> 
> +static void sd2gpfs_control(uint16_t control, struct gpfs_acl *gacl)
> +{
> +	unsigned int gpfs_aclflags = 0;
> +	gpfs_aclflags = control << 8;
> +	gpfs_aclflags &= 0x3c3c00; /* ACL4_FLAG_DACL_PROTECTED |
> +				      ACL4_FLAG_SACL_PROTECTED |
> +				      ACL4_FLAG_DACL_AUTO_INHERITED |
> +				      ACL4_FLAG_SACL_AUTO_INHERITED |
> +				      ACL4_FLAG_DACL_DEFAULTED |
> +				      ACL4_FLAG_SACL_DEFAULTED |
> +				      ACL4_FLAG_DACL_PRESENT |
> +				      ACL4_FLAG_SACL_PRESENT; */
> +	if (!(control & 0x00000400)) /* SEC_DESC_DACL_PRESENT */
> +		gpfs_aclflags |= 0x00800000; /* ACL4_FLAG_NULL_DACL; */
> +	if (!(control & 0x00001000)) /* SEC_DESC_SACL_PRESENT */
> +		gpfs_aclflags |= 0x01000000; /* ACL4_FLAG_NULL_SACL; */
> 
> The if statements should use braces. Also the SEC_DESC_... defines are
> from Samba, not GPFS, so they can be used without problems.

True.

> +	gacl->acl_level = 1; /* GPFS_ACL_LEVEL_V4FLAGS*/
> +	/* gacl->v4Level1.acl_flags requires gpfs 3.5 */
> +	*(unsigned int *)&gacl->ace_v4 = gpfs_aclflags;
> +}
> +
> +static uint16_t gpfs2sd_control(unsigned int gpfs_aclflags)
> +{
> +	uint16_t control = gpfs_aclflags >> 8;
> +	control |= SEC_DESC_SELF_RELATIVE;
> +	return control;
> +}
> 
> It is probably unlikely that someone would be modifying the flags in the
> filesystem without going through Samba, but should gpfs2sd_control use
> the same mask as sd2gpfs_control for consistency? Maybe create a
> static uint16_t gpfs_aclflags_mask = 0x3c3c00; and use it in both
> functions?

I found a different solution: Filter while we are dealing with the
security descriptor where we have the defines instead of filtering the
gpfs control flags where we need the literals.

> Also the ACL4_FLAG_NULL_DACL and ACL4_FLAG_NULL_SACL mappings are
> missing from gpfs2sd_control. Are they not needed here?

Yes, they are not relevant at the moment. The code doesn't yet
distinguish between a null dacl and an empty dacl. That's a future
change that's possible through the stored control flags.



-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-vfs-Support-NFS-control-flags-in-nfs4_acls.c.patch
Type: text/x-patch
Size: 3104 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140321/91bb86a5/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-vfs-Store-ACL-control-flags-in-gpfs-vfs-module.patch
Type: text/x-patch
Size: 8557 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140321/91bb86a5/attachment-0001.bin>