cluster: RHEL6 - fsck.gfs2: addendum to fix broken i_goal values in inodes - addendum 2 of 2

Abhijith Das adas at fedoraproject.org
Thu Jan 15 21:44:23 UTC 2015


Gitweb:        http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=407dbe2a3df3882a1e936e37e7c0a8505a2d44ac
Commit:        407dbe2a3df3882a1e936e37e7c0a8505a2d44ac
Parent:        9b64bf4d044d835d5c203e7ce8fbb5a683f84867
Author:        Abhi Das <adas at redhat.com>
AuthorDate:    Thu Jan 15 15:26:31 2015 -0600
Committer:     Abhi Das <adas at redhat.com>
CommitterDate: Thu Jan 15 15:26:31 2015 -0600

fsck.gfs2: addendum to fix broken i_goal values in inodes - addendum 2 of 2

This patch moves some code around and fixes some corner cases that
the previous patches did not address.
This patch also fixes some trailing whitespace and removes a test
that is no longer valid from test/fsck.at

This patch is part 2 of 2 addendum fixes to make fsck fix i_goal
values correctly. The original two patches to fix this problem were
checked in against bz 1149516 as well. The complete fix contains
those two patches (de32500, 7933984) and this two-patch addendum
for a total of 4 patches.

Resolves: rhbz#1149516
Signed-off-by: Abhi Das <adas at redhat.com>
---
 gfs2/fsck/metawalk.c   |   70 ++++++++++++++++++++++++++++++++++++++++++++++++
 gfs2/fsck/metawalk.h   |    2 +
 gfs2/fsck/pass1.c      |   54 +++---------------------------------
 gfs2/libgfs2/libgfs2.h |    1 +
 4 files changed, 78 insertions(+), 49 deletions(-)

diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 8dc59fb..07e5c18 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1435,6 +1435,9 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
 	uint64_t error_blk = 0;
 	int hit_error_blk = 0;
 
+	if (!height && pass->check_i_goal)
+		pass->check_i_goal(ip, ip->i_di.di_num.no_addr,
+				   pass->private);
 	if (!height && !is_dir(&ip->i_di, ip->i_sbd->gfs1))
 		return 0;
 
@@ -1830,6 +1833,72 @@ static int alloc_leaf(struct gfs2_inode *ip, uint64_t block, void *private)
 	return 0;
 }
 
+/**
+ * rgrp_contains_block - Check if the rgrp provided contains the
+ * given block. Taken directly from the gfs2 kernel code
+ * @rgd: The rgrp to search within
+ * @block: The block to search for
+ *
+ * Returns: 1 if present, 0 if not.
+ */
+static inline int rgrp_contains_block(struct rgrp_tree *rgd, uint64_t block)
+{
+	uint64_t first = rgd->ri.ri_data0;
+	uint64_t last = first + rgd->ri.ri_data;
+	return first <= block && block < last;
+}
+
+/**
+ * check_i_goal
+ * @ip
+ * @goal_blk: What the goal block should be for this inode
+ *
+ * The goal block for a regular file is typically the last
+ * data block of the file. If we can't get the right value,
+ * the inode metadata block is the next best thing.
+ *
+ * Returns: 0 if corrected, 1 if not corrected
+ */
+int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
+			void *private)
+{
+	struct gfs2_sbd *sdp = ip->i_sbd;
+	uint64_t i_block = ip->i_di.di_num.no_addr;
+
+	/* Don't fix gfs1 inodes, system inodes or inodes whose goal blocks are
+	 * set to the inode blocks themselves. */
+	if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM ||
+		ip->i_di.di_goal_meta == i_block)
+		return 0;
+	/* We default to the inode block */
+	if (!goal_blk)
+		goal_blk = i_block;
+
+	if (ip->i_di.di_goal_meta != goal_blk) {
+		/* If the existing goal block is in the same rgrp as the inode,
+		 * we give the benefit of doubt and assume the value is correct */
+		if (ip->i_rgd &&
+		    rgrp_contains_block(ip->i_rgd, ip->i_di.di_goal_meta))
+			goto skip;
+		log_err( _("Error: inode %llu (0x%llx) has invalid "
+			   "allocation goal block %llu (0x%llx). Should"
+			   " be %llu (0x%llx)\n"),
+			 (unsigned long long)i_block, (unsigned long long)i_block,
+			 (unsigned long long)ip->i_di.di_goal_meta,
+			 (unsigned long long)ip->i_di.di_goal_meta,
+			 (unsigned long long)goal_blk, (unsigned long long)goal_blk);
+		if (query( _("Fix the invalid goal block? (y/n) "))) {
+			ip->i_di.di_goal_meta = ip->i_di.di_goal_data = goal_blk;
+			bmodified(ip->i_bh);
+		} else {
+			log_err(_("Invalid goal block not fixed.\n"));
+			return 1;
+		}
+	}
+skip:
+	return 0;
+}
+
 struct metawalk_fxns alloc_fxns = {
 	.private = NULL,
 	.check_leaf = alloc_leaf,
@@ -1840,6 +1909,7 @@ struct metawalk_fxns alloc_fxns = {
 	.check_dentry = NULL,
 	.check_eattr_entry = NULL,
 	.check_eattr_extentry = NULL,
+	.check_i_goal = check_i_goal,
 	.finish_eattr_indir = NULL,
 };
 
diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h
index d4ffc62..8bd604d 100644
--- a/gfs2/fsck/metawalk.h
+++ b/gfs2/fsck/metawalk.h
@@ -51,6 +51,8 @@ extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock,
 extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk,
 			      int error_on_dinode,
 			      enum gfs2_mark_block new_blockmap_state);
