gfs2-utils: master - mkfs.gfs2: addendum to fix broken i_goal values in inodes

[Abhijith Das]

Gitweb:        http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=d07e72747f3...
Commit:        d07e72747f3f832d2f68f1a9788358fe4a4739b0
Parent:        601163a19e3cdf7f2b85afe0e2eb68fe132eb3d9
Author:        Abhi Das 
AuthorDate:    Thu Jan 15 12:49:48 2015 -0600
Committer:     Abhi Das 
CommitterDate: Thu Jan 15 12:49:48 2015 -0600

mkfs.gfs2: addendum to fix broken i_goal values in inodes

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

Resolves: rhbz#1149516
Signed-off-by: Abhi Das 
---
 gfs2/fsck/metawalk.c   |   70 ++++++++++++++++++++++++++++++++++++++++++++++++
 gfs2/fsck/metawalk.h   |    2 +
 gfs2/fsck/pass1.c      |   54 +++---------------------------------
 gfs2/libgfs2/libgfs2.h |    1 +
 tests/fsck.at          |    1 -
 5 files changed, 78 insertions(+), 50 deletions(-)

diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 217bb07..5f432d6 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1549,6 +1549,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;

@@ -1945,6 +1948,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,
@@ -1955,6 +2024,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 0d9de3f..aae9121 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 73b054c..2841b8c 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -62,7 +62,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,
@@ -100,7 +99,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,
@@ -799,48 +798,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)
@@ -1227,7 +1184,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;
@@ -1269,6 +1227,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;
@@ -1595,7 +1554,7 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin
    			 (unsigned long long)block,
    			 (unsigned long long)block);
    		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);
@@ -1678,9 +1637,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
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 5bffaed..2907e8c 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -233,6 +233,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 */
 };

/* FIXME not sure that i want to keep a record of the inodes or the
diff --git a/tests/fsck.at b/tests/fsck.at
index afe26db..e3b82bd 100644
--- a/tests/fsck.at
+++ b/tests/fsck.at
@@ -11,5 +11,4 @@ AT_CLEANUP

AT_SETUP([Fix invalid goal blocks])
 GFS_LANG_CHECK([mkfs.gfs2 -O -p lock_nolock $GFS_TGT], [set '/' { di_goal_meta: 0 }])
-GFS_LANG_CHECK([mkfs.gfs2 -O -p lock_nolock $GFS_TGT], [set '/' { di_goal_data: 0 }])
 AT_CLEANUP