diff mbox

[v1,14/22] mke2fs: add inline_data support in mke2fs

Message ID 1375436989-18948-15-git-send-email-wenqing.lz@taobao.com
State Superseded, archived
Headers show

Commit Message

Zheng Liu Aug. 2, 2013, 9:49 a.m. UTC 
From: Zheng Liu <wenqing.lz@taobao.com>

Inline_data feature doesn't be specified without ext_attr.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
 misc/mke2fs.8.in |    3 +++
 misc/mke2fs.c    |   14 +++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Darrick Wong Oct. 12, 2013, 12:27 a.m. UTC | #1 
On Fri, Aug 02, 2013 at 05:49:41PM +0800, Zheng Liu wrote:
> From: Zheng Liu <wenqing.lz@taobao.com>
> 
> Inline_data feature doesn't be specified without ext_attr.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
> ---
>  misc/mke2fs.8.in |    3 +++
>  misc/mke2fs.c    |   14 +++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/misc/mke2fs.8.in b/misc/mke2fs.8.in
> index 023ba49..5731542 100644
> --- a/misc/mke2fs.8.in
> +++ b/misc/mke2fs.8.in
> @@ -578,6 +578,9 @@ option).
>  @JDEV@must be created with the same
>  @JDEV@block size as the filesystems that will be using it.
>  .TP
> +.B inline_data
> +Allow data to be stored in inode

"..to be stored in the inode and extended attribute areas" ?

> +.TP
>  .B large_file
>  Filesystem can contain files that are greater than 2GB.  (Modern kernels
>  set this feature automatically when a file > 2GB is created.)
> diff --git a/misc/mke2fs.c b/misc/mke2fs.c
> index a59d24c..cb084ea 100644
> --- a/misc/mke2fs.c
> +++ b/misc/mke2fs.c
> @@ -902,7 +902,8 @@ static __u32 ok_features[3] = {
>  		EXT2_FEATURE_INCOMPAT_META_BG|
>  		EXT4_FEATURE_INCOMPAT_FLEX_BG|
>  		EXT4_FEATURE_INCOMPAT_MMP |
> -		EXT4_FEATURE_INCOMPAT_64BIT,
> +		EXT4_FEATURE_INCOMPAT_64BIT|
> +		EXT4_FEATURE_INCOMPAT_INLINE_DATA,
>  	/* R/O compat */
>  	EXT2_FEATURE_RO_COMPAT_LARGE_FILE|
>  		EXT4_FEATURE_RO_COMPAT_HUGE_FILE|
> @@ -2042,6 +2043,17 @@ profile_error:
>  		exit(1);
>  	}
>  
> +	/* Since inline_data depends on ext_attr, we would ask it to be
> +	 * enabled.

/*
 * Comment
 */

There needs to be an extra newline after the '/*' at the top of that multiline
comment.

> +	 */
> +	if ((fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_INLINE_DATA) &&
> +	    !(fs_param.s_feature_compat & EXT2_FEATURE_COMPAT_EXT_ATTR)) {
> +		com_err(program_name, 0,
> +			_("Ext_attr feature not enabled, so inline_data "
> +			  "feature doesn't be specified"));

"The inline_data feature requires the ext_attr feature." ?

--D

> +		exit(1);
> +	}
> +
>  	if (fs_param.s_blocks_per_group) {
>  		if (fs_param.s_blocks_per_group < 256 ||
>  		    fs_param.s_blocks_per_group > 8 * (unsigned) blocksize) {
> -- 
> 1.7.9.7
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zheng Liu Oct. 12, 2013, 8:08 a.m. UTC | #2 
On Fri, Oct 11, 2013 at 05:27:18PM -0700, Darrick J. Wong wrote:
[...]
> > +	 */
> > +	if ((fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_INLINE_DATA) &&
> > +	    !(fs_param.s_feature_compat & EXT2_FEATURE_COMPAT_EXT_ATTR)) {
> > +		com_err(program_name, 0,
> > +			_("Ext_attr feature not enabled, so inline_data "
> > +			  "feature doesn't be specified"));
> 
> "The inline_data feature requires the ext_attr feature." ?

Ah, I have noticed that now we set xattr handler by default.  So, yes,
inline_data feature doesn't require the ext_attr feature.

Thanks,
                                                - Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick Wong Oct. 12, 2013, 8:18 a.m. UTC | #3 
On Sat, Oct 12, 2013 at 04:08:46PM +0800, Zheng Liu wrote:
> On Fri, Oct 11, 2013 at 05:27:18PM -0700, Darrick J. Wong wrote:
> [...]
> > > +	 */
> > > +	if ((fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_INLINE_DATA) &&
> > > +	    !(fs_param.s_feature_compat & EXT2_FEATURE_COMPAT_EXT_ATTR)) {
> > > +		com_err(program_name, 0,
> > > +			_("Ext_attr feature not enabled, so inline_data "
> > > +			  "feature doesn't be specified"));
> > 
> > "The inline_data feature requires the ext_attr feature." ?
> 
> Ah, I have noticed that now we set xattr handler by default.  So, yes,
> inline_data feature doesn't require the ext_attr feature.

I think you /do/ still need this check, just in case someone tries to mkfs -O
^ext_attr,inline_data.  Defaults can be overridden.

(I was merely trying to clarify the English in the error message.)

--D
> 
> Thanks,
>                                                 - Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zheng Liu Oct. 12, 2013, 8:31 a.m. UTC | #4 
On Sat, Oct 12, 2013 at 01:18:25AM -0700, Darrick J. Wong wrote:
> On Sat, Oct 12, 2013 at 04:08:46PM +0800, Zheng Liu wrote:
> > On Fri, Oct 11, 2013 at 05:27:18PM -0700, Darrick J. Wong wrote:
> > [...]
> > > > +	 */
> > > > +	if ((fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_INLINE_DATA) &&
> > > > +	    !(fs_param.s_feature_compat & EXT2_FEATURE_COMPAT_EXT_ATTR)) {
> > > > +		com_err(program_name, 0,
> > > > +			_("Ext_attr feature not enabled, so inline_data "
> > > > +			  "feature doesn't be specified"));
> > > 
> > > "The inline_data feature requires the ext_attr feature." ?
> > 
> > Ah, I have noticed that now we set xattr handler by default.  So, yes,
> > inline_data feature doesn't require the ext_attr feature.
> 
> I think you /do/ still need this check, just in case someone tries to mkfs -O
> ^ext_attr,inline_data.  Defaults can be overridden.

I have tested this ('mkfs -O ^ext_attr,inline_data') just a minute ago,
and it seems that it works well.  BTW, when we use setfattr(1) to set
xattr in a ext4 file system without ext_attr feature, this feature will
be enabled in ext4_xattr_update_super_block().

                                                - Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick Wong Oct. 12, 2013, 8:32 a.m. UTC | #5 
On Sat, Oct 12, 2013 at 04:31:17PM +0800, Zheng Liu wrote:
> On Sat, Oct 12, 2013 at 01:18:25AM -0700, Darrick J. Wong wrote:
> > On Sat, Oct 12, 2013 at 04:08:46PM +0800, Zheng Liu wrote:
> > > On Fri, Oct 11, 2013 at 05:27:18PM -0700, Darrick J. Wong wrote:
> > > [...]
> > > > > +	 */
> > > > > +	if ((fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_INLINE_DATA) &&
> > > > > +	    !(fs_param.s_feature_compat & EXT2_FEATURE_COMPAT_EXT_ATTR)) {
> > > > > +		com_err(program_name, 0,
> > > > > +			_("Ext_attr feature not enabled, so inline_data "
> > > > > +			  "feature doesn't be specified"));
> > > > 
> > > > "The inline_data feature requires the ext_attr feature." ?
> > > 
> > > Ah, I have noticed that now we set xattr handler by default.  So, yes,
> > > inline_data feature doesn't require the ext_attr feature.
> > 
> > I think you /do/ still need this check, just in case someone tries to mkfs -O
> > ^ext_attr,inline_data.  Defaults can be overridden.
> 
> I have tested this ('mkfs -O ^ext_attr,inline_data') just a minute ago,
> and it seems that it works well.  BTW, when we use setfattr(1) to set
> xattr in a ext4 file system without ext_attr feature, this feature will
> be enabled in ext4_xattr_update_super_block().

Oh good!  My mistake, then.  That teaches me to squint at my computer at 1:30
in the morning. :)

--D
> 
>                                                 - Zheng
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/misc/mke2fs.8.in b/misc/mke2fs.8.in
index 023ba49..5731542 100644
--- a/misc/mke2fs.8.in
+++ b/misc/mke2fs.8.in
@@ -578,6 +578,9 @@  option).
 @JDEV@must be created with the same
 @JDEV@block size as the filesystems that will be using it.
 .TP
+.B inline_data
+Allow data to be stored in inode
+.TP
 .B large_file
 Filesystem can contain files that are greater than 2GB.  (Modern kernels
 set this feature automatically when a file > 2GB is created.)
diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index a59d24c..cb084ea 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -902,7 +902,8 @@  static __u32 ok_features[3] = {
 		EXT2_FEATURE_INCOMPAT_META_BG|
 		EXT4_FEATURE_INCOMPAT_FLEX_BG|
 		EXT4_FEATURE_INCOMPAT_MMP |
-		EXT4_FEATURE_INCOMPAT_64BIT,
+		EXT4_FEATURE_INCOMPAT_64BIT|
+		EXT4_FEATURE_INCOMPAT_INLINE_DATA,
 	/* R/O compat */
 	EXT2_FEATURE_RO_COMPAT_LARGE_FILE|
 		EXT4_FEATURE_RO_COMPAT_HUGE_FILE|
@@ -2042,6 +2043,17 @@  profile_error:
 		exit(1);
 	}
 
+	/* Since inline_data depends on ext_attr, we would ask it to be
+	 * enabled.
+	 */
+	if ((fs_param.s_feature_incompat & EXT4_FEATURE_INCOMPAT_INLINE_DATA) &&
+	    !(fs_param.s_feature_compat & EXT2_FEATURE_COMPAT_EXT_ATTR)) {
+		com_err(program_name, 0,
+			_("Ext_attr feature not enabled, so inline_data "
+			  "feature doesn't be specified"));
+		exit(1);
+	}
+
 	if (fs_param.s_blocks_per_group) {
 		if (fs_param.s_blocks_per_group < 256 ||
 		    fs_param.s_blocks_per_group > 8 * (unsigned) blocksize) {