+extern int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
+			void *private);
 extern void reprocess_inode(struct gfs2_inode *ip, const char *desc);
 extern struct duptree *dupfind(uint64_t block);
 extern struct gfs2_inode *fsck_system_inode(struct gfs2_sbd *sdp,
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 9606101..1162066 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -60,7 +60,6 @@ static int check_extended_leaf_eattr(struct gfs2_inode *ip, uint64_t *data_ptr,
 				     struct gfs2_ea_header *ea_hdr,
 				     struct gfs2_ea_header *ea_hdr_prev,
 				     void *private);
-static int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk, void *private);
 static int finish_eattr_indir(struct gfs2_inode *ip, int leaf_pointers,
 			      int leaf_pointer_errors, void *private);
 static int invalidate_metadata(struct gfs2_inode *ip, uint64_t block,
@@ -98,7 +97,7 @@ struct metawalk_fxns pass1_fxns = {
 	.check_dentry = NULL,
 	.check_eattr_entry = check_eattr_entries,
 	.check_eattr_extentry = check_extended_leaf_eattr,
-	.check_i_goal = NULL,
+	.check_i_goal = check_i_goal,
 	.finish_eattr_indir = finish_eattr_indir,
 	.big_file_msg = big_file_comfort,
 	.repair_leaf = pass1_repair_leaf,
@@ -792,48 +791,6 @@ static int check_extended_leaf_eattr(struct gfs2_inode *ip, uint64_t *data_ptr,
 	return error;
 }
 
-/**
- * check_i_goal
- * @ip
- * @goal_blk: What the goal block should be for this inode
- *
- * The goal block for a regular file is typically the last
- * data block of the file. If we can't get the right value,
- * the inode metadata block is the next best thing.
- *
- * Returns: 0 if corrected, 1 if not corrected
- */
-static int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
-			void *private)
-{
-	struct gfs2_sbd *sdp = ip->i_sbd;
-
-	if (fsck_system_inode(sdp, ip->i_di.di_num.no_addr))
-		return 0;
-	if (!goal_blk)
-		goal_blk = ip->i_di.di_num.no_addr;
-	if (ip->i_di.di_goal_meta != goal_blk ||
-	    ip->i_di.di_goal_data != goal_blk) {
-		log_err( _("Error: inode %llu (0x%llx) has invalid "
-			   "allocation goal block %llu (0x%llx). Should"
-			   " be %llu (0x%llx)\n"),
-			 (unsigned long long)ip->i_di.di_num.no_addr,
-			 (unsigned long long)ip->i_di.di_num.no_addr,
-			 (unsigned long long)ip->i_di.di_goal_meta,
-			 (unsigned long long)ip->i_di.di_goal_meta,
-			 (unsigned long long)goal_blk,
-			 (unsigned long long)goal_blk);
-		if (query( _("Fix the invalid goal block? (y/n) "))) {
-			ip->i_di.di_goal_meta = ip->i_di.di_goal_data = goal_blk;
-			bmodified(ip->i_bh);
-		} else {
-			log_err(_("Invalid goal block not fixed.\n"));
-			return 1;
-		}
-	}
-	return 0;
-}
-
 static int check_eattr_leaf(struct gfs2_inode *ip, uint64_t block,
 			    uint64_t parent, struct gfs2_buffer_head **bh,
 			    void *private)
@@ -1214,7 +1171,8 @@ bad_dinode:
  * handle_di - This is now a wrapper function that takes a gfs2_buffer_head
  *             and calls handle_ip, which takes an in-code dinode structure.
  */
-static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh)
+static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh,
+		     struct rgrp_tree *rgd)
 {
 	int error = 0;
 	uint64_t block = bh->b_blocknr;
@@ -1254,6 +1212,7 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh)
 				 (unsigned long long)block,
 				 (unsigned long long)block);
 	}
+	ip->i_rgd = rgd;
 	error = handle_ip(sdp, ip);
 	fsck_inode_put(&ip);
 	return error;
@@ -1508,9 +1467,6 @@ int pass1(struct gfs2_sbd *sdp)
 	/* Make sure the system inodes are okay & represented in the bitmap. */
 	check_system_inodes(sdp);
 
-	/* Turn the check_i_goal function ON for the non-system inodes */
-	pass1_fxns.check_i_goal = check_i_goal;
-
 	/* So, do we do a depth first search starting at the root
 	 * inode, or use the rg bitmaps, or just read every fs block
 	 * to find the inodes?  If we use the depth first search, why
@@ -1654,7 +1610,7 @@ int pass1(struct gfs2_sbd *sdp)
 					check_n_fix_bitmap(sdp, block, 0,
 							   gfs2_block_free);
 				}
-			} else if (handle_di(sdp, bh) < 0) {
+			} else if (handle_di(sdp, bh, rgd) < 0) {
 				stack;
 				brelse(bh);
 				gfs2_special_free(&gfs1_rindex_blks);
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 9985c97..9c20f11 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -131,6 +131,7 @@ struct gfs2_inode {
 	struct gfs2_dinode i_di;
 	struct gfs2_buffer_head *i_bh;
 	struct gfs2_sbd *i_sbd;
+	struct rgrp_tree *i_rgd; /* The rgrp this inode is in */
 };
 
 #define BUF_HASH_SHIFT       (13)    /* # hash buckets = 8K */


More information about the cluster-commits mailing list