Re: [PATCH v12 21/49] ext4: Add richacl feature flag

From: Austin S Hemmelgarn <ahferroin7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Andreas Gruenbacher
	<agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Alexander Viro
	<viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>,
	Theodore Ts'o <tytso-3s7WtUTddSA@public.gmane.org>,
	Andreas Dilger
	<adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>,
	"J. Bruce Fields"
	<bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>,
	Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>,
	Trond Myklebust
	<trond.myklebust-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>,
	Anna Schumaker
	<anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>,
	Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org>,
	linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	xfs-VZNHf3L845pBDgjK7y7TUQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: "Aneesh Kumar K.V"
	<aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Subject: Re: [PATCH v12 21/49] ext4: Add richacl feature flag
Date: Fri, 23 Oct 2015 15:34:28 -0400	[thread overview]
Message-ID: <562A8BC4.9050107@gmail.com> (raw)
In-Reply-To: <1445625722-13791-22-git-send-email-agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]

On 2015-10-23 14:41, Andreas Gruenbacher wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>
> This feature flag selects richacl instead of posix acl support on the
> file system. In addition, the "acl" mount option is needed for enabling
> either of the two kinds of acls.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Andreas Gruenbacher <agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>   fs/ext4/ext4.h  |  6 ++++--
>   fs/ext4/super.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
>   2 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index fd1f28b..b97a3b1 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -991,7 +991,7 @@ struct ext4_inode_info {
>   #define EXT4_MOUNT_UPDATE_JOURNAL	0x01000	/* Update the journal format */
>   #define EXT4_MOUNT_NO_UID32		0x02000  /* Disable 32-bit UIDs */
>   #define EXT4_MOUNT_XATTR_USER		0x04000	/* Extended user attributes */
> -#define EXT4_MOUNT_POSIX_ACL		0x08000	/* POSIX Access Control Lists */
> +#define EXT4_MOUNT_ACL			0x08000	/* Access Control Lists */
>   #define EXT4_MOUNT_NO_AUTO_DA_ALLOC	0x10000	/* No auto delalloc mapping */
>   #define EXT4_MOUNT_BARRIER		0x20000 /* Use block barriers */
>   #define EXT4_MOUNT_QUOTA		0x80000 /* Some quota option set */
> @@ -1582,6 +1582,7 @@ static inline int ext4_encrypted_inode(struct inode *inode)
>   #define EXT4_FEATURE_INCOMPAT_LARGEDIR		0x4000 /* >2GB or 3-lvl htree */
>   #define EXT4_FEATURE_INCOMPAT_INLINE_DATA	0x8000 /* data in inode */
>   #define EXT4_FEATURE_INCOMPAT_ENCRYPT		0x10000
> +#define EXT4_FEATURE_INCOMPAT_RICHACL		0x20000
>
>   #define EXT2_FEATURE_COMPAT_SUPP	EXT4_FEATURE_COMPAT_EXT_ATTR
>   #define EXT2_FEATURE_INCOMPAT_SUPP	(EXT4_FEATURE_INCOMPAT_FILETYPE| \
> @@ -1607,7 +1608,8 @@ static inline int ext4_encrypted_inode(struct inode *inode)
>   					 EXT4_FEATURE_INCOMPAT_FLEX_BG| \
>   					 EXT4_FEATURE_INCOMPAT_MMP | \
>   					 EXT4_FEATURE_INCOMPAT_INLINE_DATA | \
> -					 EXT4_FEATURE_INCOMPAT_ENCRYPT)
> +					 EXT4_FEATURE_INCOMPAT_ENCRYPT | \
> +					 EXT4_FEATURE_INCOMPAT_RICHACL)
If this is going to be an incompat feature, the reasoning for that being 
the case should be clearly documented in either the changelog or the 
code itself.  While the people involved in the discussion of this may 
understand the reasoning, that doesn't mean any arbitrary individual 
will at some point in the future, and documenting it properly will save 
time for people trying to figure out why this non-obvious choice was made.


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3019 bytes --]