[PATCH 06/19] ext2: replace inode uid,gid,mode initialization with helper function

[PATCH 06/19] ext2: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext2/ialloc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index 15387c9..4a4b132 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -550,16 +550,12 @@ got:
 
 	sb->s_dirt = 1;
 	mark_buffer_dirty(bh2);
-	inode->i_uid = current_fsuid();
-	if (test_opt (sb, GRPID))
+	if (test_opt (sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino;
 	inode->i_blocks = 0;
-- 
1.6.6

[PATCH 07/19] ext3: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext3/ialloc.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
index b399912..1516e6a 100644
--- a/fs/ext3/ialloc.c
+++ b/fs/ext3/ialloc.c
@@ -538,16 +538,13 @@ got:
 	if (S_ISDIR(mode))
 		percpu_counter_inc(&sbi->s_dirs_counter);
 
-	inode->i_uid = current_fsuid();
-	if (test_opt (sb, GRPID))
-		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
+
+	if (test_opt (sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino;
 	/* This is the optimal IO size (for stat), not the fs block size */
-- 
1.6.6

[PATCH 08/19] ext4: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/ialloc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index f3624ea..1b33417 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -985,16 +985,12 @@ got:
 		atomic_dec(&sbi->s_flex_groups[flex_group].free_inodes);
 	}
 
-	inode->i_uid = current_fsuid();
-	if (test_opt(sb, GRPID))
+	if (test_opt (sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino + group * EXT4_INODES_PER_GROUP(sb);
 	/* This is the optimal IO size (for stat), not the fs block size */
-- 
1.6.6

Re: [PATCH 08/19] ext4: replace inode uid,gid,mode initialization with helper function

[Andreas Dilger]

On 2010-02-17, at 11:40, Dmitry Monakhov wrote:
> @@ -985,16 +985,12 @@ got:
> 		atomic_dec(&sbi->s_flex_groups[flex_group].free_inodes);
> 	}
>
> +	if (test_opt (sb, GRPID)) {

(style) please don't add the space after "test_opt" back into the code.


> +		inode->i_mode = mode;
> +		inode->i_uid = current_fsuid();
> 		inode->i_gid = dir->i_gid;
> 	} else
> +		inode_init_owner(inode, dir, mode);


This code is a bit misleading, since it implies that "i_uid" and  
"i_mode" are set this way due to the GRPID option, even though with  
further investigation it is just set the same way in both cases.

Ted,
what do you think of just removing the "GRPID" mount option?  I  
believe this was around for ages, due to the lack of BSD setgid-on- 
parent functionality in Linux.  I don't think it is needed anymore,  
since the BSD functionality is much more flexible (it can be set on a  
per-directory basis instead of for the whole filesystem).

I suppose one way to find out positively would be to add a printk() to  
case Opt_grpid: in the option parsing that this option is deprecated,  
and to contact linux-ext4@vger.kernel.org if you are still using it.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

Re: [PATCH 06/19] ext2: replace inode uid,gid,mode initialization with helper function

[Jan Kara]

> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
>  fs/ext2/ialloc.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
> index 15387c9..4a4b132 100644
> --- a/fs/ext2/ialloc.c
> +++ b/fs/ext2/ialloc.c
> @@ -550,16 +550,12 @@ got:
>  
>  	sb->s_dirt = 1;
>  	mark_buffer_dirty(bh2);
> -	inode->i_uid = current_fsuid();
> -	if (test_opt (sb, GRPID))
> +	if (test_opt (sb, GRPID)) {
  As Andreas wrote, please don't use space after a function name.
Otherwise the patch looks fine.

							Honza

> +		inode->i_mode = mode;
> +		inode->i_uid = current_fsuid();
>  		inode->i_gid = dir->i_gid;
> -	else if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
>  	} else
> -		inode->i_gid = current_fsgid();
> -	inode->i_mode = mode;
> +		inode_init_owner(inode, dir, mode);
>  
>  	inode->i_ino = ino;
>  	inode->i_blocks = 0;
> -- 
> 1.6.6
> 
> --
> 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
-- 
Jan Kara <jack@suse.cz>
SuSE CR Labs

[PATCH 06/19] ext2: replace inode uid,gid,mode init with helper v2

[Dmitry Monakhov]

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext2/ialloc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
index 15387c9..2c484c6 100644
--- a/fs/ext2/ialloc.c
+++ b/fs/ext2/ialloc.c
@@ -550,16 +550,12 @@ got:
 
 	sb->s_dirt = 1;
 	mark_buffer_dirty(bh2);
-	inode->i_uid = current_fsuid();
-	if (test_opt (sb, GRPID))
+	if (test_opt(sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino;
 	inode->i_blocks = 0;
-- 
1.6.6

[PATCH 07/19] ext3: replace inode uid,gid,mode init with helper v2

[Dmitry Monakhov]

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

changes from v1:
 update code style according to Andreas comments


[-- Attachment #2: 0007-ext3-replace-inode-uid-gid-mode-init-with-helper-v2.patch --]
[-- Type: text/plain, Size: 1105 bytes --]

>From bd7bd775d7c1f939797ddb5df61aa3c4f50ba7b0 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Thu, 18 Feb 2010 09:44:32 +0300
Subject: [PATCH 07/19] ext3: replace inode uid,gid,mode init with helper v2


Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext3/ialloc.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/fs/ext3/ialloc.c b/fs/ext3/ialloc.c
index b399912..1a48294 100644
--- a/fs/ext3/ialloc.c
+++ b/fs/ext3/ialloc.c
@@ -538,16 +538,13 @@ got:
 	if (S_ISDIR(mode))
 		percpu_counter_inc(&sbi->s_dirs_counter);
 
-	inode->i_uid = current_fsuid();
-	if (test_opt (sb, GRPID))
-		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
+
+	if (test_opt(sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino;
 	/* This is the optimal IO size (for stat), not the fs block size */
-- 
1.6.6

[PATCH 08/19] ext4: replace inode uid,gid,mode init with helper v2

[Dmitry Monakhov]

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

Andreas Dilger <adilger@sun.com> writes:

> On 2010-02-17, at 11:40, Dmitry Monakhov wrote:
>> @@ -985,16 +985,12 @@ got:
>> 		atomic_dec(&sbi->s_flex_groups[flex_group].free_inodes);
>> 	}
>>
>> +	if (test_opt (sb, GRPID)) {
>
> (style) please don't add the space after "test_opt" back into the code.
Ok, updated, new version attached.
>
>
>> +		inode->i_mode = mode;
>> +		inode->i_uid = current_fsuid();
>> 		inode->i_gid = dir->i_gid;
>> 	} else
>> +		inode_init_owner(inode, dir, mode);
>
>
> This code is a bit misleading, since it implies that "i_uid" and
> "i_mode" are set this way due to the GRPID option, even though with
> further investigation it is just set the same way in both cases.
>
> Ted,
> what do you think of just removing the "GRPID" mount option?  I
> believe this was around for ages, due to the lack of BSD setgid-on- 
> parent functionality in Linux.  I don't think it is needed anymore,
> since the BSD functionality is much more flexible (it can be set on a
> per-directory basis instead of for the whole filesystem).
>
> I suppose one way to find out positively would be to add a printk() to
> case Opt_grpid: in the option parsing that this option is deprecated,
> and to contact linux-ext4@vger.kernel.org if you are still using it.
Let's post this as separate patch because this change existing behavior.
>
> Cheers, Andreas
> --
> Andreas Dilger
> Sr. Staff Engineer, Lustre Group
> Sun Microsystems of Canada, Inc.

[-- Attachment #2: 0008-ext4-replace-inode-uid-gid-mode-init-with-helper-v2.patch --]
[-- Type: text/plain, Size: 1135 bytes --]

>From a1336d4e57352423a7eb0f7b95ed20e78dcefc8c Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Thu, 18 Feb 2010 09:44:01 +0300
Subject: [PATCH 08/19] ext4: replace inode uid,gid,mode init with helper v2


Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/ialloc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index f3624ea..a2a35d3 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -985,16 +985,12 @@ got:
 		atomic_dec(&sbi->s_flex_groups[flex_group].free_inodes);
 	}
 
-	inode->i_uid = current_fsuid();
-	if (test_opt(sb, GRPID))
+	if (test_opt(sb, GRPID)) {
+		inode->i_mode = mode;
+		inode->i_uid = current_fsuid();
 		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
-		inode->i_gid = dir->i_gid;
-		if (S_ISDIR(mode))
-			mode |= S_ISGID;
 	} else
-		inode->i_gid = current_fsgid();
-	inode->i_mode = mode;
+		inode_init_owner(inode, dir, mode);
 
 	inode->i_ino = ino + group * EXT4_INODES_PER_GROUP(sb);
 	/* This is the optimal IO size (for stat), not the fs block size */
-- 
1.6.6

Re: [PATCH 06/19] ext2: replace inode uid,gid,mode init with helper v2

[Jan Kara]

On Thu 18-02-10 10:00:03, Dmitry Monakhov wrote:
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
  Looks good.
Acked-by: Jan Kara <jack@suse.cz>

							Honza
> ---
>  fs/ext2/ialloc.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c
> index 15387c9..2c484c6 100644
> --- a/fs/ext2/ialloc.c
> +++ b/fs/ext2/ialloc.c
> @@ -550,16 +550,12 @@ got:
>  
>  	sb->s_dirt = 1;
>  	mark_buffer_dirty(bh2);
> -	inode->i_uid = current_fsuid();
> -	if (test_opt (sb, GRPID))
> +	if (test_opt(sb, GRPID)) {
> +		inode->i_mode = mode;
> +		inode->i_uid = current_fsuid();
>  		inode->i_gid = dir->i_gid;
> -	else if (dir->i_mode & S_ISGID) {
> -		inode->i_gid = dir->i_gid;
> -		if (S_ISDIR(mode))
> -			mode |= S_ISGID;
>  	} else
> -		inode->i_gid = current_fsgid();
> -	inode->i_mode = mode;
> +		inode_init_owner(inode, dir, mode);
>  
>  	inode->i_ino = ino;
>  	inode->i_blocks = 0;
> -- 
> 1.6.6
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 08/19] ext4: replace inode uid,gid,mode initialization with helper function

[tytso]

On Wed, Feb 17, 2010 at 04:39:54PM -0700, Andreas Dilger wrote:
> 
> Ted,
> what do you think of just removing the "GRPID" mount option?  I
> believe this was around for ages, due to the lack of BSD setgid-on-
> parent functionality in Linux.  I don't think it is needed anymore,
> since the BSD functionality is much more flexible (it can be set on
> a per-directory basis instead of for the whole filesystem).
> 
> I suppose one way to find out positively would be to add a printk()
> to case Opt_grpid: in the option parsing that this option is
> deprecated, and to contact linux-ext4@vger.kernel.org if you are
> still using it.

There are whole raft of options I'd love to deprecate:

* bsddf/minixdf
* grpid/bsdgroups/nogrpiid/sysvgroups

I also wonder whether it's time that we just enable acls and xattrs
all the time, instead of making them be explicit mount options.

So anyone wants to send me a patch, feel free....  (or I may get to it
maybe this weekend.)

					- Ted

Re: [PATCH 08/19] ext4: replace inode uid,gid,mode initialization with helper function

[Dmitry Monakhov]

tytso@mit.edu writes:

> On Wed, Feb 17, 2010 at 04:39:54PM -0700, Andreas Dilger wrote:
>> 
>> Ted,
>> what do you think of just removing the "GRPID" mount option?  I
>> believe this was around for ages, due to the lack of BSD setgid-on-
>> parent functionality in Linux.  I don't think it is needed anymore,
>> since the BSD functionality is much more flexible (it can be set on
>> a per-directory basis instead of for the whole filesystem).
>> 
>> I suppose one way to find out positively would be to add a printk()
>> to case Opt_grpid: in the option parsing that this option is
>> deprecated, and to contact linux-ext4@vger.kernel.org if you are
>> still using it.
>
> There are whole raft of options I'd love to deprecate:
>
> * bsddf/minixdf
> * grpid/bsdgroups/nogrpiid/sysvgroups
>
> I also wonder whether it's time that we just enable acls and xattrs
> all the time, instead of making them be explicit mount options.
>
> So anyone wants to send me a patch, feel free....  (or I may get to it
I'll handle this, because currently there are not free bits left in
mount_flags.
> maybe this weekend.)
>
> 					- Ted

[PATCH] ext4: deprecate obsoleted mount options

[Dmitry Monakhov]

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

This patch deprecate some obsoleted functions.
It is not obvious what should we do in case of deprecated options on mount.
Just printk and continue or fail the mount, i've implemented the last one.
BTW: Do we need similar patch for e2fslib?


[-- Attachment #2: 0001-ext4-deprecate-obsoleted-mount-options.patch --]
[-- Type: text/plain, Size: 7711 bytes --]

>From 72649dbf903705df6f20c330e0fcecabf24e426e Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Fri, 19 Feb 2010 17:20:36 +0300
Subject: [PATCH] ext4: deprecate obsoleted mount options

Disable following list of obsoleted mount options:
 - bsddf, miniddf
 - grpid, bsdgroups, nogrpid, sysvgroups

Disable following list of obsoleted default mount options
 - bsdgroups

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/ext4.h   |    4 +-
 fs/ext4/ialloc.c |    4 +--
 fs/ext4/super.c  |   96 ++++++++++++++++++++++++++++++++++++-----------------
 3 files changed, 68 insertions(+), 36 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 67859fa..1fd55a7 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -731,12 +731,10 @@ struct ext4_inode_info {
  * Mount flags
  */
 #define EXT4_MOUNT_OLDALLOC		0x00002  /* Don't use the new Orlov allocator */
-#define EXT4_MOUNT_GRPID		0x00004	/* Create files with directory's group */
 #define EXT4_MOUNT_DEBUG		0x00008	/* Some debugging messages */
 #define EXT4_MOUNT_ERRORS_CONT		0x00010	/* Continue on errors */
 #define EXT4_MOUNT_ERRORS_RO		0x00020	/* Remount fs ro on errors */
 #define EXT4_MOUNT_ERRORS_PANIC		0x00040	/* Panic on errors */
-#define EXT4_MOUNT_MINIX_DF		0x00080	/* Mimics the Minix statfs */
 #define EXT4_MOUNT_NOLOAD		0x00100	/* Don't use existing journal*/
 #define EXT4_MOUNT_DATA_FLAGS		0x00C00	/* Mode for data writes: */
 #define EXT4_MOUNT_JOURNAL_DATA		0x00400	/* Write data to journal */
@@ -1185,6 +1183,8 @@ static inline void ext4_clear_inode_state(struct inode *inode, int bit)
 #define EXT4_DEFM_JMODE_DATA	0x0020
 #define EXT4_DEFM_JMODE_ORDERED	0x0040
 #define EXT4_DEFM_JMODE_WBACK	0x0060
+/* Deprecated default mount options mask */
+#define EXT4_DEFM_DEPRECATED	EXT4_DEFM_BSDGROUPS
 
 /*
  * Default journal batch times
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 2fab5ad..0bccf0d 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -986,9 +986,7 @@ got:
 	}
 
 	inode->i_uid = current_fsuid();
-	if (test_opt(sb, GRPID))
-		inode->i_gid = dir->i_gid;
-	else if (dir->i_mode & S_ISGID) {
+	if (dir->i_mode & S_ISGID) {
 		inode->i_gid = dir->i_gid;
 		if (S_ISDIR(mode))
 			mode |= S_ISGID;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 9e45e62..7fb9e77 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -823,12 +823,6 @@ static int ext4_show_options(struct seq_file *seq, struct vfsmount *vfs)
 
 	if (sbi->s_sb_block != 1)
 		seq_printf(seq, ",sb=%llu", sbi->s_sb_block);
-	if (test_opt(sb, MINIX_DF))
-		seq_puts(seq, ",minixdf");
-	if (test_opt(sb, GRPID) && !(def_mount_opts & EXT4_DEFM_BSDGROUPS))
-		seq_puts(seq, ",grpid");
-	if (!test_opt(sb, GRPID) && (def_mount_opts & EXT4_DEFM_BSDGROUPS))
-		seq_puts(seq, ",nogrpid");
 	if (sbi->s_resuid != EXT4_DEF_RESUID ||
 	    le16_to_cpu(es->s_def_resuid) != EXT4_DEF_RESUID) {
 		seq_printf(seq, ",resuid=%u", sbi->s_resuid);
@@ -1096,7 +1090,6 @@ static const struct export_operations ext4_export_ops = {
 };
 
 enum {
-	Opt_bsd_df, Opt_minix_df, Opt_grpid, Opt_nogrpid,
 	Opt_resgid, Opt_resuid, Opt_sb, Opt_err_cont, Opt_err_panic, Opt_err_ro,
 	Opt_nouid32, Opt_debug, Opt_oldalloc, Opt_orlov,
 	Opt_user_xattr, Opt_nouser_xattr, Opt_acl, Opt_noacl,
@@ -1114,16 +1107,16 @@ enum {
 	Opt_block_validity, Opt_noblock_validity,
 	Opt_inode_readahead_blks, Opt_journal_ioprio,
 	Opt_dioread_nolock, Opt_dioread_lock,
-	Opt_discard, Opt_nodiscard,
+	Opt_discard, Opt_nodiscard, Opt_deprecated,
 };
 
 static const match_table_t tokens = {
-	{Opt_bsd_df, "bsddf"},
-	{Opt_minix_df, "minixdf"},
-	{Opt_grpid, "grpid"},
-	{Opt_grpid, "bsdgroups"},
-	{Opt_nogrpid, "nogrpid"},
-	{Opt_nogrpid, "sysvgroups"},
+	{Opt_deprecated, "bsddf"},
+	{Opt_deprecated, "minixdf"},
+	{Opt_deprecated, "grpid"},
+	{Opt_deprecated, "bsdgroups"},
+	{Opt_deprecated, "nogrpid"},
+	{Opt_deprecated, "sysvgroups"},
 	{Opt_resgid, "resgid=%u"},
 	{Opt_resuid, "resuid=%u"},
 	{Opt_sb, "sb=%u"},
@@ -1188,6 +1181,18 @@ static const match_table_t tokens = {
 	{Opt_err, NULL},
 };
 
+static const match_table_t default_mountopt = {
+	{EXT4_DEFM_DEBUG, "debug"},
+	{EXT4_DEFM_BSDGROUPS, "bsdgroups"},
+	{EXT4_DEFM_XATTR_USER, "user_xattr"},
+	{EXT4_DEFM_ACL, "acl"},
+	{EXT4_DEFM_UID16, "uid16"},
+	{EXT4_DEFM_JMODE_WBACK, "journal_data_writeback"},
+	{EXT4_DEFM_JMODE_DATA, "journal_data"},
+	{EXT4_DEFM_JMODE_ORDERED, "journal_data_ordered"},
+	{0, NULL},
+};
+
 static ext4_fsblk_t get_sb_block(void **data)
 {
 	ext4_fsblk_t	sb_block;
@@ -1238,18 +1243,6 @@ static int parse_options(char *options, struct super_block *sb,
 
 		token = match_token(p, tokens, args);
 		switch (token) {
-		case Opt_bsd_df:
-			clear_opt(sbi->s_mount_opt, MINIX_DF);
-			break;
-		case Opt_minix_df:
-			set_opt(sbi->s_mount_opt, MINIX_DF);
-			break;
-		case Opt_grpid:
-			set_opt(sbi->s_mount_opt, GRPID);
-			break;
-		case Opt_nogrpid:
-			clear_opt(sbi->s_mount_opt, GRPID);
-			break;
 		case Opt_resuid:
 			if (match_int(&args[0], &option))
 				return 0;
@@ -1622,6 +1615,12 @@ set_qf_format:
 		case Opt_dioread_lock:
 			clear_opt(sbi->s_mount_opt, DIOREAD_NOLOCK);
 			break;
+		case Opt_deprecated:
+			ext4_msg(sb, KERN_ERR,
+				"Deprecated mount option \"%s\". "
+				"Please contact linux-ext4@vger.kernel.org if "
+				"you are still using it.", p);
+			return 0;
 		default:
 			ext4_msg(sb, KERN_ERR,
 			       "Unrecognized mount option \"%s\" "
@@ -2358,7 +2357,42 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly)
 	}
 	return 1;
 }
+static int handle_deprecated_defmopt(struct super_block *sb, unsigned int opt)
+{
+	int first = 1;
+	/*
+	 * When deprecated options are found they are not cleared from
+	 * super block by default. Just print error message and let
+	 * user clear it manually.
+	 */
+	printk(KERN_ERR "EXT4-fs (%s) Deprecated default mount options:",
+		sb->s_id);
+	/*
+	 * It is impossible to use simple bit traversing because,
+	 * some options use shared bits.
+	 */
+	opt &= EXT4_DEFM_DEPRECATED;
+	while (opt) {
+		const struct match_token *mt = default_mountopt;
+		while (mt->pattern != NULL) {
+			if ((opt & mt->token) == mt->token)
+				break;
+			mt++;
+		}
+		if (mt->pattern == NULL) {
+			printk("%s unknown", !first ? "," : "");
+			break;
+		} else {
+			printk("%s %s", !first ? "," : "", mt->pattern);
+			opt &= ~mt->token;
+		}
+		first = 0;
+	}
+	printk(". Please use tune2fs to disable it, or contact "
+		"linux-ext4@vger.kernel.org if you are still need it.");
 
+	return 1;
+}
 static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 				__releases(kernel_lock)
 				__acquires(kernel_lock)
@@ -2443,10 +2477,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 
 	/* Set defaults before we parse the mount options */
 	def_mount_opts = le32_to_cpu(es->s_default_mount_opts);
+	if (def_mount_opts & EXT4_DEFM_DEPRECATED) {
+		if (handle_deprecated_defmopt(sb, def_mount_opts))
+			goto failed_mount;
+	}
 	if (def_mount_opts & EXT4_DEFM_DEBUG)
 		set_opt(sbi->s_mount_opt, DEBUG);
-	if (def_mount_opts & EXT4_DEFM_BSDGROUPS)
-		set_opt(sbi->s_mount_opt, GRPID);
 	if (def_mount_opts & EXT4_DEFM_UID16)
 		set_opt(sbi->s_mount_opt, NO_UID32);
 #ifdef CONFIG_EXT4_FS_XATTR
@@ -3668,9 +3704,7 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
 	struct ext4_super_block *es = sbi->s_es;
 	u64 fsid;
 
-	if (test_opt(sb, MINIX_DF)) {
-		sbi->s_overhead_last = 0;
-	} else if (sbi->s_blocks_last != ext4_blocks_count(es)) {
+	if (sbi->s_blocks_last != ext4_blocks_count(es)) {
 		ext4_group_t i, ngroups = ext4_get_groups_count(sb);
 		ext4_fsblk_t overhead = 0;
 
-- 
1.6.6

Re: [PATCH] ext4: deprecate obsoleted mount options

[Andreas Dilger]

On 2010-02-19, at 07:39, Dmitry Monakhov wrote:
> This patch deprecate some obsoleted functions.
> It is not obvious what should we do in case of deprecated options on  
> mount.
> Just printk and continue or fail the mount, i've implemented the  
> last one.
> BTW: Do we need similar patch for e2fslib?

I think deprecating an option is not the same as removing it  
entirely.  Even though I don't think these options are in wide usage,  
I'd still prefer to add a printk() to the parsing code first, leave it  
for a year, then remove them entirely after that.

> #define EXT4_MOUNT_OLDALLOC		0x00002  /* Don't use the new Orlov  
> allocator */
> -#define EXT4_MOUNT_GRPID		0x00004	/* Create files with directory's  
> group */
> #define EXT4_MOUNT_DEBUG		0x00008	/* Some debugging messages */
> #define EXT4_MOUNT_ERRORS_CONT		0x00010	/* Continue on errors */
> #define EXT4_MOUNT_ERRORS_RO		0x00020	/* Remount fs ro on errors */
> #define EXT4_MOUNT_ERRORS_PANIC		0x00040	/* Panic on errors */
> -#define EXT4_MOUNT_MINIX_DF		0x00080	/* Mimics the Minix statfs */
> #define EXT4_MOUNT_NOLOAD		0x00100	/* Don't use existing journal*/
> #define EXT4_MOUNT_DATA_FLAGS		0x00C00	/* Mode for data writes: */
> #define EXT4_MOUNT_JOURNAL_DATA		0x00400	/* Write data to journal */

I'd prefer to leave a comment like "/* was EXT4_MOUNT_GRPID -  
2010/02/21 */" so that it is clear that these values were previously  
in use, and should not be re-used until other options are exhausted...

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

[PATCH] ext4: deprecate obsoleted mount options v2

[Dmitry Monakhov]

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

Andreas Dilger <adilger@sun.com> writes:

> On 2010-02-19, at 07:39, Dmitry Monakhov wrote:
>> This patch deprecate some obsoleted functions.
>> It is not obvious what should we do in case of deprecated options on
>> mount.
>> Just printk and continue or fail the mount, i've implemented the
>> last one.
>> BTW: Do we need similar patch for e2fslib?
>
> I think deprecating an option is not the same as removing it entirely.
Ohh.. I've hoped to reuse freed bits for new crap.
> Even though I don't think these options are in wide usage,  I'd still
> prefer to add a printk() to the parsing code first, leave it  for a
> year, then remove them entirely after that.
So option deprecation result in code boosting instead of code shrinkage.
Indeed the second law of thermodynamics is absolutely true.
New version attached.


[-- Attachment #2: 0001-ext4-deprecate-obsoleted-mount-options-v2.patch --]
[-- Type: text/plain, Size: 5021 bytes --]

>From 0d12e733a6dd87ed3bc4dea27126f6e814b5586b Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Tue, 23 Feb 2010 22:11:47 +0300
Subject: [PATCH] ext4: deprecate obsoleted mount options v2

Declare following list of mount options as deprecated:
 - bsddf, miniddf
 - grpid, bsdgroups, nogrpid, sysvgroups

Declare following list of default mount options as deprecated:
 - bsdgroups

Changes from v1
 - Mark options as deprecated instead of disabling it completely,
   they will be disabled after exportation period.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/ext4.h  |    2 +
 fs/ext4/super.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 67859fa..66ed482 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1185,6 +1185,8 @@ static inline void ext4_clear_inode_state(struct inode *inode, int bit)
 #define EXT4_DEFM_JMODE_DATA	0x0020
 #define EXT4_DEFM_JMODE_ORDERED	0x0040
 #define EXT4_DEFM_JMODE_WBACK	0x0060
+/* Deprecated default mount options mask */
+#define EXT4_DEFM_DEPRECATED	EXT4_DEFM_BSDGROUPS
 
 /*
  * Default journal batch times
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d5596ca..d07f506 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1114,7 +1114,7 @@ enum {
 	Opt_block_validity, Opt_noblock_validity,
 	Opt_inode_readahead_blks, Opt_journal_ioprio,
 	Opt_dioread_nolock, Opt_dioread_lock,
-	Opt_discard, Opt_nodiscard,
+	Opt_discard, Opt_nodiscard, Opt_deprecated, Opt_disabled,
 };
 
 static const match_table_t tokens = {
@@ -1188,6 +1188,18 @@ static const match_table_t tokens = {
 	{Opt_err, NULL},
 };
 
+static const match_table_t default_mountopt = {
+	{EXT4_DEFM_DEBUG, "debug"},
+	{EXT4_DEFM_BSDGROUPS, "bsdgroups"},
+	{EXT4_DEFM_XATTR_USER, "user_xattr"},
+	{EXT4_DEFM_ACL, "acl"},
+	{EXT4_DEFM_UID16, "uid16"},
+	{EXT4_DEFM_JMODE_WBACK, "journal_data_writeback"},
+	{EXT4_DEFM_JMODE_DATA, "journal_data"},
+	{EXT4_DEFM_JMODE_ORDERED, "journal_data_ordered"},
+	{0, NULL},
+};
+
 static ext4_fsblk_t get_sb_block(void **data)
 {
 	ext4_fsblk_t	sb_block;
@@ -1240,16 +1252,16 @@ static int parse_options(char *options, struct super_block *sb,
 		switch (token) {
 		case Opt_bsd_df:
 			clear_opt(sbi->s_mount_opt, MINIX_DF);
-			break;
+			goto deprecated;
 		case Opt_minix_df:
 			set_opt(sbi->s_mount_opt, MINIX_DF);
-			break;
+			goto deprecated;
 		case Opt_grpid:
 			set_opt(sbi->s_mount_opt, GRPID);
-			break;
+			goto deprecated;
 		case Opt_nogrpid:
 			clear_opt(sbi->s_mount_opt, GRPID);
-			break;
+			goto deprecated;
 		case Opt_resuid:
 			if (match_int(&args[0], &option))
 				return 0;
@@ -1622,6 +1634,21 @@ set_qf_format:
 		case Opt_dioread_lock:
 			clear_opt(sbi->s_mount_opt, DIOREAD_NOLOCK);
 			break;
+
+		case Opt_deprecated:
+deprecated:
+			ext4_msg(sb, KERN_ERR,
+				"Deprecated mount option \"%s\". Will be "
+				"removed soon. Please contact "
+				"linux-ext4@vger.kernel.org if you are still "
+				"using it.", p);
+			break;
+
+		case Opt_disabled:
+			ext4_msg(sb, KERN_ERR,
+				"Deprecated mount option \"%s\". And not "
+				"longer supported.", p);
+			return 0;
 		default:
 			ext4_msg(sb, KERN_ERR,
 			       "Unrecognized mount option \"%s\" "
@@ -2358,7 +2385,42 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly)
 	}
 	return 1;
 }
+static int handle_deprecated_defmopt(struct super_block *sb, unsigned int opt)
+{
+	int first = 1;
+	/*
+	 * When deprecated options are found they are not cleared from
+	 * super block by default. Just print error message and let
+	 * user clear it manually.
+	 */
+	printk(KERN_ERR "EXT4-fs (%s) Deprecated default mount options:",
+		sb->s_id);
+	/*
+	 * It is impossible to use simple bit traversing because,
+	 * some options use shared bits.
+	 */
+	opt &= EXT4_DEFM_DEPRECATED;
+	while (opt) {
+		const struct match_token *mt = default_mountopt;
+		while (mt->pattern != NULL) {
+			if ((opt & mt->token) == mt->token)
+				break;
+			mt++;
+		}
+		if (mt->pattern == NULL) {
+			printk("%s unknown", !first ? "," : "");
+			break;
+		} else {
+			printk("%s %s", !first ? "," : "", mt->pattern);
+			opt &= ~mt->token;
+		}
+		first = 0;
+	}
+	printk(". Please use tune2fs to disable it, or contact "
+		"linux-ext4@vger.kernel.org if you are still need it.");
 
+	return 0;
+}
 static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 				__releases(kernel_lock)
 				__acquires(kernel_lock)
@@ -2443,6 +2505,10 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 
 	/* Set defaults before we parse the mount options */
 	def_mount_opts = le32_to_cpu(es->s_default_mount_opts);
+	if (def_mount_opts & EXT4_DEFM_DEPRECATED) {
+		if (handle_deprecated_defmopt(sb, def_mount_opts))
+			goto failed_mount;
+	}
 	if (def_mount_opts & EXT4_DEFM_DEBUG)
 		set_opt(sbi->s_mount_opt, DEBUG);
 	if (def_mount_opts & EXT4_DEFM_BSDGROUPS)
-- 
1.6.6

Re: [PATCH] ext4: deprecate obsoleted mount options v2

[tytso]

On Tue, Feb 23, 2010 at 10:23:45PM +0300, Dmitry Monakhov wrote:
> >
> > I think deprecating an option is not the same as removing it entirely.
> Ohh.. I've hoped to reuse freed bits for new crap.

What "new crap" are you hoping to will need mount options?  One of the
things I want to do long term is to try to reduce/remove mount options
in general.

If we get general agreement that it's time to just turn on acl's and
xattr's by default, we can change the default, and in that case
removing the "noacl/noxattr" might be something that we might not need
to keep for as long, or maybe at all.  But for things like
bsddf/minixdf, we do need some kind of deprecation schedule.

The use of Opt_deprecated and Opt_disabled seems a little pointless;
nothing is using now, and nothing needs it.  All I'd probably do is
something like this:

static char deprecated_msg[] = "Mount option \"%s\" will be removed by %s\n"
       "Contact linux-ext4@vger.kernel.org if you think we should keep it.\n"

And then in each option that we want to deprecate, just add:

    ext4_msg(sb, KERN_WARN, deprecated_msg, "bsddf", "2.6.39");

The extra opt_discard, and goto deprecated, etc., seems way more
complicated than what is necessary.

						- Ted

[PATCH] ext4: deprecate obsoleted mount options v3

[Dmitry Monakhov]

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

tytso@mit.edu writes:

> On Tue, Feb 23, 2010 at 10:23:45PM +0300, Dmitry Monakhov wrote:
>> >
>> > I think deprecating an option is not the same as removing it entirely.
>> Ohh.. I've hoped to reuse freed bits for new crap.
>
> What "new crap" are you hoping to will need mount options? 
I want two bits in mount flags. But only one is available
for now. Don't you mind to extend mount flags to 64 bits?
>One of the
> things I want to do long term is to try to reduce/remove mount options
> in general.
>
> If we get general agreement that it's time to just turn on acl's and
> xattr's by default, we can change the default, and in that case
> removing the "noacl/noxattr" might be something that we might not need
> to keep for as long, or maybe at all.  But for things like
> bsddf/minixdf, we do need some kind of deprecation schedule.
>
> The use of Opt_deprecated and Opt_disabled seems a little pointless;
> nothing is using now, and nothing needs it.  All I'd probably do is
> something like this:
>
> static char deprecated_msg[] = "Mount option \"%s\" will be removed by %s\n"
>        "Contact linux-ext4@vger.kernel.org if you think we should keep it.\n"
>
> And then in each option that we want to deprecate, just add:
>
>     ext4_msg(sb, KERN_WARN, deprecated_msg, "bsddf", "2.6.39");
Yess. Definitely this look nicer.


[-- Attachment #2: 0001-ext4-deprecate-obsoleted-mount-options-v3.patch --]
[-- Type: text/plain, Size: 2515 bytes --]

>From 92ce6a649e6f9a99c9aa34b1650a7a85c8ee5b82 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Wed, 24 Feb 2010 21:00:49 +0300
Subject: [PATCH] ext4: deprecate obsoleted mount options v3

Declare following list of mount options as deprecated:
 - bsddf, miniddf
 - grpid, bsdgroups, nogrpid, sysvgroups

Declare following list of default mount options as deprecated:
 - bsdgroups

Changes from v1
 - Mark options as deprecated instead of disabling it completely,
   they will be disabled after exportation period.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/super.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d5596ca..2b413c5 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1212,6 +1212,8 @@ static ext4_fsblk_t get_sb_block(void **data)
 }
 
 #define DEFAULT_JOURNAL_IOPRIO (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, 3))
+static char deprecated_msg[] = "Mount option \"%s\" will be removed by %s\n"
+	"Contact linux-ext4@vger.kernel.org if you think we should keep it.\n";
 
 static int parse_options(char *options, struct super_block *sb,
 			 unsigned long *journal_devnum,
@@ -1239,16 +1241,23 @@ static int parse_options(char *options, struct super_block *sb,
 		token = match_token(p, tokens, args);
 		switch (token) {
 		case Opt_bsd_df:
+			ext4_msg(sb, KERN_WARNING, deprecated_msg, p, "2.6.39");
 			clear_opt(sbi->s_mount_opt, MINIX_DF);
 			break;
 		case Opt_minix_df:
+			ext4_msg(sb, KERN_WARNING, deprecated_msg, p, "2.6.39");
 			set_opt(sbi->s_mount_opt, MINIX_DF);
+
 			break;
 		case Opt_grpid:
+			ext4_msg(sb, KERN_WARNING, deprecated_msg, p, "2.6.39");
 			set_opt(sbi->s_mount_opt, GRPID);
+
 			break;
 		case Opt_nogrpid:
+			ext4_msg(sb, KERN_WARNING, deprecated_msg, p, "2.6.39");
 			clear_opt(sbi->s_mount_opt, GRPID);
+
 			break;
 		case Opt_resuid:
 			if (match_int(&args[0], &option))
@@ -2445,8 +2454,11 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	def_mount_opts = le32_to_cpu(es->s_default_mount_opts);
 	if (def_mount_opts & EXT4_DEFM_DEBUG)
 		set_opt(sbi->s_mount_opt, DEBUG);
-	if (def_mount_opts & EXT4_DEFM_BSDGROUPS)
+	if (def_mount_opts & EXT4_DEFM_BSDGROUPS) {
+		ext4_msg(sb, KERN_WARNING, deprecated_msg, "bsdgroups",
+			"2.6.39");
 		set_opt(sbi->s_mount_opt, GRPID);
+	}
 	if (def_mount_opts & EXT4_DEFM_UID16)
 		set_opt(sbi->s_mount_opt, NO_UID32);
 #ifdef CONFIG_EXT4_FS_XATTR
-- 
1.6.6

Re: [PATCH] ext4: deprecate obsoleted mount options v3

[tytso]

I've added this to the ext4 patch queue, thanks.

     	   	       	    	  	 - Ted