[Cluster-devel] [PATCH 47/66] fsck.gfs2: Bad extended attributes not deleted properly

rpeterso at redhat.com rpeterso at redhat.com
Fri Jan 20 15:10:28 UTC 2012


From: Bob Peterson <rpeterso at redhat.com>

This patch fixes a couple of problems fsck.gfs2 when it discovered
corrupt extended attributes and tried to delete them.  First, in
check_eattr_entries it wasn't reporting the corruption.  Second,
when the EA block was freed, it wasn't returning a proper return
code which caused fsck to not update the inode accordingly.
Third, it wasn't zeroing out the EA block address properly.
Fourth, I did a small amount of reformatting.

rhbz#675723
---
 gfs2/fsck/metawalk.c |   56 ++++++++++++++++++++++++++++++++++++++++---------
 gfs2/fsck/pass1.c    |    4 +--
 gfs2/fsck/pass1c.c   |    7 +++--
 gfs2/fsck/pass2.c    |    9 +++++--
 4 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 94b0148..db7168b 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -755,6 +755,13 @@ static int check_eattr_entries(struct gfs2_inode *ip,
 							      bh, ea_hdr,
 							      ea_hdr_prev,
 							      pass->private)) {
+					log_err(_("Bad extended attribute "
+						  "found at block %lld "
+						  "(0x%llx)"),
+						(unsigned long long)
+						be64_to_cpu(*ea_data_ptr),
+						(unsigned long long)
+						be64_to_cpu(*ea_data_ptr));
 					if (query( _("Repair the bad Extended "
 						     "Attribute? (y/n) "))) {
 						ea_hdr->ea_num_ptrs = i;
@@ -807,12 +814,14 @@ static int check_leaf_eattr(struct gfs2_inode *ip, uint64_t block,
 			    uint64_t parent, struct metawalk_fxns *pass)
 {
 	struct gfs2_buffer_head *bh = NULL;
-	int error = 0;
-
-	log_debug( _("Checking EA leaf block #%"PRIu64" (0x%" PRIx64 ").\n"),
-			  block, block);
 
 	if (pass->check_eattr_leaf) {
+		int error = 0;
+
+		log_debug( _("Checking EA leaf block #%llu (0x%llx).\n"),
+			   (unsigned long long)block,
+			   (unsigned long long)block);
+
 		error = pass->check_eattr_leaf(ip, block, parent, &bh,
 					       pass->private);
 		if (error < 0) {
@@ -886,13 +895,18 @@ int find_remove_dup(struct gfs2_inode *ip, uint64_t block, const char *btype)
 /**
  * free_block_if_notdup - free blocks associated with an inode, but if it's a
  *                        duplicate, just remove that designation instead.
- * Returns: 0 if the block was freed, 1 if a duplicate reference was removed
+ * Returns: 1 if the block was freed, 0 if a duplicate reference was removed
+ * Note: The return code is handled this way because there are places in
+ *       metawalk.c that assume "1" means "change was made" and "0" means
+ *       change was not made.
  */
 int free_block_if_notdup(struct gfs2_inode *ip, uint64_t block,
 			 const char *btype)
 {
-	if (!find_remove_dup(ip, block, btype))
+	if (!find_remove_dup(ip, block, btype)) { /* not a dup */
 		fsck_blockmap_set(ip, block, btype, gfs2_block_free);
+		return 1;
+	}
 	return 0;
 }
 
@@ -1446,16 +1460,36 @@ int delete_data(struct gfs2_inode *ip, uint64_t block, void *private)
 int delete_eattr_indir(struct gfs2_inode *ip, uint64_t block, uint64_t parent,
 		       struct gfs2_buffer_head **bh, void *private)
 {
-	return delete_block_if_notdup(ip, block, NULL,
-				      _("indirect extended attribute"),
-				      private);
+	int ret;
+
+	ret = delete_block_if_notdup(ip, block, NULL,
+				     _("indirect extended attribute"),
+				     private);
+	/* Even if it's a duplicate reference, we want to eliminate the
+	   reference itself, and adjust di_blocks accordingly. */
+	if (ip->i_di.di_eattr) {
+		ip->i_di.di_blocks--;
+		if (block == ip->i_di.di_eattr)
+			ip->i_di.di_eattr = 0;
+		bmodified(ip->i_bh);
+	}
+	return ret;
 }
 
 int delete_eattr_leaf(struct gfs2_inode *ip, uint64_t block, uint64_t parent,
 		      struct gfs2_buffer_head **bh, void *private)
 {
-	return delete_block_if_notdup(ip, block, NULL, _("extended attribute"),
-				      private);
+	int ret;
+
+	ret = delete_block_if_notdup(ip, block, NULL, _("extended attribute"),
+				     private);
+	if (ip->i_di.di_eattr) {
+		ip->i_di.di_blocks--;
+		if (block == ip->i_di.di_eattr)
+			ip->i_di.di_eattr = 0;
+		bmodified(ip->i_bh);
+	}
+	return ret;
 }
 
 static int alloc_metalist(struct gfs2_inode *ip, uint64_t block,
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 32ebfba..3e6b816 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -1181,9 +1181,7 @@ static int handle_ip(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
 			  (unsigned long long)ip->i_di.di_num.no_addr,
 			  (unsigned long long)ip->i_di.di_num.no_addr);
 		check_metatree(ip, &invalidate_fxns);
-		if (fsck_blockmap_set(ip, block, _("invalid mode"),
-				      gfs2_inode_invalid))
-			goto bad_dinode;
+		check_inode_eattr(ip, &invalidate_fxns);
 		return 0;
 	} else if (error)
 		goto bad_dinode;
diff --git a/gfs2/fsck/pass1c.c b/gfs2/fsck/pass1c.c
index 150dc2f..f7a7842 100644
--- a/gfs2/fsck/pass1c.c
+++ b/gfs2/fsck/pass1c.c
@@ -62,9 +62,10 @@ static int ask_remove_eattr(struct gfs2_inode *ip)
 		ip->i_di.di_eattr = 0;
 		bmodified(ip->i_bh);
 		log_err( _("Bad Extended Attribute removed.\n"));
-	} else
-		log_err( _("Bad Extended Attribute not removed.\n"));
-	return 1;
+		return 1;
+	}
+	log_err( _("Bad Extended Attribute not removed.\n"));
+	return 0;
 }
 
 static int check_eattr_indir(struct gfs2_inode *ip, uint64_t block,
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 7e20315..e02fdd0 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -203,9 +203,9 @@ static int delete_eattr_extentry(struct gfs2_inode *ip, uint64_t *ea_data_ptr,
 				 struct gfs2_ea_header *ea_hdr_prev,
 				 void *private)
 {
-        uint64_t block = be64_to_cpu(*ea_data_ptr);
+	uint64_t block = be64_to_cpu(*ea_data_ptr);
 
-        return delete_metadata(ip, block, NULL, 0, private);
+	return delete_metadata(ip, block, NULL, 0, private);
 }
 
 struct metawalk_fxns pass2_fxns_delete = {
@@ -343,7 +343,10 @@ static int check_dentry(struct gfs2_inode *ip, struct gfs2_dirent *dent,
 				entry_ip = ip;
 			else
 				entry_ip = fsck_load_inode(sdp, entryblock);
-			check_inode_eattr(entry_ip, &pass2_fxns_delete);
+			if (ip->i_di.di_eattr) {
+				check_inode_eattr(entry_ip,
+						  &pass2_fxns_delete);
+			}
 			check_metatree(entry_ip, &pass2_fxns_delete);
 			if (entry_ip != ip)
 				fsck_inode_put(&entry_ip);
-- 
1.7.7.5




More information about the Cluster-devel mailing list