Re: [PATCH] disk quotas fail when /etc/mtab is symlinked to /proc/mounts

• Previous message: Michael Thonke: "Re: 2.6.13-rc3-mm3"
• In reply to: Jan Kara: "Re: [PATCH] disk quotas fail when /etc/mtab is symlinked to /proc/mounts"
• Next in thread: Mark Bellon: "[PATCH] (REPOST/REVISION) disk quotas fail when /etc/mtab is symlinked to /proc/mounts"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Date:	Fri, 29 Jul 2005 08:47:42 -0700
To: Jan Kara <jack@suse.cz>

Jan Kara wrote:

> Hello,
>
>
>
>>If /etc/mtab is a regular file all of the mount options (of a file
>>system) are written to /etc/mtab by the mount command. The quota tools
>>look there for the quota strings for their operation. If, however,
>>/etc/mtab is a symlink to /proc/mounts (a "good thing" in some
>>environments) the tools don't write anything - they assume the kernel
>>will take care of things.
>>
>>While the quota options are sent down to the kernel via the mount system
>>call and the file system codes handle them properly unfortunately there
>>is no code to echo the quota strings into /proc/mounts and the quota
>>tools fail in the symlink case.
>>
>>
> Yes, I agree that it's a good think to have quota options in
>/proc/mounts. Doing it per filesystem is not nice but I don't know a
>nice way of doing it a VFS level either...
>
>
I'll have a new patch which includes your comments and a few I thought
up soon. I'll repost the new patch.

mark

