2016-07-07 08:08:42

by Wang Shilong

[permalink] [raw]
Subject: [PATCH] ext4: add project quota mount options

From: Wang Shilong <[email protected]>

add prjquota, prjjquota, offprjjquota mount options
for project quota.

These kind of mount options are used for old
quota design, and we can use quotas like these
way:

# mkfs.ext4 /dev/sda
# mount /dev/sda -o prjquota /mnt/test
# quotacheck -p /mnt/test
# quotaon /mnt/test

This new mount options are also useful to unify
some generic tests for xfs and ext4.

Signed-off-by: Wang Shilong <[email protected]>
---
fs/ext4/ext4.h | 1 +
fs/ext4/ialloc.c | 4 ++--
fs/ext4/inode.c | 7 ++-----
fs/ext4/ioctl.c | 6 ++++--
fs/ext4/super.c | 43 +++++++++++++++++++++++++++++++++----------
5 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index b84aa1c..315838a 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1129,6 +1129,7 @@ struct ext4_inode_info {
#define EXT4_MOUNT_POSIX_ACL 0x08000 /* POSIX Access Control Lists */
#define EXT4_MOUNT_NO_AUTO_DA_ALLOC 0x10000 /* No auto delalloc mapping */
#define EXT4_MOUNT_BARRIER 0x20000 /* Use block barriers */
+#define EXT4_MOUNT_PRJQUOTA 0x40000 /* "old" project quota */
#define EXT4_MOUNT_QUOTA 0x80000 /* Some quota option set */
#define EXT4_MOUNT_USRQUOTA 0x100000 /* "old" user quota */
#define EXT4_MOUNT_GRPQUOTA 0x200000 /* "old" group quota */
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 3da4cf8..f7209fc 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -802,8 +802,8 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
} else
inode_init_owner(inode, dir, mode);

- if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT) &&
- ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT))
+ if ((EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT) ||
+ test_opt(sb, PRJQUOTA)) && ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT))
ei->i_projid = EXT4_I(dir)->i_projid;
else
ei->i_projid = make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index f7140ca..c407f9b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4385,7 +4385,8 @@ static inline void ext4_iget_extra_inode(struct inode *inode,

int ext4_get_projid(struct inode *inode, kprojid_t *projid)
{
- if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb, EXT4_FEATURE_RO_COMPAT_PROJECT))
+ if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb, EXT4_FEATURE_RO_COMPAT_PROJECT)
+ && !test_opt(inode->i_sb, PRJQUOTA))
return -EOPNOTSUPP;
*projid = EXT4_I(inode)->i_projid;
return 0;
@@ -4856,10 +4857,6 @@ static int ext4_do_update_inode(handle_t *handle,
}
}

- BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
- EXT4_FEATURE_RO_COMPAT_PROJECT) &&
- i_projid != EXT4_DEF_PROJID);
-
if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
EXT4_FITS_IN_INODE(raw_inode, ei, i_projid))
raw_inode->i_projid = cpu_to_le32(i_projid);
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index b5a39b0..6062a81 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -311,7 +311,8 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
struct dquot *transfer_to[MAXQUOTAS] = { };

