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 <a...@redhat.com>
---
 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
-- 
1.8.1.4

Reply via email to