All of lore.kernel.org
 help / color / mirror / Atom feed
* [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling
@ 2016-02-25 17:04 Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 01/11] libgfs2: Backport rbtree fixes from upstream Bob Peterson
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi,

These patches are probably best handled as separate items, rather than as
a set, but I didn't want to send out 11 separate emails.

This is a collection of patches I developed while debugging some rather
complex problems involving the rebuilding of corrupt or missing rindex
system files by fsck.gfs2. It also fixes some problems whereby gfs2_edit
or other utils crash and burn due to corrupt rindex files.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson <rpeterso@redhat.com> 
---
Bob Peterson (11):
  libgfs2: Backport rbtree fixes from upstream
  libgfs2: Check for obvious corruption reading rindex
  libgfs2: Change rgrp counts to be uint64_t
  gfs2_edit: Don't reference an empty rgrp tree
  fsck.gfs2: Read jindex before making rindex repairs
  fsck.gfs2: better reporting of false positive rgrp identification
  fsck.gfs2: Minor reformatting
  fsck.gfs2: Ditch variable rgcount_from_index
  fsck.gfs2: Add ability to fix rindex file size
  fsck.gfs2: Do not try to overrun the max rgrps that fit inside rindex
  fsck.gfs2: Detect multiple rgrp grow segments

 gfs2/edit/hexedit.c     |   9 +-
 gfs2/fsck/fs_recovery.c | 138 +++++++++++++++++++
 gfs2/fsck/fs_recovery.h |   1 +
 gfs2/fsck/initialize.c  | 142 +-------------------
 gfs2/fsck/rgrepair.c    | 349 ++++++++++++++++++++++++++++++++++--------------
 gfs2/include/osi_tree.h |  51 ++++---
 gfs2/libgfs2/gfs2l.c    |   2 +-
 gfs2/libgfs2/libgfs2.h  |   3 +-
 gfs2/libgfs2/rgrp.c     |  30 +++--
 gfs2/libgfs2/super.c    |   8 +-
 10 files changed, 457 insertions(+), 276 deletions(-)

-- 
2.5.0



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 01/11] libgfs2: Backport rbtree fixes from upstream
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 02/11] libgfs2: Check for obvious corruption reading rindex Bob Peterson
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch changes the osi_tree.h include to more closely match
the current rbtree implementation in e2fsprogs.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/include/osi_tree.h | 51 +++++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/gfs2/include/osi_tree.h b/gfs2/include/osi_tree.h
index f0d0768..eca04a0 100644
--- a/gfs2/include/osi_tree.h
+++ b/gfs2/include/osi_tree.h
@@ -12,7 +12,6 @@ struct osi_node {
 #define	OSI_BLACK	1
 	struct osi_node *osi_left;
 	struct osi_node *osi_right;
-	struct osi_node *osi_parent;
 };
 
 #define osi_parent(r)   ((struct osi_node *)((r)->osi_parent_color & ~3))
@@ -21,22 +20,25 @@ struct osi_node {
 #define osi_is_black(r) osi_color(r)
 #define osi_set_red(r)  do { (r)->osi_parent_color &= ~1; } while (0)
 #define osi_set_black(r)  do { (r)->osi_parent_color |= 1; } while (0)
+#define OSI_EMPTY_NODE(node) (osi_parent(node) == node)
 
 struct osi_root
 {
 	struct osi_node *osi_node;
 };
 
-static inline void osi_set_color(struct osi_node *rb, int color)
-{
-	rb->osi_parent_color = (rb->osi_parent_color & ~1) | color;
-}
+#define OSI_EMPTY_ROOT(root)  ((root)->osi_node == NULL)
 
 static inline void osi_set_parent(struct osi_node *rb, struct osi_node *p)
 {
         rb->osi_parent_color = (rb->osi_parent_color & 3) | (unsigned long)p;
 }
 
+static inline void osi_set_color(struct osi_node *rb, int color)
+{
+	rb->osi_parent_color = (rb->osi_parent_color & ~1) | color;
+}
+
 static inline void osi_link_node(struct osi_node *node,
 				 struct osi_node *parent,
 				 struct osi_node **osi_link)
@@ -245,33 +247,34 @@ static inline void osi_erase(struct osi_node *node, struct osi_root *root)
 		node = node->osi_right;
 		while ((left = node->osi_left) != NULL)
 			node = left;
+
+		if (osi_parent(old)) {
+			if (osi_parent(old)->osi_left == old)
+				osi_parent(old)->osi_left = node;
+			else
+				osi_parent(old)->osi_right = node;
+		} else
+			root->osi_node = node;
+
 		child = node->osi_right;
 		parent = osi_parent(node);
 		color = osi_color(node);
 
-		if (child)
-			osi_set_parent(child, parent);
 		if (parent == old) {
-			parent->osi_right = child;
 			parent = node;
-		} else
+		} else {
+			if (child)
+				osi_set_parent(child, parent);
 			parent->osi_left = child;
 
+			node->osi_right = old->osi_right;
+			osi_set_parent(old->osi_right, node);
+		}
+
 		node->osi_parent_color = old->osi_parent_color;
-		node->osi_right = old->osi_right;
 		node->osi_left = old->osi_left;
-
-		if (osi_parent(old)) {
-			if (osi_parent(old)->osi_left == old)
-				osi_parent(old)->osi_left = node;
-			else
-				osi_parent(old)->osi_right = node;
-		} else
-			root->osi_node = node;
-
 		osi_set_parent(old->osi_left, node);
-		if (old->osi_right)
-			osi_set_parent(old->osi_right, node);
+
 		goto color;
 	}
 
@@ -326,6 +329,9 @@ static inline struct osi_node *osi_next(struct osi_node *node)
 {
 	struct osi_node *parent;
 
+	if (OSI_EMPTY_NODE(node))
+		return NULL;
+
 	/* If we have a right-hand child, go down and then left as far
 	   as we can. */
 	if (node->osi_right) {
@@ -351,6 +357,9 @@ static inline struct osi_node *osi_prev(struct osi_node *node)
 {
 	struct osi_node *parent;
 
+	if (OSI_EMPTY_NODE(node))
+		return NULL;
+
 	/* If we have a left-hand child, go down and then right as far
 	   as we can. */
 	if (node->osi_left) {
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 02/11] libgfs2: Check for obvious corruption reading rindex
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 01/11] libgfs2: Backport rbtree fixes from upstream Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 03/11] libgfs2: Change rgrp counts to be uint64_t Bob Peterson
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch changes function rindex_read so that it can detect and
bypass obvious major corruption in the file. Trying to insert highly
corrupted entries into the rgrp tree is counterproductive and only
gets us into trouble later.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/libgfs2/rgrp.c  | 10 ++++++++--
 gfs2/libgfs2/super.c |  4 +++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
index c20aa1f..c5671f0 100644
--- a/gfs2/libgfs2/rgrp.c
+++ b/gfs2/libgfs2/rgrp.c
@@ -71,8 +71,10 @@ int gfs2_compute_bitstructs(const uint32_t bsize, struct rgrp_tree *rgd)
 
 	return 0;
 errbits:
-	if (ownbits)
+	if (ownbits) {
 		free(rgd->bits);
+		rgd->bits = NULL;
+	}
 	return -1;
 }
 
