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

[Mark Bellon <mbellon@mvista.com>]

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/