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

Jan Kara
2005-07-29T13:46:50Z

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

> 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
--
Jan Kara <jack@suse.cz>
SuSE CR Labs