@@ -197,8 +199,10 @@ void gfs2_rgrp_relse(struct rgrp_tree *rgd)
 {
 	int x, length = rgd->ri.ri_length;
 
+	if (rgd->bits == NULL)
+		return;
 	for (x = 0; x < length; x++) {
-		if (rgd->bits[x].bi_bh) {
+		if (rgd->bits[x].bi_bh && rgd->bits[x].bi_bh->b_data) {
 			brelse(rgd->bits[x].bi_bh);
 			rgd->bits[x].bi_bh = NULL;
 		}
@@ -241,6 +245,8 @@ void gfs2_rgrp_free(struct osi_root *rgrp_tree)
 	struct osi_node *n;
 	struct gfs2_sbd *sdp = NULL;
 
+	if (OSI_EMPTY_ROOT(rgrp_tree))
+		return;
 	while ((n = osi_first(rgrp_tree))) {
 		rgd = (struct rgrp_tree *)n;
 
diff --git a/gfs2/libgfs2/super.c b/gfs2/libgfs2/super.c
index 73354ff..9b26cf4 100644
--- a/gfs2/libgfs2/super.c
+++ b/gfs2/libgfs2/super.c
@@ -169,7 +169,7 @@ int rindex_read(struct gfs2_sbd *sdp, int fd, int *count1, int *sane)
 		if (gfs2_check_range(sdp, ri.ri_addr) != 0) {
 			*sane = 0;
 			if (prev_rgd == NULL)
-				return -1;
+				continue;
 			ri.ri_addr = prev_rgd->ri.ri_addr + prev_rgd->length;
 		}
 		rgd = rgrp_insert(&sdp->rgtree, ri.ri_addr);
@@ -206,6 +206,8 @@ int rindex_read(struct gfs2_sbd *sdp, int fd, int *count1, int *sane)
 	}
 	if (prev_rgd)
 		prev_rgd->length = rgrp_size(prev_rgd);
+	if (*count1 == 0)
+		return -1;
 	return 0;
 }
 
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 03/11] libgfs2: Change rgrp counts to be uint64_t
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 01/11] libgfs2: Backport rbtree fixes from upstream Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 02/11] libgfs2: Check for obvious corruption reading rindex Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 04/11] gfs2_edit: Don't reference an empty rgrp tree Bob Peterson
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch changes our rgrp counts from int to uint64_t.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/edit/hexedit.c    | 3 ++-
 gfs2/libgfs2/gfs2l.c   | 2 +-
 gfs2/libgfs2/libgfs2.h | 3 ++-
 gfs2/libgfs2/super.c   | 4 ++--
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 5adf7c5..606bb5a 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1018,7 +1018,8 @@ static void read_superblock(int fd)
 static int read_rindex(void)
 {
 	struct gfs2_rindex *ri;
-	int count, sane;
+	uint64_t count;
+	int sane;
 
 	sbd.fssize = sbd.device.length;
 	if (sbd.md.riinode) /* If we found the rindex */
diff --git a/gfs2/libgfs2/gfs2l.c b/gfs2/libgfs2/gfs2l.c
index 5d9afd8..9c63ec4 100644
--- a/gfs2/libgfs2/gfs2l.c
+++ b/gfs2/libgfs2/gfs2l.c
@@ -101,7 +101,7 @@ static int openfs(const char *path, struct gfs2_sbd *sdp)
 	int fd;
 	int ret;
 	int sane;
-	int count;
+	uint64_t count;
 
 	fd = open(path, O_RDWR);
 	if (fd < 0) {
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index 12d57c7..7c87884 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -701,7 +701,8 @@ extern int build_quota_change(struct gfs2_inode *per_node, unsigned int j);
 /* super.c */
 extern int check_sb(struct gfs2_sb *sb);
 extern int read_sb(struct gfs2_sbd *sdp);
-extern int rindex_read(struct gfs2_sbd *sdp, int fd, int *count1, int *sane);
+extern int rindex_read(struct gfs2_sbd *sdp, int fd, uint64_t *count1,
+		       int *sane);
 extern int ri_update(struct gfs2_sbd *sdp, int fd, int *rgcount, int *sane);
 extern int write_sb(struct gfs2_sbd *sdp);
 
diff --git a/gfs2/libgfs2/super.c b/gfs2/libgfs2/super.c
index 9b26cf4..81834b3 100644
--- a/gfs2/libgfs2/super.c
+++ b/gfs2/libgfs2/super.c
@@ -137,7 +137,7 @@ int read_sb(struct gfs2_sbd *sdp)
  *
  * Returns: 0 on success, -1 on failure
  */
-int rindex_read(struct gfs2_sbd *sdp, int fd, int *count1, int *sane)
+int rindex_read(struct gfs2_sbd *sdp, int fd, uint64_t *count1, int *sane)
 {
 	unsigned int rg;
 	int error;
@@ -250,7 +250,7 @@ static int __ri_update(struct gfs2_sbd *sdp, int fd, int *rgcount, int *sane,
 {
 	struct rgrp_tree *rgd;
 	struct gfs2_rindex *ri;
-	int count1 = 0, count2 = 0;
+	uint64_t count1 = 0, count2 = 0;
 	uint64_t errblock = 0;
 	uint64_t rmax = 0;
 	struct osi_node *n, *next = NULL;
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 04/11] gfs2_edit: Don't reference an empty rgrp tree
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
                   ` (2 preceding siblings ...)
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 03/11] libgfs2: Change rgrp counts to be uint64_t Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 05/11] fsck.gfs2: Read jindex before making rindex repairs Bob Peterson
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch adds a check to function read_rindex in gfs2_edit which
makes sure the rgrp rb_tree is not empty before dereferencing it.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/edit/hexedit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 606bb5a..1fea017 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1025,8 +1025,10 @@ static int read_rindex(void)
 	if (sbd.md.riinode) /* If we found the rindex */
 		rindex_read(&sbd, 0, &count, &sane);
 
-	ri = &((struct rgrp_tree *)osi_last(&sbd.rgtree))->ri;
-	sbd.fssize = ri->ri_data0 + ri->ri_data;
+	if (!OSI_EMPTY_ROOT(&sbd.rgtree)) {
+		ri = &((struct rgrp_tree *)osi_last(&sbd.rgtree))->ri;
+		sbd.fssize = ri->ri_data0 + ri->ri_data;
+	}
 	return 0;
 }
 
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 05/11] fsck.gfs2: Read jindex before making rindex repairs
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
                   ` (3 preceding siblings ...)
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 04/11] gfs2_edit: Don't reference an empty rgrp tree Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 06/11] fsck.gfs2: better reporting of false positive rgrp identification Bob Peterson
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

In most cases, the rindex needs to be read into memory in case the
journals or jindex are corrupt and need repairs. However, in some
rare cases, the rindex needs repairs, and in the rindex repair code
it needs to read in the jindex and journals in order to filter out
rgrp records that appear in the journals. This prevents the rgrp
records inside journals from being treated as real rgrps, rather
than false-positives.

This patch also fixes a segfault in the rgrp code for cases of
extremely corrupt rindex files where the rgrp has no buffers.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/fsck/fs_recovery.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++
 gfs2/fsck/fs_recovery.h |   1 +
 gfs2/fsck/initialize.c  | 140 ++----------------------------------------------
 gfs2/fsck/rgrepair.c    |  38 ++++++++++---
 gfs2/libgfs2/rgrp.c     |  20 +++----
 5 files changed, 186 insertions(+), 151 deletions(-)

diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c
index ce11fb4..17b2a3a 100644
--- a/gfs2/fsck/fs_recovery.c
+++ b/gfs2/fsck/fs_recovery.c
@@ -812,3 +812,141 @@ int ji_update(struct gfs2_sbd *sdp)
 	}
 	return 0;
 }
+
+static void bad_journalname(const char *filename, int len)
+{
+	if (len >= 64)
+		len = 63;
+	log_debug(_("Journal index entry '%.*s' has an invalid filename.\n"),
+	          len, filename);
+}
+
+/**
+ * check_jindex_dent - check the jindex directory entries
+ *
+ * This function makes sure the directory entries of the jindex are valid.
+ * If they're not '.' or '..' they better have the form journalXXX.
+ */
+static int check_jindex_dent(struct gfs2_inode *ip, struct gfs2_dirent *dent,
+			     struct gfs2_dirent *prev_de,
+			     struct gfs2_buffer_head *bh, char *filename,
+			     uint32_t *count, int *lindex, void *priv)
+{
+	struct gfs2_dirent dentry, *de;
+	int i;
+
+	memset(&dentry, 0, sizeof(struct gfs2_dirent));
+	gfs2_dirent_in(&dentry, (char *)dent);
+	de = &dentry;
+
+	if (de->de_name_len == 1 && filename[0] == '.')
+		goto dirent_good;
+	if (de->de_name_len == 2 && filename[0] == '.' && filename[1] == '.')
+		goto dirent_good;
+
+	if ((de->de_name_len >= 11) || /* "journal9999" */
+	    (de->de_name_len <= 7) ||
+	    (strncmp(filename, "journal", 7))) {
+		bad_journalname(filename, de->de_name_len);
+		return -1;
+	}
+	for (i = 7; i < de->de_name_len; i++) {
+		if (filename[i] < '0' || filename[i] > '9') {
+			bad_journalname(filename, de->de_name_len);
+			return -2;
+		}
+	}
+
+dirent_good:
+	/* Return the number of leaf entries so metawalk doesn't flag this
+	   leaf as having none. */
+	*count = be16_to_cpu(((struct gfs2_leaf *)bh->b_data)->lf_entries);
+	return 0;
+}
+
+struct metawalk_fxns jindex_check_fxns = {
+	.private = NULL,
+	.check_dentry = check_jindex_dent,
+};
+
+/**
+ * init_jindex - read in the rindex file
+ */
+int init_jindex(struct gfs2_sbd *sdp, int allow_ji_rebuild)
+{
+	/*******************************************************************
+	 ******************  Fill in journal information  ******************
+	 *******************************************************************/
+
+	log_debug(_("Validating the journal index.\n"));
+	/* rgrepair requires the journals be read in in order to distinguish
+	   "real" rgrps from rgrps that are just copies left in journals. */
+	if (sdp->gfs1)
+		sdp->md.jiinode = lgfs2_inode_read(sdp, sbd1->sb_jindex_di.no_addr);
+	else
+		gfs2_lookupi(sdp->master_dir, "jindex", 6, &sdp->md.jiinode);
+
+	if (!sdp->md.jiinode) {
+		int err;
+
+		if (!allow_ji_rebuild) {
+			log_crit(_("Error: jindex and rindex files are both "
+				   "corrupt.\n"));
+			return -1;
+		}
+		if (!query( _("The gfs2 system jindex inode is missing. "
+			      "Okay to rebuild it? (y/n) "))) {
+			log_crit(_("Error: cannot proceed without a valid "
+				   "jindex file.\n"));
+			return -1;
+		}
+
+		err = build_jindex(sdp);
+		if (err) {
+			log_crit(_("Error %d rebuilding jindex\n"), err);
+			return err;
+		}
+		gfs2_lookupi(sdp->master_dir, "jindex", 6, &sdp->md.jiinode);
+	}
+
+	/* check for irrelevant entries in jindex. Can't use check_dir because
+	   that creates and destroys the inode, which we don't want. */
+	if (!sdp->gfs1) {
+		int error;
+
+		log_debug(_("Checking the integrity of the journal index.\n"));
+		if (sdp->md.jiinode->i_di.di_flags & GFS2_DIF_EXHASH)
+			error = check_leaf_blks(sdp->md.jiinode,
+						&jindex_check_fxns);
+		else
+			error = check_linear_dir(sdp->md.jiinode,
+						 sdp->md.jiinode->i_bh,
+						 &jindex_check_fxns);
+		if (error) {
+			log_err(_("The system journal index is damaged.\n"));
+			if (!query( _("Okay to rebuild it? (y/n) "))) {
+				log_crit(_("Error: cannot proceed without a "
+					   "valid jindex file.\n"));
+				return -1;
+			}
+			inode_put(&sdp->md.jiinode);
+			gfs2_dirent_del(sdp->master_dir, "jindex", 6);
+			log_err(_("Corrupt journal index was removed.\n"));
+			error = build_jindex(sdp);
+			if (error) {
+				log_err(_("Error rebuilding journal "
+					  "index: Cannot continue.\n"));
+				return error;
+			}
+			gfs2_lookupi(sdp->master_dir, "jindex", 6,
+				     &sdp->md.jiinode);
+		}
+	}
+
+	/* read in the ji data */
+	if (ji_update(sdp)){
+		log_err( _("Unable to read jindex inode.\n"));
+		return -1;
+	}
+	return 0;
+}
diff --git a/gfs2/fsck/fs_recovery.h b/gfs2/fsck/fs_recovery.h
index 1a7aae3..d687627 100644
--- a/gfs2/fsck/fs_recovery.h
+++ b/gfs2/fsck/fs_recovery.h
@@ -8,5 +8,6 @@ extern int replay_journals(struct gfs2_sbd *sdp, int preen, int force_check,
 extern int preen_is_safe(struct gfs2_sbd *sdp, int preen, int force_check);
 
 extern int ji_update(struct gfs2_sbd *sdp);
+extern int init_jindex(struct gfs2_sbd *sdp, int allow_ji_rebuild);
 #endif /* __FS_RECOVERY_H__ */
 
diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 4a31927..09ad660 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -1511,139 +1511,6 @@ static int init_rindex(struct gfs2_sbd *sdp)
 	return 0;
 }
 
-static void bad_journalname(const char *filename, int len)
-{
-	if (len >= 64)
-		len = 63;
-	log_debug(_("Journal index entry '%.*s' has an invalid filename.\n"),
-	          len, filename);
-}
-
-/**
- * check_jindex_dent - check the jindex directory entries
- *
- * This function makes sure the directory entries of the jindex are valid.
- * If they're not '.' or '..' they better have the form journalXXX.
- */
-static int check_jindex_dent(struct gfs2_inode *ip, struct gfs2_dirent *dent,
-			     struct gfs2_dirent *prev_de,
-			     struct gfs2_buffer_head *bh, char *filename,
-			     uint32_t *count, int *lindex, void *priv)
-{
-	struct gfs2_dirent dentry, *de;
-	int i;
-
-	memset(&dentry, 0, sizeof(struct gfs2_dirent));
-	gfs2_dirent_in(&dentry, (char *)dent);
-	de = &dentry;
-
-	if (de->de_name_len == 1 && filename[0] == '.')
-		goto dirent_good;
-	if (de->de_name_len == 2 && filename[0] == '.' && filename[1] == '.')
-		goto dirent_good;
-
-	if ((de->de_name_len >= 11) || /* "journal9999" */
-	    (de->de_name_len <= 7) ||
-	    (strncmp(filename, "journal", 7))) {
-		bad_journalname(filename, de->de_name_len);
-		return -1;
-	}
-	for (i = 7; i < de->de_name_len; i++) {
-		if (filename[i] < '0' || filename[i] > '9') {
-			bad_journalname(filename, de->de_name_len);
-			return -2;
-		}
-	}
-
-dirent_good:
-	/* Return the number of leaf entries so metawalk doesn't flag this
-	   leaf as having none. */
-	*count = be16_to_cpu(((struct gfs2_leaf *)bh->b_data)->lf_entries);
-	return 0;
-}
-
-struct metawalk_fxns jindex_check_fxns = {
-	.private = NULL,
-	.check_dentry = check_jindex_dent,
-};
-
-/**
- * init_jindex - read in the rindex file
- */
-static int init_jindex(struct gfs2_sbd *sdp)
-{
-	/*******************************************************************
-	 ******************  Fill in journal information  ******************
-	 *******************************************************************/
-
-	log_debug(_("Validating the journal index.\n"));
-	/* rgrepair requires the journals be read in in order to distinguish
-	   "real" rgrps from rgrps that are just copies left in journals. */
-	if (sdp->gfs1)
-		sdp->md.jiinode = lgfs2_inode_read(sdp, sbd1->sb_jindex_di.no_addr);
-	else
-		gfs2_lookupi(sdp->master_dir, "jindex", 6, &sdp->md.jiinode);
-
-	if (!sdp->md.jiinode) {
-		int err;
-
-		if (!query( _("The gfs2 system jindex inode is missing. "
-			      "Okay to rebuild it? (y/n) "))) {
-			log_crit(_("Error: cannot proceed without a valid "
-				   "jindex file.\n"));
-			return -1;
-		}
-
-		err = build_jindex(sdp);
-		if (err) {
-			log_crit(_("Error %d rebuilding jindex\n"), err);
-			return err;
-		}
-		gfs2_lookupi(sdp->master_dir, "jindex", 6, &sdp->md.jiinode);
-	}
-
-	/* check for irrelevant entries in jindex. Can't use check_dir because
-	   that creates and destroys the inode, which we don't want. */
-	if (!sdp->gfs1) {
-		int error;
-
-		log_debug(_("Checking the integrity of the journal index.\n"));
-		if (sdp->md.jiinode->i_di.di_flags & GFS2_DIF_EXHASH)
-			error = check_leaf_blks(sdp->md.jiinode,
-						&jindex_check_fxns);
-		else
-			error = check_linear_dir(sdp->md.jiinode,
-						 sdp->md.jiinode->i_bh,
-						 &jindex_check_fxns);
-		if (error) {
-			log_err(_("The system journal index is damaged.\n"));
-			if (!query( _("Okay to rebuild it? (y/n) "))) {
-				log_crit(_("Error: cannot proceed without a "
-					   "valid jindex file.\n"));
-				return -1;
-			}
-			inode_put(&sdp->md.jiinode);
-			gfs2_dirent_del(sdp->master_dir, "jindex", 6);
-			log_err(_("Corrupt journal index was removed.\n"));
-			error = build_jindex(sdp);
-			if (error) {
-				log_err(_("Error rebuilding journal "
-					  "index: Cannot continue.\n"));
-				return error;
-			}
-			gfs2_lookupi(sdp->master_dir, "jindex", 6,
-				     &sdp->md.jiinode);
-		}
-	}
-
-	/* read in the ji data */
-	if (ji_update(sdp)){
-		log_err( _("Unable to read jindex inode.\n"));
-		return -1;
-	}
-	return 0;
-}
-
 /**
  * initialize - initialize superblock pointer
  *
@@ -1735,7 +1602,10 @@ int initialize(struct gfs2_sbd *sdp, int force_check, int preen,
 		lookup_per_node(sdp, 0);
 
 	/* We need rindex first in case jindex is missing and needs to read
-	   in the rgrps before rebuilding it. */
+	   in the rgrps before rebuilding it. However, note that if the rindex
+	   is damaged, we need the journals to repair it. That's because the
+	   journals likely contain rgrps and bitmaps, which we need to ignore
+	   when we're trying to find the rgrps. */
 	if (init_rindex(sdp))
 		return FSCK_ERROR;
 
@@ -1745,7 +1615,7 @@ int initialize(struct gfs2_sbd *sdp, int force_check, int preen,
 	/* We need to read in jindex in order to replay the journals. If
 	   there's an error, we may proceed and let init_system_inodes
 	   try to rebuild it. */
-	if (init_jindex(sdp) == 0) {
+	if (init_jindex(sdp, 1) == 0) {
 		/* If GFS, rebuild the journals. If GFS2, replay them. We don't
 		   have the smarts to replay GFS1 journals (neither did
 		   gfs_fsck). */
diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index 12e474b..9ef97b8 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -14,6 +14,7 @@
 #include "libgfs2.h"
 #include "osi_list.h"
 #include "fsck.h"
+#include "fs_recovery.h"
 
 int rindex_modified = FALSE;
 struct special_blocks false_rgrps;
@@ -41,6 +42,10 @@ struct special_blocks false_rgrps;
  * case, we don't want to mistake these blocks that look just a real RG
  * for a real RG block.  These are "fake" RGs that need to be ignored for
  * the purposes of finding where things are.
+ *
+ * NOTE: This function assumes that the jindex and journals have been read in,
+ *       which isn't often the case. Normally the rindex needs to be read in
+ *       first. If the rindex is damaged, that's not an option.
  */
 static void find_journaled_rgs(struct gfs2_sbd *sdp)
 {
@@ -62,8 +67,7 @@ static void find_journaled_rgs(struct gfs2_sbd *sdp)
 				break;
 			bh = bread(sdp, dblock);
 			if (!gfs2_check_meta(bh, GFS2_METATYPE_RG)) {
-				log_debug( _("False rgrp found at block 0x%llx\n"),
-					  (unsigned long long)dblock);
+				/* False rgrp found at block dblock */
 				gfs2_special_set(&false_rgrps, dblock);
 			}
 			brelse(bh);
@@ -443,6 +447,21 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 	struct rgrp_tree *calc_rgd, *prev_rgd;
 	int number_of_rgs, rgi;
 	int rg_was_fnd = FALSE, corrupt_rgs = 0;
+	int error = -1, j;
+
+	/*
+	 * In order to continue, we need to initialize the jindex. We need
+	 * the journals in order to correctly eliminate false positives during
+	 * rgrp repair. IOW, we need to properly ignore rgrps that appear in
+	 * the journals, and we can only do that if we have the journals.
+	 * To make matters worse, journals may span several (small) rgrps,
+	 * so we can't go by the rgrps.
+	 */
+	if (init_jindex(sdp, 0) != 0) {
+		log_crit(_("Error: Can't read jindex required for rindex "
+			   "repairs.\n"));
+		return -1;
+	}
 
 	sdp->rgcalc.osi_node = NULL;
 	initial_first_rg_dist = first_rg_dist = sdp->sb_addr + 1;
@@ -466,7 +485,7 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 		calc_rgd = rgrp_insert(&sdp->rgcalc, blk);
 		if (!calc_rgd) {
 			log_crit( _("Can't allocate memory for rgrp repair.\n"));
-			return -1;
+			goto out;
 		}
 		calc_rgd->ri.ri_length = 1;
 		if (!rg_was_fnd) { /* if not an RG */
@@ -483,7 +502,7 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 				log_crit( _("Error: too many missing or "
 					    "damaged rgrps using this method. "
 					    "Time to try another method.\n"));
-				return -1;
+				goto out;
 			}
 		}
 		/* ------------------------------------------------ */
@@ -519,10 +538,10 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 		}
 		number_of_rgs++;
 		if (rg_was_fnd)
-			log_info( _("  rgrp %d at block 0x%llx intact"),
+			log_info( _("  rgrp %d at block 0x%llx intact\n"),
 				  number_of_rgs, (unsigned long long)blk);
 		else
-			log_warn( _("* rgrp %d at block 0x%llx *** DAMAGED ***"),
+			log_warn( _("* rgrp %d at block 0x%llx *** DAMAGED ***\n"),
 				  number_of_rgs, (unsigned long long)blk);
 		prev_rgd = calc_rgd;
 		/*
@@ -587,7 +606,12 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 			  calc_rgd->ri.ri_bitbytes);
         }
 	*num_rgs = number_of_rgs;
-	return 0;
+	error = 0;
+out:
+	for (j = 0; j < sdp->md.journals; j++)
+		inode_put(&sdp->md.journal[j]);
+	free(sdp->md.journal);
+	return error;
 }
 
 #define DIV_RU(x, y) (((x) + (y) - 1) / (y))
diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
index c5671f0..766bdfc 100644
--- a/gfs2/libgfs2/rgrp.c
+++ b/gfs2/libgfs2/rgrp.c
@@ -250,17 +250,19 @@ void gfs2_rgrp_free(struct osi_root *rgrp_tree)
 	while ((n = osi_first(rgrp_tree))) {
 		rgd = (struct rgrp_tree *)n;
 
-		if (rgd->bits && rgd->bits[0].bi_bh) { /* if a buffer exists */
-			rgs_since_sync++;
-			if (rgs_since_sync >= RG_SYNC_TOLERANCE) {
-				if (!sdp)
-					sdp = rgd->bits[0].bi_bh->sdp;
-				fsync(sdp->device_fd);
-				rgs_since_sync = 0;
+		if (rgd->bits) {
+			if (rgd->bits[0].bi_bh) { /* if a buffer exists */
+				rgs_since_sync++;
+				if (rgs_since_sync >= RG_SYNC_TOLERANCE) {
+					if (!sdp)
+						sdp = rgd->bits[0].bi_bh->sdp;
+					fsync(sdp->device_fd);
+					rgs_since_sync = 0;
+				}
+				gfs2_rgrp_relse(rgd); /* free them all. */
 			}
-			gfs2_rgrp_relse(rgd); /* free them all. */
+			free(rgd->bits);
 		}
-		free(rgd->bits);
 		osi_erase(&rgd->node, rgrp_tree);
 		free(rgd);
 	}
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 06/11] fsck.gfs2: better reporting of false positive rgrp identification
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
                   ` (4 preceding siblings ...)
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 05/11] fsck.gfs2: Read jindex before making rindex repairs Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 07/11] fsck.gfs2: Minor reformatting Bob Peterson
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch adds some more information to the debug messages related
to the identification of "false positive" rgrps. That is, rgrps that
aren't really rgrps because they reside inside a journal.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/fsck/rgrepair.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index 9ef97b8..f0759c1 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -55,12 +55,16 @@ static void find_journaled_rgs(struct gfs2_sbd *sdp)
 	uint32_t extlen;
 	struct gfs2_inode *ip;
 	struct gfs2_buffer_head *bh;
+	int false_count;
 
 	osi_list_init(&false_rgrps.list);
 	for (j = 0; j < sdp->md.journals; j++) {
-		log_debug( _("Checking for rgrps in journal%d.\n"), j);
 		ip = sdp->md.journal[j];
+		log_debug(_("Checking for rgrps in journal%d which starts "
+			    "at block 0x%llx.\n"), j,
+			  (unsigned long long)ip->i_di.di_num.no_addr);
 		jblocks = ip->i_di.di_size / sdp->sd_sb.sb_bsize;
+		false_count = 0;
 		for (b = 0; b < jblocks; b++) {
 			block_map(ip, b, &new, &dblock, &extlen, 0);
 			if (!dblock)
@@ -68,10 +72,12 @@ static void find_journaled_rgs(struct gfs2_sbd *sdp)
 			bh = bread(sdp, dblock);
 			if (!gfs2_check_meta(bh, GFS2_METATYPE_RG)) {
 				/* False rgrp found@block dblock */
+				false_count++;
 				gfs2_special_set(&false_rgrps, dblock);
 			}
 			brelse(bh);
 		}
+		log_debug("\n%d false positives identified.\n", false_count);
 	}
 }
 
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 07/11] fsck.gfs2: Minor reformatting
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
                   ` (5 preceding siblings ...)
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 06/11] fsck.gfs2: better reporting of false positive rgrp identification Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 08/11] fsck.gfs2: Ditch variable rgcount_from_index Bob Peterson
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch just changes some formatting inside the code and
eliminates a message that doesn't need to be there.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/fsck/rgrepair.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index f0759c1..5bf82db 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -171,8 +171,7 @@ static uint64_t find_shortest_rgdist(struct gfs2_sbd *sdp,
 			log_warn( _("rgrp 2 is damaged: getting dist from index: "));
 			*first_rg_dist = tmpndx.ri_addr - (sdp->sb_addr + 1);
 			log_warn("0x%llx\n", (unsigned long long)*first_rg_dist);
-		}
-		else {
+		} else {
 			log_warn( _("rgrp index 2 is damaged: extrapolating dist: "));
 			*first_rg_dist = sdp->device.length -
 				(sdp->rgrps - 1) *
@@ -500,8 +499,8 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 			/* ------------------------------------------------- */
 			corrupt_rgs++;
 			if (corrupt_rgs < 5)
-				log_debug( _("Missing or damaged rgrp at block "
-					  "%llu (0x%llx)\n"),
+				log_debug(_("Missing or damaged rgrp@block "
+					    "%llu (0x%llx)\n"),
 					  (unsigned long long)blk,
 					  (unsigned long long)blk);
 			else {
@@ -664,7 +663,8 @@ static uint64_t how_many_rgrps(struct gfs2_sbd *sdp, struct device *dev, int rgs
 		sdp->rgsize += GFS2_DEFAULT_RGSIZE; /* bigger rgs */
 	}
 
-	log_debug("  rg sz = %"PRIu32"\n  nrgrp = %"PRIu64"\n", sdp->rgsize, nrgrp);
+	log_debug("  rg sz = %"PRIu32"\n  nrgrp = %"PRIu64"\n", sdp->rgsize,
+		  nrgrp);
 
 	return nrgrp;
 }
@@ -728,10 +728,10 @@ static void compute_rgrp_layout(struct gfs2_sbd *sdp, struct osi_root *rgtree, i
 				(nrgrp - 1) * (dev->length / nrgrp);
 		}
 		rl->start = rgaddr;
-		printf("%d: start: %" PRIu64 " (0x%"
+		/* printf("%d: start: %" PRIu64 " (0x%"
 			 PRIx64 "), length = %"PRIu64" (0x%"
 			 PRIx64 ")\n", rgrp + 1, rl->start, rl->start,
-			 rl->length, rl->length);
+			 rl->length, rl->length);*/
 		rlast = rl;
 	}
 
@@ -909,8 +909,7 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 			gfs2_rgrp_free(&sdp->rgcalc);
 			return -1;
 		}
-	}
-	else if (trust_lvl == distrust) { /* If we can't trust RG index */
+	} else if (trust_lvl == distrust) { /* If we can't trust RG index */
 		/* Free previous incarnations in memory, if any. */
 		gfs2_rgrp_free(&sdp->rgtree);
 
@@ -921,8 +920,7 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 			return -1;
 		}
 		sdp->rgrps = calc_rg_count;
-	}
-	else if (trust_lvl == indignation) { /* If we can't trust anything */
+	} else if (trust_lvl == indignation) { /* If we can't trust anything */
 		/* Free previous incarnations in memory, if any. */
 		gfs2_rgrp_free(&sdp->rgtree);
 
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 08/11] fsck.gfs2: Ditch variable rgcount_from_index
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
                   ` (6 preceding siblings ...)
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 07/11] fsck.gfs2: Minor reformatting Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 09/11] fsck.gfs2: Add ability to fix rindex file size Bob Peterson
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch eliminates variable rgcount_from_index from function
rg_repair. It wasn't really being used anyway. The important count
of resource groups from the rindex is sdp->rgrps, so use that
instead. Also, eliminate setting that from the calculated
number of rgrps. We've already got calc_rg_count for that.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/fsck/rgrepair.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index 5bf82db..6e56f91 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -880,7 +880,7 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 {
 	struct osi_node *n, *next = NULL, *e, *enext;
 	int error, discrepancies, percent;
-	int calc_rg_count = 0, rgcount_from_index, rg;
+	int calc_rg_count = 0, rg;
 	struct gfs2_rindex buf;
 
 	if (trust_lvl == blind_faith)
@@ -919,7 +919,6 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 			gfs2_rgrp_free(&sdp->rgcalc);
 			return -1;
 		}
-		sdp->rgrps = calc_rg_count;
 	} else if (trust_lvl == indignation) { /* If we can't trust anything */
 		/* Free previous incarnations in memory, if any. */
 		gfs2_rgrp_free(&sdp->rgtree);
@@ -930,11 +929,10 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 			gfs2_rgrp_free(&sdp->rgcalc);
 			return -1;
 		}
-		sdp->rgrps = calc_rg_count;
 	}
 	/* Read in the rindex */
 	sdp->rgtree.osi_node = NULL; /* Just to be safe */
-	rindex_read(sdp, 0, &rgcount_from_index, sane);
+	rindex_read(sdp, 0, &sdp->rgrps, sane);
 	if (sdp->md.riinode->i_di.di_size % sizeof(struct gfs2_rindex)) {
 		log_warn( _("WARNING: rindex file is corrupt.\n"));
 		gfs2_rgrp_free(&sdp->rgcalc);
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 09/11] fsck.gfs2: Add ability to fix rindex file size
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
                   ` (7 preceding siblings ...)
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 08/11] fsck.gfs2: Ditch variable rgcount_from_index Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 10/11] fsck.gfs2: Do not try to overrun the max rgrps that fit inside rindex Bob Peterson
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Before this patch, fsck.gfs2 checked the size of rindex, and if
it wasn't evenly divisible by the size of an rindex entry, it
threw up its hands and gave up. This patch allows fsck.gfs2 to
repair the rindex size, basically rounding down to the largest
number of rgrps that will fit.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/fsck/rgrepair.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index 6e56f91..1fefc34 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -934,10 +934,18 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 	sdp->rgtree.osi_node = NULL; /* Just to be safe */
 	rindex_read(sdp, 0, &sdp->rgrps, sane);
 	if (sdp->md.riinode->i_di.di_size % sizeof(struct gfs2_rindex)) {
-		log_warn( _("WARNING: rindex file is corrupt.\n"));
-		gfs2_rgrp_free(&sdp->rgcalc);
-		gfs2_rgrp_free(&sdp->rgtree);
-		return -1;
+		log_warn( _("WARNING: rindex file has an invalid size.\n"));
+		if (!query( _("Truncate the rindex size? (y/n)"))) {
+			log_err(_("The rindex was not repaired.\n"));
+			gfs2_rgrp_free(&sdp->rgcalc);
+			gfs2_rgrp_free(&sdp->rgtree);
+			return -1;
+		}
+		sdp->md.riinode->i_di.di_size /= sizeof(struct gfs2_rindex);
+		sdp->md.riinode->i_di.di_size *= sizeof(struct gfs2_rindex);
+		bmodified(sdp->md.riinode->i_bh);
+		log_err(_("Changing rindex size to %lld.\n"),
+			(unsigned long long)sdp->md.riinode->i_di.di_size);
 	}
 	log_warn( _("L%d: number of rgs expected     = %lld.\n"), trust_lvl + 1,
 		 (unsigned long long)sdp->rgrps);
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 10/11] fsck.gfs2: Do not try to overrun the max rgrps that fit inside rindex
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
                   ` (8 preceding siblings ...)
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 09/11] fsck.gfs2: Add ability to fix rindex file size Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 11/11] fsck.gfs2: Detect multiple rgrp grow segments Bob Peterson
  2016-03-15 17:47 ` [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Andrew Price
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

When rindex is being repaired, it may calculate a new set of rgrps,
then replace the old rgrps with the new ones. The trouble is, if we've
calculated more rgrps than will fit inside rindex, it will try to
write past the end of rindex. That's unacceptable because we cannot
allow block allocations until after pass1 has finished, and rg repair
happens well before pass1 starts. The failing situation is for cases
where, for example, a file system was extended via lvextend, thus
allowing for many more rgrps to be calculated than originally fit,
but the rindex is still in a pre-gfs2_grow state where its size can
only fit a smaller number of rgrps. In that case, we want fsck.gfs2
to repair what rgrps it can, but stop then it reaches the limits of
the rindex file size.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/fsck/rgrepair.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index 1fefc34..e80a7af 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -950,13 +950,34 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 	log_warn( _("L%d: number of rgs expected     = %lld.\n"), trust_lvl + 1,
 		 (unsigned long long)sdp->rgrps);
 	if (calc_rg_count != sdp->rgrps) {
+		int most_that_fit;
+
 		log_warn( _("L%d: They don't match; either (1) the fs was "
 			    "extended, (2) an odd\n"), trust_lvl + 1);
 		log_warn( _("L%d: rgrp size was used, or (3) we have a corrupt "
 			    "rg index.\n"), trust_lvl + 1);
-		gfs2_rgrp_free(&sdp->rgcalc);
-		gfs2_rgrp_free(&sdp->rgtree);
-		return -1;
+		/* If the trust level is open_minded, we would have calculated
+		   the rindex based on the device size. If it's not the same
+		   number, don't trust it. Complain about the discrepancy,
+		   then try again with a little more distrust. */
+		if ((trust_lvl < distrust) ||
+		    !query( _("Attempt to use what rgrps we can? (y/n)"))) {
+			gfs2_rgrp_free(&sdp->rgcalc);
+			gfs2_rgrp_free(&sdp->rgtree);
+			log_err(_("The rindex was not repaired.\n"));
+			return -1;
+		}
+		/* We cannot grow rindex at this point. Since pass1 has not
+		   yet run, we can't allocate blocks. Therefore we must use
+		   whatever will fix in the space given. */
+		most_that_fit = sdp->md.riinode->i_di.di_size /
+			sizeof(struct gfs2_rindex);
+		log_debug(_("The most we can fit is %d rgrps\n"),
+			  most_that_fit);
+		if (most_that_fit < calc_rg_count)
+			calc_rg_count = most_that_fit;
+		log_err(_("Attempting to fix rindex with %d rgrps.\n"),
+			calc_rg_count);
 	}
 	/* ------------------------------------------------------------- */
 	/* Now compare the rindex to what we think it should be.         */
@@ -967,7 +988,7 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 	/* ------------------------------------------------------------- */
 	discrepancies = 0;
 	for (rg = 0, n = osi_first(&sdp->rgtree), e = osi_first(&sdp->rgcalc);
-	     n && e && !fsck_abort; rg++) {
+	     n && e && !fsck_abort && rg < calc_rg_count; rg++) {
 		struct rgrp_tree *expected, *actual;
 
 		next = osi_next(n);
@@ -1016,13 +1037,15 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 			return -1;
 		}
 	}
+	log_debug("Calculated %d rgrps: Total: %d Match: %d Mismatch: %d\n",
+		  calc_rg_count, rg, rg - discrepancies, discrepancies);
 	/* ------------------------------------------------------------- */
 	/* Now compare the rindex to what we think it should be.         */
 	/* Our rindex should be pretty predictable unless we've grown    */
 	/* so look for index problems first before looking@the rgs.   */
 	/* ------------------------------------------------------------- */
 	for (rg = 0, n = osi_first(&sdp->rgtree), e = osi_first(&sdp->rgcalc);
-	     e && !fsck_abort; rg++) {
+	     e && !fsck_abort && rg < calc_rg_count; rg++) {
 		struct rgrp_tree *expected, *actual;
 
 		if (n)
@@ -1091,8 +1114,8 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 	/* Now we can somewhat trust the rindex and the RG addresses,    */
 	/* so let's read them in, check them and optionally fix them.    */
 	/* ------------------------------------------------------------- */
-	for (rg = 0, n = osi_first(&sdp->rgtree); n && !fsck_abort;
-	     n = next, rg++) {
+	for (rg = 0, n = osi_first(&sdp->rgtree); n && !fsck_abort &&
+		     rg < calc_rg_count; n = next, rg++) {
 		struct rgrp_tree *rgd;
 		uint64_t prev_err = 0, errblock;
 		int i;
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 11/11] fsck.gfs2: Detect multiple rgrp grow segments
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
                   ` (9 preceding siblings ...)
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 10/11] fsck.gfs2: Do not try to overrun the max rgrps that fit inside rindex Bob Peterson
@ 2016-02-25 17:04 ` Bob Peterson
  2016-03-15 17:47 ` [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Andrew Price
  11 siblings, 0 replies; 15+ messages in thread
From: Bob Peterson @ 2016-02-25 17:04 UTC (permalink / raw)
  To: cluster-devel.redhat.com

This patch gives fsck.gfs2's rgrepair code the capability to detect
multiple gfs2_grow segments and repair them accordingly.

Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
 gfs2/fsck/initialize.c |   2 +-
 gfs2/fsck/rgrepair.c   | 226 ++++++++++++++++++++++++++++++++++---------------
 2 files changed, 160 insertions(+), 68 deletions(-)

diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 09ad660..84d39cd 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -640,7 +640,7 @@ static int fetch_rgrps(struct gfs2_sbd *sdp)
 			log_notice(_("(level %d passed)\n"), trust_lvl + 1);
 			break;
 		} else {
-			if (ret < 0)
+			if (ret == -1)
 				log_err( _("(level %d failed: %s)\n"),
 					 trust_lvl + 1, fail_desc[trust_lvl]);
 			else
diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index e80a7af..55057ce 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -21,6 +21,7 @@ struct special_blocks false_rgrps;
 
 #define BAD_RG_PERCENT_TOLERANCE 11
 #define AWAY_FROM_BITMAPS 0x1000
+#define MAX_RGSEGMENTS 20
 
 #define ri_equal(ondisk, expected, field) (ondisk.field == expected.field)
 
@@ -95,72 +96,146 @@ static int is_false_rg(uint64_t block)
  * look like twice the distance.  If we can find 6 of them, that
  * should be enough to figure out the correct layout.
  * This also figures out first_rg_dist since that's always different.
+ *
+ * This function was revised to return the number of segments, usually 2.
+ * The shortest distance is now returned in the highest entry in rg_dist
  */
-static uint64_t find_shortest_rgdist(struct gfs2_sbd *sdp,
-				     uint64_t *initial_first_rg_dist,
-				     uint64_t *first_rg_dist)
+static int find_shortest_rgdist(struct gfs2_sbd *sdp, uint64_t *dist_array,
+				int *dist_cnt)
 {
-	uint64_t blk, block_of_last_rg, shortest_dist_btwn_rgs;
+	uint64_t blk, block_last_rg, shortest_dist_btwn_rgs;
 	struct gfs2_buffer_head *bh;
-	int number_of_rgs = 0;
+	int rgs_sampled = 0;
 	struct gfs2_rindex buf, tmpndx;
+	uint64_t initial_first_rg_dist;
+	int gsegment = 0;
+	int is_rgrp;
 
 	/* Figure out if there are any RG-looking blocks in the journal we
 	   need to ignore. */
 	find_journaled_rgs(sdp);
 
-	*initial_first_rg_dist = *first_rg_dist = sdp->sb_addr + 1;
-	block_of_last_rg = sdp->sb_addr + 1;
+	initial_first_rg_dist = dist_array[0] = block_last_rg =
+		sdp->sb_addr + 1;
 	shortest_dist_btwn_rgs = sdp->device.length;
 
-	for (blk = sdp->sb_addr + 1;
-	     blk < sdp->device.length && number_of_rgs < 6; blk++) {
-		bh = bread(sdp, blk);
-		if (((blk == sdp->sb_addr + 1) ||
-		    (!gfs2_check_meta(bh, GFS2_METATYPE_RG))) &&
-		    !is_false_rg(blk)) {
-			log_debug( _("rgrp found at block 0x%llx\n"),
-				(unsigned long long)blk);
-			if (blk > sdp->sb_addr + 1) {
-				uint64_t rgdist;
-				
-				rgdist = blk - block_of_last_rg;
-				log_debug("dist 0x%llx = 0x%llx - 0x%llx",
-					  (unsigned long long)rgdist,
-					  (unsigned long long)blk,
-					  (unsigned long long)block_of_last_rg);
-				/* ----------------------------------------- */
-				/* We found an RG.  Check to see if we need  */
-				/* to set the first_rg_dist based on whether */
-				/* it's still at its initial value (i.e. the */
-				/* fs.)  The first rg distance is different  */
-				/* from the rest because of the superblock   */
-				/* and 64K dead space.                       */
-				/* ----------------------------------------- */
-				if (*first_rg_dist == *initial_first_rg_dist)
-					*first_rg_dist = rgdist;
-				if (rgdist < shortest_dist_btwn_rgs) {
-					shortest_dist_btwn_rgs = rgdist;
-					log_debug( _("(shortest so far)\n"));
+	for (blk = sdp->sb_addr + 1; blk < sdp->device.length; blk++) {
+		uint64_t dist;
+
+		if (blk == sdp->sb_addr + 1)
+			is_rgrp = 1;
+		else if (is_false_rg(blk))
+			is_rgrp = 0;
+		else {
+			bh = bread(sdp, blk);
+			is_rgrp = (gfs2_check_meta(bh, GFS2_METATYPE_RG) == 0);
+			brelse(bh);
+		}
+		if (!is_rgrp) {
+			if (rgs_sampled >= 6) {
+				uint64_t nblk;
+
+				log_info(_("rgrp not found at block 0x%llx. "
+					   "Last found rgrp was 0x%llx. "
+					   "Checking the next one.\n"),
+					 (unsigned long long)blk,
+					 (unsigned long long)block_last_rg);
+				/* check for just a damaged rgrp */
+				nblk = blk + dist_array[gsegment];
+				if (is_false_rg(nblk)) {
+					is_rgrp = 0;
+				} else {
+					bh = bread(sdp, nblk);
+					is_rgrp = (((gfs2_check_meta(bh,
+						GFS2_METATYPE_RG) == 0)));
+					brelse(bh);
+				}
+				if (is_rgrp) {
+					log_info(_("Next rgrp is intact, so "
+						   "this one is damaged.\n"));
+					blk = nblk - 1;
+					dist_cnt[gsegment]++;
+					continue;
 				}
-				else
-					log_debug("\n");
+				log_info(_("Looking for new segment.\n"));
+				blk -= 16;
+				rgs_sampled = 0;
+				shortest_dist_btwn_rgs = sdp->device.length;
+				/* That last one didn't pan out, so: */
+				dist_cnt[gsegment]--;
+				gsegment++;
 			}
-			block_of_last_rg = blk;
-			number_of_rgs++;
-			blk += 250; /* skip ahead for performance */
+			if ((blk - block_last_rg) > (524288 * 2)) {
+				log_info(_("No rgrps were found within 4GB "
+					   "of the last rgrp. Must be the "
+					   "end of the file system.\n"));
+
+				break;
+			}
+			continue;
 		}
-		brelse(bh);
+
+		dist_cnt[gsegment]++;
+		if (rgs_sampled >= 6) {
+			block_last_rg = blk;
+			blk += dist_array[gsegment] - 1; /* prev value in
+							    array minus 1. */
+			continue;
+		}
+		log_info(_("segment %d: rgrp found at block 0x%llx\n"),
+			 gsegment + 1, (unsigned long long)blk);
+		dist = blk - block_last_rg;
+		if (blk > sdp->sb_addr + 1) { /* not the very first rgrp */
+
+			log_info("dist 0x%llx = 0x%llx - 0x%llx ",
+				 (unsigned long long)dist,
+				 (unsigned long long)blk,
+				 (unsigned long long)block_last_rg);
+			/**
+			 * We found an RG.  Check to see if we need to set the
+			 * first_rg_dist based on whether it is still at its
+			 * initial value (i.e. the fs.)  The first rg distance
+			 * is different from the rest because of the
+			 * superblock and 64K dead space.
+			 **/
+			if (dist_array[0] == initial_first_rg_dist) {
+				dist_array[0] = dist;
+				dist_cnt[0] = 1;
+				rgs_sampled = 0;
+			}
+			if (dist < shortest_dist_btwn_rgs) {
+				shortest_dist_btwn_rgs = dist;
+				log_info( _("(shortest so far)"));
+			}
+			log_info("\n");
+			if (++rgs_sampled == 6) {
+				dist_array[gsegment] = shortest_dist_btwn_rgs;
+				log_info(_("Settled on distance 0x%llx for "
+					   "segment %d\n"),
+					 (unsigned long long)
+					 dist_array[gsegment], gsegment + 1);
+			}
+		} else {
+			gsegment++;
+		}
+		block_last_rg = blk;
+		if (rgs_sampled < 6)
+			blk += 250; /* skip ahead for performance */
+		else
+			blk += shortest_dist_btwn_rgs - 1;
+	}
+	if (gsegment > MAX_RGSEGMENTS) {
+		log_err(_("Maximum number of rgrp grow segments reached.\n"));
+		log_err(_("This file system cannot be repaired with fsck.\n"));
+		gsegment = 0;
+		goto out;
 	}
 	/* -------------------------------------------------------------- */
 	/* Sanity-check our first_rg_dist. If RG #2 got nuked, the        */
 	/* first_rg_dist would measure from #1 to #3, which would be bad. */
 	/* We need to take remedial measures to fix it (from the index).  */
 	/* -------------------------------------------------------------- */
-	log_debug( _("First rgrp distance: 0x%llx\n"), (unsigned long long)*first_rg_dist);
-	log_debug( _("Distance between rgrps: 0x%llx\n"),
-		  (unsigned long long)shortest_dist_btwn_rgs);
-	if (*first_rg_dist >= shortest_dist_btwn_rgs +
+	if (*dist_array >= shortest_dist_btwn_rgs +
 	    (shortest_dist_btwn_rgs / 4)) {
 		/* read in the second RG index entry for this subd. */
 		gfs2_readi(sdp->md.riinode, (char *)&buf,
@@ -169,21 +244,21 @@ static uint64_t find_shortest_rgdist(struct gfs2_sbd *sdp,
 		gfs2_rindex_in(&tmpndx, (char *)&buf);
 		if (tmpndx.ri_addr > sdp->sb_addr + 1) { /* sanity check */
 			log_warn( _("rgrp 2 is damaged: getting dist from index: "));
-			*first_rg_dist = tmpndx.ri_addr - (sdp->sb_addr + 1);
-			log_warn("0x%llx\n", (unsigned long long)*first_rg_dist);
+			*dist_array = tmpndx.ri_addr - (sdp->sb_addr + 1);
+			log_warn("0x%llx\n", (unsigned long long)*dist_array);
 		} else {
 			log_warn( _("rgrp index 2 is damaged: extrapolating dist: "));
-			*first_rg_dist = sdp->device.length -
-				(sdp->rgrps - 1) *
+			*dist_array = sdp->device.length - (sdp->rgrps - 1) *
 				(sdp->device.length / sdp->rgrps);
-			log_warn("0x%llx\n", (unsigned long long)*first_rg_dist);
+			log_warn("0x%llx\n", (unsigned long long)*dist_array);
 		}
 		log_debug( _("Adjusted first rgrp distance: 0x%llx\n"),
-			   (unsigned long long)*first_rg_dist);
+			   (unsigned long long)*dist_array);
 	} /* if first RG distance is within tolerance */
 
+out:
 	gfs2_special_free(&false_rgrps);
-	return shortest_dist_btwn_rgs;
+	return gsegment;
 }
 
 /*
@@ -445,14 +520,15 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 {
 	struct osi_node *n, *next = NULL;
 	struct gfs2_buffer_head *bh;
-	uint64_t shortest_dist_btwn_rgs;
+	uint64_t rg_dist[MAX_RGSEGMENTS] = {0, };
+	int rg_dcnt[MAX_RGSEGMENTS] = {0, };
 	uint64_t blk;
 	uint64_t fwd_block, block_bump;
-	uint64_t first_rg_dist, initial_first_rg_dist;
 	struct rgrp_tree *calc_rgd, *prev_rgd;
-	int number_of_rgs, rgi;
+	int number_of_rgs, rgi, segment_rgs;
 	int rg_was_fnd = FALSE, corrupt_rgs = 0;
-	int error = -1, j;
+	int error = -1, j, i;
+	int grow_segments, segment = 0;
 
 	/*
 	 * In order to continue, we need to initialize the jindex. We need
@@ -469,17 +545,17 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 	}
 
 	sdp->rgcalc.osi_node = NULL;
-	initial_first_rg_dist = first_rg_dist = sdp->sb_addr + 1;
-	shortest_dist_btwn_rgs = find_shortest_rgdist(sdp,
-						      &initial_first_rg_dist,
-						      &first_rg_dist);
-	number_of_rgs = 0;
+	grow_segments = find_shortest_rgdist(sdp, &rg_dist[0], &rg_dcnt[0]);
+	for (i = 0; i < grow_segments; i++)
+		log_info(_("Segment %d: rgrp distance: 0x%llx, count: %d\n"),
+			  i + 1, (unsigned long long)rg_dist[i], rg_dcnt[i]);
+	number_of_rgs = segment_rgs = 0;
 	/* -------------------------------------------------------------- */
 	/* Now go through the RGs and verify their integrity, fixing as   */
 	/* needed when corruption is encountered.                         */
 	/* -------------------------------------------------------------- */
 	prev_rgd = NULL;
-	block_bump = first_rg_dist;
+	block_bump = rg_dist[0];
 	blk = sdp->sb_addr + 1;
 	while (blk <= sdp->device.length) {
 		log_debug( _("Block 0x%llx\n"), (unsigned long long)blk);
@@ -542,6 +618,7 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 				  (unsigned long)prev_rgd->ri.ri_data);
 		}
 		number_of_rgs++;
+		segment_rgs++;
 		if (rg_was_fnd)
 			log_info( _("  rgrp %d at block 0x%llx intact\n"),
 				  number_of_rgs, (unsigned long long)blk);
@@ -552,10 +629,16 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 		/*
 		 * Figure out where our next rgrp should be.
 		 */
-		if (blk == sdp->sb_addr + 1)
-			block_bump = first_rg_dist;
-		else if (!gfs_grow) {
-			block_bump = shortest_dist_btwn_rgs;
+		if ((blk == sdp->sb_addr + 1) || (!gfs_grow)) {
+			block_bump = rg_dist[segment];
+			if (segment_rgs >= rg_dcnt[segment]) {
+				log_debug(_("End of segment %d\n"), ++segment);
+				segment_rgs = 0;
+				if (segment >= grow_segments) {
+					log_debug(_("Last segment.\n"));
+					break;
+				}
+			}
 			/* if we have uniformly-spaced rgrps, there may be
 			   some wasted space at the end of the device.
 			   Since we don't want to create a short rgrp and
@@ -1142,5 +1225,14 @@ int rg_repair(struct gfs2_sbd *sdp, int trust_lvl, int *rg_count, int *sane)
 	*rg_count = rg;
 	gfs2_rgrp_free(&sdp->rgcalc);
 	gfs2_rgrp_free(&sdp->rgtree);
+	/* We shouldn't need to worry about getting the user's permission to
+	   make changes here. If b_modified is true, they already gave their
+	   permission. */
+	if (sdp->md.riinode->i_bh->b_modified) {
+		log_debug("Syncing rindex inode changes to disk.\n");
+		gfs2_dinode_out(&sdp->md.riinode->i_di,
+				sdp->md.riinode->i_bh);
+		bwrite(sdp->md.riinode->i_bh);
+	}
 	return 0;
 }
-- 
2.5.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling
  2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
                   ` (10 preceding siblings ...)
  2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 11/11] fsck.gfs2: Detect multiple rgrp grow segments Bob Peterson
@ 2016-03-15 17:47 ` Andrew Price
  2016-03-21 14:53   ` Bob Peterson
  11 siblings, 1 reply; 15+ messages in thread
From: Andrew Price @ 2016-03-15 17:47 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi Bob,

On 25/02/16 17:04, Bob Peterson wrote:
> Hi,
>
> These patches are probably best handled as separate items, rather than as
> a set, but I didn't want to send out 11 separate emails.
>
> This is a collection of patches I developed while debugging some rather
> complex problems involving the rebuilding of corrupt or missing rindex
> system files by fsck.gfs2. It also fixes some problems whereby gfs2_edit
> or other utils crash and burn due to corrupt rindex files.
>
> Regards,
>
> Bob Peterson
> Red Hat File Systems
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> Bob Peterson (11):
>    libgfs2: Backport rbtree fixes from upstream
>    libgfs2: Check for obvious corruption reading rindex
>    libgfs2: Change rgrp counts to be uint64_t
>    gfs2_edit: Don't reference an empty rgrp tree
>    fsck.gfs2: Read jindex before making rindex repairs
>    fsck.gfs2: better reporting of false positive rgrp identification
>    fsck.gfs2: Minor reformatting
>    fsck.gfs2: Ditch variable rgcount_from_index
>    fsck.gfs2: Add ability to fix rindex file size
>    fsck.gfs2: Do not try to overrun the max rgrps that fit inside rindex
>    fsck.gfs2: Detect multiple rgrp grow segments

Consider these ACKed. I've taken a look through them and they look fine, 
though I'm not hugely familiar with that part of the fsck code. Coverity 
is happy with them too.

Outside of the scope of this set, I noticed in patch 11 that parts of 
the existing fsck.gfs2 code assume that the first rgrp lies in the first 
block after the superblock. That's likely to get confused when the file 
system is RAID stripe aligned so that's something that may need 
addressing at some point (mkfs.gfs2 -b 4096 -o swidth=16k,sunit=8k 
should mock it up for a test).

In general it would be good to have more fsck.gfs2 tests in the 
testsuite so if there's a simple way to test these patches without 
mounting (scribbling over some rindex entries?) then I'll get that added.

Cheers,
Andy

>
>   gfs2/edit/hexedit.c     |   9 +-
>   gfs2/fsck/fs_recovery.c | 138 +++++++++++++++++++
>   gfs2/fsck/fs_recovery.h |   1 +
>   gfs2/fsck/initialize.c  | 142 +-------------------
>   gfs2/fsck/rgrepair.c    | 349 ++++++++++++++++++++++++++++++++++--------------
>   gfs2/include/osi_tree.h |  51 ++++---
>   gfs2/libgfs2/gfs2l.c    |   2 +-
>   gfs2/libgfs2/libgfs2.h  |   3 +-
>   gfs2/libgfs2/rgrp.c     |  30 +++--
>   gfs2/libgfs2/super.c    |   8 +-
>   10 files changed, 457 insertions(+), 276 deletions(-)
>



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling
  2016-03-15 17:47 ` [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Andrew Price
@ 2016-03-21 14:53   ` Bob Peterson
  2016-03-21 16:44     ` Andrew Price
  0 siblings, 1 reply; 15+ messages in thread
From: Bob Peterson @ 2016-03-21 14:53 UTC (permalink / raw)
  To: cluster-devel.redhat.com

Hi Andy,

----- Original Message -----
> Hi Bob,
> 
(snip)
> Consider these ACKed. I've taken a look through them and they look fine,
> though I'm not hugely familiar with that part of the fsck code. Coverity
> is happy with them too.

Does that mean I can/should push them? I thought you said you had some
suggestions for them.
 
> Outside of the scope of this set, I noticed in patch 11 that parts of
> the existing fsck.gfs2 code assume that the first rgrp lies in the first
> block after the superblock. That's likely to get confused when the file
> system is RAID stripe aligned so that's something that may need
> addressing at some point (mkfs.gfs2 -b 4096 -o swidth=16k,sunit=8k
> should mock it up for a test).

Good to know. This could be a problem for even today's rindex repair code.
We may need to do some testing to figure out what's broken and what's not.
 
> In general it would be good to have more fsck.gfs2 tests in the
> testsuite so if there's a simple way to test these patches without
> mounting (scribbling over some rindex entries?) then I'll get that added.
> Cheers,
> Andy

This is an excellent idea. A very long time ago, I used to have a different
fsck "nightmare" test that used a little "nukerg2.c" program (source attached)
to nuke an rgrp or an rindex entry. Then I'd force fsck to fix it.
I may have originally written it for gfs1 rather than gfs2, and I know
I was especially targeting post-gfs2_grow rindexes.

I can no longer find the original script, but the original sequence went
something like:

mkfs.gfs2 <lv>
./nukerg2 -rg X <lv>
fsck.gfs2 <lv>
# make sure $? is 1
fsck.gfs2 <lv>
# make sure $? is 0
mount <lv> <mntpt>
lvextend <lv>
gfs2_grow <mntpt>
./nukerg2 -rg X <lv>
fsck.gfs2 <lv>
# make sure $? is 1
fsck.gfs2 <lv>
# make sure $? is 0

For X, I'd systematically do:
1. First rgrp
2. Second rgrp
3. Third rgrp
4. Half-way point
5. Last rgrp
6. I'd also nuke two and three rgrps in a row IIRC.

There's also an "-rgindex" parameter to kill the rindex entries themselves,
so I'd repeat each of the above steps, but killing the rindex entry rather
than the rgrp itself.

This test really ought to be resurrected and added to the test suite.

Regards,

Bob Peterson
Red Hat File Systems
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nukerg2.c
Type: text/x-c++src
Size: 6058 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/cluster-devel/attachments/20160321/04df123c/attachment.bin>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling
  2016-03-21 14:53   ` Bob Peterson
@ 2016-03-21 16:44     ` Andrew Price
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Price @ 2016-03-21 16:44 UTC (permalink / raw)
  To: cluster-devel.redhat.com

On 21/03/16 14:53, Bob Peterson wrote:
> Hi Andy,
>
> ----- Original Message -----
>> Hi Bob,
>>
> (snip)
>> Consider these ACKed. I've taken a look through them and they look fine,
>> though I'm not hugely familiar with that part of the fsck code. Coverity
>> is happy with them too.
>
> Does that mean I can/should push them?

If you're happy with them, go right ahead. We can add tests and any 
tweaks that might be needed on top.

> I thought you said you had some
> suggestions for them.

Well I thought there might be an issue with using an int for the counter 
in patch 10 but that seems like it should be sufficient on closer 
inspection. There was also one piece of code that was commented out in 
patch 7 instead of removed which seemed odd but ultimately not a bug.

>> Outside of the scope of this set, I noticed in patch 11 that parts of
>> the existing fsck.gfs2 code assume that the first rgrp lies in the first
>> block after the superblock. That's likely to get confused when the file
>> system is RAID stripe aligned so that's something that may need
>> addressing at some point (mkfs.gfs2 -b 4096 -o swidth=16k,sunit=8k
>> should mock it up for a test).
>
> Good to know. This could be a problem for even today's rindex repair code.
> We may need to do some testing to figure out what's broken and what's not.

As I understand it, the rg repair code is just using the layout 
assumptions for quickly discovering where the rgrps are in order to 
rebuild the rindex, so in most cases it shouldn't be a problem (in 
theory), but we'll need to add heuristics based on the new layout 
strategy if it gets confused. Which is where the testing comes in...

>> In general it would be good to have more fsck.gfs2 tests in the
>> testsuite so if there's a simple way to test these patches without
>> mounting (scribbling over some rindex entries?) then I'll get that added.
>> Cheers,
>> Andy
>
> This is an excellent idea. A very long time ago, I used to have a different
> fsck "nightmare" test that used a little "nukerg2.c" program (source attached)

Thanks, it could live in the tests directory to make use of in 
tests/fsck.at and perhaps gfs2l can be extended to obsolete it later.

> to nuke an rgrp or an rindex entry. Then I'd force fsck to fix it.
> I may have originally written it for gfs1 rather than gfs2, and I know
> I was especially targeting post-gfs2_grow rindexes.
>
> I can no longer find the original script, but the original sequence went
> something like:
>
> mkfs.gfs2 <lv>
> ./nukerg2 -rg X <lv>
> fsck.gfs2 <lv>
> # make sure $? is 1
> fsck.gfs2 <lv>
> # make sure $? is 0
> mount <lv> <mntpt>
> lvextend <lv>
> gfs2_grow <mntpt>
> ./nukerg2 -rg X <lv>
> fsck.gfs2 <lv>
> # make sure $? is 1
> fsck.gfs2 <lv>
> # make sure $? is 0
>
> For X, I'd systematically do:
> 1. First rgrp
> 2. Second rgrp
> 3. Third rgrp
> 4. Half-way point
> 5. Last rgrp
> 6. I'd also nuke two and three rgrps in a row IIRC.
>
> There's also an "-rgindex" parameter to kill the rindex entries themselves,
> so I'd repeat each of the above steps, but killing the rindex entry rather
> than the rgrp itself.
>
> This test really ought to be resurrected and added to the test suite.

Ok, sounds like a plan. We can do everything up to the mount in the 
gfs2-utils testsuite. Perhaps we can expand on it with some 
gfs2-specific xfstests tests for the mount+grow cases in future.

Cheers,
Andy



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-03-21 16:44 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-25 17:04 [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 01/11] libgfs2: Backport rbtree fixes from upstream Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 02/11] libgfs2: Check for obvious corruption reading rindex Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 03/11] libgfs2: Change rgrp counts to be uint64_t Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 04/11] gfs2_edit: Don't reference an empty rgrp tree Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 05/11] fsck.gfs2: Read jindex before making rindex repairs Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 06/11] fsck.gfs2: better reporting of false positive rgrp identification Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 07/11] fsck.gfs2: Minor reformatting Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 08/11] fsck.gfs2: Ditch variable rgcount_from_index Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 09/11] fsck.gfs2: Add ability to fix rindex file size Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 10/11] fsck.gfs2: Do not try to overrun the max rgrps that fit inside rindex Bob Peterson
2016-02-25 17:04 ` [Cluster-devel] [gfs2-utils PATCH 11/11] fsck.gfs2: Detect multiple rgrp grow segments Bob Peterson
2016-03-15 17:47 ` [Cluster-devel] [gfs2-utils PATCH 00/11] Misc patches regarding corrupt rindex handling Andrew Price
2016-03-21 14:53   ` Bob Peterson
2016-03-21 16:44     ` Andrew Price

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.