| Message ID | 1266432013-9241-3-git-send-email-dmonakhov@openvz.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
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. -- 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
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 -- 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
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 -- 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 --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 */
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org> --- fs/ext4/ialloc.c | 12 ++++-------- 1 files changed, 4 insertions(+), 8 deletions(-)