Quota fixes for e2fsprogs
v2 cover Quota fixes for e2fsprogs
Hello,
here is next revision of my quota fixes for e2fsprogs. When addressing the e2fsck regression Ted has pointed out, I've noticed another serious bug in e2fsck support for quotas where it just nukes existing quota limits when replaying orphan list. A fix is now included in this series together with expanded test to catch the problem.
Changes since v1:
* Rename the functions so that names match functionality
* Fix e2fsck regression when processing orphan list
* Fix e2fsck bug to preserve quota limits over orphan processing
* Expand test to verify quota limits are properly preserved
* Fix quota output headings in debugfs
Honza
PATCH v2 1/8 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
@@ -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
@@ -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 0struct 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
Theodore Ts'o
AppliedRe: [PATCH 1/8] quota: Add support to version 0 quota format
Quoted 8 lines · Jan Kara
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.
[...]
Applied, thanks!
[1/8] quota: Add support to version 0 quota format
(no commit info)
[2/8] quota: Fold quota_read_all_dquots() into quota_update_limits()
(no commit info)
[3/8] quota: Rename quota_update_limits() to quota_read_all_dquots()
(no commit info)
[4/8] tune2fs: Fix conversion of quota files
(no commit info)
[5/8] e2fsck: Do not trash user limits when processing orphan list
(no commit info)
[6/8] tests: Expand test checking quota and orphan processing interaction
(no commit info)
[7/8] debugfs: Fix headers for quota commands
(no commit info)
[8/8] quota: Drop dead code
(no commit info)
Best regards,
Signature · 1 line
Theodore Ts'o <[email protected]>
PATCH v2 2/8 quota: Fold quota_read_all_dquots() into quota_update_limits()
In reply to Jan KaraThere's just one caller of quota_read_all_dquots(), fold it into its caller quota_update_limits(). No functional changes.
--- lib/support/mkquota.c | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-)
lib/support/mkquota.c +6 -18
@@ -564,23 +564,6 @@ static int scan_dquots_callback(struct dquot *dquot, void *cb_data)return 0 ; } -/* -* Read all dquots from quota file into memory -*/ -static errcode_t quota_read_all_dquots (struct quota_handle *qh, -quota_ctx_t qctx, - int update_limits EXT2FS_ATTR((unused))) -{ -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 ; - -return qh->qh_ops->scan_dquots (qh, scan_dquots_callback, &scan_data); -} -/* * Write all memory dquots into quota file */
@@ -607,6 +590,7 @@ static errcode_t quota_write_all_dquots(struct quota_handle *qh,errcode_t quota_update_limits (quota_ctx_t qctx,ext2_ino_t qf_ino,enum quota_type qtype) { +struct scan_dquots_data scan_data;struct quota_handle *qh;errcode_t err;
@@ -625,7 +609,11 @@ errcode_t quota_update_limits(quota_ctx_t qctx, ext2_ino_t qf_ino, goto out; } -quota_read_all_dquots (qh, qctx,1 ); + 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 ; + qh->qh_ops->scan_dquots (qh, scan_dquots_callback, &scan_data); err =quota_file_close (qctx, qh);if (err) {
Signature · 1 line
2.26.2
PATCH v2 3/8 quota: Rename quota_update_limits() to quota_read_all_dquots()
In reply to Jan Karaquota_update_limits() is a misnomer because what it actually does is that it updates 'usage' counters and leaves 'limit' counters intact. Rename quota_update_limits() to quota_read_all_dquots() and while changing prototype also add a flags argument so that callers can control which quota information is actually updated from the disk.
--- e2fsck/super.c | 3 ++- lib/support/mkquota.c | 11 ++++++----- lib/support/quotaio.h | 7 +++++-- misc/tune2fs.c | 5 +++-- 4 files changed, 16 insertions(+), 10 deletions(-)
e2fsck/super.c +2 -1
@@ -281,7 +281,8 @@ static errcode_t e2fsck_read_all_quotas(e2fsck_t ctx)if (qf_ino ==0 )continue ; - retval =quota_update_limits (ctx->qctx, qf_ino, qtype); + retval =quota_read_all_dquots (ctx->qctx, qf_ino, qtype, +QREAD_USAGE );if (retval)break ; }
lib/support/mkquota.c +6 -5
@@ -585,10 +585,11 @@ static errcode_t quota_write_all_dquots(struct quota_handle *qh,#endif /* -* Updates the in-memory quota limits from the given quota inode. +* Read quotas from disk and updates the in-memory information determined by +* 'flags' from the on-disk data. */ -errcode_t quota_update_limits (quota_ctx_t qctx,ext2_ino_t qf_ino, -enum quota_type qtype) +errcode_t quota_read_all_dquots (quota_ctx_t qctx,ext2_ino_t qf_ino, +enum quota_type qtype,unsigned int flags) {struct scan_dquots_data scan_data;struct quota_handle *qh;
@@ -611,8 +612,8 @@ errcode_t quota_update_limits(quota_ctx_t qctx, ext2_ino_t qf_ino, 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 = !!(flags &QREAD_LIMITS ); + scan_data.update_usage = !!(flags &QREAD_USAGE ); qh->qh_ops->scan_dquots (qh, scan_dquots_callback, &scan_data); err =quota_file_close (qctx, qh);
lib/support/quotaio.h +5 -2
@@ -224,8 +224,11 @@ void quota_data_add(quota_ctx_t qctx, struct ext2_inode_large *inode,void quota_data_sub (quota_ctx_t qctx,struct ext2_inode_large *inode,ext2_ino_t ino,qsize_t space);errcode_t quota_write_inode (quota_ctx_t qctx,enum quota_type qtype); -errcode_t quota_update_limits (quota_ctx_t qctx,ext2_ino_t qf_ino, -enum quota_type type); +/* Flags for quota_read_all_dquots() */ +#define QREAD_USAGE 0x01 +#define QREAD_LIMITS 0x02 +errcode_t quota_read_all_dquots (quota_ctx_t qctx,ext2_ino_t qf_ino, +enum quota_type type,unsigned int flags);errcode_t quota_compute_usage (quota_ctx_t qctx);void quota_release_context (quota_ctx_t *qctx);errcode_t quota_remove_inode (ext2_filsys fs,enum quota_type qtype);
misc/tune2fs.c +3 -2
@@ -1671,8 +1671,9 @@ static int handle_quota_options(ext2_filsys fs)if (quota_enable[qtype] ==QOPT_ENABLE && *quota_sb_inump (fs->super, qtype) ==0 ) {if ((qf_ino =quota_file_exists (fs, qtype)) >0 ) { - retval =quota_update_limits (qctx, qf_ino, - qtype); + retval =quota_read_all_dquots (qctx, qf_ino, + qtype, +QREAD_USAGE );if (retval) { com_err(program_name, retval, _("while updating quota limits (%d)"),
Signature · 1 line
2.26.2
PATCH v2 4/8 tune2fs: Fix conversion of quota files
In reply to Jan KaraWhen 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.
--- misc/tune2fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
misc/tune2fs.c +1 -1
@@ -1673,7 +1673,7 @@ static int handle_quota_options(ext2_filsys fs)if ((qf_ino =quota_file_exists (fs, qtype)) >0 ) { retval =quota_read_all_dquots (qctx, qf_ino, qtype, -QREAD_USAGE ); +QREAD_LIMITS );if (retval) { com_err(program_name, retval, _("while updating quota limits (%d)"),
Signature · 1 line
2.26.2
PATCH v2 5/8 e2fsck: Do not trash user limits when processing orphan list
In reply to Jan KaraWhen e2fsck was loading quotas to process orphan list, it was loading only quota usage. However subsequent quota writeout has effectively overwritten quota limits, loosing them forever. Make sure quota limits are preserved over orphan replay.
--- e2fsck/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
e2fsck/super.c +1 -1
@@ -282,7 +282,7 @@ static errcode_t e2fsck_read_all_quotas(e2fsck_t ctx)continue ; retval =quota_read_all_dquots (ctx->qctx, qf_ino, qtype, -QREAD_USAGE ); +QREAD_USAGE |QREAD_LIMITS );if (retval)break ; }
Signature · 1 line
2.26.2
PATCH v2 6/8 tests: Expand test checking quota and orphan processing interaction
In reply to Jan KaraExpand f_orphquot test to also check handling of quotas for non-root user and verify that quota limits are properly preserved over orphan replay.
--- tests/f_orphquot/expect | 8 +++++++- tests/f_orphquot/image.bz2 | Bin 1327 -> 2083 bytes tests/f_orphquot/script | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-)
tests/f_orphquot/expect +7 -1
@@ -1,4 +1,4 @@ -Clearing orphaned inode 12 (uid=0, gid=0, mode=0100644, size=3842048) +Clearing orphaned inode 12 (uid=1000, gid=100, mode=0100644, size=3145728) Pass 1: Checking inodes, blocks, and sizes Pass 2: Checking directory structure Pass 3: Checking directory connectivity
@@ -8,3 +8,9 @@ Pass 5: Checking group summary information test_filesystem: ***** FILE SYSTEM WAS MODIFIED ***** test_filesystem: 11/512 files (9.1% non-contiguous), 1070/2048 blocks Exit status is 0 + user id blocks quota limit inodes quota limit + 0 20480 0 0 2 0 0 + 1000 0 5000 6000 0 50 60 + group id blocks quota limit inodes quota limit + 0 20480 0 0 2 0 0 + 100 0 6000 7000 0 60 70
tests/f_orphquot/image.bz2 binary +0 -0
tests/f_orphquot/script +2 -0
@@ -8,6 +8,8 @@ rm -rf $OUT $FSCK -f -y -N test_filesystem $TMPFILE > $OUT.new 2>&1 status=$? echo Exit status is $status >> $OUT.new +$DEBUGFS -R 'lq user' $TMPFILE >> $OUT.new 2>&1 +$DEBUGFS -R 'lq group' $TMPFILE >> $OUT.new 2>&1 sed -f $cmd_dir/filter.sed $OUT.new >> $OUT rm -f $OUT.new
Signature · 1 line
2.26.2
PATCH v2 7/8 debugfs: Fix headers for quota commands
In reply to Jan Karalist_quota and get_quota commands have 'blocks' header while what they actually show is a used space in bytes. Fix the header to state 'space' instead.
--- debugfs/quota.c | 4 ++-- tests/f_orphquot/expect | 4 ++-- tests/f_quota/expect.0 | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-)
debugfs/quota.c +2 -2
@@ -123,7 +123,7 @@ void do_list_quota(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)),printf ("%7s %2s %8s %8s %8s %8s %8s %8s\n" , quota_type[type],"id" , -"blocks" ,"quota" ,"limit" ,"inodes" ,"quota" ,"limit" ); +"space" ,"quota" ,"limit" ,"inodes" ,"quota" ,"limit" ); qh = current_qctx->quota_file[type]; retval = qh->qh_ops->scan_dquots (qh, list_quota_callback,NULL );if (retval) {
@@ -158,7 +158,7 @@ void do_get_quota(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)),printf ("%7s %2s %8s %8s %8s %8s %8s %8s\n" , quota_type[type],"id" , -"blocks" ,"quota" ,"limit" ,"inodes" ,"quota" ,"limit" ); +"space" ,"quota" ,"limit" ,"inodes" ,"quota" ,"limit" ); qh = current_qctx->quota_file[type];
tests/f_orphquot/expect +2 -2
@@ -8,9 +8,9 @@ Pass 5: Checking group summary information test_filesystem: ***** FILE SYSTEM WAS MODIFIED ***** test_filesystem: 11/512 files (9.1% non-contiguous), 1070/2048 blocks Exit status is 0 - user id blocks quota limit inodes quota limit + user id space quota limit inodes quota limit 0 20480 0 0 2 0 0 1000 0 5000 6000 0 50 60 - group id blocks quota limit inodes quota limit + group id space quota limit inodes quota limit 0 20480 0 0 2 0 0 100 0 6000 7000 0 60 70
tests/f_quota/expect.0 +6 -6
@@ -1,21 +1,21 @@ debugfs: list_quota user - user id blocks quota limit inodes quota limit + user id space quota limit inodes quota limit 0 13312 0 0 2 0 0 34 1024 0 0 1 0 0 100 2048 32 50 2 20 30 debugfs: list_quota group - group id blocks quota limit inodes quota limit + group id space quota limit inodes quota limit 0 16384 0 0 5 0 0 debugfs: get_quota user 0 - user id blocks quota limit inodes quota limit + user id space quota limit inodes quota limit 0 13312 0 0 2 0 0 debugfs: get_quota user 100 - user id blocks quota limit inodes quota limit + user id space quota limit inodes quota limit 100 2048 32 50 2 20 30 debugfs: get_quota user 34 - user id blocks quota limit inodes quota limit + user id space quota limit inodes quota limit 34 1024 0 0 1 0 0 debugfs: get_quota group 0 - group id blocks quota limit inodes quota limit + group id space quota limit inodes quota limit 0 16384 0 0 5 0 0 debugfs:
Signature · 1 line
2.26.2
PATCH v2 8/8 quota: Drop dead code
In reply to Jan KaraDrop unused function from quota support code.
--- lib/support/mkquota.c | 20 -------------------- 1 file changed, 20 deletions(-)
lib/support/mkquota.c +0 -20
@@ -564,26 +564,6 @@ static int scan_dquots_callback(struct dquot *dquot, void *cb_data)return 0 ; } -/* -* Write all memory dquots into quota file -*/ -#if 0 /* currently unused, but may be useful in the future? */ -static errcode_t quota_write_all_dquots (struct quota_handle *qh, -quota_ctx_t qctx) -{ -errcode_t err; - - err =ext2fs_read_bitmaps (qctx->fs); -if (err) -return err; -write_dquots (qctx->quota_dict[qh->qh_type], qh); -ext2fs_mark_bb_dirty (qctx->fs); - qctx->fs->flags &= ~EXT2_FLAG_SUPER_ONLY ; -ext2fs_write_bitmaps (qctx->fs); -return 0 ; -} -#endif -/* * Read quotas from disk and updates the in-memory information determined by * 'flags' from the on-disk data.
Signature · 1 line
2.26.2
Hello Ted!
Ping? What about these fixes?
Honza
Quoted 16 lines · Jan Kara
Hello,
here is next revision of my quota fixes for e2fsprogs. When addressing the e2fsck regression Ted has pointed out, I've noticed another serious bug in e2fsck support for quotas where it just nukes existing quota limits when replaying orphan list. A fix is now included in this series together with expanded test to catch the problem.
Changes since v1:
* Rename the functions so that names match functionality
* Fix e2fsck regression when processing orphan list
* Fix e2fsck bug to preserve quota limits over orphan processing
* Expand test to verify quota limits are properly preserved
* Fix quota output headings in debugfsHonza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR