Signed-off-by: Dmitry Monakhov <[email protected]>
---
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
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 [email protected] if you are still using it.
Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.
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 [email protected] 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
[email protected] 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 [email protected] 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
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.
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 [email protected] 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
I've added this to the ext4 patch queue, thanks.
- Ted