quota-related e2fsck fixes and tests
v3 cover quota-related e2fsck fixes and tests
Hi!
Changes since v2:
Added deallocate_inode() documentation, as suggested by Andreas Dilger (and using text he provided).
And, for reference, here's the cover-letter from v2:
I'm (re)sending two fixes to e2fsck that are related with quota handling. The fixes are the first two patches, the other two are test cases for these fixes.
As I mentioned in v1, the issues were found using fstests ext4/014 and ext4/019. And the only thing in this series that changed from v1 was the first test ("tests: new test to check quota after directory optimization"), which is now using a much simplified version of the testcase.
Note that, since the first two patches didn't change, they have already a
which I'm not including in the patches themselves. Should I? Or is that better left for the maintainer (eventually) applying them?
Cheers,
Signature · 28 lines
Luis Luis Henriques (SUSE) (4): e2fsck: update quota accounting after directory optimization e2fsck: update quota when deallocating a bad inode tests: new test to check quota after directory optimization tests: new test to check quota after a bad inode deallocation e2fsck/pass2.c | 43 ++++++++++++++++++------ e2fsck/rehash.c | 27 +++++++++++---- tests/f_quota_deallocate_inode/expect.1 | 18 ++++++++++ tests/f_quota_deallocate_inode/expect.2 | 7 ++++ tests/f_quota_deallocate_inode/image.gz | Bin 0 -> 11594 bytes tests/f_quota_deallocate_inode/name | 1 + tests/f_quota_shrinkdir/expect.1 | 18 ++++++++++ tests/f_quota_shrinkdir/expect.2 | 7 ++++ tests/f_quota_shrinkdir/image.gz | Bin 0 -> 10761 bytes tests/f_quota_shrinkdir/name | 1 + 10 files changed, 105 insertions(+), 17 deletions(-) create mode 100644 tests/f_quota_deallocate_inode/expect.1 create mode 100644 tests/f_quota_deallocate_inode/expect.2 create mode 100644 tests/f_quota_deallocate_inode/image.gz create mode 100644 tests/f_quota_deallocate_inode/name create mode 100644 tests/f_quota_shrinkdir/expect.1 create mode 100644 tests/f_quota_shrinkdir/expect.2 create mode 100644 tests/f_quota_shrinkdir/image.gz create mode 100644 tests/f_quota_shrinkdir/name
PATCH v3 1/4 e2fsck: update quota accounting after directory optimization
In "Pass 3A: Optimizing directories", a directory may have it's size reduced. If that happens and quota is enabled in the filesystem, the quota information will be incorrect because it doesn't take the rehash into account. This issue was detected by running fstest ext4/014.
This patch simply updates the quota data accordingly, after the directory is written and it's size has been updated.
--- e2fsck/rehash.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
e2fsck/rehash.c +21 -6
@@ -987,14 +987,18 @@ errcode_t e2fsck_rehash_dir(e2fsck_t ctx, ext2_ino_t ino, {ext2_filsys fs = ctx->fs;errcode_t retval; -struct ext2_inode inode; +struct ext2_inode_large inode;char *dir_buf =0 ;struct fill_dir_struct fd = {NULL ,NULL ,0 ,0 ,0 ,NULL ,0 ,0 ,0 ,0 ,0 ,0 };struct out_dir outdir = {0 ,0 ,0 ,0 }; -struct name_cmp_ctx name_cmp_ctx = {0 ,NULL }; +struct name_cmp_ctx name_cmp_ctx = {0 ,NULL }; +__u64 osize; -e2fsck_read_inode (ctx, ino, &inode,"rehash_dir" ); +e2fsck_read_inode_full (ctx, ino,EXT2_INODE (&inode), +sizeof (inode),"rehash_dir" ); + + osize =EXT2_I_SIZE (&inode);if (ext2fs_has_feature_inline_data (fs->super) && (inode.i_flags &EXT4_INLINE_DATA_FL ))
@@ -1013,7 +1017,7 @@ errcode_t e2fsck_rehash_dir(e2fsck_t ctx, ext2_ino_t ino, fd.ino = ino; fd.ctx = ctx; fd.buf = dir_buf; - fd.inode = &inode; + fd.inode =EXT2_INODE (&inode); fd.dir = ino;if (!ext2fs_has_feature_dir_index (fs->super) || (inode.i_size / fs->blocksize) <2 )
@@ -1092,14 +1096,25 @@ resort: goto errout; } - retval =write_directory (ctx, fs, &outdir, ino, &inode, fd.compress); + retval =write_directory (ctx, fs, &outdir, ino,EXT2_INODE (&inode), + fd.compress);if (retval) goto errout; +if ((osize >EXT2_I_SIZE (&inode)) && + (ino !=quota_type2inum (PRJQUOTA , fs->super)) && + (ino != fs->super->s_orphan_file_inum) && + (ino ==EXT2_ROOT_INO || ino >=EXT2_FIRST_INODE (ctx->fs->super)) && + !(inode.i_flags &EXT4_EA_INODE_FL )) { +quota_data_sub (ctx->qctx, &inode, + ino, osize -EXT2_I_SIZE (&inode)); + } +if (ctx->options &E2F_OPT_CONVERT_BMAP ) retval =e2fsck_rebuild_extents_later (ctx, ino);else - retval =e2fsck_check_rebuild_extents (ctx, ino, &inode, pctx); + retval =e2fsck_check_rebuild_extents (ctx, ino, +EXT2_INODE (&inode), pctx); errout:ext2fs_free_mem (&dir_buf);ext2fs_free_mem (&fd.harray);
PATCH v3 2/4 e2fsck: update quota when deallocating a bad inode
In reply to Luis Henriques (SUSE)If a bad inode is found it will be deallocated. However, if the filesystem has quota enabled, the quota information isn't being updated accordingly. This issue was detected by running fstest ext4/019.
This patch fixes the issue by decreasing the inode count from the quota and, if blocks are also being released, also subtract them as well.
While there, and as suggested by Andreas Dilger, the deallocate_inode() function documentation is also updated by this patch to make it clear what that function really does.
--- e2fsck/pass2.c | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-)
e2fsck/pass2.c +32 -11
@@ -1854,17 +1854,26 @@ static int deallocate_inode_block(ext2_filsys fs, }/* -* This function deallocates an inode +* This function reverts various counters and bitmaps incremented in +* pass1 for the inode, blocks, and quotas before it was decided the +* inode was corrupt and needed to be cleared. This avoids the need +* to run e2fsck a second time (or have it restart itself) to repair +* these counters. +* +* It does not modify any on-disk state, so even if the inode is bad +* it _should_ reset in-memory state to before the inode was first +* processed. */ static void deallocate_inode (e2fsck_t ctx,ext2_ino_t ino,char * block_buf) {ext2_filsys fs = ctx->fs; -struct ext2_inode inode; +struct ext2_inode_large inode;struct problem_context pctx;__u32 count;struct del_block del_block; -e2fsck_read_inode (ctx, ino, &inode,"deallocate_inode" ); +e2fsck_read_inode_full (ctx, ino,EXT2_INODE (&inode), +sizeof (inode),"deallocate_inode" );clear_problem_context (&pctx); pctx.ino = ino;
@@ -1874,29 +1883,29 @@ static void deallocate_inode(e2fsck_t ctx, ext2_ino_t ino, char* block_buf)e2fsck_read_bitmaps (ctx);ext2fs_inode_alloc_stats2 (fs, ino,-1 ,LINUX_S_ISDIR (inode.i_mode)); -if (ext2fs_file_acl_block (fs, &inode) && +if (ext2fs_file_acl_block (fs,EXT2_INODE (&inode)) &&ext2fs_has_feature_xattr (fs->super)) { pctx.errcode =ext2fs_adjust_ea_refcount3 (fs, -ext2fs_file_acl_block (fs, &inode), +ext2fs_file_acl_block (fs,EXT2_INODE (&inode)), block_buf,-1 , &count, ino);if (pctx.errcode ==EXT2_ET_BAD_EA_BLOCK_NUM ) { pctx.errcode =0 ; count =1 ; }if (pctx.errcode) { - pctx.blk =ext2fs_file_acl_block (fs, &inode); + pctx.blk =ext2fs_file_acl_block (fs,EXT2_INODE (&inode));fix_problem (ctx,PR_2_ADJ_EA_REFCOUNT , &pctx); ctx->flags |=E2F_FLAG_ABORT ;return ; }if (count ==0 ) {ext2fs_block_alloc_stats2 (fs, -ext2fs_file_acl_block (fs, &inode),-1 ); +ext2fs_file_acl_block (fs,EXT2_INODE (&inode)),-1 ); } -ext2fs_file_acl_block_set (fs, &inode,0 ); +ext2fs_file_acl_block_set (fs,EXT2_INODE (&inode),0 ); } -if (!ext2fs_inode_has_valid_blocks2 (fs, &inode)) +if (!ext2fs_inode_has_valid_blocks2 (fs,EXT2_INODE (&inode))) goto clear_inode;/* Inline data inodes don't have blocks to iterate */
@@ -1921,10 +1930,22 @@ static void deallocate_inode(e2fsck_t ctx, ext2_ino_t ino, char* block_buf) ctx->flags |=E2F_FLAG_ABORT ;return ; } + +if ((ino !=quota_type2inum (PRJQUOTA , fs->super)) && + (ino != fs->super->s_orphan_file_inum) && + (ino ==EXT2_ROOT_INO || ino >=EXT2_FIRST_INODE (ctx->fs->super)) && + !(inode.i_flags &EXT4_EA_INODE_FL )) { +if (del_block.num >0 ) +quota_data_sub (ctx->qctx, &inode, ino, + del_block.num *EXT2_CLUSTER_SIZE (fs->super)); +quota_data_inodes (ctx->qctx, (struct ext2_inode_large *)&inode, + ino,-1 ); + } + clear_inode:/* Inode may have changed by block_iterate, so reread it */ -e2fsck_read_inode (ctx, ino, &inode,"deallocate_inode" ); -e2fsck_clear_inode (ctx, ino, &inode,0 ,"deallocate_inode" ); +e2fsck_read_inode (ctx, ino,EXT2_INODE (&inode),"deallocate_inode" ); +e2fsck_clear_inode (ctx, ino,EXT2_INODE (&inode),0 ,"deallocate_inode" ); }/*
PATCH v3 3/4 tests: new test to check quota after directory optimization
In reply to Luis Henriques (SUSE)This new test validates e2fsck by verifying that quota data is updated after a directory optimization is performed. This issue was initially found by fstest ext4/014, and this test was based on it. It includes a filesystem image where the lost+found directory is unlinked after a new link to it is created:
# debugfs -w -R "ln lost+found foo" f_testnew/image # debugfs -w -R "unlink lost+found" f_testnew/image
--- tests/f_quota_shrinkdir/expect.1 | 18 ++++++++++++++++++ tests/f_quota_shrinkdir/expect.2 | 7 +++++++ tests/f_quota_shrinkdir/image.gz | Bin 0 -> 10761 bytes tests/f_quota_shrinkdir/name | 1 + 4 files changed, 26 insertions(+) create mode 100644 tests/f_quota_shrinkdir/expect.1 create mode 100644 tests/f_quota_shrinkdir/expect.2 create mode 100644 tests/f_quota_shrinkdir/image.gz create mode 100644 tests/f_quota_shrinkdir/name
tests/f_quota_shrinkdir/expect.1 new file +18 -0
@@ -0,0 +1,18 @@ +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Pass 3: Checking directory connectivity +/lost+found not found. Create? yes + +Pass 3A: Optimizing directories +Pass 4: Checking reference counts +Pass 5: Checking group summary information +[QUOTA WARNING] Usage inconsistent for ID 0:actual (3072, 3) != expected (13312, 2) +Update quota info for quota type 0? yes + +[QUOTA WARNING] Usage inconsistent for ID 0:actual (3072, 3) != expected (13312, 2) +Update quota info for quota type 1? yes + + +test_filesys: ***** FILE SYSTEM WAS MODIFIED ***** +test_filesys: 12/256 files (16.7% non-contiguous), 1145/8192 blocks +Exit status is 1
tests/f_quota_shrinkdir/expect.2 new file +7 -0
@@ -0,0 +1,7 @@ +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Pass 3: Checking directory connectivity +Pass 4: Checking reference counts +Pass 5: Checking group summary information +test_filesys: 12/256 files (16.7% non-contiguous), 1145/8192 blocks +Exit status is 0
tests/f_quota_shrinkdir/image.gz new file binary +0 -0
tests/f_quota_shrinkdir/name new file +1 -0
@@ -0,0 +1 @@ +update quota on directory optimization
PATCH v3 4/4 tests: new test to check quota after a bad inode deallocation
In reply to Luis Henriques (SUSE)This new test validates e2fsck by verifying that quota is updated after a bad inode is deallocated. It mimics fstest ext4/019 by including a filesystem image where a symbolic link was created to an existing file, using a long symlink name. This symbolic link was then wiped with:
# debugfs -w -R 'zap -f /testlink 0' f_testnew/image
--- tests/f_quota_deallocate_inode/expect.1 | 18 ++++++++++++++++++ tests/f_quota_deallocate_inode/expect.2 | 7 +++++++ tests/f_quota_deallocate_inode/image.gz | Bin 0 -> 11594 bytes tests/f_quota_deallocate_inode/name | 1 + 4 files changed, 26 insertions(+) create mode 100644 tests/f_quota_deallocate_inode/expect.1 create mode 100644 tests/f_quota_deallocate_inode/expect.2 create mode 100644 tests/f_quota_deallocate_inode/image.gz create mode 100644 tests/f_quota_deallocate_inode/name
tests/f_quota_deallocate_inode/expect.1 new file +18 -0
@@ -0,0 +1,18 @@ +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Symlink /testlink (inode #14) is invalid. +Clear? yes + +Pass 3: Checking directory connectivity +Pass 4: Checking reference counts +Pass 5: Checking group summary information +[QUOTA WARNING] Usage inconsistent for ID 0:actual (15360, 4) != expected (16384, 5) +Update quota info for quota type 0? yes + +[QUOTA WARNING] Usage inconsistent for ID 0:actual (15360, 4) != expected (16384, 5) +Update quota info for quota type 1? yes + + +test_filesys: ***** FILE SYSTEM WAS MODIFIED ***** +test_filesys: 13/256 files (15.4% non-contiguous), 1157/8192 blocks +Exit status is 1
tests/f_quota_deallocate_inode/expect.2 new file +7 -0
@@ -0,0 +1,7 @@ +Pass 1: Checking inodes, blocks, and sizes +Pass 2: Checking directory structure +Pass 3: Checking directory connectivity +Pass 4: Checking reference counts +Pass 5: Checking group summary information +test_filesys: 13/256 files (15.4% non-contiguous), 1157/8192 blocks +Exit status is 0
tests/f_quota_deallocate_inode/image.gz new file binary +0 -0
tests/f_quota_deallocate_inode/name new file +1 -0
@@ -0,0 +1 @@ +update quota when deallocating bad inode
Theodore Ts'o
AppliedQuoted 8 lines · Luis Henriques (SUSE)
Changes since v2:
Added
deallocate_inode()documentation, as suggested by Andreas Dilger (and using text he provided).And, for reference, here's the cover-letter from v2:
[...]
Applied, thanks!
[1/4] e2fsck: update quota accounting after directory optimization
commit: eb782652045e67a5379dd319613b0d3d924901dd
[2/4] e2fsck: update quota when deallocating a bad inode
commit: eb0680deb908d7da399dd331b7aa35cc1a4dfa2d
[3/4] tests: new test to check quota after directory optimization
commit: 7805308b7a111c3943d92eab412b048d059ce108
[4/4] tests: new test to check quota after a bad inode deallocation
commit: c6070404a2bed3ff675fc6500828ab6acee985f1Best regards,
Signature · 1 line
Theodore Ts'o <[email protected]>