Re: [PATCH 3/4] Adds project quota support for ext4

[Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>]

On Thu 25-09-14 09:28:24, Li Xi wrote:
> On Thu, Sep 25, 2014 at 1:31 AM, Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org> wrote:
> > On Wed 24-09-14 22:04:29, Li Xi wrote:
> >> This patch adds mount options for enabling/disabling project quota
> >> accounting and enforcement. A new specific inode is also used for
> >> project quota accounting.
> >   The patch looks mostly fine. A few smaller things below.
> >
> > ...
> >> @@ -1433,6 +1437,8 @@ 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},
> >   I think you missed to add EXT4_MOUNT_PRJQUOTA to Opt_noquota...
> >
> > ...
> >> @@ -2833,6 +2855,13 @@ static int ext4_feature_set_ok(struct super_block *sb, int readonly)
> >>                        "without CONFIG_QUOTA");
> >>               return 0;
> >>       }
> >> +     if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT) &&
> >> +         !readonly) {
> >> +             ext4_msg(sb, KERN_ERR,
> >> +                      "Filesystem with project quota feature cannot be"
> >> +                      "mounted RDWR without CONFIG_QUOTA");
> >> +             return 0;
> >> +     }
> >   Hum, I don't think this is right. EXT4_FEATURE_RO_COMPAT_PROJECT is about
> > maintaining project IDs not about quota. So it seems perfectly OK to have
> > EXT4_FEATURE_RO_COMPAT_PROJECT without CONFIG_QUOTA.
> Ah, I see. This might be my main misunderstanding. I thought it is
> about maintaining
> both project IDs and quota. And I misunderstood so that I removed all
> EXT4_FEATURE_RO_COMPAT_PROJECT checking when set/get project ID.
> If we only use EXT4_FEATURE_RO_COMPAT_PROJECT to protect imcompatibility
> of project ID, what about the change of struct ext4_super_block? I am
> still confused.
  So my vision is following:
EXT4_FEATURE_RO_COMPAT_PROJECT feature controls whether project IDs can be
stored in inode. So you don't allow project ID to be set for a superblock
without this feature. This is absolutely necessary since otherwise old
kernel could happily mount filesystem with project IDs set and would just
overwrite them. The same applies to PROJINHERIT flag.

Getting of project ID could be easily done without
EXT4_FEATURE_RO_COMPAT_PROJECT feature (just return 0) but I think
returning EOPNOTSUPP would be better for userspace programs - they
can immediatedy distinguish filesystems without project IDs and filesystems
where just the file doesn't have any project ID set.

Whether project quota is accounted and enforced is a different matter.
Without EXT4_FEATURE_RO_COMPAT_QUOTA feature it is userspace which is
responsible for properly setting up quotas so you can just use
sb_has_quota_loaded() and similar functions. With
EXT4_FEATURE_RO_COMPAT_QUOTA, kernel is responsible for setting up quotas
and we decide whether we should setup project quotas depending on whether
s_prj_quota_inum is set (tools should then make sure s_prj_quota_inum is
set only if EXT4_FEATURE_RO_COMPAT_QUOTA and EXT4_FEATURE_RO_COMPAT_PROJECT
are set but we can assert that in the kernel just to make sure).

Hope this makes things clearer.

								Honza
-- 
Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
SUSE Labs, CR