commit 6d89e9dfe83f68ebb05bfb2ef4715cc5fb1aa630 Author: Abhi Das Date: Thu Jan 15 12:49:48 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 three patches to fix this problem were checked in against a wrong bz 1149516 (which is the rhel6 version of the bug). The complete fix contains those three patches (b09dae8, d79246d, c5b09c4) and this two-patch addendum for a total of 5 patches. Resolves: rhbz#1178604 Signed-off-by: Abhi Das diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c index 82abb40..3460141 100644 --- a/gfs2/fsck/metawalk.c +++ b/gfs2/fsck/metawalk.c @@ -1497,6 +1497,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; @@ -1891,6 +1894,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, @@ -1901,6 +1970,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 b3c1299..482016a 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 60f0356..dbe60bd 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, @@ -794,48 +793,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) @@ -1217,7 +1174,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; @@ -1259,6 +1217,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; @@ -1585,7 +1544,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); @@ -1668,9 +1627,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 10df2d4..d024e56 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