diff mbox

[02/12] quota: Allow each filesystem to specify which quota types it supports

Message ID 1413902316-17997-3-git-send-email-jack@suse.cz (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kara Oct. 21, 2014, 2:38 p.m. UTC
Currently all filesystems supporting VFS quota support user and group
quotas. With introduction of project quotas this is going to change so
make sure filesystem isn't called for quota type it doesn't support by
introduction of a bitmask determining which quota types each filesystem
supports.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/quota/quota.c      | 13 +++++++++++--
 fs/super.c            |  7 +++++++
 include/linux/quota.h |  6 ++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

Comments

Christoph Hellwig Oct. 22, 2014, 4:29 p.m. UTC | #1
On Tue, Oct 21, 2014 at 04:38:26PM +0200, Jan Kara wrote:
> Currently all filesystems supporting VFS quota support user and group
> quotas. With introduction of project quotas this is going to change so
> make sure filesystem isn't called for quota type it doesn't support by
> introduction of a bitmask determining which quota types each filesystem
> supports.

Why don't you keep this bitmask in the dquot.c instead of pushing it
to the caller?  So far usage of s_dquot is mostly confined to dquot.c
(with a few leaks to the filesystems using it), so keeping it that
way seems like a good idea.
Jan Kara Oct. 22, 2014, 4:51 p.m. UTC | #2
On Wed 22-10-14 18:29:15, Christoph Hellwig wrote:
> On Tue, Oct 21, 2014 at 04:38:26PM +0200, Jan Kara wrote:
> > Currently all filesystems supporting VFS quota support user and group
> > quotas. With introduction of project quotas this is going to change so
> > make sure filesystem isn't called for quota type it doesn't support by
> > introduction of a bitmask determining which quota types each filesystem
> > supports.
> 
> Why don't you keep this bitmask in the dquot.c instead of pushing it
> to the caller?  So far usage of s_dquot is mostly confined to dquot.c
> (with a few leaks to the filesystems using it), so keeping it that
> way seems like a good idea.
  So there are two reasons:
1) Currently if you call quotactl() with invalid quota type you'll get
EINVAL. To maintain this with addition of project quotas you need to check
the types early before calling check_quotactl_permission() and other
checks.

2) I didn't want filesystem quotactl callbacks to deal with quota types
they don't support. Sure each fs could do a type check in the callback but
this looked easier.

Now I see your point about s_dquot and I can move allowed_types out of
s_dquot if that makes you happier. But otherwise what I did still seems as
the best solution to me.

								Honza
Christoph Hellwig Oct. 23, 2014, 8:53 a.m. UTC | #3
On Wed, Oct 22, 2014 at 06:51:50PM +0200, Jan Kara wrote:
>   So there are two reasons:
> 1) Currently if you call quotactl() with invalid quota type you'll get
> EINVAL. To maintain this with addition of project quotas you need to check
> the types early before calling check_quotactl_permission() and other
> checks.
> 
> 2) I didn't want filesystem quotactl callbacks to deal with quota types
> they don't support. Sure each fs could do a type check in the callback but
> this looked easier.
> 
> Now I see your point about s_dquot and I can move allowed_types out of
> s_dquot if that makes you happier. But otherwise what I did still seems as
> the best solution to me.

Moving it out of s_dquot seems very sensible to me.
diff mbox

Patch

diff --git a/fs/quota/quota.c b/fs/quota/quota.c
index 75621649dbd7..0f28eac6e638 100644
--- a/fs/quota/quota.c
+++ b/fs/quota/quota.c
@@ -47,8 +47,11 @@  static int check_quotactl_permission(struct super_block *sb, int type, int cmd,
 
 static void quota_sync_one(struct super_block *sb, void *arg)
 {
-	if (sb->s_qcop && sb->s_qcop->quota_sync)
-		sb->s_qcop->quota_sync(sb, *(int *)arg);
+	int type = *(int *)arg;
+
+	if (sb->s_qcop && sb->s_qcop->quota_sync &&
+	    (sb->s_dquot.allowed_types & (1 << type)))
+		sb->s_qcop->quota_sync(sb, type);
 }
 
 static int quota_sync_all(int type)
@@ -297,8 +300,14 @@  static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
 
 	if (type >= (XQM_COMMAND(cmd) ? XQM_MAXQUOTAS : MAXQUOTAS))
 		return -EINVAL;
+	/*
+	 * Quota not supported on this fs? Check this before allowed_types
+	 * since they needn't be set if quota is not supported.
+	 */
 	if (!sb->s_qcop)
 		return -ENOSYS;
+	if (!(sb->s_dquot.allowed_types & (1 << type)))
+		return -EINVAL;
 
 	ret = check_quotactl_permission(sb, type, cmd, id);
 	if (ret < 0)
diff --git a/fs/super.c b/fs/super.c
index eae088f6aaae..5e70cc327dae 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -218,6 +218,13 @@  static struct super_block *alloc_super(struct file_system_type *type, int flags)
 	atomic_set(&s->s_active, 1);
 	mutex_init(&s->s_vfs_rename_mutex);
 	lockdep_set_class(&s->s_vfs_rename_mutex, &type->s_vfs_rename_key);
+	/*
+	 * For now MAXQUOTAS check in do_quotactl() will limit quota type
+	 * appropriately. When each fs sets allowed_types, we can remove the
+	 * line below
+	 */
+	s->s_dquot.allowed_types = QTYPE_MASK_USR | QTYPE_MASK_GRP |
+				   QTYPE_MASK_PRJ;
 	mutex_init(&s->s_dquot.dqio_mutex);
 	mutex_init(&s->s_dquot.dqonoff_mutex);
 	s->s_maxbytes = MAX_NON_LFS;
diff --git a/include/linux/quota.h b/include/linux/quota.h
index 80d345a3524c..e0cfcce0d986 100644
--- a/include/linux/quota.h
+++ b/include/linux/quota.h
@@ -56,6 +56,11 @@  enum quota_type {
 	PRJQUOTA = 2,		/* element used for project quotas */
 };
 
+/* Masks for quota types when used as a bitmask */
+#define QTYPE_MASK_USR (1 << USRQUOTA)
+#define QTYPE_MASK_GRP (1 << GRPQUOTA)
+#define QTYPE_MASK_PRJ (1 << PRJQUOTA)
+
 typedef __kernel_uid32_t qid_t; /* Type in which we store ids in memory */
 typedef long long qsize_t;	/* Type in which we store sizes */
 
@@ -388,6 +393,7 @@  static inline void quota_send_warning(struct kqid qid, dev_t dev,
 
 struct quota_info {
 	unsigned int flags;			/* Flags for diskquotas on this device */
+	unsigned int allowed_types;		/* Bitmask of quota types this fs supports */
 	struct mutex dqio_mutex;		/* lock device while I/O in progress */
 	struct mutex dqonoff_mutex;		/* Serialize quotaon & quotaoff */
 	struct inode *files[MAXQUOTAS];		/* inodes of quotafiles */