if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
- EXT4_FEATURE_RO_COMPAT_PROJECT)) {
+ EXT4_FEATURE_RO_COMPAT_PROJECT) &&
+ !test_opt(sb, PRJQUOTA)) {
if (projid != EXT4_DEF_PROJID)
return -EOPNOTSUPP;
else
@@ -849,7 +850,8 @@ encryption_policy_out:
fa.fsx_xflags = ext4_iflags_to_xflags(ei->i_flags & EXT4_FL_USER_VISIBLE);

if (EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
- EXT4_FEATURE_RO_COMPAT_PROJECT)) {
+ EXT4_FEATURE_RO_COMPAT_PROJECT) ||
+ test_opt(inode->i_sb, PRJQUOTA)) {
fa.fsx_projid = (__u32)from_kprojid(&init_user_ns,
EXT4_I(inode)->i_projid);
}
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3822a5a..93a776f 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1184,10 +1184,10 @@ enum {
Opt_journal_path, Opt_journal_checksum, Opt_journal_async_commit,
Opt_abort, Opt_data_journal, Opt_data_ordered, Opt_data_writeback,
Opt_data_err_abort, Opt_data_err_ignore, Opt_test_dummy_encryption,
- Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
- Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_jqfmt_vfsv1, Opt_quota,
- Opt_noquota, Opt_barrier, Opt_nobarrier, Opt_err,
- Opt_usrquota, Opt_grpquota, Opt_i_version, Opt_dax,
+ Opt_usrjquota, Opt_grpjquota, Opt_prjjquota, Opt_offusrjquota,
+ Opt_offgrpjquota, Opt_offprjjquota, Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0,
+ Opt_jqfmt_vfsv1, Opt_quota, Opt_noquota, Opt_barrier, Opt_nobarrier,
+ Opt_err, Opt_usrquota, Opt_grpquota, Opt_prjquota, Opt_i_version, Opt_dax,
Opt_stripe, Opt_delalloc, Opt_nodelalloc, Opt_mblk_io_submit,
Opt_lazytime, Opt_nolazytime,
Opt_nomblk_io_submit, Opt_block_validity, Opt_noblock_validity,
@@ -1240,6 +1240,8 @@ static const match_table_t tokens = {
{Opt_usrjquota, "usrjquota=%s"},
{Opt_offgrpjquota, "grpjquota="},
{Opt_grpjquota, "grpjquota=%s"},
+ {Opt_prjjquota, "prjjquota=%s"},
+ {Opt_offprjjquota, "prjjquota="},
{Opt_jqfmt_vfsold, "jqfmt=vfsold"},
{Opt_jqfmt_vfsv0, "jqfmt=vfsv0"},
{Opt_jqfmt_vfsv1, "jqfmt=vfsv1"},
@@ -1250,6 +1252,7 @@ static const match_table_t tokens = {
{Opt_barrier, "barrier=%u"},
{Opt_barrier, "barrier"},
{Opt_nobarrier, "nobarrier"},
+ {Opt_prjquota, "prjquota"},
{Opt_i_version, "i_version"},
{Opt_dax, "dax"},
{Opt_stripe, "stripe=%u"},
@@ -1466,12 +1469,17 @@ static const struct mount_opts {
MOPT_SET | MOPT_Q},
{Opt_grpquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_GRPQUOTA,
MOPT_SET | MOPT_Q},
+ {Opt_prjquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_PRJQUOTA,
+ MOPT_SET | MOPT_Q},
{Opt_noquota, (EXT4_MOUNT_QUOTA | EXT4_MOUNT_USRQUOTA |
- EXT4_MOUNT_GRPQUOTA), MOPT_CLEAR | MOPT_Q},
+ EXT4_MOUNT_GRPQUOTA | EXT4_MOUNT_PRJQUOTA),
+ MOPT_CLEAR | MOPT_Q},
{Opt_usrjquota, 0, MOPT_Q},
{Opt_grpjquota, 0, MOPT_Q},
+ {Opt_prjjquota, 0, MOPT_Q},
{Opt_offusrjquota, 0, MOPT_Q},
{Opt_offgrpjquota, 0, MOPT_Q},
+ {Opt_offprjjquota, 0, MOPT_Q},
{Opt_jqfmt_vfsold, QFMT_VFS_OLD, MOPT_QFMT},
{Opt_jqfmt_vfsv0, QFMT_VFS_V0, MOPT_QFMT},
{Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
@@ -1495,10 +1503,14 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
return set_qf_name(sb, USRQUOTA, &args[0]);
else if (token == Opt_grpjquota)
return set_qf_name(sb, GRPQUOTA, &args[0]);
+ else if (token == Opt_prjjquota)
+ return set_qf_name(sb, PRJQUOTA, &args[0]);
else if (token == Opt_offusrjquota)
return clear_qf_name(sb, USRQUOTA);
else if (token == Opt_offgrpjquota)
return clear_qf_name(sb, GRPQUOTA);
+ else if (token == Opt_offprjjquota)
+ return clear_qf_name(sb, PRJQUOTA);
#endif
switch (token) {
case Opt_noacl:
@@ -1757,19 +1769,26 @@ static int parse_options(char *options, struct super_block *sb,
}
#ifdef CONFIG_QUOTA
if (ext4_has_feature_quota(sb) &&
- (test_opt(sb, USRQUOTA) || test_opt(sb, GRPQUOTA))) {
- ext4_msg(sb, KERN_INFO, "Quota feature enabled, usrquota and grpquota "
+ (test_opt(sb, USRQUOTA) || test_opt(sb, GRPQUOTA) ||
+ test_opt(sb, PRJQUOTA))) {
+ ext4_msg(sb, KERN_INFO, "Quota feature enabled, usrquota,grpquota,prjquota "
"mount options ignored.");
clear_opt(sb, USRQUOTA);
clear_opt(sb, GRPQUOTA);
- } else if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
+ clear_opt(sb, PRJQUOTA);
+ } else if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA] ||
+ sbi->s_qf_names[PRJQUOTA]) {
if (test_opt(sb, USRQUOTA) && sbi->s_qf_names[USRQUOTA])
clear_opt(sb, USRQUOTA);

if (test_opt(sb, GRPQUOTA) && sbi->s_qf_names[GRPQUOTA])
clear_opt(sb, GRPQUOTA);

- if (test_opt(sb, GRPQUOTA) || test_opt(sb, USRQUOTA)) {
+ if (test_opt(sb, PRJQUOTA) && sbi->s_qf_names[PRJQUOTA])
+ clear_opt(sb, GRPQUOTA);
+
+ if (test_opt(sb, GRPQUOTA) || test_opt(sb, USRQUOTA) ||
+ test_opt(sb, PRJQUOTA)) {
ext4_msg(sb, KERN_ERR, "old and new quota "
"format mixing");
return 0;
@@ -1829,6 +1848,9 @@ static inline void ext4_show_quota_options(struct seq_file *seq,

if (sbi->s_qf_names[GRPQUOTA])
seq_show_option(seq, "grpjquota", sbi->s_qf_names[GRPQUOTA]);
+
+ if (sbi->s_qf_names[PRJQUOTA])
+ seq_show_option(seq, "prjjquota", sbi->s_qf_names[PRJQUOTA]);
#endif
}

@@ -4992,7 +5014,8 @@ static int ext4_mark_dquot_dirty(struct dquot *dquot)

/* Are we journaling quotas? */
if (ext4_has_feature_quota(sb) ||
- sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
+ sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA] ||
+ sbi->s_qf_names[PRJQUOTA]) {
dquot_mark_dquot_dirty(dquot);
return ext4_write_dquot(dquot);
} else {
--
2.7.4



2016-07-07 16:07:19

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] ext4: add project quota mount options

On 7/7/16 2:09 AM, Wang Shilong wrote:
> From: Wang Shilong <[email protected]>
>
> add prjquota, prjjquota, offprjjquota mount options
> for project quota.
>
> These kind of mount options are used for old
> quota design, and we can use quotas like these
> way:
>
> # mkfs.ext4 /dev/sda
> # mount /dev/sda -o prjquota /mnt/test
> # quotacheck -p /mnt/test
> # quotaon /mnt/test
>
> This new mount options are also useful to unify
> some generic tests for xfs and ext4.

Thanks, I definitely think this makes sense for symmetry, at least.

The new mount options should also be documented in
Documentation/filesystems/ext4.txt as well as the ext4(5) manpage
in e2fsprogs.

But speaking of documentation, is the whole "old vs new" quota
behavior well-documented somewhere? I'm afraid I've lost the
thread on all that. mount -o usrquota vs. mount -o usrjquota
vs tune2fs vs mkfs vs. oh my! ;)

> Signed-off-by: Wang Shilong <[email protected]>
> ---
> fs/ext4/ext4.h | 1 +
> fs/ext4/ialloc.c | 4 ++--
> fs/ext4/inode.c | 7 ++-----
> fs/ext4/ioctl.c | 6 ++++--
> fs/ext4/super.c | 43 +++++++++++++++++++++++++++++++++----------
> 5 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index b84aa1c..315838a 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1129,6 +1129,7 @@ struct ext4_inode_info {
> #define EXT4_MOUNT_POSIX_ACL 0x08000 /* POSIX Access Control Lists */
> #define EXT4_MOUNT_NO_AUTO_DA_ALLOC 0x10000 /* No auto delalloc mapping */
> #define EXT4_MOUNT_BARRIER 0x20000 /* Use block barriers */
> +#define EXT4_MOUNT_PRJQUOTA 0x40000 /* "old" project quota */
> #define EXT4_MOUNT_QUOTA 0x80000 /* Some quota option set */
> #define EXT4_MOUNT_USRQUOTA 0x100000 /* "old" user quota */
> #define EXT4_MOUNT_GRPQUOTA 0x200000 /* "old" group quota */
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 3da4cf8..f7209fc 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -802,8 +802,8 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
> } else
> inode_init_owner(inode, dir, mode);
>
> - if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT) &&
> - ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT))
> + if ((EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT) ||
> + test_opt(sb, PRJQUOTA)) && ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT))
> ei->i_projid = EXT4_I(dir)->i_projid;
> else
> ei->i_projid = make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index f7140ca..c407f9b 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4385,7 +4385,8 @@ static inline void ext4_iget_extra_inode(struct inode *inode,
>
> int ext4_get_projid(struct inode *inode, kprojid_t *projid)
> {
> - if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb, EXT4_FEATURE_RO_COMPAT_PROJECT))
> + if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb, EXT4_FEATURE_RO_COMPAT_PROJECT)
> + && !test_opt(inode->i_sb, PRJQUOTA))

