[Cluster-devel] [gfs2-utils PATCH 05/11] fsck.gfs2: Read jindex before making rindex repairs

Bob Peterson rpeterso at redhat.com
Thu Feb 25 17:04:32 UTC 2016


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 at 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




More information about the Cluster-devel mailing list