[PATCH RESEND 1/4] e2fsck: fix orphaned extent files handling

[Etienne AUJAMES <eaujames@ddn.com>]

release_inode_blocks() does not handle corectly multi-levels extent
files: it does not count the non-leaf blocks directly released by
ext2fs_block_iterate3().

This patch relies on ext2fs_get_stat_i_blocks() count for quota update
and ext2fs_free_blocks_count() to count number of blocks released by
release_inode_blocks().

Add regression test: f_orphan_truncate_extents_inode

Signed-off-by: Etienne AUJAMES <eaujames@ddn.com>
Change-Id: Ib0c3aaaa685e7bcfae896617cda03005d19539ff
Lustre-bug-id: https://jira.whamcloud.com/browse/LU-20049
---
 e2fsck/super.c                                | 244 +++++++++---------
 .../f_orphan_truncate_extents_inode/expect.1  |   3 +
 .../f_orphan_truncate_extents_inode/expect.2  |   7 +
 .../f_orphan_truncate_extents_inode/image.gz  | Bin 0 -> 2854 bytes
 tests/f_orphan_truncate_extents_inode/name    |   1 +
 tests/f_orphan_truncate_extents_inode/script  |   3 +
 6 files changed, 133 insertions(+), 125 deletions(-)
 create mode 100644 tests/f_orphan_truncate_extents_inode/expect.1
 create mode 100644 tests/f_orphan_truncate_extents_inode/expect.2
 create mode 100644 tests/f_orphan_truncate_extents_inode/image.gz
 create mode 100644 tests/f_orphan_truncate_extents_inode/name
 create mode 100644 tests/f_orphan_truncate_extents_inode/script

diff --git a/e2fsck/super.c b/e2fsck/super.c
index cfc0919a2..c2ccefd54 100644
--- a/e2fsck/super.c
+++ b/e2fsck/super.c
@@ -62,21 +62,14 @@ static int check_super_value64(e2fsck_t ctx, const char *descr,
 	return 1;
 }
 
