Gitweb: http://git.fedorahosted.org/git/?p=cluster.git;a=commitdiff;h=75fa0e0b3bdb8…
Commit: 75fa0e0b3bdb82b368cd71f5fa43b8705aa93357
Parent: 353cec33e438d32a4be1a3eeb165abad57133864
Author: Abhi Das <adas(a)redhat.com>
AuthorDate: Sun Jul 5 23:49:05 2015 -0500
Committer: Abhi Das <adas(a)redhat.com>
CommitterDate: Fri Jul 10 11:03:06 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.
This patch is already upstream and in RHEL7
Resolves: rhbz#1238754
Signed-off-by: Abhi Das <adas(a)redhat.com>
---
gfs2/fsck/metawalk.c | 86 +++---------------------------------------------
gfs2/fsck/metawalk.h | 5 ---
gfs2/fsck/pass1.c | 35 +++++++++++++++++---
gfs2/libgfs2/libgfs2.h | 1 -
4 files changed, 35 insertions(+), 92 deletions(-)
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 07e5c18..959b905 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1333,8 +1333,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;
@@ -1349,7 +1348,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
@@ -1429,15 +1428,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;
@@ -1456,9 +1452,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 */
@@ -1482,10 +1475,10 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
head_size = hdr_size(bh, height);
if (!head_size)
continue;
-
+
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);
@@ -1498,8 +1491,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;
@@ -1833,72 +1824,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;
- /* 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,
@@ -1909,7 +1834,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 8bd604d..5e30bfe 100644
--- a/gfs2/fsck/metawalk.h
+++ b/gfs2/fsck/metawalk.h
@@ -51,8 +51,6 @@ 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,
@@ -92,7 +90,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 1162066..4d31cff 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -97,7 +97,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,
@@ -1167,12 +1166,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;
@@ -1212,7 +1236,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;
@@ -1315,6 +1339,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;
}
@@ -1610,7 +1635,7 @@ int pass1(struct gfs2_sbd *sdp)
check_n_fix_bitmap(sdp, block, 0,
gfs2_block_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 25286d1..3de693f 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -131,7 +131,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 */
};
#define BUF_HASH_SHIFT (13) /* # hash buckets = 8K */
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=f9b17da3f2…
Commit: f9b17da3f2e4b278bfcb04465a7a22c45c441d04
Parent: a523af584645a01bb4900b10d0c9232d0ae50339
Author: Bob Peterson <rpeterso(a)redhat.com>
AuthorDate: Fri Jun 26 12:18:59 2015 -0500
Committer: Bob Peterson <rpeterso(a)redhat.com>
CommitterDate: Thu Jul 2 13:01:12 2015 -0500
fsck.gfs2: Change duptree structure to have generic flags
This patch does not change any functionality. It merely changes the
specialized first_ref_found flag to a flag within a multi-flag var.
rhbz#1236669
---
gfs2/fsck/fsck.h | 4 +++-
gfs2/fsck/util.c | 9 ++++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/gfs2/fsck/fsck.h b/gfs2/fsck/fsck.h
index 09db668..78e8ad4 100644
--- a/gfs2/fsck/fsck.h
+++ b/gfs2/fsck/fsck.h
@@ -57,9 +57,11 @@ struct dir_status {
uint32_t entry_count;
};
+#define DUPFLAG_REF1_FOUND 1 /* Has the original reference been found? */
+
struct duptree {
struct osi_node node;
- int first_ref_found; /* Has the original reference been found? */
+ int dup_flags;
int refs;
uint64_t block;
osi_list_t ref_inode_list; /* list of inodes referencing a dup block */
diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c
index efdd132..4022fee 100644
--- a/gfs2/fsck/util.c
+++ b/gfs2/fsck/util.c
@@ -261,7 +261,6 @@ static struct duptree *gfs2_dup_set(uint64_t dblock, int create)
dt->block = dblock;
dt->refs = 1; /* reference 1 is actually the reference we need to
discover in pass1b. */
- dt->first_ref_found = 0;
osi_list_init(&dt->ref_inode_list);
osi_list_init(&dt->ref_invinode_list);
osi_link_node(&dt->node, parent, newn);
@@ -326,7 +325,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
/* If we found the duplicate reference but we've already discovered
the first reference (in pass1b) and the other references in pass1,
we don't need to count it, so just return. */
- if (dt->first_ref_found)
+ if (dt->dup_flags & DUPFLAG_REF1_FOUND)
return meta_is_good;
/* Check for a previous reference to this duplicate */
@@ -338,7 +337,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
that case, we don't want to be confused and consider this second
reference the same as the first. If we do, we'll never be able to
resolve it. The first reference can't be the second reference. */
- if (id && first && !dt->first_ref_found) {
+ if (id && first && !(dt->dup_flags & DUPFLAG_REF1_FOUND)) {
log_info(_("Original reference to block %llu (0x%llx) was "
"previously found to be bad and deleted.\n"),
(unsigned long long)block,
@@ -347,7 +346,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
"(0x%llx) the first reference.\n"),
(unsigned long long)ip->i_di.di_num.no_addr,
(unsigned long long)ip->i_di.di_num.no_addr);
- dt->first_ref_found = 1;
+ dt->dup_flags |= DUPFLAG_REF1_FOUND;
return meta_is_good;
}
@@ -356,7 +355,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
reference, we don't want to increment the reference count because
it's already accounted for. */
if (first) {
- dt->first_ref_found = 1;
+ dt->dup_flags |= DUPFLAG_REF1_FOUND;
dups_found_first++; /* We found another first ref. */
} else {
dt->refs++;
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=b90dbab5aa…
Commit: b90dbab5aa516f356cca24739564e2c4e3c0c815
Parent: 335552bcf1345d9083690e1ef892fa856ac1e59e
Author: Bob Peterson <rpeterso(a)redhat.com>
AuthorDate: Fri Jun 26 12:18:59 2015 -0500
Committer: Bob Peterson <rpeterso(a)redhat.com>
CommitterDate: Wed Jul 1 09:47:27 2015 -0500
fsck.gfs2: Change duptree structure to have generic flags
This patch does not change any functionality. It merely changes the
specialized first_ref_found flag to a flag within a multi-flag var.
---
gfs2/fsck/fsck.h | 4 +++-
gfs2/fsck/util.c | 9 ++++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/gfs2/fsck/fsck.h b/gfs2/fsck/fsck.h
index 09db668..78e8ad4 100644
--- a/gfs2/fsck/fsck.h
+++ b/gfs2/fsck/fsck.h
@@ -57,9 +57,11 @@ struct dir_status {
uint32_t entry_count;
};
+#define DUPFLAG_REF1_FOUND 1 /* Has the original reference been found? */
+
struct duptree {
struct osi_node node;
- int first_ref_found; /* Has the original reference been found? */
+ int dup_flags;
int refs;
uint64_t block;
osi_list_t ref_inode_list; /* list of inodes referencing a dup block */
diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c
index efdd132..4022fee 100644
--- a/gfs2/fsck/util.c
+++ b/gfs2/fsck/util.c
@@ -261,7 +261,6 @@ static struct duptree *gfs2_dup_set(uint64_t dblock, int create)
dt->block = dblock;
dt->refs = 1; /* reference 1 is actually the reference we need to
discover in pass1b. */
- dt->first_ref_found = 0;
osi_list_init(&dt->ref_inode_list);
osi_list_init(&dt->ref_invinode_list);
osi_link_node(&dt->node, parent, newn);
@@ -326,7 +325,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
/* If we found the duplicate reference but we've already discovered
the first reference (in pass1b) and the other references in pass1,
we don't need to count it, so just return. */
- if (dt->first_ref_found)
+ if (dt->dup_flags & DUPFLAG_REF1_FOUND)
return meta_is_good;
/* Check for a previous reference to this duplicate */
@@ -338,7 +337,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
that case, we don't want to be confused and consider this second
reference the same as the first. If we do, we'll never be able to
resolve it. The first reference can't be the second reference. */
- if (id && first && !dt->first_ref_found) {
+ if (id && first && !(dt->dup_flags & DUPFLAG_REF1_FOUND)) {
log_info(_("Original reference to block %llu (0x%llx) was "
"previously found to be bad and deleted.\n"),
(unsigned long long)block,
@@ -347,7 +346,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
"(0x%llx) the first reference.\n"),
(unsigned long long)ip->i_di.di_num.no_addr,
(unsigned long long)ip->i_di.di_num.no_addr);
- dt->first_ref_found = 1;
+ dt->dup_flags |= DUPFLAG_REF1_FOUND;
return meta_is_good;
}
@@ -356,7 +355,7 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
reference, we don't want to increment the reference count because
it's already accounted for. */
if (first) {
- dt->first_ref_found = 1;
+ dt->dup_flags |= DUPFLAG_REF1_FOUND;
dups_found_first++; /* We found another first ref. */
} else {
dt->refs++;