Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=6d89e9dfe8…
Commit: 6d89e9dfe83f68ebb05bfb2ef4715cc5fb1aa630
Parent: 5eccbb33c28604436d38f4148fbb651151af1aef
Author: Abhi Das <adas(a)redhat.com>
AuthorDate: Thu Jan 15 12:49:48 2015 -0600
Committer: Abhi Das <adas(a)redhat.com>
CommitterDate: Thu Jan 15 13:54:33 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 <adas(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 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
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=818232fbf5…
Commit: 818232fbf5b4adb48488877abc7c022894372cd8
Parent: 9c750fe0f06a9ab1b7d36c68041d0a61d8cdcede
Author: Bob Peterson <rpeterso(a)redhat.com>
AuthorDate: Thu Jan 15 09:49:12 2015 -0600
Committer: Bob Peterson <rpeterso(a)redhat.com>
CommitterDate: Thu Jan 15 10:39:17 2015 -0600
fsck.gfs2: fix double-free bug
This patch fixes a double-free bug that can occur when errors are found
in an inode and later, it's freed. The problem is that function
free_metalist was using osi_list_del when removing buffer_heads from the
inode in memory. It should be using osi_list_del_init so that the list
head is reinitialized. Otherwise the list head is left with an unknown
value which gets interpreted later on as !list_empty(), so it tries to
free the list a second time.
This also fixes a minor issue with function rangecheck_jmeta, which
frees the buffer_head when it hits an error. The patch sets the returned
*bh value back to NULL to ensure it's not reused. This is just a
precaution I spotted while debugging, and it's worth doing.
---
gfs2/fsck/fs_recovery.c | 1 +
gfs2/fsck/metawalk.c | 2 +-
2 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c
index 3d8fe98..095d118 100644
--- a/gfs2/fsck/fs_recovery.c
+++ b/gfs2/fsck/fs_recovery.c
@@ -631,6 +631,7 @@ static int rangecheck_jmeta(struct gfs2_inode *ip, uint64_t block,
(unsigned long long)block,
(unsigned long long)block);
brelse(*bh);
+ *bh = NULL;
return meta_skip_further;
}
}
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 1875b24..16faafb 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1203,7 +1203,7 @@ static void free_metalist(struct gfs2_inode *ip, osi_list_t *mlp)
nbh = osi_list_entry(list->next,
struct gfs2_buffer_head, b_altlist);
if (nbh == ip->i_bh)
- osi_list_del(&nbh->b_altlist);
+ osi_list_del_init(&nbh->b_altlist);
else
brelse(nbh);
}
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=9aedcc0e2e…
Commit: 9aedcc0e2e1d1b801bcb0b7d33d1f6e104fd400a
Parent: 3a629eccf04f4f7ee960c3ed980352a3d022f8cd
Author: Bob Peterson <rpeterso(a)redhat.com>
AuthorDate: Fri Jan 9 07:35:46 2015 -0600
Committer: Bob Peterson <rpeterso(a)redhat.com>
CommitterDate: Fri Jan 9 13:31:45 2015 -0600
fsck.gfs2: remove duplicate designation during undo
This patch fixes a problem whereby fsck.gfs2's pass1 would perform this
sequence of events:
1. Metadata block X is identified as being referenced from dinode D1.
2. Metadata block X is identified as being referenced from another dinode, D2,
which makes it a duplicate reference, but so far, no serious errors were
found for that dinode.
3. Dinode D2 is found later to be irreparably damaged, and needs to be removed.
When D2 is deleted, the duplicate reference from D2 is removed and block X is
not freed because D1 still references it. However, it's still marked as a
duplicate and requires processing in pass1b.
Later, pass1b resolves the duplicate and determine's there is really only one
reference remaining, so it makes the correct decision. However, it should not
be necessary. The "undo" functions should remove the duplicate reference if
(and only if) the only reference was from D2. Note, though, that if the
corruption is found later in the cycle (after "undo" is possible) the duplicate
reference MUST remain and be resolved by pass1b.
---
gfs2/fsck/pass1.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index a4ba04c..73b054c 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -364,6 +364,11 @@ static int undo_reference(struct gfs2_inode *ip, uint64_t block, int meta,
"from another inode; not freeing.\n"),
(unsigned long long)block,
(unsigned long long)block);
+ if (dt->refs == 1) {
+ log_err(_("This was the only duplicate "
+ "reference so far; removing it.\n"));
+ dup_delete(dt);
+ }
return 1;
}
}
@@ -1055,8 +1060,13 @@ static int rangecheck_block(struct gfs2_inode *ip, uint64_t block,
(unsigned long long)ip->i_di.di_num.no_addr);
if ((*bad_pointers) <= BAD_POINTER_TOLERANCE)
return meta_is_good;
- else
+ else {
+ log_debug(_("Inode 0x%llx bad pointer tolerance "
+ "exceeded: block 0x%llx.\n"),
+ (unsigned long long)ip->i_di.di_num.no_addr,
+ (unsigned long long)block);
return meta_error; /* Exits check_metatree quicker */
+ }
}
return meta_is_good;
}
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=3a629eccf0…
Commit: 3a629eccf04f4f7ee960c3ed980352a3d022f8cd
Parent: 060e7a17bc411d7e44e6e433ed7e8220c088f839
Author: Bob Peterson <rpeterso(a)redhat.com>
AuthorDate: Wed Jan 7 08:18:44 2015 -0600
Committer: Bob Peterson <rpeterso(a)redhat.com>
CommitterDate: Fri Jan 9 13:27:11 2015 -0600
fsck.gfs2: Revise "undo" processing
This patch has to do with "undo" processing. This is where pass1 has
processed a bunch of data blocks before encountering an error. If the
error is "fatal" processing is stopped immediately and the work done
to that point (up to the fatal error block) is backed out. If the
error is not fatal, processing may continue for the rest of the
indirect block it's processing. The problem occurs when it finds a
non-fatal error block, then later it decides it cannot continue. In
that case, the "undo" function needs to back out changes, but it
needs to finish working through the data blocks on the rest of that
indirect block. This patch allows it to finish out the rest of the
"undo" work properly.
---
gfs2/fsck/metawalk.c | 46 ++++++++++++++++++++++++++++++++++++----------
1 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index cd224fe..1875b24 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1446,17 +1446,28 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
pass1. Therefore the individual check_data functions
should do a range check. */
rc = pass->check_data(ip, metablock, block, pass->private);
- if (!error && rc) {
- error = rc;
+ if (rc && (!error || (rc < error))) {
log_info("\n");
if (rc < 0) {
- *error_blk = block;
+ /* A fatal error trumps a non-fatal one. */
+ if ((*error_blk == 0) || (rc < error)) {
+ log_debug(_("Fatal error on block 0x"
+ "%llx preempts non-fatal "
+ "error on block 0x%llx\n"),
+ (unsigned long long)block,
+ (unsigned long long)*error_blk);
+ *error_blk = block;
+ }
log_info(_("Unrecoverable "));
+ } else { /* nonfatal error */
+ if ((*error_blk) == 0)
+ *error_blk = block;
}
log_info(_("data block error %d on block %llu "
"(0x%llx).\n"), rc,
(unsigned long long)block,
(unsigned long long)block);
+ error = rc;
}
if (rc < 0)
return rc;
@@ -1467,10 +1478,11 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
static int undo_check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
uint64_t *ptr_start, char *ptr_end,
- uint64_t error_blk)
+ uint64_t error_blk, int error)
{
int rc = 0;
uint64_t block, *ptr;
+ int found_error_blk = 0;
/* If there isn't much pointer corruption check the pointers */
for (ptr = ptr_start ; (char *)ptr < ptr_end && !fsck_abort; ptr++) {
@@ -1480,13 +1492,25 @@ static int undo_check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
if (skip_this_pass || fsck_abort)
return 1;
block = be64_to_cpu(*ptr);
- if (block == error_blk)
- return 1;
+ if (block == error_blk) {
+ if (error < 0) { /* A fatal error that stopped it? */
+ log_debug(_("Stopping the undo process: "
+ "fatal error block 0x%llx was "
+ "found.\n"),
+ (unsigned long long)error_blk);
+ return 1;
+ }
+ found_error_blk = 1;
+ log_debug(_("The non-fatal error block 0x%llx was "
+ "found, but undo processing will continue "
+ "until the end of this metadata block.\n"),
+ (unsigned long long)error_blk);
+ }
rc = pass->undo_check_data(ip, block, pass->private);
if (rc < 0)
return rc;
}
- return 0;
+ return found_error_blk;
}
static int hdr_size(struct gfs2_buffer_head *bh, int height)
@@ -1589,9 +1613,11 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
undo_metalist:
if (!error)
goto out;
- log_err( _("Error: inode %llu (0x%llx) had unrecoverable errors.\n"),
+ log_err( _("Error: inode %llu (0x%llx) had unrecoverable errors at "
+ "%lld (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_num.no_addr,
+ (unsigned long long)error_blk, (unsigned long long)error_blk);
if (!query( _("Remove the invalid inode? (y/n) "))) {
free_metalist(ip, &metalist[0]);
log_err(_("Invalid inode not deleted.\n"));
@@ -1620,7 +1646,7 @@ undo_metalist:
(uint64_t *)
(bh->b_data + head_size),
(bh->b_data + ip->i_sbd->bsize),
- error_blk);
+ error_blk, error);
if (rc > 0) {
hit_error_blk = 1;
rc = 0;
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=158ebfe22a…
Commit: 158ebfe22afff6c3c1a8357c8cbe31c8982cac34
Parent: ab8b0e0ba2fff7585a386673ff378f3cdc992063
Author: Bob Peterson <rpeterso(a)redhat.com>
AuthorDate: Wed Dec 17 10:22:25 2014 -0600
Committer: Bob Peterson <rpeterso(a)redhat.com>
CommitterDate: Fri Jan 9 13:14:30 2015 -0600
fsck.gfs2: Print out block number when pass3 finds a bad directory
This patch changes pass3 so that it prints out the directory inode
number when it finds a directory containing a bad block.
---
gfs2/fsck/pass3.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c
index 9582b5b..33865df 100644
--- a/gfs2/fsck/pass3.c
+++ b/gfs2/fsck/pass3.c
@@ -246,7 +246,10 @@ int pass3(struct gfs2_sbd *sdp)
q = block_type(di->dinode.no_addr);
if (q == gfs2_bad_block) {
log_err( _("Found unlinked directory "
- "containing bad block\n"));
+ "containing bad block at block %llu"
+ " (0x%llx)\n"),
+ (unsigned long long)di->dinode.no_addr,
+ (unsigned long long)di->dinode.no_addr);
if (query(_("Clear unlinked directory "
"with bad blocks? (y/n) "))) {
log_warn( _("inode %lld (0x%llx) is "