-/*
- * helper function to release an inode
- */
 struct process_block_struct {
-	e2fsck_t 	ctx;
-	char 		*buf;
+	e2fsck_t	ctx;
+	char		*buf;
 	struct problem_context *pctx;
-	int		truncating;
-	int		truncate_offset;
 	e2_blkcnt_t	truncate_block;
-	int		truncated_blocks;
-	int		abort;
+	e2_blkcnt_t	truncated_blocks;
 	errcode_t	errcode;
 	blk64_t last_cluster;
-	struct ext2_inode_large *inode;
 };
 
 static int release_inode_block(ext2_filsys fs,
@@ -91,7 +84,6 @@ static int release_inode_block(ext2_filsys fs,
 	struct problem_context	*pctx;
 	blk64_t			blk = *block_nr;
 	blk64_t			cluster = EXT2FS_B2C(fs, *block_nr);
-	int			retval = 0;
 
 	pb = (struct process_block_struct *) priv_data;
 	ctx = pb->ctx;
@@ -111,155 +103,157 @@ static int release_inode_block(ext2_filsys fs,
 	if ((blk < fs->super->s_first_data_block) ||
 	    (blk >= ext2fs_blocks_count(fs->super))) {
 		fix_problem(ctx, PR_0_ORPHAN_ILLEGAL_BLOCK_NUM, pctx);
-	return_abort:
-		pb->abort = 1;
+		pb->errcode = EXT2_ET_BAD_BLOCK_NUM;
 		return BLOCK_ABORT;
 	}
 
 	if (!ext2fs_test_block_bitmap2(fs->block_map, blk)) {
 		fix_problem(ctx, PR_0_ORPHAN_ALREADY_CLEARED_BLOCK, pctx);
-		goto return_abort;
+		pb->errcode = EXT2_ET_BAD_BLOCK_NUM;
+		return BLOCK_ABORT;
 	}
 
 	/*
-	 * If we are deleting an orphan, then we leave the fields alone.
-	 * If we are truncating an orphan, then update the inode fields
-	 * and clean up any partial block data.
+	 * We don't remove direct blocks until we've reached
+	 * the truncation block.
 	 */
-	if (pb->truncating) {
-		/*
-		 * We only remove indirect blocks if they are
-		 * completely empty.
-		 */
-		if (blockcnt < 0) {
-			int	i, limit;
-			blk_t	*bp;
-
-			pb->errcode = io_channel_read_blk64(fs->io, blk, 1,
-							pb->buf);
-			if (pb->errcode)
-				goto return_abort;
-
-			limit = fs->blocksize >> 2;
-			for (i = 0, bp = (blk_t *) pb->buf;
-			     i < limit;	 i++, bp++)
-				if (*bp)
-					return 0;
-		}
-		/*
-		 * We don't remove direct blocks until we've reached
-		 * the truncation block.
-		 */
-		if (blockcnt >= 0 && blockcnt < pb->truncate_block)
-			return 0;
-		/*
-		 * If part of the last block needs truncating, we do
-		 * it here.
-		 */
-		if ((blockcnt == pb->truncate_block) && pb->truncate_offset) {
-			pb->errcode = io_channel_read_blk64(fs->io, blk, 1,
-							pb->buf);
-			if (pb->errcode)
-				goto return_abort;
-			memset(pb->buf + pb->truncate_offset, 0,
-			       fs->blocksize - pb->truncate_offset);
-			pb->errcode = io_channel_write_blk64(fs->io, blk, 1,
-							 pb->buf);
-			if (pb->errcode)
-				goto return_abort;
-		}
-		pb->truncated_blocks++;
-		*block_nr = 0;
-		retval |= BLOCK_CHANGED;
+	if (blockcnt >= 0 && blockcnt < pb->truncate_block)
+		return 0;
+
+	/*
+	 * We only remove indirect blocks if they are
+	 * completely empty.
+	 */
+	if (blockcnt < 0) {
+		int	i, limit;
+		blk_t	*bp;
+
+		pb->errcode = io_channel_read_blk64(fs->io, blk, 1,
+						    pb->buf);
+		if (pb->errcode)
+			return BLOCK_ABORT;
+
+		limit = fs->blocksize >> 2;
+		for (i = 0, bp = (blk_t *) pb->buf;
+		     i < limit;	 i++, bp++)
+			if (*bp)
+				return 0;
 	}
 
-	if (ctx->qctx)
-		quota_data_sub(ctx->qctx, pb->inode, 0, ctx->fs->blocksize);
 	ext2fs_block_alloc_stats2(fs, blk, -1);
-	ctx->free_blocks++;
-	return retval;
+	pb->truncated_blocks++;
+	*block_nr = 0;
+
+	return BLOCK_CHANGED;
 }
 
-/*
- * This function releases an inode.  Returns 1 if an inconsistency was
- * found.  If the inode has a link count, then it is being truncated and
- * not deleted.
- */
-static int release_inode_blocks(e2fsck_t ctx, ext2_ino_t ino,
-				struct ext2_inode_large *inode, char *block_buf,
-				struct problem_context *pctx)
+static errcode_t truncate_inode_blocks(e2fsck_t ctx, ext2_ino_t ino,
+				       struct ext2_inode_large *inode,
+				       char *block_buf,
+				       struct problem_context *pctx)
 {
-	struct process_block_struct 	pb;
-	ext2_filsys			fs = ctx->fs;
-	blk64_t				blk;
-	errcode_t			retval;
-	__u32				count;
+	ext2_filsys fs = ctx->fs;
+	struct process_block_struct pb = { 0 };
+	e2_blkcnt_t truncate_block = 0;
+	__u32 truncate_offset = 0;
+	blk64_t blk;
+	int ret_flags;
+	errcode_t retval = 0;
 
 	if (!ext2fs_inode_has_valid_blocks2(fs, EXT2_INODE(inode)))
-		goto release_acl;
+		return 0;
 
-	pb.buf = block_buf + 3 * ctx->fs->blocksize;
-	pb.ctx = ctx;
-	pb.abort = 0;
-	pb.errcode = 0;
-	pb.pctx = pctx;
-	pb.last_cluster = 0;
-	pb.inode = inode;
 	if (inode->i_links_count) {
-		pb.truncating = 1;
-		pb.truncate_block = (e2_blkcnt_t)
+		truncate_offset = inode->i_size % fs->blocksize;
+		truncate_block = (e2_blkcnt_t)
 			((EXT2_I_SIZE(inode) + fs->blocksize - 1) /
 			 fs->blocksize);
-		pb.truncate_offset = inode->i_size % fs->blocksize;
-	} else {
-		pb.truncating = 0;
-		pb.truncate_block = 0;
-		pb.truncate_offset = 0;
 	}
-	pb.truncated_blocks = 0;
+
+	pb.buf = block_buf;
+	pb.ctx = ctx;
+	pb.pctx = pctx;
+	pb.truncate_block = truncate_block;
 	retval = ext2fs_block_iterate3(fs, ino, BLOCK_FLAG_DEPTH_TRAVERSE,
 				      block_buf, release_inode_block, &pb);
 	if (retval) {
 		com_err("release_inode_blocks", retval,
 			_("while calling ext2fs_block_iterate for inode %u"),
 			ino);
-		return 1;
+		return retval;
 	}
-	if (pb.abort)
-		return 1;
+	if (pb.errcode)
+		return pb.errcode;
 
 	/* Refresh the inode since ext2fs_block_iterate may have changed it */
 	e2fsck_read_inode_full(ctx, ino, EXT2_INODE(inode), sizeof(*inode),
 			"release_inode_blocks");
 
-	if (pb.truncated_blocks)
-		ext2fs_iblk_sub_blocks(fs, EXT2_INODE(inode),
-				pb.truncated_blocks);
-release_acl:
-	blk = ext2fs_file_acl_block(fs, EXT2_INODE(inode));
-	if (blk) {
-		retval = ext2fs_adjust_ea_refcount3(fs, blk, block_buf, -1,
-				&count, ino);
-		if (retval == EXT2_ET_BAD_EA_BLOCK_NUM) {
-			retval = 0;
-			count = 1;
-		}
-		if (retval) {
-			com_err("release_inode_blocks", retval,
-		_("while calling ext2fs_adjust_ea_refcount2 for inode %u"),
-				ino);
-			return 1;
-		}
-		if (count == 0) {
-			if (ctx->qctx)
-				quota_data_sub(ctx->qctx, inode, 0,
-						ctx->fs->blocksize);
-			ext2fs_block_alloc_stats2(fs, blk, -1);
-			ctx->free_blocks++;
-		}
-		ext2fs_file_acl_block_set(fs, EXT2_INODE(inode), 0);
+	ext2fs_iblk_sub_blocks(fs, EXT2_INODE(inode), pb.truncated_blocks);
+	if (!truncate_offset)
+		return 0;
+
+	/* Is there an initialized block at the end? */
+	retval = ext2fs_bmap2(fs, ino, NULL, NULL, 0,
+			   truncate_block, &ret_flags, &blk);
+	if (retval)
+		return retval;
+	if ((blk == 0) || (ret_flags & BMAP_RET_UNINIT))
+		return 0;
+
+	retval = io_channel_read_blk64(fs->io, blk, 1, block_buf);
+	if (retval)
+		return retval;
+
+	memset(block_buf + truncate_offset, 0, fs->blocksize - truncate_offset);
+	retval = io_channel_write_blk64(fs->io, blk, 1, block_buf);
+
+	return retval;
+}
+
+/*
+ * This function releases an inode.  Returns 1 if an inconsistency was
+ * found.  If the inode has a link count, then it is being truncated and
+ * not deleted.
+ */
+static int release_inode_blocks(e2fsck_t ctx, ext2_ino_t ino,
+				struct ext2_inode_large *inode, char *block_buf,
+				struct problem_context *pctx)
+{
+	ext2_filsys			fs = ctx->fs;
+	blk64_t				free_blks, ino_blks;
+	char				*buf;
+	errcode_t			err;
+	int				rc = 0;
+
+	free_blks = ext2fs_free_blocks_count(fs->super);
+	ino_blks = ext2fs_get_stat_i_blocks(fs, EXT2_INODE(inode));
+	buf = block_buf + 3 * ctx->fs->blocksize;
+	if (truncate_inode_blocks(ctx, ino, inode, buf, pctx)) {
+		rc = 1;
+		goto update_counts;
 	}
-	return 0;
+	if (inode->i_links_count)
+		goto update_counts;
+
+	err = ext2fs_free_ext_attr(fs, ino, inode);
+	if (err) {
+		com_err(__func__, err,
+			_("while calling ext2fs_free_ext_attr for inode %u"),
+			ino);
+		rc = 1;
+		goto update_counts;
+	}
+
+	rc = 0;
+
+update_counts:
+	ctx->free_blocks += ext2fs_free_blocks_count(fs->super) - free_blks;
+	ino_blks -= ext2fs_get_stat_i_blocks(fs, EXT2_INODE(inode));
+	if (ctx->qctx)
+		quota_data_sub(ctx->qctx, inode, 0, ino_blks << 9);
+
+	return rc;
 }
 
 /* Load all quota data in preparation for orphan clearing. */
diff --git a/tests/f_orphan_truncate_extents_inode/expect.1 b/tests/f_orphan_truncate_extents_inode/expect.1
new file mode 100644
index 000000000..b24aae7ad
--- /dev/null
+++ b/tests/f_orphan_truncate_extents_inode/expect.1
@@ -0,0 +1,3 @@
+test_filesys: Truncating orphaned inode 12 (uid=0, gid=0, mode=0100644, size=4096)
+test_filesys: clean, 12/128 files, 75/1024 blocks
+Exit status is 0
diff --git a/tests/f_orphan_truncate_extents_inode/expect.2 b/tests/f_orphan_truncate_extents_inode/expect.2
new file mode 100644
index 000000000..7edff9bce
--- /dev/null
+++ b/tests/f_orphan_truncate_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/128 files (16.7% non-contiguous), 75/1024 blocks
+Exit status is 0
diff --git a/tests/f_orphan_truncate_extents_inode/image.gz b/tests/f_orphan_truncate_extents_inode/image.gz
new file mode 100644
index 0000000000000000000000000000000000000000..30681b879455b936e05d4dcbb4feaed6c4ff1eb9
GIT binary patch
literal 2854
zcmeHI{Z|ub7RHY)J?j?rU>6Hbay;x&EDLO;6|5w)E)secX|xtmD~TnV2mvC7HJUid
zQceX863bCRLDH29M-);L`3R7JUqnJ68v*MeBqBnh8Iq!8LP#>R!|wj*U*PHdF!S7d
zp7*))-g)mcB<cI_uW5H?EnSwC`z_~iz|44l$zvCxXV$_uLK&f*VL@Hf4~zB?`7d(G
z9Z{!g&3Sg&a>DO7t*O4TcjJk~J3vBKc=8_z<Sx_ZjK+W5OQMCD&zoA-ZaHK-k{JHC
zZM3Gff_sHcoAcz(H>U;-V<l(x4%b;(x0@n(<rf>BqHdjt*wL`GuHnw>6mHu1+Nyoz
zW#pC>z>l3<rY6NLyqxh`dRgPmwBTT=F8iEJ6rWe+`*)uuF5F_14mkHWWP@|oHuvqk
zFtsZg@btzm3uh`mVERrPNbQ_SpU6QtArK8AfQ6rCaPSmduP5Cr`@p3+K0lm-sT=o1
z<%}^hZj+oB9rI3o${4Y@Q+VB9n~x6jB~<Rlj7xR8Q=6t^&$)n|E!E7`0sp&trZS&*
zN$6dy6=sCTJUS6JNg!<9@XwVyQqmY<>0I@?)`7e%^~C9E?mTr@Do-uCS66sakURfa
zW69B;b{JjAXwR$EWrD^|QH!GH=*g;l(*nlPsL{fW*GAR_*hxR5OWtxT?7yS4mTGsR
z-Qz{f4kh`)haPQK(8^OkjS!YrD%XD=EexyN<QpQ@MPt2z+0Hrd@>&xA*JqnXp0Db;
z@R>W?h~4>m$^%q(oWgL9Y<7tSE54y~V@noy^{m!D0kZ7tIiJk9Exa3{G2eaS_1|D_
zuJ_aksd10YC-MpwE{|3*BGdA!W{E!kIQd@69%FRuNuW#a1==a%mmR}?CfC>H9(;l|
zG-HiC2x}U<q^`@RR(!qis=;$A|1!j5VKLN|K(&`C=|_#~gBWntYZ0CS7yjY|c7HSJ
zbLQr1hCLQfGzKfSU=xYCw-eBpnqimXdOoNaWMXFDytSd){e0FKs758Nn*1LW`bjPK
z*!pf1ygqYRCz)EndT+j1c=LU1VzVZeXRq3mCRJ`Ag=K{BLC{=-wPDLR^scY!65ji>
zT=#;t?f)P2t&e9#>EbU%9;j(3v#DewfZX;C{}M?EUF5dkN(EFu?JuVcKk`wZO}ZLJ
zt0*z8_n@lhh#%@MBle-mb4L(srYJ(h9TR`*l{Qmq{7>8chmh`TbWV)yBO@c2pZD4s
zB|o5no@S1!NF^@~X2Q@#{}($1NHCcK1+s)Hx%6efn9BYZIa7(k>8w8<mY>^qDMOJq
zqKC_DEs&}<j%_?=E(ck22a)e=1V~ydIIJik>KP<WoQOYn6ah_Mm4GHY1gxQ4@$a4c
z7hHc$#s&yrg7@fLAIvh<tLRBGjx0&uFQ#;Cf7#kjA60Q^0=DstL(d?NU)8Md*dW?n
zz(ZPXS~&1p2-{iaFpB>q+=T?2`jsnb3WGRWn2WqIg{kP<d3`7g)*VDvn6@ghP)(#r
zG`ACZRH=b(W}a5X-G`0l(AaWNVmN{nk&Bh?`yUUOL-+~azZ&{r4(m$}X`@wwNT+;8
zvAP-?x&qc!V^A<2)~t5#N=I(l95A<5q_ifx*_H!U2e7tkCI))6Pba|@Drc#~$RKJW
zM5%L1IQ1~)5HHfc&ReJ?Dg_m;^ZqaPt%T?oT<5``ZxzE<z3`z}i-TaC*S-I7A_Cz&
zd%i`+7F<vy<4u*ZjZsXQvU?S{$=i>&h$kh=6M{@uWd*Hb!{0-#$+%n?u3}zX?8jAr
zy*Q}BRooZxB0u8VoPOa$>Q{JHx>)1@4))@U7PB=G_H~_&IHe5db8tST9uL%0!t)eB
z81Jn+MtR8C*%S!1RoJ&7S53vric2^+Ynz0)de_0%YZs$w+bry)$@@|9$-51W5Kx+D
zM6(JwNX)fPrM%QJh^7|Mkyw)kl9V|5(G<^{t(JD0_VGmi(zhP--;(ce2YNa`^Bc;u
z`+mo3>m6G?M1`W#MlOTkj`ST-(;byJS+B@#_jLtUosXJl>q8euJ;ek<5-Fq7f7OP<
z&ZHPUx(%PI2joaq`u$r243dg0;u|i(-puz@f?oKcID(yyu*iuJ{Q*26{+u1}J!(K<
z7C9WM&!nkznL&rUiTqDHqk<mI!k0}ORMzeC!I}_C4Y+$w4S&NOC>nd>j&GfTBK6=8
z8fr(Rh`$Ae+)3_3_)rgsBRXQd&9?6$dXk$15Hu0EZz*x#io|_OF`x}^4O3P09#26U
zo&>RZB{OAkWApe$P?A%uB$dvXVM;S$&>ZsA4+Um!E%)c-B&%fik(~%`##j8u@G>0z
ztg$9S2Z(5J{|Ve;_|Px3^r!)G%f}cTJ2lVgW|T>eQwB#I@JEYAv~LiDAslF1acg>`
z_sBuk7EHy9#(k?1e<f!Lqe>GmWFfC@Q4ml@G%!BYgnLc449ITRBLDrzzQYIY9o*Wl
Qs3(SfSGqhPU{%0>0O93B*Z=?k

literal 0
HcmV?d00001

diff --git a/tests/f_orphan_truncate_extents_inode/name b/tests/f_orphan_truncate_extents_inode/name
new file mode 100644
index 000000000..6f16502b3
--- /dev/null
+++ b/tests/f_orphan_truncate_extents_inode/name
@@ -0,0 +1 @@
+truncating an orphaned extent-mapped inode in preen mode
diff --git a/tests/f_orphan_truncate_extents_inode/script b/tests/f_orphan_truncate_extents_inode/script
new file mode 100644
index 000000000..fb895e9a4
--- /dev/null
+++ b/tests/f_orphan_truncate_extents_inode/script
@@ -0,0 +1,3 @@
+FSCK_OPT=-p
+SECOND_FSCK_OPT="-yf -E no_optimize_extents"
+. $cmd_dir/run_e2fsck
-- 
2.43.7