By the time we start processing the orphan inode list, we have already
calculated the total expected number of free blocks and inodes in
ctx->free_{blocks,inodes}. This is used to set the free blocks/inodes
count in the superblock in the case where we don't need to do a full
e2fsck.
We need to update these expected free block counts as we process the
orphan inode list so that superblock values are set correctly.
Otherwise we could have the following happen:
% e2fsck /tmp/test.img
e2fsck 1.42.3 (14-May-2012)
Truncating orphaned inode 12 (uid=0, gid=0, mode=0100644, size=0)
Setting free blocks count to 46 (was 79)
/tmp/test.img: clean, 12/16 files, 54/100 blocks
% e2fsck /tmp/test.img
e2fsck 1.42.3 (14-May-2012)
Setting free blocks count to 79 (was 46)
/tmp/test.img: clean, 12/16 files, 21/100 blocks
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
e2fsck/super.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/e2fsck/super.c b/e2fsck/super.c
index 6c18d0e..160991d 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -143,6 +143,7 @@ static int release_inode_block(ext2_filsys fs,
}
ext2fs_block_alloc_stats2(fs, blk, -1);
+ ctx->free_blocks++;
return retval;
}
@@ -211,9 +212,11 @@ static int release_inode_blocks(e2fsck_t ctx, ext2_ino_t ino,
ino);
return 1;
}
- if (count == 0)
+ if (count == 0) {
ext2fs_block_alloc_stats2(fs,
ext2fs_file_acl_block(fs, inode), -1);
+ ctx->free_blocks++;
+ }
ext2fs_file_acl_block_set(fs, inode, 0);
}
return 0;
@@ -286,6 +289,7 @@ static int release_orphan_inodes(e2fsck_t ctx)
if (!inode.i_links_count) {
ext2fs_inode_alloc_stats2(fs, ino, -1,
LINUX_S_ISDIR(inode.i_mode));
+ ctx->free_inodes++;
inode.i_dtime = ctx->now;
} else {
inode.i_dtime = 0;
--
1.7.10.2.552.gaa3bb87
Add two tests, f_orphan_indirect_inode, and f_orphan_extents_inode,
which tests the bug fixes in the two previous commits:
e2fsck: update global free blocks/inodes count when truncating orphan inodes
libext2fs: fix block iterator for extents when truncating inodes
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
tests/f_orphan_extents_inode/expect.1 | 10 ++++++++++
tests/f_orphan_extents_inode/expect.2 | 7 +++++++
tests/f_orphan_extents_inode/image.gz | Bin 0 -> 564 bytes
tests/f_orphan_extents_inode/name | 1 +
tests/f_orphan_indirect_inode/expect.1 | 2 ++
tests/f_orphan_indirect_inode/expect.2 | 7 +++++++
tests/f_orphan_indirect_inode/image.gz | Bin 0 -> 592 bytes
tests/f_orphan_indirect_inode/name | 1 +
tests/f_orphan_indirect_inode/script | 2 ++
9 files changed, 30 insertions(+)
create mode 100644 tests/f_orphan_extents_inode/expect.1
create mode 100644 tests/f_orphan_extents_inode/expect.2
create mode 100644 tests/f_orphan_extents_inode/image.gz
create mode 100644 tests/f_orphan_extents_inode/name
create mode 100644 tests/f_orphan_indirect_inode/expect.1
create mode 100644 tests/f_orphan_indirect_inode/expect.2
create mode 100644 tests/f_orphan_indirect_inode/image.gz
create mode 100644 tests/f_orphan_indirect_inode/name
create mode 100644 tests/f_orphan_indirect_inode/script
diff --git a/tests/f_orphan_extents_inode/expect.1 b/tests/f_orphan_extents_inode/expect.1
new file mode 100644
index 0000000..2eaab78
--- /dev/null
+++ b/tests/f_orphan_extents_inode/expect.1
@@ -0,0 +1,10 @@
+Truncating orphaned inode 12 (uid=0, gid=0, mode=0100644, size=0)
+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: ***** FILE SYSTEM WAS MODIFIED *****
+test_filesys: 12/16 files (0.0% non-contiguous), 21/100 blocks
+Exit status is 1
diff --git a/tests/f_orphan_extents_inode/expect.2 b/tests/f_orphan_extents_inode/expect.2
new file mode 100644
index 0000000..4ee5d96
--- /dev/null
+++ b/tests/f_orphan_extents_inode/expect.2
@@ -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/16 files (0.0% non-contiguous), 21/100 blocks
+Exit status is 0
diff --git a/tests/f_orphan_extents_inode/image.gz b/tests/f_orphan_extents_inode/image.gz
new file mode 100644
index 0000000000000000000000000000000000000000..6c57fef553f44be6dd97bb2449827dd0ce5a0007
GIT binary patch
literal 564
zc-oWi=HT#Lc+H=QIlrhNBQZ}mwW1_7ucTNvGcP|SRWCC)o#E|W`)uJv8TJp)cX~VC
zc34$bARuyNAxGx&S1TN?cBFg|73rFs%dUOHPxpt}624NwazPoZj?K+3uc8G?FF3|L
zn!KBPhKWMP)C)CsAI{27SHCmgX21OQCqhgHE7O9*f|mt}1!ZY|yv{iN)R$wn_g`<(
zw|TiG`9<0Ln$Dv;{%%=ozvchVnN#LYd9Artr)BMynypuJYin**efjZq_S>53pNBsb
z>a1Ct|L^qYHOp;we|u!Gzc%~j(@zio-2T=7ed8^;wsVgyjtbtcx?XYeN;1zA&F4XS
z;$P#rBMq&8fBy1$mNtue`1T7L*Y=k;dQKAke!wt$S?bPtsb|wF`R&D*ZLXI7ZodBB
z-HHwSqV8$$Kb;(N(d?ni49Qjh^D86&A81*zlV!m_#;^P<>K*<vzmgC5-|&n5ial6}
zfuUz{-r@WOi9s(L7*K#g-M8;nZ|nX&ud~0)bM*7^s^7LNg7!@o|L-nyXr;jA*I%w%
ze3-FG!e(2~{_UG*$BAWcUHsWT?DPBca|<hv-*a65U#IU?eWXSF*ZBsE{$B}R*0=cm
zyRhShOa31%NM7*&Df937zB_|-r8SE+#aHjED9eyv{ybdn*D^*1N<hQ?pPcCj7~e8X
HU}OLQ$zlQb
literal 0
Hc-jL100001
diff --git a/tests/f_orphan_extents_inode/name b/tests/f_orphan_extents_inode/name
new file mode 100644
index 0000000..fc48ec2
--- /dev/null
+++ b/tests/f_orphan_extents_inode/name
@@ -0,0 +1 @@
+truncating an orphaned extent-mapped inode
diff --git a/tests/f_orphan_indirect_inode/expect.1 b/tests/f_orphan_indirect_inode/expect.1
new file mode 100644
index 0000000..33cdd65
--- /dev/null
+++ b/tests/f_orphan_indirect_inode/expect.1
@@ -0,0 +1,2 @@
+test_filesys: clean, 12/16 files, 21/100 blocks
+Exit status is 0
diff --git a/tests/f_orphan_indirect_inode/expect.2 b/tests/f_orphan_indirect_inode/expect.2
new file mode 100644
index 0000000..4ee5d96
--- /dev/null
+++ b/tests/f_orphan_indirect_inode/expect.2
@@ -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/16 files (0.0% non-contiguous), 21/100 blocks
+Exit status is 0
diff --git a/tests/f_orphan_indirect_inode/image.gz b/tests/f_orphan_indirect_inode/image.gz
new file mode 100644
index 0000000000000000000000000000000000000000..54b39094350bfb528a4202b9cb3d3be38ddab209
GIT binary patch
literal 592
zc-oWi=HQsW@R~mpbAC}lMq-|BW?o8WQEGAtkddE~s+XCY&hYlGZMJZs%z=;pW0kU2
zZ_8TCzx7)CuZZ5SoFY~$niqL5T&L$$D3E#Z)jhx1Ilq`h9FI5(91(x;Oj<y_uslGV
zeftmZcS;N2DqRga^PKs<8K?NAKh^t{%e{k`tU~imB-eIl1)83^Wqu?6O7y+jUvC~)
zOndb;`u^r;W&DrovM*ozyle5z7fDm5_v`PeFUs8T|Jmc~@yFEH-Jg9j{C90$*S&o^
zYv0~J@bqE0eq5jL-G~`yP949z==(?W)H1_s8@wz(AN>;^oaeb9`O56JS=;NcHTGRf
z=3EfGbld0qVXJ@seEfRqBbFr}cYRJ>YdrZ|@x-rot3L9dId`=GVpi$PfAiP=h}m)e
z)4Iy{Qpe|CeA-uEdF$%7yKNcCS3=|N=fs%bk;(0>I`w)*^y1u$-|Q>?J}*A{{-^5k
z{W`I^xAWYu$iMB8s=xj&!{0>UZ7<v6+KT~|C%-w|F1D{dcl^)tNWXh`rf=T!?RVL{
z>sd$b?oHqI>Gx%0e)o5N_OoW`eEZ2iRcCkd%?JB0?9KRBAJX-c-OA;vP{U95GxiDp
z7(esRs6X(B`I)@If3Oe(GT0!*zS#L<L5YQxkZf<C>+!|H7d=Xhtforc7o2zL<&{6D
g)|9AONzd&*=Ps_J&Oj+RFzY*a!K>vl3=<d`05fVA1poj5
literal 0
Hc-jL100001
diff --git a/tests/f_orphan_indirect_inode/name b/tests/f_orphan_indirect_inode/name
new file mode 100644
index 0000000..04ccb31
--- /dev/null
+++ b/tests/f_orphan_indirect_inode/name
@@ -0,0 +1 @@
+truncating an orphaned inode in preen mode
diff --git a/tests/f_orphan_indirect_inode/script b/tests/f_orphan_indirect_inode/script
new file mode 100644
index 0000000..ea3b93a
--- /dev/null
+++ b/tests/f_orphan_indirect_inode/script
@@ -0,0 +1,2 @@
+FSCK_OPT=-p
+. $cmd_dir/run_e2fsck
--
1.7.10.2.552.gaa3bb87
When e2fsck uses the block iterator to release the blocks in an
extent-mapped inode, when the last block in an extent is removed, the
current extent has been removed and the extent cursor is now pointing
at the next inode. But the block iterator code doesn't know that. So
when it tries to go the next extent, it will end up skipping an
extent, and so the inode will be incompletely truncated.
The fix is to go to the next extent before calling the callback
function for the current extent. This way, regardless of whether the
current extent gets removed, the extent cursor is still pointing at
the right place.
Reported-by: Andreas Dilger <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
lib/ext2fs/block.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/lib/ext2fs/block.c b/lib/ext2fs/block.c
index 85a1803..68dcb03 100644
--- a/lib/ext2fs/block.c
+++ b/lib/ext2fs/block.c
@@ -389,7 +389,7 @@ errcode_t ext2fs_block_iterate3(ext2_filsys fs,
if (inode.i_flags & EXT4_EXTENTS_FL) {
ext2_extent_handle_t handle;
- struct ext2fs_extent extent;
+ struct ext2fs_extent extent, next;
e2_blkcnt_t blockcnt = 0;
blk64_t blk, new_blk;
int op = EXT2_EXTENT_ROOT;
@@ -401,7 +401,11 @@ errcode_t ext2fs_block_iterate3(ext2_filsys fs,
goto abort_exit;
while (1) {
- ctx.errcode = ext2fs_extent_get(handle, op, &extent);
+ if (op == EXT2_EXTENT_CURRENT)
+ ctx.errcode = 0;
+ else
+ ctx.errcode = ext2fs_extent_get(handle, op,
+ &extent);
if (ctx.errcode) {
if (ctx.errcode != EXT2_ET_EXTENT_NO_NEXT)
break;
@@ -456,6 +460,13 @@ errcode_t ext2fs_block_iterate3(ext2_filsys fs,
uninit = 0;
if (extent.e_flags & EXT2_EXTENT_FLAGS_UNINIT)
uninit = EXT2_EXTENT_SET_BMAP_UNINIT;
+
+ /*
+ * Get the next extent before we start messing
+ * with the current extent
+ */
+ retval = ext2fs_extent_get(handle, op, &next);
+
#if 0
printf("lblk %llu pblk %llu len %d blockcnt %llu\n",
extent.e_lblk, extent.e_pblk,
@@ -487,6 +498,10 @@ errcode_t ext2fs_block_iterate3(ext2_filsys fs,
if (ret & BLOCK_ABORT)
goto extent_done;
}
+ if (retval == 0) {
+ extent = next;
+ op = EXT2_EXTENT_CURRENT;
+ }
}
extent_done:
--
1.7.10.2.552.gaa3bb87
On 2012-06-11, at 11:45 PM, Theodore Ts'o wrote:
> Add two tests, f_orphan_indirect_inode, and f_orphan_extents_inode,
> which tests the bug fixes in the two previous commits:
>
> e2fsck: update global free blocks/inodes count when truncating orphan inodes
> libext2fs: fix block iterator for extents when truncating inodes
>
> diff --git a/tests/f_orphan_indirect_inode/expect.1 b/tests/f_orphan_indirect_inode/expect.1
> new file mode 100644
> index 0000000..33cdd65
> --- /dev/null
> +++ b/tests/f_orphan_indirect_inode/expect.1
> @@ -0,0 +1,2 @@
> +test_filesys: clean, 12/16 files, 21/100 blocks
> +Exit status is 0
Hmm, this doesn't appear to be truncating an orphan inode, unlike
the f_orphan_extent_inode test is doing?
Cheers, Andreas
--
Andreas Dilger Whamcloud, Inc.
Principal Lustre Engineer http://www.whamcloud.com/
On Wed, Jun 13, 2012 at 12:44:10AM -0600, Andreas Dilger wrote:
> > diff --git a/tests/f_orphan_indirect_inode/expect.1 b/tests/f_orphan_indirect_inode/expect.1
> > new file mode 100644
> > index 0000000..33cdd65
> > --- /dev/null
> > +++ b/tests/f_orphan_indirect_inode/expect.1
> > @@ -0,0 +1,2 @@
> > +test_filesys: clean, 12/16 files, 21/100 blocks
> > +Exit status is 0
>
> Hmm, this doesn't appear to be truncating an orphan inode, unlike
> the f_orphan_extent_inode test is doing?
That's because we're running the test in preen mode, where messages
about the orphan inode processing are suppressed. The bug that this
test is specifically testing for only happens we don't do a full file
system check, and normally tests run with -fy, which forces a full
check.
I overrode the default options used for the first fs check in this
test to be -p. I probably should have used -y instead, so that the
orpan inode message didn't get hidden. I'll fix that for the next
release.
In any case, if you take the image from this test, and run "e2fsck -y
/tmp/image; e2fsck -fy /tmp/image" using the e2fsck from 1.42.3, it'll
become obvious what this test is checking....
- Ted