From 9b97d62de89c991b7a47475818af1b7ecb440552 Mon Sep 17 00:00:00 2001 From: Andrew Price Date: Sat, 18 Apr 2015 18:43:42 +0100 Subject: [PATCH] * Sat Apr 18 2015 Andrew Price - 3.1.8-2 - fsck.gfs2: replace recent i_goal fixes with simple logic --- ...ecent_i_goal_fixes_with_simple_logic.patch | 304 ++++++++++++++++++ gfs2-utils.spec | 7 +- 2 files changed, 310 insertions(+), 1 deletion(-) create mode 100644 fsck_gfs2_replace_recent_i_goal_fixes_with_simple_logic.patch diff --git a/fsck_gfs2_replace_recent_i_goal_fixes_with_simple_logic.patch b/fsck_gfs2_replace_recent_i_goal_fixes_with_simple_logic.patch new file mode 100644 index 0000000..7411fd8 --- /dev/null +++ b/fsck_gfs2_replace_recent_i_goal_fixes_with_simple_logic.patch @@ -0,0 +1,304 @@ +commit f1028ec054f2d7f85a449b2bf0894e0435c01d6a +Author: Abhi Das +Date: Tue Apr 14 19:51:55 2015 -0500 + + fsck.gfs2: replace recent i_goal fixes with simple logic + + This patch reverses the recent set of i_goal fixes for fsck.gfs2. + This is because of two problems. + 1. It is not possible to determine if a valid block within the fs + is the correct goal block for a given inode. + 2. Conversely, given an inode, it is also not possible to accurately + determine what its goal block should be. + + The previous patches assumed that the last block of a file is its + goal block, but that is not true if the file is a directory or if + its blocks are not allocated sequentially. fsck.gfs2 would flag + these inodes incorrectly as having bad i_goal values. + + This patch takes a simple approach. It checks if the i_goal of a + given inode is out of bounds of the fs. If so, we can be certain + that it is wrong and we set it to the inode metadata block. This + is a safe starting point for gfs2 to determine where to allocate + from next. + + Resolves: rhbz#1186515 + Signed-off-by: Abhi Das + +diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c +index f05fb51..4d5a660 100644 +--- a/gfs2/fsck/metawalk.c ++++ b/gfs2/fsck/metawalk.c +@@ -1428,8 +1428,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp, + */ + static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, + struct gfs2_buffer_head *bh, int head_size, +- uint64_t *last_block, uint64_t *blks_checked, +- uint64_t *error_blk) ++ uint64_t *blks_checked, uint64_t *error_blk) + { + int error = 0, rc = 0; + uint64_t block, *ptr; +@@ -1444,7 +1443,7 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass, + + if (skip_this_pass || fsck_abort) + return error; +- *last_block = block = be64_to_cpu(*ptr); ++ block = be64_to_cpu(*ptr); + /* It's important that we don't call valid_block() and + bypass calling check_data on invalid blocks because that + would defeat the rangecheck_block related functions in +@@ -1548,15 +1547,12 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) + struct gfs2_buffer_head *bh; + uint32_t height = ip->i_di.di_height; + int i, head_size; +- uint64_t blks_checked = 0, last_block = 0; ++ uint64_t blks_checked = 0; + int error, rc; + int metadata_clean = 0; + 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; + +@@ -1575,9 +1571,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) + * comprise the directory hash table, so we perform the directory + * checks and exit. */ + if (is_dir(&ip->i_di, ip->i_sbd->gfs1)) { +- last_block = ip->i_di.di_num.no_addr; +- if (pass->check_i_goal) +- pass->check_i_goal(ip, last_block, pass->private); + if (!(ip->i_di.di_flags & GFS2_DIF_EXHASH)) + goto out; + /* check validity of leaf blocks and leaf chains */ +@@ -1604,7 +1597,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) + + if (pass->check_data) + error = check_data(ip, pass, bh, head_size, +- &last_block, &blks_checked, &error_blk); ++ &blks_checked, &error_blk); + if (pass->big_file_msg && ip->i_di.di_blocks > COMFORTABLE_BLKS) + pass->big_file_msg(ip, blks_checked); + } +@@ -1616,8 +1609,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass) + (unsigned long long)ip->i_di.di_num.no_addr); + fflush(stdout); + } +- if (!error && pass->check_i_goal) +- pass->check_i_goal(ip, last_block, pass->private); + undo_metalist: + if (!error) + goto out; +@@ -1958,80 +1949,6 @@ 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; +- /* Don't fix directory goal blocks unless we know they're wrong. +- * i.e. out of bounds of the fs. Directories can easily have blocks +- * outside of the dinode's rgrp and thus we have no way of knowing +- * if the goal block is bogus or not. */ +- if (is_dir(&ip->i_di, ip->i_sbd->gfs1) && +- (ip->i_di.di_goal_meta > sdp->sb_addr && +- ip->i_di.di_goal_meta <= sdp->fssize)) +- 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, +@@ -2042,7 +1959,6 @@ 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 779360e..fa4c850 100644 +--- a/gfs2/fsck/metawalk.h ++++ b/gfs2/fsck/metawalk.h +@@ -50,8 +50,6 @@ extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock, + const char *caller, int line); + extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk, + int error_on_dinode, int 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, +@@ -91,7 +89,6 @@ enum meta_check_rc { + * check_dentry: + * check_eattr_entry: + * check_eattr_extentry: +- * check_i_goal: + */ + struct metawalk_fxns { + void *private; +@@ -143,8 +140,6 @@ struct metawalk_fxns { + struct gfs2_ea_header *ea_hdr, + struct gfs2_ea_header *ea_hdr_prev, + void *private); +- int (*check_i_goal) (struct gfs2_inode *ip, uint64_t goal_blk, +- void *private); + int (*finish_eattr_indir) (struct gfs2_inode *ip, int leaf_pointers, + int leaf_pointer_errors, void *private); + void (*big_file_msg) (struct gfs2_inode *ip, uint64_t blks_checked); +diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c +index 69c88f4..0909873 100644 +--- a/gfs2/fsck/pass1.c ++++ b/gfs2/fsck/pass1.c +@@ -100,7 +100,6 @@ struct metawalk_fxns pass1_fxns = { + .check_dentry = NULL, + .check_eattr_entry = check_eattr_entries, + .check_eattr_extentry = check_extended_leaf_eattr, +- .check_i_goal = check_i_goal, + .finish_eattr_indir = finish_eattr_indir, + .big_file_msg = big_file_comfort, + .repair_leaf = pass1_repair_leaf, +@@ -1205,12 +1204,37 @@ bad_dinode: + return -1; + } + ++static void check_i_goal(struct gfs2_sbd *sdp, struct gfs2_inode *ip) ++{ ++ if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM) ++ return; ++ ++ if (ip->i_di.di_goal_meta <= sdp->sb_addr || ++ ip->i_di.di_goal_meta > sdp->fssize) { ++ log_err(_("Inode #%llu (0x%llx): Bad allocation goal block " ++ "found: %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); ++ if (query( _("Fix goal block in inode #%llu (0x%llx)? (y/n) "), ++ (unsigned long long)ip->i_di.di_num.no_addr, ++ (unsigned long long)ip->i_di.di_num.no_addr)) { ++ ip->i_di.di_goal_meta = ip->i_di.di_num.no_addr; ++ bmodified(ip->i_bh); ++ } else ++ log_err(_("Allocation goal block in inode #%lld " ++ "(0x%llx) not fixed\n"), ++ (unsigned long long)ip->i_di.di_num.no_addr, ++ (unsigned long long)ip->i_di.di_num.no_addr); ++ } ++} ++ + /* + * 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, +- struct rgrp_tree *rgd) ++static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh) + { + int error = 0; + uint64_t block = bh->b_blocknr; +@@ -1252,7 +1276,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; ++ check_i_goal(sdp, ip); + error = handle_ip(sdp, ip); + fsck_inode_put(&ip); + return error; +@@ -1378,6 +1402,7 @@ static int check_system_inode(struct gfs2_sbd *sdp, + "directory entries.\n"), filename); + } + } ++ check_i_goal(sdp, *sysinode); + error = handle_ip(sdp, *sysinode); + return error ? error : err; + } +@@ -1602,7 +1627,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_BLKST_FREE); +- } else if (handle_di(sdp, bh, rgd) < 0) { ++ } else if (handle_di(sdp, bh) < 0) { + stack; + brelse(bh); + gfs2_special_free(&gfs1_rindex_blks); +diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h +index f1f81d3..ccae721 100644 +--- a/gfs2/libgfs2/libgfs2.h ++++ b/gfs2/libgfs2/libgfs2.h +@@ -233,7 +233,6 @@ 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 */ + }; + + struct master_dir diff --git a/gfs2-utils.spec b/gfs2-utils.spec index 0a787cf..d951e6d 100644 --- a/gfs2-utils.spec +++ b/gfs2-utils.spec @@ -12,7 +12,7 @@ Name: gfs2-utils Version: 3.1.8 -Release: 1%{?dist} +Release: 2%{?dist} License: GPLv2+ and LGPLv2+ Group: System Environment/Kernel Summary: Utilities for managing the global file system (GFS2) @@ -31,9 +31,11 @@ BuildRequires: libblkid-devel BuildRequires: check-devel Source: https://fedorahosted.org/released/gfs2-utils/gfs2-utils-%{version}.tar.gz URL: https://fedorahosted.org/cluster/wiki/HomePage +Patch0: fsck_gfs2_replace_recent_i_goal_fixes_with_simple_logic.patch %prep %setup -q -n gfs2-utils-%{version} +%patch0 -p 1 -b .fsck_gfs2_replace_recent_i_goal_fixes_with_simple_logic %build ./autogen.sh @@ -70,6 +72,9 @@ file systems. %{_mandir}/man5/* %changelog +* Sat Apr 18 2015 Andrew Price - 3.1.8-2 +- fsck.gfs2: replace recent i_goal fixes with simple logic + * Tue Apr 07 2015 Andrew Price - 3.1.8-1 - New upstream release - Remove perl dependency