cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: rpeterso@redhat.com <rpeterso@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH 27/66] fsck.gfs2 pass3: Refactor mark_and_return_parent
Date: Fri, 20 Jan 2012 09:10:08 -0600	[thread overview]
Message-ID: <1327072247-26275-28-git-send-email-rpeterso@redhat.com> (raw)
In-Reply-To: <1327072247-26275-1-git-send-email-rpeterso@redhat.com>

From: Bob Peterson <rpeterso@redhat.com>

This patch refactors pass3 function "mark_and_return_parent".  Before
it was so complex and indented you could hardly interpret it.  Now you
can actually follow the logic.

rhbz#675723
---
 gfs2/fsck/pass3.c |  162 ++++++++++++++++++++++++++---------------------------
 1 files changed, 79 insertions(+), 83 deletions(-)

diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c
index 580c662..b904814 100644
--- a/gfs2/fsck/pass3.c
+++ b/gfs2/fsck/pass3.c
@@ -73,102 +73,98 @@ static struct dir_info *mark_and_return_parent(struct gfs2_sbd *sdp,
 {
 	struct dir_info *pdi;
 	uint8_t q_dotdot, q_treewalk;
+	int error = 0;
 
 	di->checked = 1;
 
 	if (!di->treewalk_parent)
 		return NULL;
 
-	if (di->dotdot_parent != di->treewalk_parent) {
-		log_warn( _("Directory '..' and treewalk connections disagree for inode %llu"
-				 " (0x%llx)\n"), (unsigned long long)di->dinode,
-			(unsigned long long)di->dinode);
-		log_notice( _("'..' has %llu (0x%llx), treewalk has %llu"
-			      " (0x%llx)\n"),
-			   (unsigned long long)di->dotdot_parent,
-			   (unsigned long long)di->dotdot_parent,
-			   (unsigned long long)di->treewalk_parent,
-			   (unsigned long long)di->treewalk_parent);
+	if (di->dotdot_parent == di->treewalk_parent) {
 		q_dotdot = block_type(di->dotdot_parent);
-		q_treewalk = block_type(di->treewalk_parent);
-		/* if the dotdot entry isn't a directory, but the
-		 * treewalk is, treewalk is correct - if the treewalk
-		 * entry isn't a directory, but the dotdot is, dotdot
-		 * is correct - if both are directories, which do we
-		 * choose? if neither are directories, we have a
-		 * problem - need to move this directory into lost+found
-		 */
 		if (q_dotdot != gfs2_inode_dir) {
-			if (q_treewalk != gfs2_inode_dir) {
-				log_err( _("Orphaned directory, move to lost+found\n"));
-				return NULL;
-			}
-			else {
-				log_warn( _("Treewalk parent is correct,"
-					    " fixing dotdot -> %llu (0x%llx)\n"),
-					 (unsigned long long)di->treewalk_parent,
-					 (unsigned long long)di->treewalk_parent);
-				attach_dotdot_to(sdp, di->treewalk_parent,
-						 di->dotdot_parent, di->dinode);
-				di->dotdot_parent = di->treewalk_parent;
-			}
-		} else {
-			if (q_treewalk != gfs2_inode_dir) {
-				int error = 0;
-				log_warn( _(".. parent is valid, but treewalk"
-						 "is bad - reattaching to lost+found"));
-
-				/* FIXME: add a dinode for this entry instead? */
-
-				if (query( _("Remove directory entry for bad"
-					    " inode %llu (0x%llx) in %llu"
-					    " (0x%llx)? (y/n)"),
-					(unsigned long long)di->dinode,
-					(unsigned long long)di->dinode,
-					(unsigned long long)di->treewalk_parent,
-					(unsigned long long)di->treewalk_parent)) {
-					error = remove_dentry_from_dir(sdp, di->treewalk_parent,
-												   di->dinode);
-					if (error < 0) {
-						stack;
-						return NULL;
-					}
-					if (error > 0) {
-						log_warn( _("Unable to find dentry for block %llu"
-							" (0x%llx) in %llu (0x%llx)\n"),
-							 (unsigned long long)di->dinode,
-							(unsigned long long)di->dinode,
-							(unsigned long long)di->treewalk_parent,
-							(unsigned long long)di->treewalk_parent);
-					}
-					log_warn( _("Directory entry removed\n"));
-				} else {
-					log_err( _("Directory entry to invalid inode remains\n"));
-				}
-				log_info( _("Marking directory unlinked\n"));
-
-				return NULL;
-			}
-			else {
-				log_err( _("Both .. and treewalk parents are "
-					   "directories, going with treewalk "
-					   "for now...\n"));
-				attach_dotdot_to(sdp, di->treewalk_parent,
-						 di->dotdot_parent,
-						 di->dinode);
-				di->dotdot_parent = di->treewalk_parent;
-			}
+			log_err( _("Orphaned directory at block %llu (0x%llx) "
+				   "moved to lost+found\n"),
+				 (unsigned long long)di->dinode,
+				 (unsigned long long)di->dinode);
+			return NULL;
 		}
+		goto out;
 	}
-	else {
-		q_dotdot = block_type(di->dotdot_parent);
-		if (q_dotdot != gfs2_inode_dir) {
-			log_err( _("Orphaned directory at block %llu (0x%llx) moved to lost+found\n"),
-				(unsigned long long)di->dinode,
-				(unsigned long long)di->dinode);
+
+	log_warn( _("Directory '..' and treewalk connections disagree for "
+		    "inode %llu (0x%llx)\n"), (unsigned long long)di->dinode,
+		  (unsigned long long)di->dinode);
+	log_notice( _("'..' has %llu (0x%llx), treewalk has %llu (0x%llx)\n"),
+		    (unsigned long long)di->dotdot_parent,
+		    (unsigned long long)di->dotdot_parent,
+		    (unsigned long long)di->treewalk_parent,
+		    (unsigned long long)di->treewalk_parent);
+	q_dotdot = block_type(di->dotdot_parent);
+	q_treewalk = block_type(di->treewalk_parent);
+	/* if the dotdot entry isn't a directory, but the
+	 * treewalk is, treewalk is correct - if the treewalk
+	 * entry isn't a directory, but the dotdot is, dotdot
+	 * is correct - if both are directories, which do we
+	 * choose? if neither are directories, we have a
+	 * problem - need to move this directory into lost+found
+	 */
+	if (q_dotdot != gfs2_inode_dir) {
+		if (q_treewalk != gfs2_inode_dir) {
+			log_err( _("Orphaned directory, move to "
+				   "lost+found\n"));
 			return NULL;
+		} else {
+			log_warn( _("Treewalk parent is correct, fixing "
+				    "dotdot -> %llu (0x%llx)\n"),
+				  (unsigned long long)di->treewalk_parent,
+				  (unsigned long long)di->treewalk_parent);
+			attach_dotdot_to(sdp, di->treewalk_parent,
+					 di->dotdot_parent, di->dinode);
+			di->dotdot_parent = di->treewalk_parent;
 		}
+		goto out;
+	}
+	if (q_treewalk == gfs2_inode_dir) {
+		log_err( _("Both .. and treewalk parents are directories, "
+			   "going with treewalk...\n"));
+		attach_dotdot_to(sdp, di->treewalk_parent,
+				 di->dotdot_parent, di->dinode);
+		di->dotdot_parent = di->treewalk_parent;
+		goto out;
 	}
+	log_warn( _(".. parent is valid, but treewalk is bad - reattaching to "
+		    "lost+found"));
+
+	/* FIXME: add a dinode for this entry instead? */
+
+	if (!query( _("Remove directory entry for bad inode %llu (0x%llx) in "
+		      "%llu (0x%llx)? (y/n)"),
+		    (unsigned long long)di->dinode,
+		    (unsigned long long)di->dinode,
+		    (unsigned long long)di->treewalk_parent,
+		    (unsigned long long)di->treewalk_parent)) {
+		log_err( _("Directory entry to invalid inode remains\n"));
+		return NULL;
+	}
+	error = remove_dentry_from_dir(sdp, di->treewalk_parent, di->dinode);
+	if (error < 0) {
+		stack;
+		return NULL;
+	}
+	if (error > 0)
+		log_warn( _("Unable to find dentry for block %llu"
+			    " (0x%llx) in %llu (0x%llx)\n"),
+			  (unsigned long long)di->dinode,
+			  (unsigned long long)di->dinode,
+			  (unsigned long long)di->treewalk_parent,
+			  (unsigned long long)di->treewalk_parent);
+	log_warn( _("Directory entry removed\n"));
+	log_info( _("Marking directory unlinked\n"));
+
+	return NULL;
+
+out:
 	pdi = dirtree_find(di->dotdot_parent);
 
 	return pdi;
-- 
1.7.7.5



  parent reply	other threads:[~2012-01-20 15:10 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-20 15:09 [Cluster-devel] [PATCH 00/66] fsck.gfs2: add ability to fix GFS (gfs1) file systems rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 01/66] fsck.gfs2: Make functions use sdp rather than sbp rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 02/66] fsck.gfs2: Change "if(" to "if (" rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 03/66] libgfs1: Add gfs1 variable to superblock structure rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 04/66] libgfs2: Make check_sb and read_sb operate on gfs1 rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 05/66] libgfs2: move gfs1 structures to libgfs2 rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 06/66] fsck.gfs2: Check for blocks wrongly inside resource groups rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 07/66] fsck.gfs2: Rename check_leaf to check_ealeaf_block rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 08/66] fsck.gfs2: fsck.gfs2: Delete vestigial buffer_head in check_leaf rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 09/66] fsck.gfs2: fsck.gfs2: Rename nlink functions to be intuitive rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 10/66] fsck.gfs2: fsck.gfs2: Sync di_nlink adding links for lost+found rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 11/66] fsck.gfs2: fsck.gfs2: Make dir entry count 32 bits rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 12/66] fsck.gfs2: get rid of triple negative logic rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 13/66] dirent_repair needs to mark the buffer as modified rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 14/66] fsck.gfs2: fsck.gfs2: Ask to reclaim unlinked meta per-rgrp only rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 15/66] fsck.gfs2: fsck.gfs2: Refactor add_dotdot function in lost+found rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 16/66] libgfs2: libgfs2: Use __FUNCTION__ rather than __FILE__ rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 17/66] fsck.gfs2: fsck.gfs2: Don't stop invalidating blocks on invalid rpeterso
2012-01-20 15:09 ` [Cluster-devel] [PATCH 18/66] fsck.gfs2: fsck.gfs2: Find and clear duplicate leaf blocks refs rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 19/66] fsck.gfs2: fsck.gfs2: Move check_num_ptrs from metawalk to pass1 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 20/66] fsck.gfs2: fsck.gfs2: Duplicate ref processing for leaf blocks rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 21/66] fsck.gfs2: fsck.gfs2: split check_leaf_blks to be more readable rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 22/66] fsck.gfs2: Shorten output rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 23/66] fsck.gfs2: Make output messages more sensible rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 24/66] fsck.gfs pass2: Refactor function set_dotdot_dir rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 25/66] fsck.gfs2 pass2: Delete extended attributes with inode rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 26/66] fsck.gfs2 pass2: Don't delete invalid inode metadata rpeterso
2012-01-20 15:10 ` rpeterso [this message]
2012-01-20 15:10 ` [Cluster-devel] [PATCH 28/66] fsck.gfs2: misc cosmetic changes rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 29/66] fsck.gfs2: Don't use old_leaf if it was a duplicate rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 30/66] fsck.gfs2: Add find_remove_dup, free_block_if_notdup rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 31/66] fsck.gfs2: don't free prev rgrp list repairing rgrps rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 32/66] libgfs2: eliminate gfs1_readi in favor of gfs2_readi rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 33/66] libgfs2: Mark buffer modified adding a new GFS1 block rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 34/66] libgfs2: Use dinode buffer to map gfs1 dinode blocks rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 35/66] libgfs2: move block_map functions to fsck.gfs2 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 36/66] libgfs2: eliminate gfs1_rindex_read rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 37/66] libgfs2: combine ri_update and gfs1_ri_update rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 38/66] libgfs2: combine gfs_inode_read and gfs_inode_get rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 39/66] libgfs2: move gfs1 functions from edit to libgfs2 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 40/66] gfs2_edit savemeta: save_inode_data backward for gfs1 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 41/66] libgfs2: expand capabilities to operate on gfs1 rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 42/66] fsck.gfs2: Combine block and char device inode types rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 43/66] fsck.gfs2: four-step duplicate elimination process rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 44/66] fsck.gfs2: Add ability to check gfs1 file systems rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 45/66] fsck.gfs2: Remove bad inodes from duplicate tree rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 46/66] fsck.gfs2: Handle duplicate reference to dinode blocks rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 47/66] fsck.gfs2: Bad extended attributes not deleted properly rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 48/66] libgfs2: Make rebuild functions not re-read ip rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 49/66] fsck.gfs2: Shorten debug output rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 50/66] fsck.gfs2: Increment link count reporting wrong dinode rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 51/66] fsck.gfs2: system dinodes take priority over user dinodes rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 52/66] fsck.gfs2: Recognize partially gfs2-converted dinodes rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 53/66] fsck.gfs2: Print step 2 duplicate debug msg first rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 54/66] fsck.gfs2: pass1c counts percentage backward rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 55/66] fsck.gfs2: Speed up rangecheck functions rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 56/66] libgfs2: Make in-core rgrps use rbtree rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 57/66] fsck.gfs2: Fix memory leaks rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 58/66] Change man pages and gfs2_convert messages to include GFS rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 59/66] gfs2_edit: Fix memory leaks rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 60/66] fsck.gfs2: Journals not properly checked rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 61/66] fsck.gfs2: Rearrange block types to group all inode types rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 62/66] fsck.gfs2: Fix initialization error return codes rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 63/66] fsck.gfs2: Don't use strerror for libgfs2 errors rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 64/66] fsck.gfs2: Fix memory leak in initialize.c rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 65/66] fsck.gfs2: Add return code checks and initializations rpeterso
2012-01-20 15:10 ` [Cluster-devel] [PATCH 66/66] libgfs2: Fix null pointer dereference in linked_leaf_search rpeterso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1327072247-26275-28-git-send-email-rpeterso@redhat.com \
    --to=rpeterso@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).