I wonder if this should be encapsulated in an ext4_prjquota_enabled() or something;
not that big of a deal but it gets repeated a few times, now.

> return -EOPNOTSUPP;
> *projid = EXT4_I(inode)->i_projid;
> return 0;
> @@ -4856,10 +4857,6 @@ static int ext4_do_update_inode(handle_t *handle,
> }
> }
>
> - BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
> - EXT4_FEATURE_RO_COMPAT_PROJECT) &&
> - i_projid != EXT4_DEF_PROJID);
> -

Why was this removed vs. adding the mount option test?

> if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
> EXT4_FITS_IN_INODE(raw_inode, ei, i_projid))
> raw_inode->i_projid = cpu_to_le32(i_projid);
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index b5a39b0..6062a81 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -311,7 +311,8 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
> struct dquot *transfer_to[MAXQUOTAS] = { };
>
> if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
> - EXT4_FEATURE_RO_COMPAT_PROJECT)) {
> + EXT4_FEATURE_RO_COMPAT_PROJECT) &&
> + !test_opt(sb, PRJQUOTA)) {
> if (projid != EXT4_DEF_PROJID)
> return -EOPNOTSUPP;
> else
> @@ -849,7 +850,8 @@ encryption_policy_out:
> fa.fsx_xflags = ext4_iflags_to_xflags(ei->i_flags & EXT4_FL_USER_VISIBLE);
>
> if (EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
> - EXT4_FEATURE_RO_COMPAT_PROJECT)) {
> + EXT4_FEATURE_RO_COMPAT_PROJECT) ||
> + test_opt(inode->i_sb, PRJQUOTA)) {
> fa.fsx_projid = (__u32)from_kprojid(&init_user_ns,
> EXT4_I(inode)->i_projid);
> }
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 3822a5a..93a776f 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1184,10 +1184,10 @@ enum {
> Opt_journal_path, Opt_journal_checksum, Opt_journal_async_commit,
> Opt_abort, Opt_data_journal, Opt_data_ordered, Opt_data_writeback,
> Opt_data_err_abort, Opt_data_err_ignore, Opt_test_dummy_encryption,
> - Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
> - Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_jqfmt_vfsv1, Opt_quota,
> - Opt_noquota, Opt_barrier, Opt_nobarrier, Opt_err,
> - Opt_usrquota, Opt_grpquota, Opt_i_version, Opt_dax,
> + Opt_usrjquota, Opt_grpjquota, Opt_prjjquota, Opt_offusrjquota,
> + Opt_offgrpjquota, Opt_offprjjquota, Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0,
> + Opt_jqfmt_vfsv1, Opt_quota, Opt_noquota, Opt_barrier, Opt_nobarrier,

small whitespace damage at the beginning of that line.

> + Opt_err, Opt_usrquota, Opt_grpquota, Opt_prjquota, Opt_i_version, Opt_dax,
> Opt_stripe, Opt_delalloc, Opt_nodelalloc, Opt_mblk_io_submit,
> Opt_lazytime, Opt_nolazytime,
> Opt_nomblk_io_submit, Opt_block_validity, Opt_noblock_validity,
> @@ -1240,6 +1240,8 @@ static const match_table_t tokens = {
> {Opt_usrjquota, "usrjquota=%s"},
> {Opt_offgrpjquota, "grpjquota="},
> {Opt_grpjquota, "grpjquota=%s"},
> + {Opt_prjjquota, "prjjquota=%s"},
> + {Opt_offprjjquota, "prjjquota="},
> {Opt_jqfmt_vfsold, "jqfmt=vfsold"},
> {Opt_jqfmt_vfsv0, "jqfmt=vfsv0"},
> {Opt_jqfmt_vfsv1, "jqfmt=vfsv1"},
> @@ -1250,6 +1252,7 @@ static const match_table_t tokens = {
> {Opt_barrier, "barrier=%u"},
> {Opt_barrier, "barrier"},
> {Opt_nobarrier, "nobarrier"},
> + {Opt_prjquota, "prjquota"},
> {Opt_i_version, "i_version"},
> {Opt_dax, "dax"},
> {Opt_stripe, "stripe=%u"},
> @@ -1466,12 +1469,17 @@ static const struct mount_opts {
> MOPT_SET | MOPT_Q},
> {Opt_grpquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_GRPQUOTA,
> MOPT_SET | MOPT_Q},
> + {Opt_prjquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_PRJQUOTA,
> + MOPT_SET | MOPT_Q},
> {Opt_noquota, (EXT4_MOUNT_QUOTA | EXT4_MOUNT_USRQUOTA |
> - EXT4_MOUNT_GRPQUOTA), MOPT_CLEAR | MOPT_Q},
> + EXT4_MOUNT_GRPQUOTA | EXT4_MOUNT_PRJQUOTA),
> + MOPT_CLEAR | MOPT_Q},
> {Opt_usrjquota, 0, MOPT_Q},
> {Opt_grpjquota, 0, MOPT_Q},
> + {Opt_prjjquota, 0, MOPT_Q},
> {Opt_offusrjquota, 0, MOPT_Q},
> {Opt_offgrpjquota, 0, MOPT_Q},
> + {Opt_offprjjquota, 0, MOPT_Q},
> {Opt_jqfmt_vfsold, QFMT_VFS_OLD, MOPT_QFMT},
> {Opt_jqfmt_vfsv0, QFMT_VFS_V0, MOPT_QFMT},
> {Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
> @@ -1495,10 +1503,14 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
> return set_qf_name(sb, USRQUOTA, &args[0]);
> else if (token == Opt_grpjquota)
> return set_qf_name(sb, GRPQUOTA, &args[0]);
> + else if (token == Opt_prjjquota)
> + return set_qf_name(sb, PRJQUOTA, &args[0]);
> else if (token == Opt_offusrjquota)
> return clear_qf_name(sb, USRQUOTA);
> else if (token == Opt_offgrpjquota)
> return clear_qf_name(sb, GRPQUOTA);
> + else if (token == Opt_offprjjquota)
> + return clear_qf_name(sb, PRJQUOTA);
> #endif
> switch (token) {
> case Opt_noacl:
> @@ -1757,19 +1769,26 @@ static int parse_options(char *options, struct super_block *sb,
> }
> #ifdef CONFIG_QUOTA
> if (ext4_has_feature_quota(sb) &&
> - (test_opt(sb, USRQUOTA) || test_opt(sb, GRPQUOTA))) {
> - ext4_msg(sb, KERN_INFO, "Quota feature enabled, usrquota and grpquota "
> + (test_opt(sb, USRQUOTA) || test_opt(sb, GRPQUOTA) ||
> + test_opt(sb, PRJQUOTA))) {
> + ext4_msg(sb, KERN_INFO, "Quota feature enabled, usrquota,grpquota,prjquota "
> "mount options ignored.");
> clear_opt(sb, USRQUOTA);
> clear_opt(sb, GRPQUOTA);
> - } else if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
> + clear_opt(sb, PRJQUOTA);
> + } else if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA] ||
> + sbi->s_qf_names[PRJQUOTA]) {
> if (test_opt(sb, USRQUOTA) && sbi->s_qf_names[USRQUOTA])
> clear_opt(sb, USRQUOTA);
>
> if (test_opt(sb, GRPQUOTA) && sbi->s_qf_names[GRPQUOTA])
> clear_opt(sb, GRPQUOTA);
>
> - if (test_opt(sb, GRPQUOTA) || test_opt(sb, USRQUOTA)) {
> + if (test_opt(sb, PRJQUOTA) && sbi->s_qf_names[PRJQUOTA])
> + clear_opt(sb, GRPQUOTA);
> +
> + if (test_opt(sb, GRPQUOTA) || test_opt(sb, USRQUOTA) ||
> + test_opt(sb, PRJQUOTA)) {
> ext4_msg(sb, KERN_ERR, "old and new quota "
> "format mixing");
> return 0;
> @@ -1829,6 +1848,9 @@ static inline void ext4_show_quota_options(struct seq_file *seq,
>
> if (sbi->s_qf_names[GRPQUOTA])
> seq_show_option(seq, "grpjquota", sbi->s_qf_names[GRPQUOTA]);
> +
> + if (sbi->s_qf_names[PRJQUOTA])
> + seq_show_option(seq, "prjjquota", sbi->s_qf_names[PRJQUOTA]);
> #endif
> }
>
> @@ -4992,7 +5014,8 @@ static int ext4_mark_dquot_dirty(struct dquot *dquot)
>
> /* Are we journaling quotas? */
> if (ext4_has_feature_quota(sb) ||
> - sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
> + sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA] ||
> + sbi->s_qf_names[PRJQUOTA]) {
> dquot_mark_dquot_dirty(dquot);
> return ext4_write_dquot(dquot);
> } else {
>


2016-07-07 18:15:55

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] ext4: add project quota mount options

On Jul 7, 2016, at 1:09 AM, Wang Shilong <[email protected]> wrote:
>
> From: Wang Shilong <[email protected]>
>
> add prjquota, prjjquota, offprjjquota mount options
> for project quota.
>
> These kind of mount options are used for old
> quota design, and we can use quotas like these
> way:
>
> # mkfs.ext4 /dev/sda
> # mount /dev/sda -o prjquota /mnt/test
> # quotacheck -p /mnt/test
> # quotaon /mnt/test
>
> This new mount options are also useful to unify
> some generic tests for xfs and ext4.
>
> Signed-off-by: Wang Shilong <[email protected]>
> ---
> fs/ext4/ext4.h | 1 +
> fs/ext4/ialloc.c | 4 ++--
> fs/ext4/inode.c | 7 ++-----
> fs/ext4/ioctl.c | 6 ++++--
> fs/ext4/super.c | 43 +++++++++++++++++++++++++++++++++----------
> 5 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index b84aa1c..315838a 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1129,6 +1129,7 @@ struct ext4_inode_info {
> #define EXT4_MOUNT_POSIX_ACL 0x08000 /* POSIX Access Control Lists */
> #define EXT4_MOUNT_NO_AUTO_DA_ALLOC 0x10000 /* No auto delalloc mapping */
> #define EXT4_MOUNT_BARRIER 0x20000 /* Use block barriers */
> +#define EXT4_MOUNT_PRJQUOTA 0x40000 /* "old" project quota */
> #define EXT4_MOUNT_QUOTA 0x80000 /* Some quota option set */
> #define EXT4_MOUNT_USRQUOTA 0x100000 /* "old" user quota */
> #define EXT4_MOUNT_GRPQUOTA 0x200000 /* "old" group quota */
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 3da4cf8..f7209fc 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -802,8 +802,8 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
> } else
> inode_init_owner(inode, dir, mode);
>
> - if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT) &&
> - ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT))
> + if ((EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT) ||
> + test_opt(sb, PRJQUOTA)) && ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT))

