[Ocfs2-tools-devel] [PATCH] fsck.ocfs2(1.2): Modify e_clusters if it isn't equal to eb contains.

[Joel Becker]

sob

On Thu, Nov 20, 2008 at 11:29:41AM +0800, Tao Ma wrote:
> In bug 7553939, we meet with a scenario that the clusters haven't
> been allocated to the extent block(no update) while the inode already
> have the i_clusters and e_clusters updated. The commit
> 09166ad03c83f39f2dcd77c74cd4672c3a371dfa has fixed the problem
> of i_clusters corruption, so this one intends to fix e_clusters.
> We calculate the total clusters within the extent block and then
> modify the extent record if it doesn't show the right value.
> 
> Please note that to be safe, it only fix the corruption happens in the
> last extent block. If there is any corruption happens in other extent block,
> we don't touch it(actually if we modify it, we may leave a hole there and
> make the system corrupted). The good thing is that in ocfs2-1.2, we don't
> support sparse files, and we only extend files, so if there is one corruption
> like 7553939, it must happen in the last extent block.
> 
> One more thing, in ocfs2-1.4, we support sparse files, so it can be handled
> gracefully. So this is only a fix for ocfs2-tools-1.2.
> 
> Signed-off-by: Tao Ma <tao.ma at oracle.com>
> ---
>  fsck.ocfs2/extent.c               |   36 ++++++++++++++++++++++++++++--------
>  fsck.ocfs2/fsck.ocfs2.checks.8.in |    7 +++++++
>  2 files changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/fsck.ocfs2/extent.c b/fsck.ocfs2/extent.c
> index 0690408..d4b5364 100644
> --- a/fsck.ocfs2/extent.c
> +++ b/fsck.ocfs2/extent.c
> @@ -57,11 +57,12 @@ struct extent_info {
>  static errcode_t check_el(o2fsck_state *ost, struct extent_info *ei,
>  			  struct ocfs2_dinode *di,
>  			  struct ocfs2_extent_list *el,
> -			  uint16_t max_recs, int *changed);
> +			  uint16_t max_recs, int *changed,
> +			  uint32_t *total_clusters);
>  
>  static errcode_t check_eb(o2fsck_state *ost, struct extent_info *ei,
>  			  struct ocfs2_dinode *di, uint64_t blkno,
> -			  int *is_valid)
> +			  int *is_valid, uint32_t *total_clusters)
>  {
>  	int changed = 0;
>  	char *buf = NULL;
> @@ -128,7 +129,7 @@ static errcode_t check_eb(o2fsck_state *ost, struct extent_info *ei,
>  
>  	check_el(ost, ei, di, &eb->h_list,
>  		 ocfs2_extent_recs_per_eb(ost->ost_fs->fs_blocksize), 
> -		 &changed);
> +		 &changed, total_clusters);
>  
>  	if (changed) {
>  		ret = ocfs2_write_extent_block(ost->ost_fs, blkno, buf);
> @@ -155,7 +156,7 @@ static errcode_t check_er(o2fsck_state *ost, struct extent_info *ei,
>  {
>  	errcode_t ret = 0;
>  	uint64_t first_block;
> -	uint32_t last_cluster;
> +	uint32_t last_cluster, total_clusters;
>  
>  	verbosef("cpos %u clusters %u blkno %"PRIu64"\n", er->e_cpos,
>  		 er->e_clusters, er->e_blkno);
> @@ -170,7 +171,9 @@ static errcode_t check_er(o2fsck_state *ost, struct extent_info *ei,
>  		 * is checked */
>  		ei->ei_expect_depth = 1;
>  		ei->ei_expected_depth = el->l_tree_depth - 1;
> -		check_eb(ost, ei, di, er->e_blkno, &is_valid);
> +		ret = 0;
> +		total_clusters = 0;
> +		check_eb(ost, ei, di, er->e_blkno, &is_valid, &total_clusters);
>  		if (!is_valid && 
>  		    prompt(ost, PY, PR_EXTENT_EB_INVALID,
>  			   "The extent record for cluster offset "
> @@ -181,8 +184,22 @@ static errcode_t check_er(o2fsck_state *ost, struct extent_info *ei,
>  
>  			er->e_blkno = 0;
>  			*changed = 1;
> +			goto out;
>  		}
> -		ret = 0;
> +
> +		if (el->l_recs[el->l_next_free_rec-1].e_blkno == er->e_blkno &&
> +		    total_clusters != er->e_clusters &&
> +		    prompt(ost, PY, PR_EXTENT_EB_CLUSTER_RANGE,
> +			   "The extent record for cluster offset %"PRIu32" "
> +			   "in inode %"PRIu64" has a cluster count of %"PRIu32
> +			   " while its extent block has %"PRIu32" clusters "
> +			   "allocated. Change it to what extent block "
> +			   "represents?", er->e_cpos, di->i_blkno,
> +			   er->e_clusters, total_clusters)) {
> +			er->e_clusters = total_clusters;
> +			*changed = 1;
> +		}
> +
>  		goto out;
>  	}
>  
> @@ -230,7 +247,8 @@ out:
>  static errcode_t check_el(o2fsck_state *ost, struct extent_info *ei,
>  			  struct ocfs2_dinode *di,
>  			  struct ocfs2_extent_list *el,
> -			  uint16_t max_recs, int *changed)
> +			  uint16_t max_recs, int *changed,
> +			  uint32_t *total_clusters)
>  {
>  	int trust_next_free = 1;
>  	struct ocfs2_extent_rec *er;
> @@ -319,6 +337,8 @@ static errcode_t check_el(o2fsck_state *ost, struct extent_info *ei,
>  			continue;
>  		}
>  
> +		if (total_clusters)
> +			*total_clusters += er->e_clusters;
>  
>  		/* we've already accounted for the extent block as part of
>  		 * the extent block chain groups */
> @@ -350,7 +370,7 @@ errcode_t o2fsck_check_extents(o2fsck_state *ost,
>  	
>  	ret = check_el(ost, &ei, di, &di->id2.i_list, 
>  	         ocfs2_extent_recs_per_inode(ost->ost_fs->fs_blocksize),
> -		 &changed);
> +		 &changed, NULL);
>  
>  	if (changed)
>  		o2fsck_write_inode(ost, di->i_blkno, di);
> diff --git a/fsck.ocfs2/fsck.ocfs2.checks.8.in b/fsck.ocfs2/fsck.ocfs2.checks.8.in
> index 2e30f9d..8551018 100644
> --- a/fsck.ocfs2/fsck.ocfs2.checks.8.in
> +++ b/fsck.ocfs2/fsck.ocfs2.checks.8.in
> @@ -71,6 +71,13 @@ extent tree references an invalid extent block.
>  Answering yes stops the tree from referencing the invalid extent block.  This
>  may truncate data from the file which contains the tree.
>  
> +.SS "EXTENT_EB_CLUSTER_RANGE"
> +Deep extent trees are built by forming a tree out of extent blocks.  An
> +extent record represent a wrong clusters count than the extent block contains.
> +
> +Answering yes change clusters count in the extent record to the number the below
> +extent block contains.
> +
>  .SS "EXTENT_LIST_DEPTH"
>  Extent lists contain a record of their depth in the tree.  An extent list
>  was found whose recorded depth doesn't match the position they have in the
> -- 
> 1.5.5
> 

-- 

 There are morethings in heaven and earth, Horatio,
 Than are dreamt of in your philosophy.

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127