>
>
>>The attached patchs modify the EXT[2|3] and [X|J]FS codes to add the
>>necessary hooks. The show_options function of each file system in these
>>patches currently deal with only those things that seemed related to
>>quotas; especially in the EXT3 case more can be done (later?).
>>
>>The EXT3 has added error checking and has two minor changes:
>> The "quota" option is considered part of the older style quotas
>> Journalled quotas and older style quotas are mutually exclusive.
>> - both discussable topics
>>
>>
> Ack.
>
>
>
>>mark
>>
>>Signed-off-by: Mark Bellon <mbellon@mvista.com>
>>
>>
>>
> Thanks for the patch - I have some comments below...
>
> <snip>
>
>
>> #ifdef CONFIG_QUOTA
>>+static int ext3_show_options(struct seq_file *seq, struct vfsmount *vfs)
>>+{
>>+ struct ext3_sb_info *sbi = EXT3_SB(vfs->mnt_sb);
>>+
>>+ if (sbi->s_mount_opt & EXT3_MOUNT_JOURNAL_DATA)
>>+ seq_puts(seq, ",data=journal");
>>+
>>+ if (sbi->s_mount_opt & EXT3_MOUNT_ORDERED_DATA)
>>+ seq_puts(seq, ",data=ordered");
>>+
>>+ if (sbi->s_mount_opt & EXT3_MOUNT_WRITEBACK_DATA)
>>+ seq_puts(seq, ",data=writeback");
>>
>>
> Showing 'data' option only when quota is compile is ugly... Please move
>CONFIG_QUOTA inside only around quota specific stuff.
>
>
>
>>+
>>+ if (sbi->s_jquota_fmt)
>>+ seq_printf(seq, ",jqfmt=%s",
>>+ (sbi->s_jquota_fmt == QFMT_VFS_OLD) ? "vfsold": "vfsv0");
>>+
>>+ if (sbi->s_qf_names[USRQUOTA])
>>+ seq_printf(seq, ",usrjquota=%s", sbi->s_qf_names[USRQUOTA]);
>>+
>>+ if (sbi->s_qf_names[GRPQUOTA])
>>+ seq_printf(seq, ",grpjquota=%s", sbi->s_qf_names[GRPQUOTA]);
>>+
>>+ if (sbi->s_mount_opt & EXT3_MOUNT_USRQUOTA)
>>+ seq_puts(seq, ",usrquota");
>>+
>>+ if (sbi->s_mount_opt & EXT3_MOUNT_GRPQUOTA)
>>+ seq_puts(seq, ",grpquota");
>>+
>>+ return 0;
>>+}
>>
>> #define QTYPE2NAME(t) ((t)==USRQUOTA?"user":"group")
>> #define QTYPE2MOPT(on, t) ((t)==USRQUOTA?((on)##USRJQUOTA):((on)##GRPJQUOTA))
>>@@ -572,6 +604,7 @@
>> #ifdef CONFIG_QUOTA
>> .quota_read = ext3_quota_read,
>> .quota_write = ext3_quota_write,
>>+ .show_options = ext3_show_options,
>>
>>
> Probably set this function everytime...
>
><snip>
>
>
>
>>+ case Opt_quota:
>>+ case Opt_usrquota:
>>+ case Opt_grpquota:
>> case Opt_usrjquota:
>> case Opt_grpjquota:
>> case Opt_offusrjquota:
>>@@ -924,7 +973,6 @@
>> "EXT3-fs: journalled quota options not "
>> "supported.\n");
>> break;
>>- case Opt_quota:
>> case Opt_noquota:
>> break;
>>
>>
> I'm not sure with the above change.. Previously if you mounted a
>filesystem with 'usrquota' option without a kernel quota support the mount
>succeeded. With your patch it will fail. I agree that that makes more
>sense but I'm not sure it's correct to change a behaviour so suddently.
>Maybe just issue a warning but let the mount succeed.
>
>
>
>> #endif
>>@@ -962,11 +1010,36 @@
>> }
>> }
>> #ifdef CONFIG_QUOTA
>>- if (!sbi->s_jquota_fmt && (sbi->s_qf_names[USRQUOTA] ||
>>- sbi->s_qf_names[GRPQUOTA])) {
>>- printk(KERN_ERR
>>+ if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
>>+ if ((sbi->s_mount_opt & EXT3_MOUNT_USRQUOTA) ||
>>+ (sbi->s_mount_opt & EXT3_MOUNT_GRPQUOTA)) {
>>+ printk(KERN_ERR
>>+ "EXT3-fs: only one type of quotas allowed.\n");
>>+
>>+ return 0;
>>+ }
>>+
>>+ if (!sbi->s_jquota_fmt) {
>>+ printk(KERN_ERR
>> "EXT3-fs: journalled quota format not specified.\n");
>>- return 0;
>>+
>>+ return 0;
>>+ }
>>+
>>+ if ((sbi->s_mount_opt & EXT3_MOUNT_JOURNAL_DATA) == 0) {
>>
>>
> This test does not make sense - journaled quota in recent kernels
>works with arbitrary journaling data mode.
>
>
>
>>+ printk(KERN_ERR
>>+ "EXT3-fs: journalled quota specified when data journalling is not.\n");
>>+
>>+ return 0;
>>+ }
>>+ }
>>+ else {
>>+ if (sbi->s_jquota_fmt) {
>>+ printk(KERN_ERR
>>+"EXT3-fs: journalled quota format specified with no journalling enabled.\n");
>>+
>>+ return 0;
>>+ }
>> }
>> #endif
>>
>>+#if defined(CONFIG_QUOTA)
>>+ case Opt_usrquota:
>>+ set_opt(sbi->s_mount_opt, USRQUOTA);
>>+ break;
>>+
>>+ case Opt_grpquota:
>>+ set_opt(sbi->s_mount_opt, GRPQUOTA);
>>+ break;
>>+
>>+ case Opt_quota:
>>+ set_opt(sbi->s_mount_opt, GRPQUOTA);
>>+ set_opt(sbi->s_mount_opt, USRQUOTA);
>>+ break;
>>
>>
> The old 'quota' option means the same as 'usrquota' - at least tools
>consider it like that.
>
>
>
>>+#else
>>+ case Opt_quota:
>>+ case Opt_usrquota:
>>+ case Opt_grpquota:
>>+ printk(KERN_ERR
>>+ "EXT2-fs: quota operations not supported.\n");
>>+
>>+ break;
>>+#endif
>>
>>
> The same as with ext3 - I don't like this change...
>
> <snip>
>
> Honza
>
>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

• Previous message: Michael Thonke: "Re: 2.6.13-rc3-mm3"
• In reply to: Jan Kara: "Re: [PATCH] disk quotas fail when /etc/mtab is symlinked to /proc/mounts"
• Next in thread: Mark Bellon: "[PATCH] (REPOST/REVISION) disk quotas fail when /etc/mtab is symlinked to /proc/mounts"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]