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

[Dmitry Monakhov <dmonakhov@openvz.org>]

[-- 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