[Cluster-devel] [PATCH 60/66] fsck.gfs2: Journals not properly checked

[rpeterso@redhat.com <rpeterso@redhat.com>]

From: Bob Peterson <rpeterso@redhat.com>

The coverity tool found a problem with the previous set of patches that
led me to discover a problem with how journals were being processed
under the new code.  The problem was that the journal index was read
in after journal replay.  But journal replay relies upon information
in the jindex.  This patch rearranges the initializeation sequence
of fsck.gfs2 so that the journal index and rindex are read in and
processed prior to journal recovery.

rhbz#675723
---
 gfs2/fsck/fs_recovery.c |   12 ---
 gfs2/fsck/initialize.c  |  183 +++++++++++++++++++++++++++++++----------------
 2 files changed, 122 insertions(+), 73 deletions(-)

diff --git a/gfs2/fsck/fs_recovery.c b/gfs2/fsck/fs_recovery.c
index d3c742a..52303dd 100644
--- a/gfs2/fsck/fs_recovery.c
+++ b/gfs2/fsck/fs_recovery.c
@@ -572,14 +572,6 @@ int replay_journals(struct gfs2_sbd *sdp, int preen, int force_check,
 	*clean_journals = 0;
 
 	sdp->jsize = GFS2_DEFAULT_JSIZE;
-	/* Get master dinode */
-	gfs2_lookupi(sdp->master_dir, "jindex", 6, &sdp->md.jiinode);
-
-	/* read in the journal index data */
-	if (ji_update(sdp)){
-		log_err( _("Unable to read in jindex inode.\n"));
-		return -1;
-	}
 
 	for(i = 0; i < sdp->md.journals; i++) {
 		if (!sdp->md.journal[i]) {
@@ -608,11 +600,7 @@ int replay_journals(struct gfs2_sbd *sdp, int preen, int force_check,
 			}
 			*clean_journals += clean;
 		}
-		inode_put(&sdp->md.journal[i]);
 	}
-	inode_put(&sdp->md.jiinode);
-	free(sdp->md.journal);
-	sdp->md.journal = NULL;
 	/* Sync the buffers to disk so we get a fresh start. */
 	fsync(sdp->device_fd);
 	return error;
diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
index 95a6a65..608eb32 100644
--- a/gfs2/fsck/initialize.c
+++ b/gfs2/fsck/initialize.c
@@ -477,18 +477,13 @@ static void lookup_per_node(struct gfs2_sbd *sdp, int allow_rebuild)
 }
 
 /**
- * init_system_inodes
- *
- * Returns: 0 on success, -1 on failure
+ * fetch_rgrps - fetch the resource groups from disk, and check their integrity
  */
-static int init_system_inodes(struct gfs2_sbd *sdp)
+static int fetch_rgrps(struct gfs2_sbd *sdp)
 {
-	uint64_t inumbuf;
-	char *buf;
-	struct gfs2_statfs_change sc;
-	int rgcount, sane = 1, err = 0;
 	enum rgindex_trust_level trust_lvl;
-	uint64_t addl_mem_needed;
+	int rgcount, sane = 1;
+
 	const char *level_desc[] = {
 		_("Checking if all rgrp and rindex values are good"),
 		_("Checking if rindex values may be easily repaired"),
@@ -503,48 +498,6 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 		_("Too many rgrp misses: rgrps must be unevenly spaced"),
 		_("Too much damage found: we cannot rebuild this rindex"),
 	};
-
-	/*******************************************************************
-	 ******************  Initialize important inodes  ******************
-	 *******************************************************************/
-
-	log_info( _("Initializing special inodes...\n"));
-
-	/* Get root dinode */
-	sdp->md.rooti = inode_read(sdp, sdp->sd_sb.sb_root_dir.no_addr);
-
-	if (sdp->gfs1)
-		sdp->md.riinode = inode_read(sdp, sbd1->sb_rindex_di.no_addr);
-	else
-		gfs2_lookupi(sdp->master_dir, "rindex", 6, &sdp->md.riinode);
-	if (!sdp->md.riinode) {
-		if (query( _("The gfs2 system rindex inode is missing. "
-			     "Okay to rebuild it? (y/n) ")))
-			build_rindex(sdp);
-	}
-
-	/*******************************************************************
-	 ******************  Fill in journal information  ******************
-	 *******************************************************************/
-
-	/* 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 = inode_read(sdp, sbd1->sb_jindex_di.no_addr);
-	else
-		gfs2_lookupi(sdp->master_dir, "jindex", 6, &sdp->md.jiinode);
-	if (!sdp->md.jiinode) {
-		if (query( _("The gfs2 system jindex inode is missing. "
-			     "Okay to rebuild it? (y/n) ")))
-			build_jindex(sdp);
-	}
-
-	/* read in the ji data */
-	if (ji_update(sdp)){
-		log_err( _("Unable to read in jindex inode.\n"));
-		return -1;
-	}
-
 	/*******************************************************************
 	 ********  Validate and read in resource group information  ********
 	 *******************************************************************/
@@ -580,6 +533,34 @@ static int init_system_inodes(struct gfs2_sbd *sdp)
 	log_info( _("%u resource groups found.\n"), rgcount);
 
 	check_rgrps_integrity(sdp);
+	return 0;
+}
+
+/**
+ * init_system_inodes
+ *
+ * Returns: 0 on success, -1 on failure
+ */
+static int init_system_inodes(struct gfs2_sbd *sdp)
+{
+	uint64_t inumbuf = 0;
+	char *buf;
+	struct gfs2_statfs_change sc;
+	uint64_t addl_mem_needed;
+	int err;
+
+	/*******************************************************************
+	 ******************  Initialize important inodes  ******************
+	 *******************************************************************/
+
+	log_info( _("Initializing special inodes...\n"));
+
+	/* Get root dinode */
+	sdp->md.rooti = inode_read(sdp, sdp->sd_sb.sb_root_dir.no_addr);
+
+	err = fetch_rgrps(sdp);
+	if (err)
+		return err;
 
 	/*******************************************************************
 	 *****************  Initialize more system inodes  *****************
@@ -1232,8 +1213,6 @@ static int reconstruct_single_journal(struct gfs2_sbd *sdp, int jnum,
 	log_info("Clearing journal %d\n", jnum);
 
 	for (seg = 0; seg < ji_nsegment; seg++){
-		bh = bget(sdp, lh.lh_first * sdp->bsize);
-		memset(bh->b_data, 0, sdp->bsize);
 		memset(&lh, 0, sizeof(struct gfs_log_header));
 
 		lh.lh_header.mh_magic = GFS2_MAGIC;
@@ -1245,6 +1224,8 @@ static int reconstruct_single_journal(struct gfs2_sbd *sdp, int jnum,
 			(seg * sbd1->sb_seg_size);
 		lh.lh_sequence = sequence;
 
+		bh = bget(sdp, lh.lh_first * sdp->bsize);
+		memset(bh->b_data, 0, sdp->bsize);
 		gfs_log_header_out(&lh, bh->b_data);
 		gfs_log_header_out(&lh, bh->b_data + GFS2_BASIC_BLOCK -
 				   sizeof(struct gfs_log_header));
@@ -1269,7 +1250,7 @@ static int reconstruct_journals(struct gfs2_sbd *sdp)
 	struct gfs_jindex ji;
 	char buf[sizeof(struct gfs_jindex)];
 
-	log_err("Clearing GFS journals (this may take a while)");
+	log_err("Clearing GFS journals (this may take a while)\n");
 	for (i = 0; i < sdp->md.journals; i++) {
 		gfs2_readi(sdp->md.jiinode, buf, i * sizeof(struct gfs_jindex),
 			   sizeof(struct gfs_jindex));
@@ -1284,6 +1265,81 @@ static int reconstruct_journals(struct gfs2_sbd *sdp)
 }
 
 /**
+ * init_rindex - read in the rindex file
+ */
+static int init_rindex(struct gfs2_sbd *sdp)
+{
+	int err;
+
+	if (sdp->gfs1)
+		sdp->md.riinode = inode_read(sdp, sbd1->sb_rindex_di.no_addr);
+	else
+		gfs2_lookupi(sdp->master_dir, "rindex", 6, &sdp->md.riinode);
+
+	if (sdp->md.riinode)
+		return 0;
+
+	if (!query( _("The gfs2 system rindex inode is missing. "
+		      "Okay to rebuild it? (y/n) "))) {
+		log_crit(_("Error: Cannot proceed without a valid rindex.\n"));
+		return -1;
+	}
+	if ((err = build_rindex(sdp))) {
+		log_crit(_("Error %d rebuilding rindex\n"), err);
+		return -1;
+	}
+	return 0;
+}
+
+/**
+ * init_jindex - read in the rindex file
+ */
+static int init_jindex(struct gfs2_sbd *sdp)
+{
+	/*******************************************************************
+	 ******************  Fill in journal information  ******************
+	 *******************************************************************/
+
+	/* 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 = 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;
+		}
+		/* In order to rebuild jindex, we need some valid
+		   rgrps in memory.  Temporarily read those in. */
+		err = fetch_rgrps(sdp);
+		if (err)
+			return err;
+
+		err = build_jindex(sdp);
+		/* Free rgrps read in earlier (re-read them later) */
+		gfs2_rgrp_free(&sdp->rgtree);
+		if (err) {
+			log_crit(_("Error %d rebuilding jindex\n"), err);
+			return err;
+		}
+	}
+
+	/* read in the ji data */
+	if (ji_update(sdp)){
+		log_err( _("Unable to read in jindex inode.\n"));
+		return -1;
+	}
+	return 0;
+}
+
+/**
  * initialize - initialize superblock pointer
  *
  */
@@ -1372,7 +1428,17 @@ int initialize(struct gfs2_sbd *sdp, int force_check, int preen,
 	if (!sdp->gfs1)
 		lookup_per_node(sdp, 0);
 
-	/* verify various things */
+	/* We need rindex first in case jindex is missing and needs to read
+	   in the rgrps before rebuilding it. */
+	if (init_rindex(sdp))
+		return FSCK_ERROR;
+
+	/* We need to read in jindex in order to replay the journals */
+	if (init_jindex(sdp))
+		return FSCK_ERROR;
+
+	/* If GFS, rebuild the journals.  If GFS2, replay them.  We don't have
+	   the smarts to replay GFS1 journals (neither did gfs_fsck). */
 
 	if (sdp->gfs1) {
 		if (reconstruct_journals(sdp))
@@ -1403,7 +1469,7 @@ mount_fail:
 	return FSCK_USAGE;
 }
 
-static void destroy_sdp(struct gfs2_sbd *sdp)
+void destroy(struct gfs2_sbd *sdp)
 {
 	if (!opts.no) {
 		if (block_mounters(sdp, 0)) {
@@ -1425,8 +1491,3 @@ static void destroy_sdp(struct gfs2_sbd *sdp)
 				   "caches.\n"));
 	}
 }
-
-void destroy(struct gfs2_sbd *sdp)
-{
-	destroy_sdp(sdp);
-}
-- 
1.7.7.5