(style) line over 80 columns

It might be easier to change the code to enable the PRJQUOTA mount option
when EXT4_FEATURE_RO_COMPAT_PROJECT is in the superblock, and then check
only test_opt(sb, PRJQUOTA) in the code instead of checking both.

Cheers, Andreas

> ei->i_projid = EXT4_I(dir)->i_projid;
> else
> ei->i_projid = make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index f7140ca..c407f9b 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4385,7 +4385,8 @@ static inline void ext4_iget_extra_inode(struct inode *inode,
>
> int ext4_get_projid(struct inode *inode, kprojid_t *projid)
> {
> - if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb, EXT4_FEATURE_RO_COMPAT_PROJECT))
> + if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb, EXT4_FEATURE_RO_COMPAT_PROJECT)
> + && !test_opt(inode->i_sb, PRJQUOTA))
> return -EOPNOTSUPP;
> *projid = EXT4_I(inode)->i_projid;
> return 0;
> @@ -4856,10 +4857,6 @@ static int ext4_do_update_inode(handle_t *handle,
> }
> }
>
> - BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
> - EXT4_FEATURE_RO_COMPAT_PROJECT) &&
> - i_projid != EXT4_DEF_PROJID);
> -
> if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
> EXT4_FITS_IN_INODE(raw_inode, ei, i_projid))
> raw_inode->i_projid = cpu_to_le32(i_projid);
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index b5a39b0..6062a81 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -311,7 +311,8 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
> struct dquot *transfer_to[MAXQUOTAS] = { };
>
> if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
> - EXT4_FEATURE_RO_COMPAT_PROJECT)) {
> + EXT4_FEATURE_RO_COMPAT_PROJECT) &&
> + !test_opt(sb, PRJQUOTA)) {
> if (projid != EXT4_DEF_PROJID)
> return -EOPNOTSUPP;
> else
> @@ -849,7 +850,8 @@ encryption_policy_out:
> fa.fsx_xflags = ext4_iflags_to_xflags(ei->i_flags & EXT4_FL_USER_VISIBLE);
>
> if (EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
> - EXT4_FEATURE_RO_COMPAT_PROJECT)) {
> + EXT4_FEATURE_RO_COMPAT_PROJECT) ||
> + test_opt(inode->i_sb, PRJQUOTA)) {
> fa.fsx_projid = (__u32)from_kprojid(&init_user_ns,
> EXT4_I(inode)->i_projid);
> }
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 3822a5a..93a776f 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1184,10 +1184,10 @@ enum {
> Opt_journal_path, Opt_journal_checksum, Opt_journal_async_commit,
> Opt_abort, Opt_data_journal, Opt_data_ordered, Opt_data_writeback,
> Opt_data_err_abort, Opt_data_err_ignore, Opt_test_dummy_encryption,
> - Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
> - Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_jqfmt_vfsv1, Opt_quota,
> - Opt_noquota, Opt_barrier, Opt_nobarrier, Opt_err,
> - Opt_usrquota, Opt_grpquota, Opt_i_version, Opt_dax,
> + Opt_usrjquota, Opt_grpjquota, Opt_prjjquota, Opt_offusrjquota,
> + Opt_offgrpjquota, Opt_offprjjquota, Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0,
> + Opt_jqfmt_vfsv1, Opt_quota, Opt_noquota, Opt_barrier, Opt_nobarrier,
> + Opt_err, Opt_usrquota, Opt_grpquota, Opt_prjquota, Opt_i_version, Opt_dax,
> Opt_stripe, Opt_delalloc, Opt_nodelalloc, Opt_mblk_io_submit,
> Opt_lazytime, Opt_nolazytime,
> Opt_nomblk_io_submit, Opt_block_validity, Opt_noblock_validity,
> @@ -1240,6 +1240,8 @@ static const match_table_t tokens = {
> {Opt_usrjquota, "usrjquota=%s"},
> {Opt_offgrpjquota, "grpjquota="},
> {Opt_grpjquota, "grpjquota=%s"},
> + {Opt_prjjquota, "prjjquota=%s"},
> + {Opt_offprjjquota, "prjjquota="},
> {Opt_jqfmt_vfsold, "jqfmt=vfsold"},
> {Opt_jqfmt_vfsv0, "jqfmt=vfsv0"},
> {Opt_jqfmt_vfsv1, "jqfmt=vfsv1"},
> @@ -1250,6 +1252,7 @@ static const match_table_t tokens = {
> {Opt_barrier, "barrier=%u"},
> {Opt_barrier, "barrier"},
> {Opt_nobarrier, "nobarrier"},
> + {Opt_prjquota, "prjquota"},
> {Opt_i_version, "i_version"},
> {Opt_dax, "dax"},
> {Opt_stripe, "stripe=%u"},
> @@ -1466,12 +1469,17 @@ static const struct mount_opts {
> MOPT_SET | MOPT_Q},
> {Opt_grpquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_GRPQUOTA,
> MOPT_SET | MOPT_Q},
> + {Opt_prjquota, EXT4_MOUNT_QUOTA | EXT4_MOUNT_PRJQUOTA,
> + MOPT_SET | MOPT_Q},
> {Opt_noquota, (EXT4_MOUNT_QUOTA | EXT4_MOUNT_USRQUOTA |
> - EXT4_MOUNT_GRPQUOTA), MOPT_CLEAR | MOPT_Q},
> + EXT4_MOUNT_GRPQUOTA | EXT4_MOUNT_PRJQUOTA),
> + MOPT_CLEAR | MOPT_Q},
> {Opt_usrjquota, 0, MOPT_Q},
> {Opt_grpjquota, 0, MOPT_Q},
> + {Opt_prjjquota, 0, MOPT_Q},
> {Opt_offusrjquota, 0, MOPT_Q},
> {Opt_offgrpjquota, 0, MOPT_Q},
> + {Opt_offprjjquota, 0, MOPT_Q},
> {Opt_jqfmt_vfsold, QFMT_VFS_OLD, MOPT_QFMT},
> {Opt_jqfmt_vfsv0, QFMT_VFS_V0, MOPT_QFMT},
> {Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
> @@ -1495,10 +1503,14 @@ static int handle_mount_opt(struct super_block *sb, char *opt, int token,
> return set_qf_name(sb, USRQUOTA, &args[0]);
> else if (token == Opt_grpjquota)
> return set_qf_name(sb, GRPQUOTA, &args[0]);
> + else if (token == Opt_prjjquota)
> + return set_qf_name(sb, PRJQUOTA, &args[0]);
> else if (token == Opt_offusrjquota)
> return clear_qf_name(sb, USRQUOTA);
> else if (token == Opt_offgrpjquota)
> return clear_qf_name(sb, GRPQUOTA);
> + else if (token == Opt_offprjjquota)
> + return clear_qf_name(sb, PRJQUOTA);
> #endif
> switch (token) {
> case Opt_noacl:
> @@ -1757,19 +1769,26 @@ static int parse_options(char *options, struct super_block *sb,
> }
> #ifdef CONFIG_QUOTA
> if (ext4_has_feature_quota(sb) &&
> - (test_opt(sb, USRQUOTA) || test_opt(sb, GRPQUOTA))) {
> - ext4_msg(sb, KERN_INFO, "Quota feature enabled, usrquota and grpquota "
> + (test_opt(sb, USRQUOTA) || test_opt(sb, GRPQUOTA) ||
> + test_opt(sb, PRJQUOTA))) {
> + ext4_msg(sb, KERN_INFO, "Quota feature enabled, usrquota,grpquota,prjquota "
> "mount options ignored.");
> clear_opt(sb, USRQUOTA);
> clear_opt(sb, GRPQUOTA);
> - } else if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
> + clear_opt(sb, PRJQUOTA);
> + } else if (sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA] ||
> + sbi->s_qf_names[PRJQUOTA]) {
> if (test_opt(sb, USRQUOTA) && sbi->s_qf_names[USRQUOTA])
> clear_opt(sb, USRQUOTA);
>
> if (test_opt(sb, GRPQUOTA) && sbi->s_qf_names[GRPQUOTA])
> clear_opt(sb, GRPQUOTA);
>
> - if (test_opt(sb, GRPQUOTA) || test_opt(sb, USRQUOTA)) {
> + if (test_opt(sb, PRJQUOTA) && sbi->s_qf_names[PRJQUOTA])
> + clear_opt(sb, GRPQUOTA);
> +
> + if (test_opt(sb, GRPQUOTA) || test_opt(sb, USRQUOTA) ||
> + test_opt(sb, PRJQUOTA)) {
> ext4_msg(sb, KERN_ERR, "old and new quota "
> "format mixing");
> return 0;
> @@ -1829,6 +1848,9 @@ static inline void ext4_show_quota_options(struct seq_file *seq,
>
> if (sbi->s_qf_names[GRPQUOTA])
> seq_show_option(seq, "grpjquota", sbi->s_qf_names[GRPQUOTA]);
> +
> + if (sbi->s_qf_names[PRJQUOTA])
> + seq_show_option(seq, "prjjquota", sbi->s_qf_names[PRJQUOTA]);
> #endif
> }
>
> @@ -4992,7 +5014,8 @@ static int ext4_mark_dquot_dirty(struct dquot *dquot)
>
> /* Are we journaling quotas? */
> if (ext4_has_feature_quota(sb) ||
> - sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA]) {
> + sbi->s_qf_names[USRQUOTA] || sbi->s_qf_names[GRPQUOTA] ||
> + sbi->s_qf_names[PRJQUOTA]) {
> dquot_mark_dquot_dirty(dquot);
> return ext4_write_dquot(dquot);
> } else {
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






Attachments:
signature.asc (833.00 B)
Message signed with OpenPGP using GPGMail

2016-07-08 00:41:11

by Wang Shilong

[permalink] [raw]
Subject: Re: [PATCH] ext4: add project quota mount options

Hell Eric,

On Fri, Jul 8, 2016 at 1:06 AM, Eric Sandeen <[email protected]> wrote:
> On 7/7/16 2:09 AM, Wang Shilong wrote:
>> From: Wang Shilong <[email protected]>
>>
>> add prjquota, prjjquota, offprjjquota mount options
>> for project quota.
>>
>> These kind of mount options are used for old
>> quota design, and we can use quotas like these
>> way:
>>
>> # mkfs.ext4 /dev/sda
>> # mount /dev/sda -o prjquota /mnt/test
>> # quotacheck -p /mnt/test
>> # quotaon /mnt/test
>>
>> This new mount options are also useful to unify
>> some generic tests for xfs and ext4.
>
> Thanks, I definitely think this makes sense for symmetry, at least.
>
> The new mount options should also be documented in
> Documentation/filesystems/ext4.txt as well as the ext4(5) manpage
> in e2fsprogs.

Yes, i will add it.

>
> But speaking of documentation, is the whole "old vs new" quota
> behavior well-documented somewhere? I'm afraid I've lost the
> thread on all that. mount -o usrquota vs. mount -o usrjquota
> vs tune2fs vs mkfs vs. oh my! ;)

This is my confusing before writting this patch.

Old quota design depends on mount options, and every mount
must follow same 'usrquota' etc mount options to make sure
space accounting is right.

I guess that is why new quota design is there, just enable
quota when mkfs, and space accounting will be enabled from then.
and we won't have to run 'quotacheck' to correct space accounting
hopely.

I don't know well what is usrjquota mean here too...maybe
Jan Kara have some ideas about that?


>
>> Signed-off-by: Wang Shilong <[email protected]>
>> ---
>> fs/ext4/ext4.h | 1 +
>> fs/ext4/ialloc.c | 4 ++--
>> fs/ext4/inode.c | 7 ++-----
>> fs/ext4/ioctl.c | 6 ++++--
>> fs/ext4/super.c | 43 +++++++++++++++++++++++++++++++++----------
>> 5 files changed, 42 insertions(+), 19 deletions(-)

<..snip..>


>> return -EOPNOTSUPP;
>> *projid = EXT4_I(inode)->i_projid;
>> return 0;
>> @@ -4856,10 +4857,6 @@ static int ext4_do_update_inode(handle_t *handle,
>> }
>> }
>>
>> - BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
>> - EXT4_FEATURE_RO_COMPAT_PROJECT) &&
>> - i_projid != EXT4_DEF_PROJID);
>> -
>
> Why was this removed vs. adding the mount option test?

As you can see with new mount options introduced, if next time
without prjquota mount options, some existed inode projid is
still none zero, so this assumptions will be wrong.



>
>> if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
>> EXT4_FITS_IN_INODE(raw_inode, ei, i_projid))
>> raw_inode->i_projid = cpu_to_le32(i_projid);
>> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
>> index b5a39b0..6062a81 100644
>> --- a/fs/ext4/ioctl.c
>> +++ b/fs/ext4/ioctl.c
>> @@ -311,7 +311,8 @@ static int ext4_ioctl_setproject(struct file *filp, __u32 projid)
>> struct dquot *transfer_to[MAXQUOTAS] = { };
>>
>> if (!EXT4_HAS_RO_COMPAT_FEATURE(sb,
>> - EXT4_FEATURE_RO_COMPAT_PROJECT)) {
>> + EXT4_FEATURE_RO_COMPAT_PROJECT) &&
>> + !test_opt(sb, PRJQUOTA)) {
>> if (projid != EXT4_DEF_PROJID)
>> return -EOPNOTSUPP;
>> else

2016-07-08 03:10:08

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: add project quota mount options

On Thu, Jul 07, 2016 at 11:06:10AM -0500, Eric Sandeen wrote:
> > add prjquota, prjjquota, offprjjquota mount options
> > for project quota.
> >
> > These kind of mount options are used for old
> > quota design, and we can use quotas like these
> > way:
> >
> > # mkfs.ext4 /dev/sda
> > # mount /dev/sda -o prjquota /mnt/test
> > # quotacheck -p /mnt/test
> > # quotaon /mnt/test
> >
> > This new mount options are also useful to unify
> > some generic tests for xfs and ext4.

I think we want to really think about whether we want to support this
mode. Right now we have so *many* different permutations of quota
that it's a whole lot more than "old" vs "new". We have different
versions of the quota files, "journaled quota", "first class quota"
(where the quota files are hidden), etc.

For user quota and project quota, we do need to support the older
quota systems, where usrquota and grpquota are in fact no-ops as far
as the kernel is concerned, and are only used as a signal to some init
files so they know how to run quotacheck and to the quotatools
userspace tools, and where quotaon is used to pass the quota files to
the file system using the quotactl system call.

Then there is the "journalled quota" system, where filenames are
passed via the mount options usrjquota= and grpjquota= and where quota
updates are done using data journal mode. In this mode, at least in
theory, you don't need to run quotacheck, so the fact that usrquota
and grpquota mount options aren't present don't matter as far as the
init scripts are concerned. (Although if they get corrupted, good
luck to you.)

With both of the above systems, there are three possible quota
formats: old, v0, and v1. I never remember the details of the
differences between them, but IIRC there are differences in terms of
the width (e.g., number of bits) used for the uids, gids, and number
of blocks and files.

OK, so if you look at the above, the test matrix is at six: journaled
vs. non-journaled quota files, old vs v0 vs v1. And this doesn't take
into account whether or not project quota is enabled or not. I propose
that we ***not*** support any of this for project quota.


The mode that I *do* suggest supporting for project quota for ext4
requires that you create a file system using -O quota, which enables
"internal quotas", which is sometimes also referred to as "first class
quotas". In this mode, the quota files become hidden files so
userspace can't screw them up or otherwise corrupt them. The quota
files are journalled, and are checked and consistency corrected by
e2fsck. This is *much* faster than having quotacheck do a separate
user-space walk of the entire file system, which is slow, and subject
to files being hidden underneath mountpoints, etc., etc.

This mode is supported by quotatools, so it's not a matter of "xfs"
versus "vfs/quotatools". It's just a question of a different way of
setting up file system. And for ext4, it means "don't use any magic
mount option", and creating the file system with "mke2fs -O
project,quota".

Yes, for the sake of those poor, unfortunate souls who have to support
RHEL 6 (really, you couldn't pay me enough :-), it probably makes
sense to support the legacy style quota system using either the
usrquota/grpquota mount options, and so it makes sense for xfstests to
user the old-style aquota.user files that can be messed with by
confused userspace utilities, and which are dog-slow to check becuase
a separate quotacheck run is required.

But RHEL 6 doesn't have project quota, and so I don't see any reason
to try to support the legacy quota setup for project quota. Yes, we
still want to test the quotatools VFS interfaces, and that means using
quotatools binaries --- but it doesn't follow from that choice that we
have to support the ancient ways of storing the quota files as
user-visible files, or using the ancient mount options which are just
horrible hacks to keep the old enterprise linux initscripts from
breaking.

Does this make sense?

- Ted

2016-07-08 03:36:59

by Wang Shilong

[permalink] [raw]
Subject: Re: [PATCH] ext4: add project quota mount options

On Fri, Jul 8, 2016 at 12:10 PM, Theodore Ts'o <[email protected]> wrote:
> On Thu, Jul 07, 2016 at 11:06:10AM -0500, Eric Sandeen wrote:
>> > add prjquota, prjjquota, offprjjquota mount options
>> > for project quota.
>> >
>> > These kind of mount options are used for old
>> > quota design, and we can use quotas like these
>> > way:
>> >
>> > # mkfs.ext4 /dev/sda
>> > # mount /dev/sda -o prjquota /mnt/test
>> > # quotacheck -p /mnt/test
>> > # quotaon /mnt/test
>> >
>> > This new mount options are also useful to unify
>> > some generic tests for xfs and ext4.
>

<...SNIP...>


>
> Yes, for the sake of those poor, unfortunate souls who have to support
> RHEL 6 (really, you couldn't pay me enough :-), it probably makes
> sense to support the legacy style quota system using either the
> usrquota/grpquota mount options, and so it makes sense for xfstests to
> user the old-style aquota.user files that can be messed with by
> confused userspace utilities, and which are dog-slow to check becuase
> a separate quotacheck run is required.
>
> But RHEL 6 doesn't have project quota, and so I don't see any reason
> to try to support the legacy quota setup for project quota. Yes, we
> still want to test the quotatools VFS interfaces, and that means using
> quotatools binaries --- but it doesn't follow from that choice that we
> have to support the ancient ways of storing the quota files as
> user-visible files, or using the ancient mount options which are just
> horrible hacks to keep the old enterprise linux initscripts from
> breaking.
>
> Does this make sense?


I do agree with your point here, but the problem is as you can see come
from xfstests arguments from Dave Chinner.

I don't think currently users will try to old interface with project quota.
But as your request, i think you need some tests for project quota...

I think it make sense to add a test for lsattr/chattr interface for ext4,
whatever it can be moved to ext4 or generic.


Thanks,
Shilong



>
> - Ted

2016-07-08 04:48:27

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: add project quota mount options

On Fri, Jul 08, 2016 at 12:36:56PM +0900, Wang Shilong wrote:
>
> I do agree with your point here, but the problem is as you can see come
> from xfstests arguments from Dave Chinner.
>
> I don't think currently users will try to old interface with project quota.
> But as your request, i think you need some tests for project quota...

I think we need some real clarity about exactly what Dave is asking
for. I think what Eric and Dave have been asking for is testing both
of these two different quota management stacks:

(1) XFS has ioctl's which come from the Irix days, and these are used by
the xfs quota utilities.

(2) Quotatools uses the quotactl(2) system call, and so there is an
entirely **different** code path which we can exercise.

This is a completely orthogonal question from which mount options and
which mkfs options are used to set up project quota support for ext4.
That's an ext4-specific concern, and I don't see why Dave would have
any interest in what ext4 chooses to support here.

- Ted

2016-07-11 15:29:43

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH] ext4: add project quota mount options

On Thu 07-07-16 23:10:02, Ted Tso wrote:
> On Thu, Jul 07, 2016 at 11:06:10AM -0500, Eric Sandeen wrote:
> > > add prjquota, prjjquota, offprjjquota mount options
> > > for project quota.
> > >
> > > These kind of mount options are used for old
> > > quota design, and we can use quotas like these
> > > way:
> > >
> > > # mkfs.ext4 /dev/sda
> > > # mount /dev/sda -o prjquota /mnt/test
> > > # quotacheck -p /mnt/test
> > > # quotaon /mnt/test
> > >
> > > This new mount options are also useful to unify
> > > some generic tests for xfs and ext4.
>
> I think we want to really think about whether we want to support this
> mode. Right now we have so *many* different permutations of quota
> that it's a whole lot more than "old" vs "new". We have different
> versions of the quota files, "journaled quota", "first class quota"
> (where the quota files are hidden), etc.
>
> For user quota and project quota, we do need to support the older
> quota systems, where usrquota and grpquota are in fact no-ops as far
> as the kernel is concerned, and are only used as a signal to some init
> files so they know how to run quotacheck and to the quotatools
> userspace tools, and where quotaon is used to pass the quota files to
> the file system using the quotactl system call.
>
> Then there is the "journalled quota" system, where filenames are
> passed via the mount options usrjquota= and grpjquota= and where quota
> updates are done using data journal mode. In this mode, at least in
> theory, you don't need to run quotacheck, so the fact that usrquota
> and grpquota mount options aren't present don't matter as far as the
> init scripts are concerned. (Although if they get corrupted, good
> luck to you.)

When you use "journalled quota" mode, you still need to run quotacheck
after e2fsck fixes anything or when quota files get corrupted due to bugs,
HW errors, etc. Also quotaon(8) still uses mount options to determine on
which filesystems it should turn on quotas in this mode.

> With both of the above systems, there are three possible quota
> formats: old, v0, and v1. I never remember the details of the
> differences between them, but IIRC there are differences in terms of
> the width (e.g., number of bits) used for the uids, gids, and number
> of blocks and files.

This is correct.

> OK, so if you look at the above, the test matrix is at six: journaled
> vs. non-journaled quota files, old vs v0 vs v1. And this doesn't take
> into account whether or not project quota is enabled or not. I propose
> that we ***not*** support any of this for project quota.
>
> The mode that I *do* suggest supporting for project quota for ext4
> requires that you create a file system using -O quota, which enables
> "internal quotas", which is sometimes also referred to as "first class
> quotas". In this mode, the quota files become hidden files so
> userspace can't screw them up or otherwise corrupt them. The quota
> files are journalled, and are checked and consistency corrected by
> e2fsck. This is *much* faster than having quotacheck do a separate
> user-space walk of the entire file system, which is slow, and subject
> to files being hidden underneath mountpoints, etc., etc.

Completely agreed. Old modes of operation for project quota were actually
originally implemented but we never merged that code exactly because we
didn't find legacy mode of operation useful for project quotas.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR