Skip to contentSkip to conversation

Quota fixes for e2fsprogs

PATCH Superseded Jan Kara 2 participants 5 messages Aug 20, 2021 – Aug 23, 2021

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jan Kara

author
#0 lore raw

cover Quota fixes for e2fsprogs

Hello,

I've been testing how transition from old-style quota files to ext4 quota feature works and I've found two serious bugs which make the transation problematic. First, e2fsprogs do not support commonly used version 0 quota format (which has 32-bit inode and block limit counters) and second, the transfer of quota information from old quota files is broken. This series addresses both problems.

Honza

author
#1 lore raw

PATCH 1/2 quota: Add support to version 0 quota format

Version 0 quota format differs from version 1 by having only 32-bit counters for inodes and block limits. For many installations this is not limiting and thus the format is widely used. Also quota tools still create quota files with this format by default. Add support for this quota format to e2fsprogs so that we can seamlessly convert quota files in this format into our internal quota files.

---
 lib/support/quotaio_v2.c | 85 +++++++++++++++++++++++++++++++++++++---
 lib/support/quotaio_v2.h | 17 +++++++-
 2 files changed, 96 insertions(+), 6 deletions(-)
lib/support/quotaio_v2.c +80 -5
index 23717f03f428..a49aa6ac8c2f 100644
@@ -41,6 +41,68 @@ struct quotafile_ops quotafile_ops_2 = {
 	.report		= v2_report,
 };
 
+/*
+ * Copy dquot from disk to memory
+ */
+static void v2r0_disk2memdqblk(struct dquot *dquot, void *dp)
+{
+	struct util_dqblk *m = &dquot->dq_dqb;
+	struct v2r0_disk_dqblk *d = dp, empty;
+
+	dquot->dq_id = ext2fs_le32_to_cpu(d->dqb_id);
+	m->dqb_ihardlimit = ext2fs_le32_to_cpu(d->dqb_ihardlimit);
+	m->dqb_isoftlimit = ext2fs_le32_to_cpu(d->dqb_isoftlimit);
+	m->dqb_bhardlimit = ext2fs_le32_to_cpu(d->dqb_bhardlimit);
+	m->dqb_bsoftlimit = ext2fs_le32_to_cpu(d->dqb_bsoftlimit);
+	m->dqb_curinodes = ext2fs_le32_to_cpu(d->dqb_curinodes);
+	m->dqb_curspace = ext2fs_le64_to_cpu(d->dqb_curspace);
+	m->dqb_itime = ext2fs_le64_to_cpu(d->dqb_itime);
+	m->dqb_btime = ext2fs_le64_to_cpu(d->dqb_btime);
+
+	memset(&empty, 0, sizeof(struct v2r0_disk_dqblk));
+	empty.dqb_itime = ext2fs_cpu_to_le64(1);
+	if (!memcmp(&empty, dp, sizeof(struct v2r0_disk_dqblk)))
+		m->dqb_itime = 0;
+}
+
+/*
+ * Copy dquot from memory to disk
+ */
+static void v2r0_mem2diskdqblk(void *dp, struct dquot *dquot)
+{
+	struct util_dqblk *m = &dquot->dq_dqb;
+	struct v2r0_disk_dqblk *d = dp;
+
+	d->dqb_ihardlimit = ext2fs_cpu_to_le32(m->dqb_ihardlimit);
+	d->dqb_isoftlimit = ext2fs_cpu_to_le32(m->dqb_isoftlimit);
+	d->dqb_bhardlimit = ext2fs_cpu_to_le32(m->dqb_bhardlimit);
+	d->dqb_bsoftlimit = ext2fs_cpu_to_le32(m->dqb_bsoftlimit);
+	d->dqb_curinodes = ext2fs_cpu_to_le32(m->dqb_curinodes);
+	d->dqb_curspace = ext2fs_cpu_to_le64(m->dqb_curspace);
+	d->dqb_itime = ext2fs_cpu_to_le64(m->dqb_itime);
+	d->dqb_btime = ext2fs_cpu_to_le64(m->dqb_btime);
+	d->dqb_id = ext2fs_cpu_to_le32(dquot->dq_id);
+	if (qtree_entry_unused(&dquot->dq_h->qh_info.u.v2_mdqi.dqi_qtree, dp))
+		d->dqb_itime = ext2fs_cpu_to_le64(1);
+}
+
+static int v2r0_is_id(void *dp, struct dquot *dquot)
+{
+	struct v2r0_disk_dqblk *d = dp;
+	struct qtree_mem_dqinfo *info =
+			&dquot->dq_h->qh_info.u.v2_mdqi.dqi_qtree;
+
+	if (qtree_entry_unused(info, dp))
+		return 0;
+	return ext2fs_le32_to_cpu(d->dqb_id) == dquot->dq_id;
+}
+
+static struct qtree_fmt_operations v2r0_fmt_ops = {
+	.mem2disk_dqblk = v2r0_mem2diskdqblk,
+	.disk2mem_dqblk = v2r0_disk2memdqblk,
+	.is_id = v2r0_is_id,
+};
+
 /*
  * Copy dquot from disk to memory
  */
@@ -164,7 +226,8 @@ static int v2_check_file(struct quota_handle *h, int type, int fmt)
 		log_err("Your quota file is stored in wrong endianity");
 		return 0;
 	}
-	if (V2_VERSION != ext2fs_le32_to_cpu(dqh.dqh_version))
+	if (V2_VERSION_R0 != ext2fs_le32_to_cpu(dqh.dqh_version) &&
+	    V2_VERSION_R1 != ext2fs_le32_to_cpu(dqh.dqh_version))
 		return 0;
 	return 1;
 }
@@ -174,13 +237,25 @@ static int v2_check_file(struct quota_handle *h, int type, int fmt)
  */
 static int v2_init_io(struct quota_handle *h)
 {
+	struct v2_disk_dqheader dqh;
 	struct v2_disk_dqinfo ddqinfo;
 	struct v2_mem_dqinfo *info;
 	__u64 filesize;
+	int version;
 
-	h->qh_info.u.v2_mdqi.dqi_qtree.dqi_entry_size =
-		sizeof(struct v2r1_disk_dqblk);
-	h->qh_info.u.v2_mdqi.dqi_qtree.dqi_ops = &v2r1_fmt_ops;
+	if (!v2_read_header(h, &dqh))
+		return -1;
+	version = ext2fs_le32_to_cpu(dqh.dqh_version);
+
+	if (version == V2_VERSION_R0) {
+		h->qh_info.u.v2_mdqi.dqi_qtree.dqi_entry_size =
+			sizeof(struct v2r0_disk_dqblk);
+		h->qh_info.u.v2_mdqi.dqi_qtree.dqi_ops = &v2r0_fmt_ops;
+	} else {
+		h->qh_info.u.v2_mdqi.dqi_qtree.dqi_entry_size =
+			sizeof(struct v2r1_disk_dqblk);
+		h->qh_info.u.v2_mdqi.dqi_qtree.dqi_ops = &v2r1_fmt_ops;
+	}
 
 	/* Read information about quotafile */
 	if (h->e2fs_read(&h->qh_qf, V2_DQINFOOFF, &ddqinfo,
@@ -231,7 +306,7 @@ static int v2_new_io(struct quota_handle *h)
 
 	/* Write basic quota header */
 	ddqheader.dqh_magic = ext2fs_cpu_to_le32(file_magics[h->qh_type]);
-	ddqheader.dqh_version = ext2fs_cpu_to_le32(V2_VERSION);
+	ddqheader.dqh_version = ext2fs_cpu_to_le32(V2_VERSION_R1);
 	if (h->e2fs_write(&h->qh_qf, 0, &ddqheader, sizeof(ddqheader)) !=
 			sizeof(ddqheader))
 		return -1;
lib/support/quotaio_v2.h +16 -1
index de2db2785cb0..35054cafaa23 100644
@@ -13,7 +13,8 @@
 /* Offset of info header in file */
 #define V2_DQINFOOFF		sizeof(struct v2_disk_dqheader)
 /* Supported version of quota-tree format */
-#define V2_VERSION 1
+#define V2_VERSION_R1 1
+#define V2_VERSION_R0 0
 
 struct v2_disk_dqheader {
 	__le32 dqh_magic;	/* Magic number identifying file */
@@ -36,6 +37,20 @@ struct v2_disk_dqinfo {
 					 * free entry */
 } __attribute__ ((packed));
 
+struct v2r0_disk_dqblk {
+	__le32 dqb_id;	/* id this quota applies to */
+	__le32 dqb_ihardlimit;	/* absolute limit on allocated inodes */
+	__le32 dqb_isoftlimit;	/* preferred inode limit */
+	__le32 dqb_curinodes;	/* current # allocated inodes */
+	__le32 dqb_bhardlimit;	/* absolute limit on disk space
+					 * (in QUOTABLOCK_SIZE) */
+	__le32 dqb_bsoftlimit;	/* preferred limit on disk space
+					 * (in QUOTABLOCK_SIZE) */
+	__le64 dqb_curspace;	/* current space occupied (in bytes) */
+	__le64 dqb_btime;	/* time limit for excessive disk use */
+	__le64 dqb_itime;	/* time limit for excessive inode use */
+} __attribute__ ((packed));
+
 struct v2r1_disk_dqblk {
 	__le32 dqb_id;	/* id this quota applies to */
 	__le32 dqb_pad;
Signature · 1 line
2.26.2
author
#2 lore raw

PATCH 2/2 tune2fs: Fix conversion of quota files

In reply to Jan Kara

When tune2fs is enabling quota feature, it looks for old-style quota files and tries to transfer limits stored in these files into newly created hidded quota files. However the code doing the transfer setups the quota scan wrongly and instead of transferring limits we transfer usage. So not only quota limits are lost (at least they can still be recovered from the old quota files) but also usage information may be wrong if the accounting in e2fsprogs does not exactly match the accounting in quota-tools (which is actually the case). Fix the setup of the quota scan.

---
 lib/support/mkquota.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
lib/support/mkquota.c +3 -3
index fbc3833aee98..34ab632fb81c 100644
@@ -569,14 +569,14 @@ static int scan_dquots_callback(struct dquot *dquot, void *cb_data)
  */
 static errcode_t quota_read_all_dquots(struct quota_handle *qh,
                                        quota_ctx_t qctx,
-				       int update_limits EXT2FS_ATTR((unused)))
+				       int update_limits)
 {
 	struct scan_dquots_data scan_data;
 
 	scan_data.quota_dict = qctx->quota_dict[qh->qh_type];
 	scan_data.check_consistency = 0;
-	scan_data.update_limits = 0;
-	scan_data.update_usage = 1;
+	scan_data.update_limits = update_limits;
+	scan_data.update_usage = 0;
 
 	return qh->qh_ops->scan_dquots(qh, scan_dquots_callback, &scan_data);
 }
Signature · 1 line
2.26.2
#3 lore raw

Re: [PATCH 2/2] tune2fs: Fix conversion of quota files

Quoted 13 lines · Jan Kara
lib/support/mkquota.c +1 -1
index fbc3833aee98..34ab632fb81c 100644
@@ -569,14 +569,14 @@ static int scan_dquots_callback(struct dquot *dquot, void *cb_data)
  */
 static errcode_t quota_read_all_dquots(struct quota_handle *qh,
                                        quota_ctx_t qctx,
-				       int update_limits EXT2FS_ATTR((unused)))
+				       int update_limits)
 {
 	struct scan_dquots_data scan_data;

lib/support/mkquota.c static int scan_dquots_callback(struct dquot *dquot, void *cb_data) · line 582

Quoted 10 lines
lib/support/mkquota.c +2 -2
 
 	scan_data.quota_dict = qctx->quota_dict[qh->qh_type];
 	scan_data.check_consistency = 0;
-	scan_data.update_limits = 0;
-	scan_data.update_usage = 1;
+	scan_data.update_limits = update_limits;
+	scan_data.update_usage = 0;
 
 	return qh->qh_ops->scan_dquots(qh, scan_dquots_callback, &scan_data);
 }

This change, while it is correct for tune2fs, is breaking e2fsck's f_orphquot test. The root cause is that e2fsck_read_all_quotas() in e2fsck/super.c is calling quota_update_limits(), where it had wanted to be reading the quota usage data, not the limits.

I think what we need to do is to take quota_read_all_dquots(), which is only used by quota_update_limits(), and then fodl it into quota_update_limits(). And then add a flags parameter to indicate whether we want to be reading the limits or the usage, and then rename quota_update_limits() to quota_read_all_dquots().

Does that make sense to you? (One of the reasons why the quota functions are all in libsupport is precisely because I knew these functions still needed more polishing.)

- Ted

author
#4 lore raw

Re: [PATCH 2/2] tune2fs: Fix conversion of quota files

Quoted 36 lines · Theodore Ts'o
Quoted 13 lines · Jan Kara
lib/support/mkquota.c +1 -1
index fbc3833aee98..34ab632fb81c 100644
@@ -569,14 +569,14 @@ static int scan_dquots_callback(struct dquot *dquot, void *cb_data)
  */
 static errcode_t quota_read_all_dquots(struct quota_handle *qh,
                                        quota_ctx_t qctx,
-				       int update_limits EXT2FS_ATTR((unused)))
+				       int update_limits)
 {
 	struct scan_dquots_data scan_data;

lib/support/mkquota.c static int scan_dquots_callback(struct dquot *dquot, void *cb_data) · line 582

Quoted 10 lines
lib/support/mkquota.c +2 -2
 
 	scan_data.quota_dict = qctx->quota_dict[qh->qh_type];
 	scan_data.check_consistency = 0;
-	scan_data.update_limits = 0;
-	scan_data.update_usage = 1;
+	scan_data.update_limits = update_limits;
+	scan_data.update_usage = 0;
 
 	return qh->qh_ops->scan_dquots(qh, scan_dquots_callback, &scan_data);
 }

This change, while it is correct for tune2fs, is breaking e2fsck's f_orphquot test. The root cause is that e2fsck_read_all_quotas() in e2fsck/super.c is calling quota_update_limits(), where it had wanted to be reading the quota usage data, not the limits.

I think what we need to do is to take quota_read_all_dquots(), which is only used by quota_update_limits(), and then fodl it into quota_update_limits(). And then add a flags parameter to indicate whether we want to be reading the limits or the usage, and then rename quota_update_limits() to quota_read_all_dquots().

Quoted 3 lines

Does that make sense to you? (One of the reasons why the quota functions are all in libsupport is precisely because I knew these functions still needed more polishing.)

Yes, makes sense. I'll send patches.

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR