Re: [Cluster-devel] [Patch 17/44] fsck.gfs2: Don't stop invalidating blocks if an invalid one is found

2011-08-12 Thread Steven Whitehouse
ACK,

Steve.

On Thu, 2011-08-11 at 17:05 -0400, Bob Peterson wrote:
 From 0f424f6c6a2b4fda8c5b9b2bc1cb246d868d3fec Mon Sep 17 00:00:00 2001
 From: Bob Peterson rpete...@redhat.com
 Date: Mon, 8 Aug 2011 16:20:14 -0500
 Subject: [PATCH 17/44] fsck.gfs2: Don't stop invalidating blocks if an
  invalid one is found
 
 When fsck found a duplicate reference to a block it invalidated the dinode's
 metadata.  But if it encountered an invalid block, for example, out of range,
 the invalidating would stop.  If we encounter a block that isn't valid, we
 obviously can't invalidate it.  However, if we return an error, all future
 invalidating will stop for that dinode.  That's wrong because we need it to
 continue to invalidate the other valid blocks.  If we don't do this, block
 references that follow the bad one that are also referenced elsewhere
 (duplicates) won't be flagged as such.  As a result, they'll be freed when
 this corrupt dinode is deleted, despite being used by another dinode as a
 valid block.  This patch makes it return a good return code so the 
 invalidating
 continues.
 
 rhbz#675723
 ---
  gfs2/fsck/pass1.c |   11 +--
  1 files changed, 9 insertions(+), 2 deletions(-)
 
 diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
 index b9aa165..2b04227 100644
 --- a/gfs2/fsck/pass1.c
 +++ b/gfs2/fsck/pass1.c
 @@ -827,8 +827,15 @@ static int mark_block_invalid(struct gfs2_inode *ip, 
 uint64_t block,
  {
   uint8_t q;
  
 - if (!valid_block(ip-i_sbd, block) != 0)
 - return -EFAULT;
 + /* If the block isn't valid, we obviously can't invalidate it.
 +  * However, if we return an error, invalidating will stop, and
 +  * we want it to continue to invalidate the valid blocks.  If we
 +  * don't do this, block references that follow that are also
 +  * referenced elsewhere (duplicates) won't be flagged as such,
 +  * and as a result, they'll be freed when this dinode is deleted,
 +  * despite being used by another dinode as a valid block. */
 + if (!valid_block(ip-i_sbd, block))
 + return 0;
  
   q = block_type(block);
   if (q != gfs2_block_free) {




[Cluster-devel] [Patch 17/44] fsck.gfs2: Don't stop invalidating blocks if an invalid one is found

2011-08-11 Thread Bob Peterson
From 0f424f6c6a2b4fda8c5b9b2bc1cb246d868d3fec Mon Sep 17 00:00:00 2001
From: Bob Peterson rpete...@redhat.com
Date: Mon, 8 Aug 2011 16:20:14 -0500
Subject: [PATCH 17/44] fsck.gfs2: Don't stop invalidating blocks if an
 invalid one is found

When fsck found a duplicate reference to a block it invalidated the dinode's
metadata.  But if it encountered an invalid block, for example, out of range,
the invalidating would stop.  If we encounter a block that isn't valid, we
obviously can't invalidate it.  However, if we return an error, all future
invalidating will stop for that dinode.  That's wrong because we need it to
continue to invalidate the other valid blocks.  If we don't do this, block
references that follow the bad one that are also referenced elsewhere
(duplicates) won't be flagged as such.  As a result, they'll be freed when
this corrupt dinode is deleted, despite being used by another dinode as a
valid block.  This patch makes it return a good return code so the invalidating
continues.

rhbz#675723
---
 gfs2/fsck/pass1.c |   11 +--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index b9aa165..2b04227 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -827,8 +827,15 @@ static int mark_block_invalid(struct gfs2_inode *ip, 
uint64_t block,
 {
uint8_t q;
 
-   if (!valid_block(ip-i_sbd, block) != 0)
-   return -EFAULT;
+   /* If the block isn't valid, we obviously can't invalidate it.
+* However, if we return an error, invalidating will stop, and
+* we want it to continue to invalidate the valid blocks.  If we
+* don't do this, block references that follow that are also
+* referenced elsewhere (duplicates) won't be flagged as such,
+* and as a result, they'll be freed when this dinode is deleted,
+* despite being used by another dinode as a valid block. */
+   if (!valid_block(ip-i_sbd, block))
+   return 0;
 
q = block_type(block);
if (q != gfs2_block_free) {
-- 
1.7.4.4