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 <[EMAIL PROTECTED]>
> 
  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 <[EMAIL PROTECTED]>
SuSE CR Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to