[PATCH 0/13] xfs_repair: recombine cut&waste code in dir2.c/attr_repair.c

[PATCH 0/13] xfs_repair: recombine cut&waste code in dir2.c/attr_repair.c

[Eric Sandeen]

repair/dir2.c and repair/attr_repair.c got cut and pasted
long, long ago and have diverged since.

This series brings them back together, presumably fixes some
bugs, and loses a few hundred lines in the process.

o/~ reunited, and it feels so so good ... o/~

The series accomplishes this through a combination of trivial changes
(removing unused structure members, whitespace, etc) as well
as by "cross-porting" changes & fixes which happened to one
file but not the other over the past many years.

Along the way, a graphical diff of dir2 vs. attr_repair should
show the convergence.

Up until the last patch, I don't worry about dir vs. attr naming
in comments or error messages; the goal is to make these chunks
of the two files sufficiently similar so that by patch 11, the
reviewer can do a diff and say "yeah, ok, those really are
substantially the same now."

Also instructive is to apply up to patch 11, copy dir2.c and
attr_repair.c to /tmp, apply patch 12, and do a 3-way graphical
diff of the 3 files to see that the move really is OK and
didn't play any significant tricks.

The last patch fixes up the dir vs. attr text in error messages
and comments.  I do have a question about whether this is ok
for i8n:

	printf(_("This string is %s"), _("awesome"));

because that's essentially the trick I used...

Thanks,
-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 01/13] xfs_repair: remove trace-only 'n' member from da_level_state

[Eric Sandeen]

The da_level_state structure contains an 'n' member
when XR_DIR_TRACE is enabled, which is a) write only, and
b) set by a macro which doesn't exist (XFS_BUF_TO_DA_INTNODE)

Removing this structure member fixes compilation with
XR_DIR_TRACE enabled, and also makes da_level_state identical
to dir2_level_state, so the two can be combined later.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index debe9e8..d63bc87 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -59,9 +59,6 @@ typedef unsigned char	da_freemap_t;
  */
 typedef struct da_level_state  {
 	xfs_buf_t	*bp;		/* block bp */
-#ifdef XR_DIR_TRACE
-	xfs_da_intnode_t *n;		/* bp data */
-#endif
 	xfs_dablk_t	bno;		/* file block number */
 	xfs_dahash_t	hashval;	/* last verified hashval */
 	int		index;		/* current index in block */
@@ -232,9 +229,6 @@ traverse_int_dablock(xfs_mount_t	*mp,
 		da_cursor->level[i].bp = bp;
 		da_cursor->level[i].bno = bno;
 		da_cursor->level[i].index = 0;
-#ifdef XR_DIR_TRACE
-		da_cursor->level[i].n = XFS_BUF_TO_DA_INTNODE(bp);
-#endif
 
 		/*
 		 * set up new bno for next level down
@@ -624,9 +618,6 @@ verify_da_path(xfs_mount_t	*mp,
 		cursor->level[this_level].bno = dabno;
 		cursor->level[this_level].hashval =
 					be32_to_cpu(btree[0].hashval);
-#ifdef XR_DIR_TRACE
-		cursor->level[this_level].n = newnode;
-#endif
 		entry = cursor->level[this_level].index = 0;
 
 		/*
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 01/13] xfs_repair: remove trace-only 'n' member from da_level_state

[Brian Foster]

On Wed, Sep 09, 2015 at 02:33:59PM -0500, Eric Sandeen wrote:
> The da_level_state structure contains an 'n' member
> when XR_DIR_TRACE is enabled, which is a) write only, and
> b) set by a macro which doesn't exist (XFS_BUF_TO_DA_INTNODE)
> 
> Removing this structure member fixes compilation with
> XR_DIR_TRACE enabled, and also makes da_level_state identical
> to dir2_level_state, so the two can be combined later.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  repair/attr_repair.c |    9 ---------
>  1 files changed, 0 insertions(+), 9 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index debe9e8..d63bc87 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -59,9 +59,6 @@ typedef unsigned char	da_freemap_t;
>   */
>  typedef struct da_level_state  {
>  	xfs_buf_t	*bp;		/* block bp */
> -#ifdef XR_DIR_TRACE
> -	xfs_da_intnode_t *n;		/* bp data */
> -#endif
>  	xfs_dablk_t	bno;		/* file block number */
>  	xfs_dahash_t	hashval;	/* last verified hashval */
>  	int		index;		/* current index in block */
> @@ -232,9 +229,6 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  		da_cursor->level[i].bp = bp;
>  		da_cursor->level[i].bno = bno;
>  		da_cursor->level[i].index = 0;
> -#ifdef XR_DIR_TRACE
> -		da_cursor->level[i].n = XFS_BUF_TO_DA_INTNODE(bp);
> -#endif
>  
>  		/*
>  		 * set up new bno for next level down
> @@ -624,9 +618,6 @@ verify_da_path(xfs_mount_t	*mp,
>  		cursor->level[this_level].bno = dabno;
>  		cursor->level[this_level].hashval =
>  					be32_to_cpu(btree[0].hashval);
> -#ifdef XR_DIR_TRACE
> -		cursor->level[this_level].n = newnode;
> -#endif
>  		entry = cursor->level[this_level].index = 0;
>  
>  		/*
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 02/13] xfs_repair: remove type from da & dir2 cursors

[Eric Sandeen]

The type field in these cursors is only set (and only
in the attr code), and it's never read; just remove
it.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |    2 --
 repair/dir2.h        |    1 -
 2 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index d63bc87..f29a5bd 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -67,7 +67,6 @@ typedef struct da_level_state  {
 
 typedef struct da_bt_cursor  {
 	int			active;	/* highest level in tree (# levels-1) */
-	int			type;	/* 0 if dir, 1 if attr */
 	xfs_ino_t		ino;
 	xfs_dablk_t		greatest_bno;
 	xfs_dinode_t		*dip;
@@ -1477,7 +1476,6 @@ process_node_attr(
 	 */
 	memset(&da_cursor, 0, sizeof(da_bt_cursor_t));
 	da_cursor.active = 0;
-	da_cursor.type = 0;
 	da_cursor.ino = ino;
 	da_cursor.dip = dip;
 	da_cursor.greatest_bno = 0;
diff --git a/repair/dir2.h b/repair/dir2.h
index df68d5c..3cc1941 100644
--- a/repair/dir2.h
+++ b/repair/dir2.h
@@ -51,7 +51,6 @@ typedef struct dir2_level_state  {
 
 typedef struct dir2_bt_cursor  {
 	int			active;	/* highest level in tree (# levels-1) */
-	int			type;	/* 0 if dir, 1 if attr */
 	xfs_ino_t		ino;
 	xfs_dablk_t		greatest_bno;
 	xfs_dinode_t		*dip;
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 02/13] xfs_repair: remove type from da & dir2 cursors

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:00PM -0500, Eric Sandeen wrote:
> The type field in these cursors is only set (and only
> in the attr code), and it's never read; just remove
> it.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  repair/attr_repair.c |    2 --
>  repair/dir2.h        |    1 -
>  2 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index d63bc87..f29a5bd 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -67,7 +67,6 @@ typedef struct da_level_state  {
>  
>  typedef struct da_bt_cursor  {
>  	int			active;	/* highest level in tree (# levels-1) */
> -	int			type;	/* 0 if dir, 1 if attr */
>  	xfs_ino_t		ino;
>  	xfs_dablk_t		greatest_bno;
>  	xfs_dinode_t		*dip;
> @@ -1477,7 +1476,6 @@ process_node_attr(
>  	 */
>  	memset(&da_cursor, 0, sizeof(da_bt_cursor_t));
>  	da_cursor.active = 0;
> -	da_cursor.type = 0;
>  	da_cursor.ino = ino;
>  	da_cursor.dip = dip;
>  	da_cursor.greatest_bno = 0;
> diff --git a/repair/dir2.h b/repair/dir2.h
> index df68d5c..3cc1941 100644
> --- a/repair/dir2.h
> +++ b/repair/dir2.h
> @@ -51,7 +51,6 @@ typedef struct dir2_level_state  {
>  
>  typedef struct dir2_bt_cursor  {
>  	int			active;	/* highest level in tree (# levels-1) */
> -	int			type;	/* 0 if dir, 1 if attr */
>  	xfs_ino_t		ino;
>  	xfs_dablk_t		greatest_bno;
>  	xfs_dinode_t		*dip;
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 03/13] xfs_repair: make CRC checking consistent in path verification

[Eric Sandeen]

verify_da_path and verify_dir2_path both take steps to
re-compute the CRC of the block if it otherwise looks
ok and no other changes are needed.  They do this inside
a loop, but the approach differs; verify_da_path expects
its caller to check the first buffer prior to the loop,
and verify_dir2_path expects its caller to check the last
buffer after the loop.

Make this consistent by semi-arbitrarily choosing to make
verify_da_path (and its caller) match the method used by
verify_dir2_path, and check the last buffer after the
loop is done.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index f29a5bd..aba0782 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -606,6 +606,14 @@ verify_da_path(xfs_mount_t	*mp,
 		ASSERT(cursor->level[this_level].dirty == 0 ||
 			(cursor->level[this_level].dirty && !no_modify));
 
+		/*
+		 * If block looks ok but CRC didn't match, make sure to
+		 * recompute it.
+		 */
+		if (!no_modify &&
+		    cursor->level[this_level].bp->b_error == -EFSBADCRC)
+			cursor->level[this_level].dirty = 1;
+
 		if (cursor->level[this_level].dirty && !no_modify)
 			libxfs_writebuf(cursor->level[this_level].bp, 0);
 		else
@@ -618,14 +626,6 @@ verify_da_path(xfs_mount_t	*mp,
 		cursor->level[this_level].hashval =
 					be32_to_cpu(btree[0].hashval);
 		entry = cursor->level[this_level].index = 0;
-
-		/*
-		 * We want to rewrite the buffer on a CRC error seeing as it
-		 * contains what appears to be a valid node block, but only if
-		 * we are fixing errors.
-		 */
-		if (bp->b_error == -EFSBADCRC && !no_modify)
-			cursor->level[this_level].dirty++;
 	}
 	/*
 	 * ditto for block numbers
@@ -1363,8 +1363,6 @@ process_leaf_attr_level(xfs_mount_t	*mp,
 				da_bno, dev_bno, ino);
 			goto error_out;
 		}
-		if (bp->b_error == -EFSBADCRC)
-			repair++;
 
 		leaf = bp->b_addr;
 		xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
@@ -1419,6 +1417,12 @@ process_leaf_attr_level(xfs_mount_t	*mp,
 		}
 
 		current_hashval = greatest_hashval;
+                /*
+		 * If block looks ok but CRC didn't match, make sure to
+		 * recompute it.
+		 */
+		if (!no_modify && bp->b_error == -EFSBADCRC)
+			repair++;
 
 		if (repair && !no_modify)
 			libxfs_writebuf(bp, 0);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 03/13] xfs_repair: make CRC checking consistent in path verification

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:01PM -0500, Eric Sandeen wrote:
> verify_da_path and verify_dir2_path both take steps to
> re-compute the CRC of the block if it otherwise looks
> ok and no other changes are needed.  They do this inside
> a loop, but the approach differs; verify_da_path expects
> its caller to check the first buffer prior to the loop,
> and verify_dir2_path expects its caller to check the last
> buffer after the loop.
> 
> Make this consistent by semi-arbitrarily choosing to make
> verify_da_path (and its caller) match the method used by
> verify_dir2_path, and check the last buffer after the
> loop is done.
> 

The code here seems Ok, but I don't think the commit log description
describes what the code is doing. I could also just have misread it.
This code is recursive and hairy enough as it is...

As I follow it, we init the cursor to the leftmost descendant in
traverse_int_dablock(). I don't see any crc verification in there. We
get into process_leaf_attr_level() and it reads and walks through the
leaf blocks, doing the crc check of each one. Each leaf block we verify
corresponds to an entry in the node block, so the verify_da_path() walks
the node entry index along until we slide over to the next node block.
At that point, we verify the crc of the current node buffer, write it
out and replace that level in the cursor with the next one. Eventually
we hit the end of the leaf block chain and call verify_final_da_path().

If I'm following all that correctly, it looks to me that before this
change we would have never verified the crc of the first node block. At
least, I can't find where that might happen. After this change, that
first node block crc is checked immediately before it's written out. But
now that the post-read check is gone, I don't see where the last node
block is crc-checked. Perhaps this should be checked in
verify_final_da_path()..?

Taking a quick look at the dir2 side, it looks like it checks the crc in
traverse_int_dir2block() and bails out on -EFSBADCRC (presumably to
rebuild the whole thing..?). As noted above, it does the pre-write crc
check and I don't see where it would check the final node block(s)
either. Hmm, this code is hairy enough it might be worth running through
a debugger or doing a targeted corruption to see if I'm missing
something...

Brian

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  repair/attr_repair.c |   24 ++++++++++++++----------
>  1 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index f29a5bd..aba0782 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -606,6 +606,14 @@ verify_da_path(xfs_mount_t	*mp,
>  		ASSERT(cursor->level[this_level].dirty == 0 ||
>  			(cursor->level[this_level].dirty && !no_modify));
>  
> +		/*
> +		 * If block looks ok but CRC didn't match, make sure to
> +		 * recompute it.
> +		 */
> +		if (!no_modify &&
> +		    cursor->level[this_level].bp->b_error == -EFSBADCRC)
> +			cursor->level[this_level].dirty = 1;
> +
>  		if (cursor->level[this_level].dirty && !no_modify)
>  			libxfs_writebuf(cursor->level[this_level].bp, 0);
>  		else
> @@ -618,14 +626,6 @@ verify_da_path(xfs_mount_t	*mp,
>  		cursor->level[this_level].hashval =
>  					be32_to_cpu(btree[0].hashval);
>  		entry = cursor->level[this_level].index = 0;
> -
> -		/*
> -		 * We want to rewrite the buffer on a CRC error seeing as it
> -		 * contains what appears to be a valid node block, but only if
> -		 * we are fixing errors.
> -		 */
> -		if (bp->b_error == -EFSBADCRC && !no_modify)
> -			cursor->level[this_level].dirty++;
>  	}
>  	/*
>  	 * ditto for block numbers
> @@ -1363,8 +1363,6 @@ process_leaf_attr_level(xfs_mount_t	*mp,
>  				da_bno, dev_bno, ino);
>  			goto error_out;
>  		}
> -		if (bp->b_error == -EFSBADCRC)
> -			repair++;
>  
>  		leaf = bp->b_addr;
>  		xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
> @@ -1419,6 +1417,12 @@ process_leaf_attr_level(xfs_mount_t	*mp,
>  		}
>  
>  		current_hashval = greatest_hashval;
> +                /*
> +		 * If block looks ok but CRC didn't match, make sure to
> +		 * recompute it.
> +		 */
> +		if (!no_modify && bp->b_error == -EFSBADCRC)
> +			repair++;
>  
>  		if (repair && !no_modify)
>  			libxfs_writebuf(bp, 0);
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 04/13] xfs_repair: use multibuffer read routines in attr_repair.c

[Eric Sandeen]

verify_da_path and traverse_int_dablock are similar to
verify_dir2_path and traverse_int_dir2block, but one
difference is that the dir2 code reads using the
multibuffer capable da_read_buf() routine, whereas
the attr code doesn't need to, and just calls
libxfs_readbuf.

The multibuffer code falls back just fine when the
geometry indicates that it's not needed, so use that
same code in the attribute routines, and remove
another dir2 / da difference.  We make da_read_buf()
non-static to facilitate this.

Finally, add a local *geo to these routines,
to make the code even more similar at this point.
The geometry will get passed in later in the series.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |   49 +++++++++++++++++++++++++++++++++----------------
 repair/dir2.c        |   22 +++++++++++++---------
 repair/dir2.h        |    8 ++++++++
 3 files changed, 54 insertions(+), 25 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index aba0782..fe81df4 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -138,14 +138,20 @@ traverse_int_dablock(xfs_mount_t	*mp,
 		xfs_dablk_t		*rbno,
 		int			whichfork)
 {
+	bmap_ext_t		*bmp;
 	xfs_dablk_t		bno;
 	int			i;
+	int			nex;
 	xfs_da_intnode_t	*node;
+	bmap_ext_t		lbmp;
 	xfs_fsblock_t		fsbno;
 	xfs_buf_t		*bp;
+	struct xfs_da_geometry	*geo;
 	struct xfs_da_node_entry *btree;
 	struct xfs_da3_icnode_hdr nodehdr;
 
+	geo = mp->m_attr_geo;
+
 	/*
 	 * traverse down left-side of tree until we hit the
 	 * left-most leaf block setting up the btree cursor along
@@ -160,13 +166,16 @@ traverse_int_dablock(xfs_mount_t	*mp,
 		/*
 		 * read in each block along the way and set up cursor
 		 */
-		fsbno = blkmap_get(da_cursor->blkmap, bno);
+		nex = blkmap_getn(da_cursor->blkmap, bno,
+				geo->fsbcount, &bmp, &lbmp);
 
-		if (fsbno == NULLFSBLOCK)
+		if (nex == 0)
 			goto error_out;
 
-		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
-				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
+		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
+		if (bmp != &lbmp)
+			free(bmp);
+
 		if (!bp) {
 			if (whichfork == XFS_DATA_FORK)
 				do_warn(
@@ -192,12 +201,10 @@ traverse_int_dablock(xfs_mount_t	*mp,
 			goto error_out;
 		}
 
-		if (nodehdr.count > mp->m_attr_geo->node_ents)  {
+		if (nodehdr.count > geo->node_ents)  {
 			do_warn(_("bad record count in inode %" PRIu64 ", "
 				  "count = %d, max = %d\n"),
-				da_cursor->ino,
-				nodehdr.count,
-				mp->m_attr_geo->node_ents);
+				da_cursor->ino, nodehdr.count, geo->node_ents);
 			libxfs_putbuf(bp);
 			goto error_out;
 		}
@@ -492,9 +499,15 @@ verify_da_path(xfs_mount_t	*mp,
 	int			bad;
 	int			entry;
 	int			this_level = p_level + 1;
+	bmap_ext_t		*bmp;
+	int			nex;
+	bmap_ext_t		lbmp;
+	struct xfs_da_geometry	*geo;
 	struct xfs_da_node_entry *btree;
 	struct xfs_da3_icnode_hdr nodehdr;
 
+	geo = mp->m_attr_geo;
+
 	/*
 	 * index is currently set to point to the entry that
 	 * should be processed now in this level.
@@ -536,17 +549,21 @@ verify_da_path(xfs_mount_t	*mp,
 		 */
 		dabno = nodehdr.forw;
 		ASSERT(dabno != 0);
-		fsbno = blkmap_get(cursor->blkmap, dabno);
-
-		if (fsbno == NULLFSBLOCK) {
-			do_warn(_("can't get map info for block %u "
-				  "of directory inode %" PRIu64 "\n"),
+		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
+			&bmp, &lbmp);
+		if (nex == 0) {
+			do_warn(
+_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
 				dabno, cursor->ino);
 			return(1);
 		}
 
-		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
-				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
+		fsbno = bmp[0].startblock;
+
+		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
+		if (bmp != &lbmp)
+			free(bmp);
+
 		if (!bp) {
 			do_warn(
 	_("can't read block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
@@ -577,7 +594,7 @@ verify_da_path(xfs_mount_t	*mp,
 				dabno, fsbno, cursor->ino);
 			bad++;
 		}
-		if (nodehdr.count > mp->m_attr_geo->node_ents) {
+		if (nodehdr.count > geo->node_ents) {
 			do_warn(
 	_("entry count %d too large in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
 				nodehdr.count,
diff --git a/repair/dir2.c b/repair/dir2.c
index 54c49eb..44367c6 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -92,7 +92,7 @@ namecheck(char *name, int length)
  * Multibuffer handling.
  * V2 directory blocks can be noncontiguous, needing multiple buffers.
  */
-static struct xfs_buf *
+struct xfs_buf *
 da_read_buf(
 	xfs_mount_t	*mp,
 	int		nex,
@@ -143,9 +143,12 @@ traverse_int_dir2block(xfs_mount_t	*mp,
 	int			nex;
 	xfs_da_intnode_t	*node;
 	bmap_ext_t		lbmp;
+	struct xfs_da_geometry	*geo;
 	struct xfs_da_node_entry *btree;
 	struct xfs_da3_icnode_hdr nodehdr;
 
+	geo = mp->m_dir_geo;
+
 	/*
 	 * traverse down left-side of tree until we hit the
 	 * left-most leaf block setting up the btree cursor along
@@ -161,7 +164,7 @@ traverse_int_dir2block(xfs_mount_t	*mp,
 		 * read in each block along the way and set up cursor
 		 */
 		nex = blkmap_getn(da_cursor->blkmap, bno,
-				mp->m_dir_geo->fsbcount, &bmp, &lbmp);
+				geo->fsbcount, &bmp, &lbmp);
 
 		if (nex == 0)
 			goto error_out;
@@ -207,13 +210,11 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
 			goto error_out;
 		}
 		btree = M_DIROPS(mp)->node_tree_p(node);
-		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
-			libxfs_putbuf(bp);
+		if (nodehdr.count > geo->node_ents)  {
 			do_warn(
 _("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
-				da_cursor->ino,
-				nodehdr.count,
-				mp->m_dir_geo->node_ents);
+				da_cursor->ino, nodehdr.count, geo->node_ents);
+			libxfs_putbuf(bp);
 			goto error_out;
 		}
 		/*
@@ -488,9 +489,12 @@ verify_dir2_path(xfs_mount_t	*mp,
 	bmap_ext_t		*bmp;
 	int			nex;
 	bmap_ext_t		lbmp;
+	struct xfs_da_geometry	*geo;
 	struct xfs_da_node_entry *btree;
 	struct xfs_da3_icnode_hdr nodehdr;
 
+	geo = mp->m_dir_geo;
+
 	/*
 	 * index is currently set to point to the entry that
 	 * should be processed now in this level.
@@ -532,7 +536,7 @@ verify_dir2_path(xfs_mount_t	*mp,
 		 */
 		dabno = nodehdr.forw;
 		ASSERT(dabno != 0);
-		nex = blkmap_getn(cursor->blkmap, dabno, mp->m_dir_geo->fsbcount,
+		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
 			&bmp, &lbmp);
 		if (nex == 0) {
 			do_warn(
@@ -574,7 +578,7 @@ _("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
 				dabno, cursor->ino);
 			bad++;
 		}
-		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
+		if (nodehdr.count > geo->node_ents)  {
 			do_warn(
 _("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
 				nodehdr.count,
diff --git a/repair/dir2.h b/repair/dir2.h
index 3cc1941..186633f 100644
--- a/repair/dir2.h
+++ b/repair/dir2.h
@@ -58,6 +58,14 @@ typedef struct dir2_bt_cursor  {
 	struct blkmap		*blkmap;
 } dir2_bt_cursor_t;
 
+#include "bmap.h"	/* Goes away in later refactoring */
+struct xfs_buf *
+da_read_buf(
+	xfs_mount_t	*mp,
+	int		nex,
+	bmap_ext_t	*bmp,
+	const struct xfs_buf_ops *ops);
+
 int
 process_dir2(
 	xfs_mount_t	*mp,
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 04/13] xfs_repair: use multibuffer read routines in attr_repair.c

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:02PM -0500, Eric Sandeen wrote:
> verify_da_path and traverse_int_dablock are similar to
> verify_dir2_path and traverse_int_dir2block, but one
> difference is that the dir2 code reads using the
> multibuffer capable da_read_buf() routine, whereas
> the attr code doesn't need to, and just calls
> libxfs_readbuf.
> 
> The multibuffer code falls back just fine when the
> geometry indicates that it's not needed, so use that
> same code in the attribute routines, and remove
> another dir2 / da difference.  We make da_read_buf()
> non-static to facilitate this.
> 
> Finally, add a local *geo to these routines,
> to make the code even more similar at this point.
> The geometry will get passed in later in the series.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  repair/attr_repair.c |   49 +++++++++++++++++++++++++++++++++----------------
>  repair/dir2.c        |   22 +++++++++++++---------
>  repair/dir2.h        |    8 ++++++++
>  3 files changed, 54 insertions(+), 25 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index aba0782..fe81df4 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -138,14 +138,20 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  		xfs_dablk_t		*rbno,
>  		int			whichfork)
>  {
> +	bmap_ext_t		*bmp;

struct bmap_ext

(and several other places below)

>  	xfs_dablk_t		bno;
>  	int			i;
> +	int			nex;
>  	xfs_da_intnode_t	*node;
> +	bmap_ext_t		lbmp;
>  	xfs_fsblock_t		fsbno;
>  	xfs_buf_t		*bp;
> +	struct xfs_da_geometry	*geo;
>  	struct xfs_da_node_entry *btree;
>  	struct xfs_da3_icnode_hdr nodehdr;
>  
> +	geo = mp->m_attr_geo;
> +
>  	/*
>  	 * traverse down left-side of tree until we hit the
>  	 * left-most leaf block setting up the btree cursor along
> @@ -160,13 +166,16 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  		/*
>  		 * read in each block along the way and set up cursor
>  		 */
> -		fsbno = blkmap_get(da_cursor->blkmap, bno);
> +		nex = blkmap_getn(da_cursor->blkmap, bno,
> +				geo->fsbcount, &bmp, &lbmp);
>  

...
attr_repair.c: In function ‘process_attributes’:
attr_repair.c:185:5: warning: ‘fsbno’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     do_warn(
...

Brian

> -		if (fsbno == NULLFSBLOCK)
> +		if (nex == 0)
>  			goto error_out;
>  
> -		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
> -				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
> +		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
> +		if (bmp != &lbmp)
> +			free(bmp);
> +
>  		if (!bp) {
>  			if (whichfork == XFS_DATA_FORK)
>  				do_warn(
> @@ -192,12 +201,10 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  			goto error_out;
>  		}
>  
> -		if (nodehdr.count > mp->m_attr_geo->node_ents)  {
> +		if (nodehdr.count > geo->node_ents)  {
>  			do_warn(_("bad record count in inode %" PRIu64 ", "
>  				  "count = %d, max = %d\n"),
> -				da_cursor->ino,
> -				nodehdr.count,
> -				mp->m_attr_geo->node_ents);
> +				da_cursor->ino, nodehdr.count, geo->node_ents);
>  			libxfs_putbuf(bp);
>  			goto error_out;
>  		}
> @@ -492,9 +499,15 @@ verify_da_path(xfs_mount_t	*mp,
>  	int			bad;
>  	int			entry;
>  	int			this_level = p_level + 1;
> +	bmap_ext_t		*bmp;
> +	int			nex;
> +	bmap_ext_t		lbmp;
> +	struct xfs_da_geometry	*geo;
>  	struct xfs_da_node_entry *btree;
>  	struct xfs_da3_icnode_hdr nodehdr;
>  
> +	geo = mp->m_attr_geo;
> +
>  	/*
>  	 * index is currently set to point to the entry that
>  	 * should be processed now in this level.
> @@ -536,17 +549,21 @@ verify_da_path(xfs_mount_t	*mp,
>  		 */
>  		dabno = nodehdr.forw;
>  		ASSERT(dabno != 0);
> -		fsbno = blkmap_get(cursor->blkmap, dabno);
> -
> -		if (fsbno == NULLFSBLOCK) {
> -			do_warn(_("can't get map info for block %u "
> -				  "of directory inode %" PRIu64 "\n"),
> +		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
> +			&bmp, &lbmp);
> +		if (nex == 0) {
> +			do_warn(
> +_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  				dabno, cursor->ino);
>  			return(1);
>  		}
>  
> -		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
> -				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
> +		fsbno = bmp[0].startblock;
> +
> +		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
> +		if (bmp != &lbmp)
> +			free(bmp);
> +
>  		if (!bp) {
>  			do_warn(
>  	_("can't read block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
> @@ -577,7 +594,7 @@ verify_da_path(xfs_mount_t	*mp,
>  				dabno, fsbno, cursor->ino);
>  			bad++;
>  		}
> -		if (nodehdr.count > mp->m_attr_geo->node_ents) {
> +		if (nodehdr.count > geo->node_ents) {
>  			do_warn(
>  	_("entry count %d too large in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
>  				nodehdr.count,
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 54c49eb..44367c6 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -92,7 +92,7 @@ namecheck(char *name, int length)
>   * Multibuffer handling.
>   * V2 directory blocks can be noncontiguous, needing multiple buffers.
>   */
> -static struct xfs_buf *
> +struct xfs_buf *
>  da_read_buf(
>  	xfs_mount_t	*mp,
>  	int		nex,
> @@ -143,9 +143,12 @@ traverse_int_dir2block(xfs_mount_t	*mp,
>  	int			nex;
>  	xfs_da_intnode_t	*node;
>  	bmap_ext_t		lbmp;
> +	struct xfs_da_geometry	*geo;
>  	struct xfs_da_node_entry *btree;
>  	struct xfs_da3_icnode_hdr nodehdr;
>  
> +	geo = mp->m_dir_geo;
> +
>  	/*
>  	 * traverse down left-side of tree until we hit the
>  	 * left-most leaf block setting up the btree cursor along
> @@ -161,7 +164,7 @@ traverse_int_dir2block(xfs_mount_t	*mp,
>  		 * read in each block along the way and set up cursor
>  		 */
>  		nex = blkmap_getn(da_cursor->blkmap, bno,
> -				mp->m_dir_geo->fsbcount, &bmp, &lbmp);
> +				geo->fsbcount, &bmp, &lbmp);
>  
>  		if (nex == 0)
>  			goto error_out;
> @@ -207,13 +210,11 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
>  			goto error_out;
>  		}
>  		btree = M_DIROPS(mp)->node_tree_p(node);
> -		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
> -			libxfs_putbuf(bp);
> +		if (nodehdr.count > geo->node_ents)  {
>  			do_warn(
>  _("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
> -				da_cursor->ino,
> -				nodehdr.count,
> -				mp->m_dir_geo->node_ents);
> +				da_cursor->ino, nodehdr.count, geo->node_ents);
> +			libxfs_putbuf(bp);
>  			goto error_out;
>  		}
>  		/*
> @@ -488,9 +489,12 @@ verify_dir2_path(xfs_mount_t	*mp,
>  	bmap_ext_t		*bmp;
>  	int			nex;
>  	bmap_ext_t		lbmp;
> +	struct xfs_da_geometry	*geo;
>  	struct xfs_da_node_entry *btree;
>  	struct xfs_da3_icnode_hdr nodehdr;
>  
> +	geo = mp->m_dir_geo;
> +
>  	/*
>  	 * index is currently set to point to the entry that
>  	 * should be processed now in this level.
> @@ -532,7 +536,7 @@ verify_dir2_path(xfs_mount_t	*mp,
>  		 */
>  		dabno = nodehdr.forw;
>  		ASSERT(dabno != 0);
> -		nex = blkmap_getn(cursor->blkmap, dabno, mp->m_dir_geo->fsbcount,
> +		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
>  			&bmp, &lbmp);
>  		if (nex == 0) {
>  			do_warn(
> @@ -574,7 +578,7 @@ _("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
>  				dabno, cursor->ino);
>  			bad++;
>  		}
> -		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
> +		if (nodehdr.count > geo->node_ents)  {
>  			do_warn(
>  _("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
>  				nodehdr.count,
> diff --git a/repair/dir2.h b/repair/dir2.h
> index 3cc1941..186633f 100644
> --- a/repair/dir2.h
> +++ b/repair/dir2.h
> @@ -58,6 +58,14 @@ typedef struct dir2_bt_cursor  {
>  	struct blkmap		*blkmap;
>  } dir2_bt_cursor_t;
>  
> +#include "bmap.h"	/* Goes away in later refactoring */
> +struct xfs_buf *
> +da_read_buf(
> +	xfs_mount_t	*mp,
> +	int		nex,
> +	bmap_ext_t	*bmp,
> +	const struct xfs_buf_ops *ops);
> +
>  int
>  process_dir2(
>  	xfs_mount_t	*mp,
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 04/13] xfs_repair: use multibuffer read routines in attr_repair.c

[Eric Sandeen]

On 9/14/15 2:18 PM, Brian Foster wrote:
> On Wed, Sep 09, 2015 at 02:34:02PM -0500, Eric Sandeen wrote:
>> verify_da_path and traverse_int_dablock are similar to
>> verify_dir2_path and traverse_int_dir2block, but one
>> difference is that the dir2 code reads using the
>> multibuffer capable da_read_buf() routine, whereas
>> the attr code doesn't need to, and just calls
>> libxfs_readbuf.
>>
>> The multibuffer code falls back just fine when the
>> geometry indicates that it's not needed, so use that
>> same code in the attribute routines, and remove
>> another dir2 / da difference.  We make da_read_buf()
>> non-static to facilitate this.
>>
>> Finally, add a local *geo to these routines,
>> to make the code even more similar at this point.
>> The geometry will get passed in later in the series.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>> ---
>>  repair/attr_repair.c |   49 +++++++++++++++++++++++++++++++++----------------
>>  repair/dir2.c        |   22 +++++++++++++---------
>>  repair/dir2.h        |    8 ++++++++
>>  3 files changed, 54 insertions(+), 25 deletions(-)
>>
>> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
>> index aba0782..fe81df4 100644
>> --- a/repair/attr_repair.c
>> +++ b/repair/attr_repair.c
>> @@ -138,14 +138,20 @@ traverse_int_dablock(xfs_mount_t	*mp,
>>  		xfs_dablk_t		*rbno,
>>  		int			whichfork)
>>  {
>> +	bmap_ext_t		*bmp;

Well, yeah - I thought about that.  The goal was to make it as close
as possible to the other code, so I decided against the struct foo's.

I could do a "get rid of typedefs" as the last patch to the new util
code if you like.

> struct bmap_ext
> 
> (and several other places below)
> 
>>  	xfs_dablk_t		bno;
>>  	int			i;
>> +	int			nex;
>>  	xfs_da_intnode_t	*node;
>> +	bmap_ext_t		lbmp;
>>  	xfs_fsblock_t		fsbno;
>>  	xfs_buf_t		*bp;
>> +	struct xfs_da_geometry	*geo;
>>  	struct xfs_da_node_entry *btree;
>>  	struct xfs_da3_icnode_hdr nodehdr;
>>  
>> +	geo = mp->m_attr_geo;
>> +
>>  	/*
>>  	 * traverse down left-side of tree until we hit the
>>  	 * left-most leaf block setting up the btree cursor along
>> @@ -160,13 +166,16 @@ traverse_int_dablock(xfs_mount_t	*mp,
>>  		/*
>>  		 * read in each block along the way and set up cursor
>>  		 */
>> -		fsbno = blkmap_get(da_cursor->blkmap, bno);
>> +		nex = blkmap_getn(da_cursor->blkmap, bno,
>> +				geo->fsbcount, &bmp, &lbmp);
>>  
> 
> ...
> attr_repair.c: In function ‘process_attributes’:
> attr_repair.c:185:5: warning: ‘fsbno’ may be used uninitialized in this function [-Wmaybe-uninitialized]

hm, whoops.  Well, it goes away in the long run, but I can still fix
that up.

Thanks,
-Eric

>      do_warn(
> ...
> 
> Brian
> 
>> -		if (fsbno == NULLFSBLOCK)
>> +		if (nex == 0)
>>  			goto error_out;
>>  
>> -		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
>> -				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
>> +		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
>> +		if (bmp != &lbmp)
>> +			free(bmp);
>> +
>>  		if (!bp) {
>>  			if (whichfork == XFS_DATA_FORK)
>>  				do_warn(
>> @@ -192,12 +201,10 @@ traverse_int_dablock(xfs_mount_t	*mp,
>>  			goto error_out;
>>  		}
>>  
>> -		if (nodehdr.count > mp->m_attr_geo->node_ents)  {
>> +		if (nodehdr.count > geo->node_ents)  {
>>  			do_warn(_("bad record count in inode %" PRIu64 ", "
>>  				  "count = %d, max = %d\n"),
>> -				da_cursor->ino,
>> -				nodehdr.count,
>> -				mp->m_attr_geo->node_ents);
>> +				da_cursor->ino, nodehdr.count, geo->node_ents);
>>  			libxfs_putbuf(bp);
>>  			goto error_out;
>>  		}
>> @@ -492,9 +499,15 @@ verify_da_path(xfs_mount_t	*mp,
>>  	int			bad;
>>  	int			entry;
>>  	int			this_level = p_level + 1;
>> +	bmap_ext_t		*bmp;
>> +	int			nex;
>> +	bmap_ext_t		lbmp;
>> +	struct xfs_da_geometry	*geo;
>>  	struct xfs_da_node_entry *btree;
>>  	struct xfs_da3_icnode_hdr nodehdr;
>>  
>> +	geo = mp->m_attr_geo;
>> +
>>  	/*
>>  	 * index is currently set to point to the entry that
>>  	 * should be processed now in this level.
>> @@ -536,17 +549,21 @@ verify_da_path(xfs_mount_t	*mp,
>>  		 */
>>  		dabno = nodehdr.forw;
>>  		ASSERT(dabno != 0);
>> -		fsbno = blkmap_get(cursor->blkmap, dabno);
>> -
>> -		if (fsbno == NULLFSBLOCK) {
>> -			do_warn(_("can't get map info for block %u "
>> -				  "of directory inode %" PRIu64 "\n"),
>> +		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
>> +			&bmp, &lbmp);
>> +		if (nex == 0) {
>> +			do_warn(
>> +_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>>  				dabno, cursor->ino);
>>  			return(1);
>>  		}
>>  
>> -		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
>> -				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
>> +		fsbno = bmp[0].startblock;
>> +
>> +		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
>> +		if (bmp != &lbmp)
>> +			free(bmp);
>> +
>>  		if (!bp) {
>>  			do_warn(
>>  	_("can't read block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
>> @@ -577,7 +594,7 @@ verify_da_path(xfs_mount_t	*mp,
>>  				dabno, fsbno, cursor->ino);
>>  			bad++;
>>  		}
>> -		if (nodehdr.count > mp->m_attr_geo->node_ents) {
>> +		if (nodehdr.count > geo->node_ents) {
>>  			do_warn(
>>  	_("entry count %d too large in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
>>  				nodehdr.count,
>> diff --git a/repair/dir2.c b/repair/dir2.c
>> index 54c49eb..44367c6 100644
>> --- a/repair/dir2.c
>> +++ b/repair/dir2.c
>> @@ -92,7 +92,7 @@ namecheck(char *name, int length)
>>   * Multibuffer handling.
>>   * V2 directory blocks can be noncontiguous, needing multiple buffers.
>>   */
>> -static struct xfs_buf *
>> +struct xfs_buf *
>>  da_read_buf(
>>  	xfs_mount_t	*mp,
>>  	int		nex,
>> @@ -143,9 +143,12 @@ traverse_int_dir2block(xfs_mount_t	*mp,
>>  	int			nex;
>>  	xfs_da_intnode_t	*node;
>>  	bmap_ext_t		lbmp;
>> +	struct xfs_da_geometry	*geo;
>>  	struct xfs_da_node_entry *btree;
>>  	struct xfs_da3_icnode_hdr nodehdr;
>>  
>> +	geo = mp->m_dir_geo;
>> +
>>  	/*
>>  	 * traverse down left-side of tree until we hit the
>>  	 * left-most leaf block setting up the btree cursor along
>> @@ -161,7 +164,7 @@ traverse_int_dir2block(xfs_mount_t	*mp,
>>  		 * read in each block along the way and set up cursor
>>  		 */
>>  		nex = blkmap_getn(da_cursor->blkmap, bno,
>> -				mp->m_dir_geo->fsbcount, &bmp, &lbmp);
>> +				geo->fsbcount, &bmp, &lbmp);
>>  
>>  		if (nex == 0)
>>  			goto error_out;
>> @@ -207,13 +210,11 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
>>  			goto error_out;
>>  		}
>>  		btree = M_DIROPS(mp)->node_tree_p(node);
>> -		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
>> -			libxfs_putbuf(bp);
>> +		if (nodehdr.count > geo->node_ents)  {
>>  			do_warn(
>>  _("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
>> -				da_cursor->ino,
>> -				nodehdr.count,
>> -				mp->m_dir_geo->node_ents);
>> +				da_cursor->ino, nodehdr.count, geo->node_ents);
>> +			libxfs_putbuf(bp);
>>  			goto error_out;
>>  		}
>>  		/*
>> @@ -488,9 +489,12 @@ verify_dir2_path(xfs_mount_t	*mp,
>>  	bmap_ext_t		*bmp;
>>  	int			nex;
>>  	bmap_ext_t		lbmp;
>> +	struct xfs_da_geometry	*geo;
>>  	struct xfs_da_node_entry *btree;
>>  	struct xfs_da3_icnode_hdr nodehdr;
>>  
>> +	geo = mp->m_dir_geo;
>> +
>>  	/*
>>  	 * index is currently set to point to the entry that
>>  	 * should be processed now in this level.
>> @@ -532,7 +536,7 @@ verify_dir2_path(xfs_mount_t	*mp,
>>  		 */
>>  		dabno = nodehdr.forw;
>>  		ASSERT(dabno != 0);
>> -		nex = blkmap_getn(cursor->blkmap, dabno, mp->m_dir_geo->fsbcount,
>> +		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
>>  			&bmp, &lbmp);
>>  		if (nex == 0) {
>>  			do_warn(
>> @@ -574,7 +578,7 @@ _("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
>>  				dabno, cursor->ino);
>>  			bad++;
>>  		}
>> -		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
>> +		if (nodehdr.count > geo->node_ents)  {
>>  			do_warn(
>>  _("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
>>  				nodehdr.count,
>> diff --git a/repair/dir2.h b/repair/dir2.h
>> index 3cc1941..186633f 100644
>> --- a/repair/dir2.h
>> +++ b/repair/dir2.h
>> @@ -58,6 +58,14 @@ typedef struct dir2_bt_cursor  {
>>  	struct blkmap		*blkmap;
>>  } dir2_bt_cursor_t;
>>  
>> +#include "bmap.h"	/* Goes away in later refactoring */
>> +struct xfs_buf *
>> +da_read_buf(
>> +	xfs_mount_t	*mp,
>> +	int		nex,
>> +	bmap_ext_t	*bmp,
>> +	const struct xfs_buf_ops *ops);
>> +
>>  int
>>  process_dir2(
>>  	xfs_mount_t	*mp,
>> -- 
>> 1.7.1
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 04/13] xfs_repair: use multibuffer read routines in attr_repair.c

[Brian Foster]

On Mon, Sep 14, 2015 at 02:24:53PM -0500, Eric Sandeen wrote:
> 
> 
> On 9/14/15 2:18 PM, Brian Foster wrote:
> > On Wed, Sep 09, 2015 at 02:34:02PM -0500, Eric Sandeen wrote:
> >> verify_da_path and traverse_int_dablock are similar to
> >> verify_dir2_path and traverse_int_dir2block, but one
> >> difference is that the dir2 code reads using the
> >> multibuffer capable da_read_buf() routine, whereas
> >> the attr code doesn't need to, and just calls
> >> libxfs_readbuf.
> >>
> >> The multibuffer code falls back just fine when the
> >> geometry indicates that it's not needed, so use that
> >> same code in the attribute routines, and remove
> >> another dir2 / da difference.  We make da_read_buf()
> >> non-static to facilitate this.
> >>
> >> Finally, add a local *geo to these routines,
> >> to make the code even more similar at this point.
> >> The geometry will get passed in later in the series.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> >> ---
> >>  repair/attr_repair.c |   49 +++++++++++++++++++++++++++++++++----------------
> >>  repair/dir2.c        |   22 +++++++++++++---------
> >>  repair/dir2.h        |    8 ++++++++
> >>  3 files changed, 54 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> >> index aba0782..fe81df4 100644
> >> --- a/repair/attr_repair.c
> >> +++ b/repair/attr_repair.c
> >> @@ -138,14 +138,20 @@ traverse_int_dablock(xfs_mount_t	*mp,
> >>  		xfs_dablk_t		*rbno,
> >>  		int			whichfork)
> >>  {
> >> +	bmap_ext_t		*bmp;
> 
> Well, yeah - I thought about that.  The goal was to make it as close
> as possible to the other code, so I decided against the struct foo's.
> 
> I could do a "get rid of typedefs" as the last patch to the new util
> code if you like.
> 

Ah, right. Don't worry about this then.

> > struct bmap_ext
> > 
> > (and several other places below)
> > 
> >>  	xfs_dablk_t		bno;
> >>  	int			i;
> >> +	int			nex;
> >>  	xfs_da_intnode_t	*node;
> >> +	bmap_ext_t		lbmp;
> >>  	xfs_fsblock_t		fsbno;
> >>  	xfs_buf_t		*bp;
> >> +	struct xfs_da_geometry	*geo;
> >>  	struct xfs_da_node_entry *btree;
> >>  	struct xfs_da3_icnode_hdr nodehdr;
> >>  
> >> +	geo = mp->m_attr_geo;
> >> +
> >>  	/*
> >>  	 * traverse down left-side of tree until we hit the
> >>  	 * left-most leaf block setting up the btree cursor along
> >> @@ -160,13 +166,16 @@ traverse_int_dablock(xfs_mount_t	*mp,
> >>  		/*
> >>  		 * read in each block along the way and set up cursor
> >>  		 */
> >> -		fsbno = blkmap_get(da_cursor->blkmap, bno);
> >> +		nex = blkmap_getn(da_cursor->blkmap, bno,
> >> +				geo->fsbcount, &bmp, &lbmp);
> >>  
> > 
> > ...
> > attr_repair.c: In function ‘process_attributes’:
> > attr_repair.c:185:5: warning: ‘fsbno’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> hm, whoops.  Well, it goes away in the long run, but I can still fix
> that up.
> 

I figured it might (haven't got that far yet), but I'd still rather not
have regressions in the history if we can help it, even if transient.

Brian

> Thanks,
> -Eric
> 
> >      do_warn(
> > ...
> > 
> > Brian
> > 
> >> -		if (fsbno == NULLFSBLOCK)
> >> +		if (nex == 0)
> >>  			goto error_out;
> >>  
> >> -		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
> >> -				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
> >> +		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
> >> +		if (bmp != &lbmp)
> >> +			free(bmp);
> >> +
> >>  		if (!bp) {
> >>  			if (whichfork == XFS_DATA_FORK)
> >>  				do_warn(
> >> @@ -192,12 +201,10 @@ traverse_int_dablock(xfs_mount_t	*mp,
> >>  			goto error_out;
> >>  		}
> >>  
> >> -		if (nodehdr.count > mp->m_attr_geo->node_ents)  {
> >> +		if (nodehdr.count > geo->node_ents)  {
> >>  			do_warn(_("bad record count in inode %" PRIu64 ", "
> >>  				  "count = %d, max = %d\n"),
> >> -				da_cursor->ino,
> >> -				nodehdr.count,
> >> -				mp->m_attr_geo->node_ents);
> >> +				da_cursor->ino, nodehdr.count, geo->node_ents);
> >>  			libxfs_putbuf(bp);
> >>  			goto error_out;
> >>  		}
> >> @@ -492,9 +499,15 @@ verify_da_path(xfs_mount_t	*mp,
> >>  	int			bad;
> >>  	int			entry;
> >>  	int			this_level = p_level + 1;
> >> +	bmap_ext_t		*bmp;
> >> +	int			nex;
> >> +	bmap_ext_t		lbmp;
> >> +	struct xfs_da_geometry	*geo;
> >>  	struct xfs_da_node_entry *btree;
> >>  	struct xfs_da3_icnode_hdr nodehdr;
> >>  
> >> +	geo = mp->m_attr_geo;
> >> +
> >>  	/*
> >>  	 * index is currently set to point to the entry that
> >>  	 * should be processed now in this level.
> >> @@ -536,17 +549,21 @@ verify_da_path(xfs_mount_t	*mp,
> >>  		 */
> >>  		dabno = nodehdr.forw;
> >>  		ASSERT(dabno != 0);
> >> -		fsbno = blkmap_get(cursor->blkmap, dabno);
> >> -
> >> -		if (fsbno == NULLFSBLOCK) {
> >> -			do_warn(_("can't get map info for block %u "
> >> -				  "of directory inode %" PRIu64 "\n"),
> >> +		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
> >> +			&bmp, &lbmp);
> >> +		if (nex == 0) {
> >> +			do_warn(
> >> +_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
> >>  				dabno, cursor->ino);
> >>  			return(1);
> >>  		}
> >>  
> >> -		bp = libxfs_readbuf(mp->m_dev, XFS_FSB_TO_DADDR(mp, fsbno),
> >> -				XFS_FSB_TO_BB(mp, 1), 0, &xfs_da3_node_buf_ops);
> >> +		fsbno = bmp[0].startblock;
> >> +
> >> +		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
> >> +		if (bmp != &lbmp)
> >> +			free(bmp);
> >> +
> >>  		if (!bp) {
> >>  			do_warn(
> >>  	_("can't read block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
> >> @@ -577,7 +594,7 @@ verify_da_path(xfs_mount_t	*mp,
> >>  				dabno, fsbno, cursor->ino);
> >>  			bad++;
> >>  		}
> >> -		if (nodehdr.count > mp->m_attr_geo->node_ents) {
> >> +		if (nodehdr.count > geo->node_ents) {
> >>  			do_warn(
> >>  	_("entry count %d too large in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
> >>  				nodehdr.count,
> >> diff --git a/repair/dir2.c b/repair/dir2.c
> >> index 54c49eb..44367c6 100644
> >> --- a/repair/dir2.c
> >> +++ b/repair/dir2.c
> >> @@ -92,7 +92,7 @@ namecheck(char *name, int length)
> >>   * Multibuffer handling.
> >>   * V2 directory blocks can be noncontiguous, needing multiple buffers.
> >>   */
> >> -static struct xfs_buf *
> >> +struct xfs_buf *
> >>  da_read_buf(
> >>  	xfs_mount_t	*mp,
> >>  	int		nex,
> >> @@ -143,9 +143,12 @@ traverse_int_dir2block(xfs_mount_t	*mp,
> >>  	int			nex;
> >>  	xfs_da_intnode_t	*node;
> >>  	bmap_ext_t		lbmp;
> >> +	struct xfs_da_geometry	*geo;
> >>  	struct xfs_da_node_entry *btree;
> >>  	struct xfs_da3_icnode_hdr nodehdr;
> >>  
> >> +	geo = mp->m_dir_geo;
> >> +
> >>  	/*
> >>  	 * traverse down left-side of tree until we hit the
> >>  	 * left-most leaf block setting up the btree cursor along
> >> @@ -161,7 +164,7 @@ traverse_int_dir2block(xfs_mount_t	*mp,
> >>  		 * read in each block along the way and set up cursor
> >>  		 */
> >>  		nex = blkmap_getn(da_cursor->blkmap, bno,
> >> -				mp->m_dir_geo->fsbcount, &bmp, &lbmp);
> >> +				geo->fsbcount, &bmp, &lbmp);
> >>  
> >>  		if (nex == 0)
> >>  			goto error_out;
> >> @@ -207,13 +210,11 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
> >>  			goto error_out;
> >>  		}
> >>  		btree = M_DIROPS(mp)->node_tree_p(node);
> >> -		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
> >> -			libxfs_putbuf(bp);
> >> +		if (nodehdr.count > geo->node_ents)  {
> >>  			do_warn(
> >>  _("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
> >> -				da_cursor->ino,
> >> -				nodehdr.count,
> >> -				mp->m_dir_geo->node_ents);
> >> +				da_cursor->ino, nodehdr.count, geo->node_ents);
> >> +			libxfs_putbuf(bp);
> >>  			goto error_out;
> >>  		}
> >>  		/*
> >> @@ -488,9 +489,12 @@ verify_dir2_path(xfs_mount_t	*mp,
> >>  	bmap_ext_t		*bmp;
> >>  	int			nex;
> >>  	bmap_ext_t		lbmp;
> >> +	struct xfs_da_geometry	*geo;
> >>  	struct xfs_da_node_entry *btree;
> >>  	struct xfs_da3_icnode_hdr nodehdr;
> >>  
> >> +	geo = mp->m_dir_geo;
> >> +
> >>  	/*
> >>  	 * index is currently set to point to the entry that
> >>  	 * should be processed now in this level.
> >> @@ -532,7 +536,7 @@ verify_dir2_path(xfs_mount_t	*mp,
> >>  		 */
> >>  		dabno = nodehdr.forw;
> >>  		ASSERT(dabno != 0);
> >> -		nex = blkmap_getn(cursor->blkmap, dabno, mp->m_dir_geo->fsbcount,
> >> +		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
> >>  			&bmp, &lbmp);
> >>  		if (nex == 0) {
> >>  			do_warn(
> >> @@ -574,7 +578,7 @@ _("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
> >>  				dabno, cursor->ino);
> >>  			bad++;
> >>  		}
> >> -		if (nodehdr.count > mp->m_dir_geo->node_ents)  {
> >> +		if (nodehdr.count > geo->node_ents)  {
> >>  			do_warn(
> >>  _("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
> >>  				nodehdr.count,
> >> diff --git a/repair/dir2.h b/repair/dir2.h
> >> index 3cc1941..186633f 100644
> >> --- a/repair/dir2.h
> >> +++ b/repair/dir2.h
> >> @@ -58,6 +58,14 @@ typedef struct dir2_bt_cursor  {
> >>  	struct blkmap		*blkmap;
> >>  } dir2_bt_cursor_t;
> >>  
> >> +#include "bmap.h"	/* Goes away in later refactoring */
> >> +struct xfs_buf *
> >> +da_read_buf(
> >> +	xfs_mount_t	*mp,
> >> +	int		nex,
> >> +	bmap_ext_t	*bmp,
> >> +	const struct xfs_buf_ops *ops);
> >> +
> >>  int
> >>  process_dir2(
> >>  	xfs_mount_t	*mp,
> >> -- 
> >> 1.7.1
> >>
> >> _______________________________________________
> >> xfs mailing list
> >> xfs@oss.sgi.com
> >> http://oss.sgi.com/mailman/listinfo/xfs
> > 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 05/13] xfs_repair: fix use-after-free in verify_final_dir2_path

[Eric Sandeen]

Way back in 2002, commit 948ce18 fixed a potential use-after-free
in verify_final_da_path, but the same fix was not applied to
verify_final_dir2_path; apply it now.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/dir2.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/repair/dir2.c b/repair/dir2.c
index 44367c6..898b27e 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -330,6 +330,7 @@ verify_final_dir2_path(xfs_mount_t	*mp,
 		const int		p_level)
 {
 	xfs_da_intnode_t	*node;
+	xfs_dahash_t		hashval;
 	int			bad = 0;
 	int			entry;
 	int			this_level = p_level + 1;
@@ -409,6 +410,12 @@ _("would correct bad hashval in non-leaf dir block\n"
 	}
 
 	/*
+	 * Note: squirrel hashval away _before_ releasing the
+	 * buffer, preventing a use-after-free problem.
+	 */
+	hashval = be32_to_cpu(btree[entry].hashval);
+
+	/*
 	 * release/write buffer
 	 */
 	ASSERT(cursor->level[this_level].dirty == 0 ||
@@ -430,7 +437,7 @@ _("would correct bad hashval in non-leaf dir block\n"
 	 * set hashvalue to correctl reflect the now-validated
 	 * last entry in this block and continue upwards validation
 	 */
-	cursor->level[this_level].hashval = be32_to_cpu(btree[entry].hashval);
+	cursor->level[this_level].hashval = hashval;
 
 	return(verify_final_dir2_path(mp, cursor, this_level));
 }
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 05/13] xfs_repair: fix use-after-free in verify_final_dir2_path

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:03PM -0500, Eric Sandeen wrote:
> Way back in 2002, commit 948ce18 fixed a potential use-after-free
> in verify_final_da_path, but the same fix was not applied to
> verify_final_dir2_path; apply it now.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  repair/dir2.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 44367c6..898b27e 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -330,6 +330,7 @@ verify_final_dir2_path(xfs_mount_t	*mp,
>  		const int		p_level)
>  {
>  	xfs_da_intnode_t	*node;
> +	xfs_dahash_t		hashval;
>  	int			bad = 0;
>  	int			entry;
>  	int			this_level = p_level + 1;
> @@ -409,6 +410,12 @@ _("would correct bad hashval in non-leaf dir block\n"
>  	}
>  
>  	/*
> +	 * Note: squirrel hashval away _before_ releasing the
> +	 * buffer, preventing a use-after-free problem.
> +	 */
> +	hashval = be32_to_cpu(btree[entry].hashval);
> +
> +	/*
>  	 * release/write buffer
>  	 */
>  	ASSERT(cursor->level[this_level].dirty == 0 ||
> @@ -430,7 +437,7 @@ _("would correct bad hashval in non-leaf dir block\n"
>  	 * set hashvalue to correctl reflect the now-validated
>  	 * last entry in this block and continue upwards validation
>  	 */
> -	cursor->level[this_level].hashval = be32_to_cpu(btree[entry].hashval);
> +	cursor->level[this_level].hashval = hashval;
>  
>  	return(verify_final_dir2_path(mp, cursor, this_level));
>  }
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 06/13] xfs_repair: add XR_DIR_TRACE to dir2.c

[Eric Sandeen]

attr_repair.c has many printf-tracepoints under
#ifdef XR_DIR_TRACE, but the similar code in dir2.c does not.

Add these same tracepoints to remove more differences between
these two pieces of code.

Not all messages are quite correct; those will be fixed up last.
For now we just make the code more obviously similar.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/dir2.c |   39 ++++++++++++++++++++++++++++++++++++---
 1 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/repair/dir2.c b/repair/dir2.c
index 898b27e..d9bd5fc 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -337,6 +337,11 @@ verify_final_dir2_path(xfs_mount_t	*mp,
 	struct xfs_da_node_entry *btree;
 	struct xfs_da3_icnode_hdr nodehdr;
 
+#ifdef XR_DIR_TRACE
+	fprintf(stderr, "in verify_final_dir2_path, this_level = %d\n",
+		this_level);
+#endif
+
 	/*
 	 * the index should point to the next "unprocessed" entry
 	 * in the block which should be the final (rightmost) entry
@@ -388,8 +393,17 @@ verify_final_dir2_path(xfs_mount_t	*mp,
 	/*
 	 * ok, now check descendant block number against this level
 	 */
-	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before))
+	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
+#ifdef XR_DIR_TRACE
+		fprintf(stderr, "bad directory btree pointer, child bno should "
+				"be %d, block bno is %d, hashval is %u\n",
+			be16_to_cpu(btree[entry].before),
+			cursor->level[p_level].bno,
+			cursor->level[p_level].hashval);
+		fprintf(stderr, "verify_final_dir2_path returns 1 (bad) #1a\n");
+#endif
 		return(1);
+	}
 
 	if (cursor->level[p_level].hashval !=
 				be32_to_cpu(btree[entry].hashval))  {
@@ -431,8 +445,12 @@ _("would correct bad hashval in non-leaf dir block\n"
 	/*
 	 * bail out if this is the root block (top of tree)
 	 */
-	if (this_level >= cursor->active)
+	if (this_level >= cursor->active) {
+#ifdef XR_DIR_TRACE
+		fprintf(stderr, "verify_final_dir2_path returns 0 (ok)\n");
+#endif
 		return(0);
+	}
 	/*
 	 * set hashvalue to correctl reflect the now-validated
 	 * last entry in this block and continue upwards validation
@@ -600,6 +618,9 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
 			bad++;
 		}
 		if (bad)  {
+#ifdef XR_DIR_TRACE
+			fprintf(stderr, "verify_dir2_path returns 1 (bad) #4\n");
+#endif
 			libxfs_putbuf(bp);
 			return(1);
 		}
@@ -631,8 +652,17 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
 	/*
 	 * ditto for block numbers
 	 */
-	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before))
+	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
+#ifdef XR_DIR_TRACE
+		fprintf(stderr, "bad directory btree pointer, child bno "
+			"should be %d, block bno is %d, hashval is %u\n",
+			be32_to_cpu(btree[entry].before),
+			cursor->level[p_level].bno,
+			cursor->level[p_level].hashval);
+		fprintf(stderr, "verify_dir2_path returns 1 (bad) #1a\n");
+#endif
 		return(1);
+	}
 	/*
 	 * ok, now validate last hashvalue in the descendant
 	 * block against the hashval in the current entry
@@ -659,6 +689,9 @@ _("would correct bad hashval in interior dir block\n"
 	 * (which should point to the next descendant block)
 	 */
 	cursor->level[this_level].index++;
+#ifdef XR_DIR_TRACE
+       fprintf(stderr, "verify_dir2_path returns 0 (ok)\n");
+#endif
 	return(0);
 }
 
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 06/13] xfs_repair: add XR_DIR_TRACE to dir2.c

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:04PM -0500, Eric Sandeen wrote:
> attr_repair.c has many printf-tracepoints under
> #ifdef XR_DIR_TRACE, but the similar code in dir2.c does not.
> 
> Add these same tracepoints to remove more differences between
> these two pieces of code.
> 
> Not all messages are quite correct; those will be fixed up last.
> For now we just make the code more obviously similar.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  repair/dir2.c |   39 ++++++++++++++++++++++++++++++++++++---
>  1 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 898b27e..d9bd5fc 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -337,6 +337,11 @@ verify_final_dir2_path(xfs_mount_t	*mp,
>  	struct xfs_da_node_entry *btree;
>  	struct xfs_da3_icnode_hdr nodehdr;
>  
> +#ifdef XR_DIR_TRACE
> +	fprintf(stderr, "in verify_final_dir2_path, this_level = %d\n",
> +		this_level);
> +#endif
> +
>  	/*
>  	 * the index should point to the next "unprocessed" entry
>  	 * in the block which should be the final (rightmost) entry
> @@ -388,8 +393,17 @@ verify_final_dir2_path(xfs_mount_t	*mp,
>  	/*
>  	 * ok, now check descendant block number against this level
>  	 */
> -	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before))
> +	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
> +#ifdef XR_DIR_TRACE
> +		fprintf(stderr, "bad directory btree pointer, child bno should "
> +				"be %d, block bno is %d, hashval is %u\n",
> +			be16_to_cpu(btree[entry].before),
> +			cursor->level[p_level].bno,
> +			cursor->level[p_level].hashval);
> +		fprintf(stderr, "verify_final_dir2_path returns 1 (bad) #1a\n");
> +#endif
>  		return(1);
> +	}
>  
>  	if (cursor->level[p_level].hashval !=
>  				be32_to_cpu(btree[entry].hashval))  {
> @@ -431,8 +445,12 @@ _("would correct bad hashval in non-leaf dir block\n"
>  	/*
>  	 * bail out if this is the root block (top of tree)
>  	 */
> -	if (this_level >= cursor->active)
> +	if (this_level >= cursor->active) {
> +#ifdef XR_DIR_TRACE
> +		fprintf(stderr, "verify_final_dir2_path returns 0 (ok)\n");
> +#endif
>  		return(0);
> +	}
>  	/*
>  	 * set hashvalue to correctl reflect the now-validated
>  	 * last entry in this block and continue upwards validation
> @@ -600,6 +618,9 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>  			bad++;
>  		}
>  		if (bad)  {
> +#ifdef XR_DIR_TRACE
> +			fprintf(stderr, "verify_dir2_path returns 1 (bad) #4\n");
> +#endif
>  			libxfs_putbuf(bp);
>  			return(1);
>  		}
> @@ -631,8 +652,17 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>  	/*
>  	 * ditto for block numbers
>  	 */
> -	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before))
> +	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
> +#ifdef XR_DIR_TRACE
> +		fprintf(stderr, "bad directory btree pointer, child bno "
> +			"should be %d, block bno is %d, hashval is %u\n",
> +			be32_to_cpu(btree[entry].before),
> +			cursor->level[p_level].bno,
> +			cursor->level[p_level].hashval);
> +		fprintf(stderr, "verify_dir2_path returns 1 (bad) #1a\n");
> +#endif
>  		return(1);
> +	}
>  	/*
>  	 * ok, now validate last hashvalue in the descendant
>  	 * block against the hashval in the current entry
> @@ -659,6 +689,9 @@ _("would correct bad hashval in interior dir block\n"
>  	 * (which should point to the next descendant block)
>  	 */
>  	cursor->level[this_level].index++;
> +#ifdef XR_DIR_TRACE
> +       fprintf(stderr, "verify_dir2_path returns 0 (ok)\n");
> +#endif
>  	return(0);
>  }
>  
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 07/13] xfs_repair: Remove BUF_PTR from attr_repair.c

[Eric Sandeen]

The BUF_PTR macro was removed from kernelspace a while ago
(6292604 xfs: Remove the macro XFS_BUF_PTR) but it lives
on in some parts of xfsprogs.  dir2.c doesn't use it,
but similar code in attr_repair.c does.  remove it from
attr_repair.c to converge the code.

Remove a related but unnecessary cast from a *void b_addr
in dir2.c while we're at it.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |   12 ++++++------
 repair/dir2.c        |    2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index fe81df4..5ae2356 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -188,7 +188,7 @@ traverse_int_dablock(xfs_mount_t	*mp,
 			goto error_out;
 		}
 
-		node = (xfs_da_intnode_t *)XFS_BUF_PTR(bp);
+		node = bp->b_addr;
 		btree = M_DIROPS(mp)->node_tree_p(node);
 		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
 
@@ -335,7 +335,7 @@ verify_final_da_path(xfs_mount_t	*mp,
 	 * in the block which should be the final (rightmost) entry
 	 */
 	entry = cursor->level[this_level].index;
-	node = (xfs_da_intnode_t *)XFS_BUF_PTR(cursor->level[this_level].bp);
+	node = cursor->level[this_level].bp->b_addr;
 	btree = M_DIROPS(mp)->node_tree_p(node);
 	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
 
@@ -513,7 +513,7 @@ verify_da_path(xfs_mount_t	*mp,
 	 * should be processed now in this level.
 	 */
 	entry = cursor->level[this_level].index;
-	node = (xfs_da_intnode_t *)XFS_BUF_PTR(cursor->level[this_level].bp);
+	node = cursor->level[this_level].bp->b_addr;
 	btree = M_DIROPS(mp)->node_tree_p(node);
 	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
 
@@ -571,7 +571,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
 			return(1);
 		}
 
-		newnode = (xfs_da_intnode_t *)XFS_BUF_PTR(bp);
+		newnode = bp->b_addr;
 		btree = M_DIROPS(mp)->node_tree_p(newnode);
 		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, newnode);
 
@@ -1040,7 +1040,7 @@ rmtval_get(xfs_mount_t *mp, xfs_ino_t ino, blkmap_t *blkmap,
 		ASSERT(mp->m_sb.sb_blocksize == XFS_BUF_COUNT(bp));
 
 		length = MIN(XFS_BUF_COUNT(bp) - hdrsize, valuelen - amountdone);
-		memmove(value, XFS_BUF_PTR(bp) + hdrsize, length);
+		memmove(value, bp->b_addr + hdrsize, length);
 		amountdone += length;
 		value += length;
 		i++;
@@ -1620,7 +1620,7 @@ process_longform_attr(
 	}
 
 	/* verify leaf block */
-	leaf = (xfs_attr_leafblock_t *)XFS_BUF_PTR(bp);
+	leaf = bp->b_addr;
 	xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
 
 	/* check sibling pointers in leaf block or root block 0 before
diff --git a/repair/dir2.c b/repair/dir2.c
index d9bd5fc..9398df5 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -347,7 +347,7 @@ verify_final_dir2_path(xfs_mount_t	*mp,
 	 * in the block which should be the final (rightmost) entry
 	 */
 	entry = cursor->level[this_level].index;
-	node = (xfs_da_intnode_t *)(cursor->level[this_level].bp->b_addr);
+	node = cursor->level[this_level].bp->b_addr;
 	btree = M_DIROPS(mp)->node_tree_p(node);
 	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
 
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 07/13] xfs_repair: Remove BUF_PTR from attr_repair.c

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:05PM -0500, Eric Sandeen wrote:
> The BUF_PTR macro was removed from kernelspace a while ago
> (6292604 xfs: Remove the macro XFS_BUF_PTR) but it lives
> on in some parts of xfsprogs.  dir2.c doesn't use it,
> but similar code in attr_repair.c does.  remove it from
> attr_repair.c to converge the code.
> 
> Remove a related but unnecessary cast from a *void b_addr
> in dir2.c while we're at it.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  repair/attr_repair.c |   12 ++++++------
>  repair/dir2.c        |    2 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index fe81df4..5ae2356 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -188,7 +188,7 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  			goto error_out;
>  		}
>  
> -		node = (xfs_da_intnode_t *)XFS_BUF_PTR(bp);
> +		node = bp->b_addr;
>  		btree = M_DIROPS(mp)->node_tree_p(node);
>  		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
>  
> @@ -335,7 +335,7 @@ verify_final_da_path(xfs_mount_t	*mp,
>  	 * in the block which should be the final (rightmost) entry
>  	 */
>  	entry = cursor->level[this_level].index;
> -	node = (xfs_da_intnode_t *)XFS_BUF_PTR(cursor->level[this_level].bp);
> +	node = cursor->level[this_level].bp->b_addr;
>  	btree = M_DIROPS(mp)->node_tree_p(node);
>  	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
>  
> @@ -513,7 +513,7 @@ verify_da_path(xfs_mount_t	*mp,
>  	 * should be processed now in this level.
>  	 */
>  	entry = cursor->level[this_level].index;
> -	node = (xfs_da_intnode_t *)XFS_BUF_PTR(cursor->level[this_level].bp);
> +	node = cursor->level[this_level].bp->b_addr;
>  	btree = M_DIROPS(mp)->node_tree_p(node);
>  	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
>  
> @@ -571,7 +571,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  			return(1);
>  		}
>  
> -		newnode = (xfs_da_intnode_t *)XFS_BUF_PTR(bp);
> +		newnode = bp->b_addr;
>  		btree = M_DIROPS(mp)->node_tree_p(newnode);
>  		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, newnode);
>  
> @@ -1040,7 +1040,7 @@ rmtval_get(xfs_mount_t *mp, xfs_ino_t ino, blkmap_t *blkmap,
>  		ASSERT(mp->m_sb.sb_blocksize == XFS_BUF_COUNT(bp));
>  
>  		length = MIN(XFS_BUF_COUNT(bp) - hdrsize, valuelen - amountdone);
> -		memmove(value, XFS_BUF_PTR(bp) + hdrsize, length);
> +		memmove(value, bp->b_addr + hdrsize, length);
>  		amountdone += length;
>  		value += length;
>  		i++;
> @@ -1620,7 +1620,7 @@ process_longform_attr(
>  	}
>  
>  	/* verify leaf block */
> -	leaf = (xfs_attr_leafblock_t *)XFS_BUF_PTR(bp);
> +	leaf = bp->b_addr;
>  	xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
>  
>  	/* check sibling pointers in leaf block or root block 0 before
> diff --git a/repair/dir2.c b/repair/dir2.c
> index d9bd5fc..9398df5 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -347,7 +347,7 @@ verify_final_dir2_path(xfs_mount_t	*mp,
>  	 * in the block which should be the final (rightmost) entry
>  	 */
>  	entry = cursor->level[this_level].index;
> -	node = (xfs_da_intnode_t *)(cursor->level[this_level].bp->b_addr);
> +	node = cursor->level[this_level].bp->b_addr;
>  	btree = M_DIROPS(mp)->node_tree_p(node);
>  	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
>  
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 08/13] xfs_repair: catch bad level/depth in da node

[Eric Sandeen]

Two tests added some time ago to dir2.c:

44dae5e xfs_repair: test for bad level in dir2 node
28148f6 xfs_repair: catch bad depth in traverse_int_dir2block

never made it to the similar tree-walking code in attr_repair.c;
fix that up here.  The error string details will be fixed up
later.

Signed-off-by; Eric Sandeen <sandeen@redhat.com>

Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 5ae2356..2aafdf6 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -212,9 +212,17 @@ traverse_int_dablock(xfs_mount_t	*mp,
 		/*
 		 * maintain level counter
 		 */
-		if (i == -1)
+		if (i == -1) {
 			i = da_cursor->active = nodehdr.level;
-		else  {
+			if (i < 1 || i >= XFS_DA_NODE_MAXDEPTH) {
+				do_warn(
+_("bad header depth for directory inode %" PRIu64 "\n"),
+					da_cursor->ino);
+				libxfs_putbuf(bp);
+				i = -1;
+				goto error_out;
+			}
+		} else  {
 			if (nodehdr.level == i - 1)  {
 				i--;
 			} else  {
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 08/13] xfs_repair: catch bad level/depth in da node

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:06PM -0500, Eric Sandeen wrote:
> Two tests added some time ago to dir2.c:
> 
> 44dae5e xfs_repair: test for bad level in dir2 node
> 28148f6 xfs_repair: catch bad depth in traverse_int_dir2block
> 
> never made it to the similar tree-walking code in attr_repair.c;
> fix that up here.  The error string details will be fixed up
> later.
> 
> Signed-off-by; Eric Sandeen <sandeen@redhat.com>
> 
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  repair/attr_repair.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index 5ae2356..2aafdf6 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -212,9 +212,17 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  		/*
>  		 * maintain level counter
>  		 */
> -		if (i == -1)
> +		if (i == -1) {
>  			i = da_cursor->active = nodehdr.level;
> -		else  {
> +			if (i < 1 || i >= XFS_DA_NODE_MAXDEPTH) {
> +				do_warn(
> +_("bad header depth for directory inode %" PRIu64 "\n"),
> +					da_cursor->ino);
> +				libxfs_putbuf(bp);
> +				i = -1;
> +				goto error_out;
> +			}
> +		} else  {
>  			if (nodehdr.level == i - 1)  {
>  				i--;
>  			} else  {
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 09/13] xfs_repair: better checking of v5 attributes

[Eric Sandeen]

The commit:

0519f66 xfs_repair: better checking of v5 metadata fields

added new corruption checks to dir2.c but missed the similar
code in attr_repair.c; add that here.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 2aafdf6..c8ba484 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -201,6 +201,15 @@ traverse_int_dablock(xfs_mount_t	*mp,
 			goto error_out;
 		}
 
+		/* corrupt node; rebuild the dir. */
+		if (bp->b_error == -EFSBADCRC || bp->b_error == -EFSCORRUPTED) {
+			libxfs_putbuf(bp);
+			do_warn(
+_("corrupt tree block %u for directory inode %" PRIu64 "\n"),
+				bno, da_cursor->ino);
+			goto error_out;
+		}
+
 		if (nodehdr.count > geo->node_ents)  {
 			do_warn(_("bad record count in inode %" PRIu64 ", "
 				  "count = %d, max = %d\n"),
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 09/13] xfs_repair: better checking of v5 attributes

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:07PM -0500, Eric Sandeen wrote:
> The commit:
> 
> 0519f66 xfs_repair: better checking of v5 metadata fields
> 
> added new corruption checks to dir2.c but missed the similar
> code in attr_repair.c; add that here.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---
>  repair/attr_repair.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index 2aafdf6..c8ba484 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -201,6 +201,15 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  			goto error_out;
>  		}
>  
> +		/* corrupt node; rebuild the dir. */
> +		if (bp->b_error == -EFSBADCRC || bp->b_error == -EFSCORRUPTED) {
> +			libxfs_putbuf(bp);
> +			do_warn(
> +_("corrupt tree block %u for directory inode %" PRIu64 "\n"),
> +				bno, da_cursor->ino);
> +			goto error_out;
> +		}
> +

Hmm, well this certainly looks similar, but is it the right thing to do
for xattrs? I haven't followed through how exactly directories are
rebuilt, but there does appear to be a recovery path in the dir2
context. A failure there simply puts the inode on a "bad" list to be
rebuilt later, presumably from data collected from all of the inodes.

If we fail here, it looks like we just clear the attribute fork. So are
we failing too hard, too soon here if a dablock crc happens to be
incorrect?

Brian

>  		if (nodehdr.count > geo->node_ents)  {
>  			do_warn(_("bad record count in inode %" PRIu64 ", "
>  				  "count = %d, max = %d\n"),
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 09/13] xfs_repair: better checking of v5 attributes

[Eric Sandeen]

On 9/14/15 2:44 PM, Brian Foster wrote:
> On Wed, Sep 09, 2015 at 02:34:07PM -0500, Eric Sandeen wrote:
>> The commit:
>>
>> 0519f66 xfs_repair: better checking of v5 metadata fields
>>
>> added new corruption checks to dir2.c but missed the similar
>> code in attr_repair.c; add that here.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>> ---
>>  repair/attr_repair.c |    9 +++++++++
>>  1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
>> index 2aafdf6..c8ba484 100644
>> --- a/repair/attr_repair.c
>> +++ b/repair/attr_repair.c
>> @@ -201,6 +201,15 @@ traverse_int_dablock(xfs_mount_t	*mp,
>>  			goto error_out;
>>  		}
>>  
>> +		/* corrupt node; rebuild the dir. */
>> +		if (bp->b_error == -EFSBADCRC || bp->b_error == -EFSCORRUPTED) {
>> +			libxfs_putbuf(bp);
>> +			do_warn(
>> +_("corrupt tree block %u for directory inode %" PRIu64 "\n"),
>> +				bno, da_cursor->ino);
>> +			goto error_out;
>> +		}
>> +
> 
> Hmm, well this certainly looks similar, but is it the right thing to do
> for xattrs? I haven't followed through how exactly directories are
> rebuilt, but there does appear to be a recovery path in the dir2
> context. A failure there simply puts the inode on a "bad" list to be
> rebuilt later, presumably from data collected from all of the inodes.
> 
> If we fail here, it looks like we just clear the attribute fork. So are
> we failing too hard, too soon here if a dablock crc happens to be
> incorrect?

Brian & I talked about this briefly on IRC.  The upshot:

attr checking already has many failure points, and if any one fails, the attr
may get nuked.

dir checking already has many failure points, and if any one fails, the dir
can get rebuilt.

All this patch does is add another check to several existing checks in the
attr code, and if it fails, whatever action was taken before for any other
error will also be taken for a bad CRC or a verifier failure.  So, this 
doesn't really introduce any new or more draconian behavior; it simply adds
one more check (the CRC, which had previously been ignored) to a host of other
verifications in this code, with the same results as before if this new check
fails.  So I think it's fine as it is.

Thanks,
-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 10/13] xfs_repair: Remove more differences between attr & dir2

[Eric Sandeen]

This is a hodgepodge of unrelated but not-completely-trivial
chagnes to both the dir2 and attr code to make their common
code more similar.

* It removes the whichfork checking in attr_repair, because we
  only get there with XFS_ATTR_FORK.
* It changes the magic-checking logic slightly to match.
* It swaps some (bp == NULL) tests for (!bp)

These should be purely cosmetic changes.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |   20 +++++---------------
 repair/dir2.c        |   14 ++++++++------
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index c8ba484..26a0e71 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -177,19 +177,13 @@ traverse_int_dablock(xfs_mount_t	*mp,
 			free(bmp);
 
 		if (!bp) {
-			if (whichfork == XFS_DATA_FORK)
-				do_warn(
-	_("can't read block %u (fsbno %" PRIu64 ") for directory inode %" PRIu64 "\n"),
-					bno, fsbno, da_cursor->ino);
-			else
-				do_warn(
+			do_warn(
 	_("can't read block %u (fsbno %" PRIu64 ") for attrbute fork of inode %" PRIu64 "\n"),
 					bno, fsbno, da_cursor->ino);
 			goto error_out;
 		}
 
 		node = bp->b_addr;
-		btree = M_DIROPS(mp)->node_tree_p(node);
 		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
 
 		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
@@ -210,6 +204,7 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
 			goto error_out;
 		}
 
+		btree = M_DIROPS(mp)->node_tree_p(node);
 		if (nodehdr.count > geo->node_ents)  {
 			do_warn(_("bad record count in inode %" PRIu64 ", "
 				  "count = %d, max = %d\n"),
@@ -235,14 +230,9 @@ _("bad header depth for directory inode %" PRIu64 "\n"),
 			if (nodehdr.level == i - 1)  {
 				i--;
 			} else  {
-				if (whichfork == XFS_DATA_FORK)
-					do_warn(_("bad directory btree for "
-						  "directory inode %" PRIu64 "\n"),
-						da_cursor->ino);
-				else
-					do_warn(_("bad attribute fork btree "
-						  "for inode %" PRIu64 "\n"),
-						da_cursor->ino);
+				do_warn(_("bad attribute fork btree "
+					  "for inode %" PRIu64 "\n"),
+					da_cursor->ino);
 				libxfs_putbuf(bp);
 				goto error_out;
 			}
diff --git a/repair/dir2.c b/repair/dir2.c
index 9398df5..8cf981f 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -172,7 +172,7 @@ traverse_int_dir2block(xfs_mount_t	*mp,
 		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
 		if (bmp != &lbmp)
 			free(bmp);
-		if (bp == NULL) {
+		if (!bp) {
 			do_warn(
 _("can't read block %u for directory inode %" PRIu64 "\n"),
 				bno, da_cursor->ino);
@@ -192,8 +192,10 @@ _("found non-root LEAFN node in inode %" PRIu64 " bno = %u\n"),
 			*rbno = 0;
 			libxfs_putbuf(bp);
 			return(1);
-		} else if (!(nodehdr.magic == XFS_DA_NODE_MAGIC ||
-			     nodehdr.magic == XFS_DA3_NODE_MAGIC))  {
+		}
+
+		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
+		    nodehdr.magic != XFS_DA3_NODE_MAGIC)  {
 			libxfs_putbuf(bp);
 			do_warn(
 _("bad dir magic number 0x%x in inode %" PRIu64 " bno = %u\n"),
@@ -574,7 +576,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
 		if (bmp != &lbmp)
 			free(bmp);
 
-		if (bp == NULL) {
+		if (!bp) {
 			do_warn(
 _("can't read block %u for directory inode %" PRIu64 "\n"),
 				dabno, cursor->ino);
@@ -589,8 +591,8 @@ _("can't read block %u for directory inode %" PRIu64 "\n"),
 		 * entry count, verify level
 		 */
 		bad = 0;
-		if (!(nodehdr.magic == XFS_DA_NODE_MAGIC ||
-		      nodehdr.magic == XFS_DA3_NODE_MAGIC)) {
+		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
+		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
 			do_warn(
 _("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
 				nodehdr.magic,
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 10/13] xfs_repair: Remove more differences between attr & dir2

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:08PM -0500, Eric Sandeen wrote:
> This is a hodgepodge of unrelated but not-completely-trivial
> chagnes to both the dir2 and attr code to make their common
> code more similar.
> 
> * It removes the whichfork checking in attr_repair, because we
>   only get there with XFS_ATTR_FORK.
> * It changes the magic-checking logic slightly to match.
> * It swaps some (bp == NULL) tests for (!bp)
> 
> These should be purely cosmetic changes.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  repair/attr_repair.c |   20 +++++---------------
>  repair/dir2.c        |   14 ++++++++------
>  2 files changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index c8ba484..26a0e71 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -177,19 +177,13 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  			free(bmp);
>  
>  		if (!bp) {
> -			if (whichfork == XFS_DATA_FORK)
> -				do_warn(
> -	_("can't read block %u (fsbno %" PRIu64 ") for directory inode %" PRIu64 "\n"),
> -					bno, fsbno, da_cursor->ino);
> -			else
> -				do_warn(
> +			do_warn(
>  	_("can't read block %u (fsbno %" PRIu64 ") for attrbute fork of inode %" PRIu64 "\n"),
>  					bno, fsbno, da_cursor->ino);
>  			goto error_out;
>  		}
>  
>  		node = bp->b_addr;
> -		btree = M_DIROPS(mp)->node_tree_p(node);
>  		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
>  
>  		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
> @@ -210,6 +204,7 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
>  			goto error_out;
>  		}
>  
> +		btree = M_DIROPS(mp)->node_tree_p(node);
>  		if (nodehdr.count > geo->node_ents)  {
>  			do_warn(_("bad record count in inode %" PRIu64 ", "
>  				  "count = %d, max = %d\n"),
> @@ -235,14 +230,9 @@ _("bad header depth for directory inode %" PRIu64 "\n"),
>  			if (nodehdr.level == i - 1)  {
>  				i--;
>  			} else  {
> -				if (whichfork == XFS_DATA_FORK)
> -					do_warn(_("bad directory btree for "
> -						  "directory inode %" PRIu64 "\n"),
> -						da_cursor->ino);
> -				else
> -					do_warn(_("bad attribute fork btree "
> -						  "for inode %" PRIu64 "\n"),
> -						da_cursor->ino);
> +				do_warn(_("bad attribute fork btree "
> +					  "for inode %" PRIu64 "\n"),
> +					da_cursor->ino);
>  				libxfs_putbuf(bp);
>  				goto error_out;
>  			}
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 9398df5..8cf981f 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -172,7 +172,7 @@ traverse_int_dir2block(xfs_mount_t	*mp,
>  		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
>  		if (bmp != &lbmp)
>  			free(bmp);
> -		if (bp == NULL) {
> +		if (!bp) {
>  			do_warn(
>  _("can't read block %u for directory inode %" PRIu64 "\n"),
>  				bno, da_cursor->ino);
> @@ -192,8 +192,10 @@ _("found non-root LEAFN node in inode %" PRIu64 " bno = %u\n"),
>  			*rbno = 0;
>  			libxfs_putbuf(bp);
>  			return(1);
> -		} else if (!(nodehdr.magic == XFS_DA_NODE_MAGIC ||
> -			     nodehdr.magic == XFS_DA3_NODE_MAGIC))  {
> +		}
> +
> +		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
> +		    nodehdr.magic != XFS_DA3_NODE_MAGIC)  {
>  			libxfs_putbuf(bp);
>  			do_warn(
>  _("bad dir magic number 0x%x in inode %" PRIu64 " bno = %u\n"),
> @@ -574,7 +576,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  		if (bmp != &lbmp)
>  			free(bmp);
>  
> -		if (bp == NULL) {
> +		if (!bp) {
>  			do_warn(
>  _("can't read block %u for directory inode %" PRIu64 "\n"),
>  				dabno, cursor->ino);
> @@ -589,8 +591,8 @@ _("can't read block %u for directory inode %" PRIu64 "\n"),
>  		 * entry count, verify level
>  		 */
>  		bad = 0;
> -		if (!(nodehdr.magic == XFS_DA_NODE_MAGIC ||
> -		      nodehdr.magic == XFS_DA3_NODE_MAGIC)) {
> +		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
> +		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
>  			do_warn(
>  _("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
>  				nodehdr.magic,
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 11/13] xfs_repair: whitespace & comments

[Eric Sandeen]

This patch does nothing but fix up whitespace and comments
to match across dir2.c and attr_repair.c

At this point, a diff of repair/dir2.c and attr_repair.c
show them to be identical in function.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |   36 ++++++++++++++++++------------------
 repair/dir2.c        |   46 ++++++++++++++++++++++++----------------------
 2 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 26a0e71..0804a22 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -187,7 +187,7 @@ traverse_int_dablock(xfs_mount_t	*mp,
 		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
 
 		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
-		    nodehdr.magic != XFS_DA3_NODE_MAGIC)  {
+		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
 			do_warn(_("bad dir/attr magic number in inode %" PRIu64 ", "
 				  "file bno = %u, fsbno = %" PRIu64 "\n"),
 				da_cursor->ino, bno, fsbno);
@@ -205,7 +205,7 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
 		}
 
 		btree = M_DIROPS(mp)->node_tree_p(node);
-		if (nodehdr.count > geo->node_ents)  {
+		if (nodehdr.count > geo->node_ents) {
 			do_warn(_("bad record count in inode %" PRIu64 ", "
 				  "count = %d, max = %d\n"),
 				da_cursor->ino, nodehdr.count, geo->node_ents);
@@ -226,10 +226,10 @@ _("bad header depth for directory inode %" PRIu64 "\n"),
 				i = -1;
 				goto error_out;
 			}
-		} else  {
-			if (nodehdr.level == i - 1)  {
+		} else {
+			if (nodehdr.level == i - 1) {
 				i--;
-			} else  {
+			} else {
 				do_warn(_("bad attribute fork btree "
 					  "for inode %" PRIu64 "\n"),
 					da_cursor->ino);
@@ -256,7 +256,7 @@ _("bad header depth for directory inode %" PRIu64 "\n"),
 	return(1);
 
 error_out:
-	while (i > 1 && i <= da_cursor->active)  {
+	while (i > 1 && i <= da_cursor->active) {
 		libxfs_putbuf(da_cursor->level[i].bp);
 		i++;
 	}
@@ -351,7 +351,7 @@ verify_final_da_path(xfs_mount_t	*mp,
 	 * that all entries are used, encountered and expected hashvals
 	 * match, etc.
 	 */
-	if (entry != nodehdr.count - 1)  {
+	if (entry != nodehdr.count - 1) {
 		do_warn(_("directory/attribute block used/count "
 			  "inconsistency - %d/%hu\n"),
 			entry, nodehdr.count);
@@ -368,7 +368,7 @@ verify_final_da_path(xfs_mount_t	*mp,
 			be32_to_cpu(btree[entry].hashval));
 		bad++;
 	}
-	if (nodehdr.forw != 0)  {
+	if (nodehdr.forw != 0) {
 		do_warn(_("bad directory/attribute forward block pointer, "
 			  "expected 0, saw %u\n"),
 			nodehdr.forw);
@@ -402,7 +402,7 @@ verify_final_da_path(xfs_mount_t	*mp,
 	}
 
 	if (cursor->level[p_level].hashval != be32_to_cpu(btree[entry].hashval)) {
-		if (!no_modify)  {
+		if (!no_modify) {
 			do_warn(_("correcting bad hashval in non-leaf "
 				  "dir/attr block\n\tin (level %d) in "
 				  "inode %" PRIu64 ".\n"),
@@ -410,7 +410,7 @@ verify_final_da_path(xfs_mount_t	*mp,
 			btree[entry].hashval = cpu_to_be32(
 						cursor->level[p_level].hashval);
 			cursor->level[this_level].dirty++;
-		} else  {
+		} else {
 			do_warn(_("would correct bad hashval in non-leaf "
 				  "dir/attr block\n\tin (level %d) in "
 				  "inode %" PRIu64 ".\n"),
@@ -440,7 +440,7 @@ verify_final_da_path(xfs_mount_t	*mp,
 	/*
 	 * bail out if this is the root block (top of tree)
 	 */
-	if (this_level >= cursor->active)  {
+	if (this_level >= cursor->active) {
 #ifdef XR_DIR_TRACE
 		fprintf(stderr, "verify_final_da_path returns 0 (ok)\n");
 #endif
@@ -529,7 +529,7 @@ verify_da_path(xfs_mount_t	*mp,
 	 * block and move on to the next block.
 	 * and update cursor value for said level
 	 */
-	if (entry >= nodehdr.count)  {
+	if (entry >= nodehdr.count) {
 		/*
 		 * update the hash value for this level before
 		 * validating it.  bno value should be ok since
@@ -588,7 +588,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
 		 */
 		bad = 0;
 		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
-		    nodehdr.magic != XFS_DA3_NODE_MAGIC)  {
+		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
 			do_warn(
 	_("bad magic number %x in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
 				nodehdr.magic,
@@ -615,7 +615,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
 				dabno, fsbno, cursor->ino);
 			bad++;
 		}
-		if (bad)  {
+		if (bad) {
 #ifdef XR_DIR_TRACE
 			fprintf(stderr, "verify_da_path returns 1 (bad) #4\n");
 #endif
@@ -654,7 +654,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
 	/*
 	 * ditto for block numbers
 	 */
-	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before))  {
+	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
 #ifdef XR_DIR_TRACE
 		fprintf(stderr, "bad directory btree pointer, child bno "
 			"should be %d, block bno is %d, hashval is %u\n",
@@ -670,8 +670,8 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
 	 * block against the hashval in the current entry
 	 */
 	if (cursor->level[p_level].hashval !=
-				be32_to_cpu(btree[entry].hashval))  {
-		if (!no_modify)  {
+				be32_to_cpu(btree[entry].hashval)) {
+		if (!no_modify) {
 			do_warn(_("correcting bad hashval in interior "
 				  "dir/attr block\n\tin (level %d) in "
 				  "inode %" PRIu64 ".\n"),
@@ -679,7 +679,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
 			btree[entry].hashval = cpu_to_be32(
 						cursor->level[p_level].hashval);
 			cursor->level[this_level].dirty++;
-		} else  {
+		} else {
 			do_warn(_("would correct bad hashval in interior "
 				  "dir/attr block\n\tin (level %d) in "
 				  "inode %" PRIu64 ".\n"),
diff --git a/repair/dir2.c b/repair/dir2.c
index 8cf981f..7b47a9e 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -183,7 +183,7 @@ _("can't read block %u for directory inode %" PRIu64 "\n"),
 		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
 
 		if (nodehdr.magic == XFS_DIR2_LEAFN_MAGIC ||
-		    nodehdr.magic == XFS_DIR3_LEAFN_MAGIC)  {
+		    nodehdr.magic == XFS_DIR3_LEAFN_MAGIC) {
 			if ( i != -1 ) {
 				do_warn(
 _("found non-root LEAFN node in inode %" PRIu64 " bno = %u\n"),
@@ -195,7 +195,7 @@ _("found non-root LEAFN node in inode %" PRIu64 " bno = %u\n"),
 		}
 
 		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
-		    nodehdr.magic != XFS_DA3_NODE_MAGIC)  {
+		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
 			libxfs_putbuf(bp);
 			do_warn(
 _("bad dir magic number 0x%x in inode %" PRIu64 " bno = %u\n"),
@@ -212,7 +212,7 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
 			goto error_out;
 		}
 		btree = M_DIROPS(mp)->node_tree_p(node);
-		if (nodehdr.count > geo->node_ents)  {
+		if (nodehdr.count > geo->node_ents) {
 			do_warn(
 _("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
 				da_cursor->ino, nodehdr.count, geo->node_ents);
@@ -233,9 +233,9 @@ _("bad header depth for directory inode %" PRIu64 "\n"),
 				goto error_out;
 			}
 		} else {
-			if (nodehdr.level == i - 1)  {
+			if (nodehdr.level == i - 1) {
 				i--;
-			} else  {
+			} else {
 				do_warn(
 _("bad directory btree for directory inode %" PRIu64 "\n"),
 					da_cursor->ino);
@@ -262,7 +262,7 @@ _("bad directory btree for directory inode %" PRIu64 "\n"),
 	return(1);
 
 error_out:
-	while (i > 1 && i <= da_cursor->active)  {
+	while (i > 1 && i <= da_cursor->active) {
 		libxfs_putbuf(da_cursor->level[i].bp);
 		i++;
 	}
@@ -358,7 +358,7 @@ verify_final_dir2_path(xfs_mount_t	*mp,
 	 * that all entries are used, encountered and expected hashvals
 	 * match, etc.
 	 */
-	if (entry != nodehdr.count - 1)  {
+	if (entry != nodehdr.count - 1) {
 		do_warn(
 		_("directory block used/count inconsistency - %d / %hu\n"),
 			entry, nodehdr.count);
@@ -368,20 +368,20 @@ verify_final_dir2_path(xfs_mount_t	*mp,
 	 * hash values monotonically increasing ???
 	 */
 	if (cursor->level[this_level].hashval >=
-				be32_to_cpu(btree[entry].hashval))  {
+				be32_to_cpu(btree[entry].hashval)) {
 		do_warn(_("directory/attribute block hashvalue inconsistency, "
 			  "expected > %u / saw %u\n"),
 			cursor->level[this_level].hashval,
 			be32_to_cpu(btree[entry].hashval));
 		bad++;
 	}
-	if (nodehdr.forw != 0)  {
+	if (nodehdr.forw != 0) {
 		do_warn(_("bad directory/attribute forward block pointer, "
 			  "expected 0, saw %u\n"),
 			nodehdr.forw);
 		bad++;
 	}
-	if (bad)  {
+	if (bad) {
 		do_warn(_("bad directory block in inode %" PRIu64 "\n"), cursor->ino);
 		return(1);
 	}
@@ -408,8 +408,8 @@ verify_final_dir2_path(xfs_mount_t	*mp,
 	}
 
 	if (cursor->level[p_level].hashval !=
-				be32_to_cpu(btree[entry].hashval))  {
-		if (!no_modify)  {
+				be32_to_cpu(btree[entry].hashval)) {
+		if (!no_modify) {
 			do_warn(
 _("correcting bad hashval in non-leaf dir block\n"
   "\tin (level %d) in inode %" PRIu64 ".\n"),
@@ -417,7 +417,7 @@ _("correcting bad hashval in non-leaf dir block\n"
 			btree[entry].hashval = cpu_to_be32(
 						cursor->level[p_level].hashval);
 			cursor->level[this_level].dirty++;
-		} else  {
+		} else {
 			do_warn(
 _("would correct bad hashval in non-leaf dir block\n"
   "\tin (level %d) in inode %" PRIu64 ".\n"),
@@ -454,7 +454,7 @@ _("would correct bad hashval in non-leaf dir block\n"
 		return(0);
 	}
 	/*
-	 * set hashvalue to correctl reflect the now-validated
+	 * set hashvalue to correctly reflect the now-validated
 	 * last entry in this block and continue upwards validation
 	 */
 	cursor->level[this_level].hashval = hashval;
@@ -536,7 +536,7 @@ verify_dir2_path(xfs_mount_t	*mp,
 	 * block and move on to the next block.
 	 * and update cursor value for said level
 	 */
-	if (entry >= nodehdr.count)  {
+	if (entry >= nodehdr.count) {
 		/*
 		 * update the hash value for this level before
 		 * validating it.  bno value should be ok since
@@ -599,27 +599,27 @@ _("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
 				dabno, cursor->ino);
 			bad++;
 		}
-		if (nodehdr.back != cursor->level[this_level].bno)  {
+		if (nodehdr.back != cursor->level[this_level].bno) {
 			do_warn(
 _("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
 				dabno, cursor->ino);
 			bad++;
 		}
-		if (nodehdr.count > geo->node_ents)  {
+		if (nodehdr.count > geo->node_ents) {
 			do_warn(
 _("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
 				nodehdr.count,
 				dabno, cursor->ino);
 			bad++;
 		}
-		if (nodehdr.level != this_level)  {
+		if (nodehdr.level != this_level) {
 			do_warn(
 _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
 				nodehdr.level,
 				dabno, cursor->ino);
 			bad++;
 		}
-		if (bad)  {
+		if (bad) {
 #ifdef XR_DIR_TRACE
 			fprintf(stderr, "verify_dir2_path returns 1 (bad) #4\n");
 #endif
@@ -643,6 +643,8 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
 			libxfs_writebuf(cursor->level[this_level].bp, 0);
 		else
 			libxfs_putbuf(cursor->level[this_level].bp);
+
+		/* switch cursor to point at the new buffer we just read */
 		cursor->level[this_level].bp = bp;
 		cursor->level[this_level].dirty = 0;
 		cursor->level[this_level].bno = dabno;
@@ -670,8 +672,8 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
 	 * block against the hashval in the current entry
 	 */
 	if (cursor->level[p_level].hashval !=
-				be32_to_cpu(btree[entry].hashval))  {
-		if (!no_modify)  {
+				be32_to_cpu(btree[entry].hashval)) {
+		if (!no_modify) {
 			do_warn(
 _("correcting bad hashval in interior dir block\n"
   "\tin (level %d) in inode %" PRIu64 ".\n"),
@@ -679,7 +681,7 @@ _("correcting bad hashval in interior dir block\n"
 			btree[entry].hashval = cpu_to_be32(
 					cursor->level[p_level].hashval);
 			cursor->level[this_level].dirty++;
-		} else  {
+		} else {
 			do_warn(
 _("would correct bad hashval in interior dir block\n"
   "\tin (level %d) in inode %" PRIu64 ".\n"),
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 11/13] xfs_repair: whitespace & comments

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:09PM -0500, Eric Sandeen wrote:
> This patch does nothing but fix up whitespace and comments
> to match across dir2.c and attr_repair.c
> 
> At this point, a diff of repair/dir2.c and attr_repair.c
> show them to be identical in function.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  repair/attr_repair.c |   36 ++++++++++++++++++------------------
>  repair/dir2.c        |   46 ++++++++++++++++++++++++----------------------
>  2 files changed, 42 insertions(+), 40 deletions(-)
> 
> diff --git a/repair/attr_repair.c b/repair/attr_repair.c
> index 26a0e71..0804a22 100644
> --- a/repair/attr_repair.c
> +++ b/repair/attr_repair.c
> @@ -187,7 +187,7 @@ traverse_int_dablock(xfs_mount_t	*mp,
>  		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
>  
>  		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
> -		    nodehdr.magic != XFS_DA3_NODE_MAGIC)  {
> +		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
>  			do_warn(_("bad dir/attr magic number in inode %" PRIu64 ", "
>  				  "file bno = %u, fsbno = %" PRIu64 "\n"),
>  				da_cursor->ino, bno, fsbno);
> @@ -205,7 +205,7 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
>  		}
>  
>  		btree = M_DIROPS(mp)->node_tree_p(node);
> -		if (nodehdr.count > geo->node_ents)  {
> +		if (nodehdr.count > geo->node_ents) {
>  			do_warn(_("bad record count in inode %" PRIu64 ", "
>  				  "count = %d, max = %d\n"),
>  				da_cursor->ino, nodehdr.count, geo->node_ents);
> @@ -226,10 +226,10 @@ _("bad header depth for directory inode %" PRIu64 "\n"),
>  				i = -1;
>  				goto error_out;
>  			}
> -		} else  {
> -			if (nodehdr.level == i - 1)  {
> +		} else {
> +			if (nodehdr.level == i - 1) {
>  				i--;
> -			} else  {
> +			} else {
>  				do_warn(_("bad attribute fork btree "
>  					  "for inode %" PRIu64 "\n"),
>  					da_cursor->ino);
> @@ -256,7 +256,7 @@ _("bad header depth for directory inode %" PRIu64 "\n"),
>  	return(1);
>  
>  error_out:
> -	while (i > 1 && i <= da_cursor->active)  {
> +	while (i > 1 && i <= da_cursor->active) {
>  		libxfs_putbuf(da_cursor->level[i].bp);
>  		i++;
>  	}
> @@ -351,7 +351,7 @@ verify_final_da_path(xfs_mount_t	*mp,
>  	 * that all entries are used, encountered and expected hashvals
>  	 * match, etc.
>  	 */
> -	if (entry != nodehdr.count - 1)  {
> +	if (entry != nodehdr.count - 1) {
>  		do_warn(_("directory/attribute block used/count "
>  			  "inconsistency - %d/%hu\n"),
>  			entry, nodehdr.count);
> @@ -368,7 +368,7 @@ verify_final_da_path(xfs_mount_t	*mp,
>  			be32_to_cpu(btree[entry].hashval));
>  		bad++;
>  	}
> -	if (nodehdr.forw != 0)  {
> +	if (nodehdr.forw != 0) {
>  		do_warn(_("bad directory/attribute forward block pointer, "
>  			  "expected 0, saw %u\n"),
>  			nodehdr.forw);
> @@ -402,7 +402,7 @@ verify_final_da_path(xfs_mount_t	*mp,
>  	}
>  
>  	if (cursor->level[p_level].hashval != be32_to_cpu(btree[entry].hashval)) {
> -		if (!no_modify)  {
> +		if (!no_modify) {
>  			do_warn(_("correcting bad hashval in non-leaf "
>  				  "dir/attr block\n\tin (level %d) in "
>  				  "inode %" PRIu64 ".\n"),
> @@ -410,7 +410,7 @@ verify_final_da_path(xfs_mount_t	*mp,
>  			btree[entry].hashval = cpu_to_be32(
>  						cursor->level[p_level].hashval);
>  			cursor->level[this_level].dirty++;
> -		} else  {
> +		} else {
>  			do_warn(_("would correct bad hashval in non-leaf "
>  				  "dir/attr block\n\tin (level %d) in "
>  				  "inode %" PRIu64 ".\n"),
> @@ -440,7 +440,7 @@ verify_final_da_path(xfs_mount_t	*mp,
>  	/*
>  	 * bail out if this is the root block (top of tree)
>  	 */
> -	if (this_level >= cursor->active)  {
> +	if (this_level >= cursor->active) {
>  #ifdef XR_DIR_TRACE
>  		fprintf(stderr, "verify_final_da_path returns 0 (ok)\n");
>  #endif
> @@ -529,7 +529,7 @@ verify_da_path(xfs_mount_t	*mp,
>  	 * block and move on to the next block.
>  	 * and update cursor value for said level
>  	 */
> -	if (entry >= nodehdr.count)  {
> +	if (entry >= nodehdr.count) {
>  		/*
>  		 * update the hash value for this level before
>  		 * validating it.  bno value should be ok since
> @@ -588,7 +588,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  		 */
>  		bad = 0;
>  		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
> -		    nodehdr.magic != XFS_DA3_NODE_MAGIC)  {
> +		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
>  			do_warn(
>  	_("bad magic number %x in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
>  				nodehdr.magic,
> @@ -615,7 +615,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  				dabno, fsbno, cursor->ino);
>  			bad++;
>  		}
> -		if (bad)  {
> +		if (bad) {
>  #ifdef XR_DIR_TRACE
>  			fprintf(stderr, "verify_da_path returns 1 (bad) #4\n");
>  #endif
> @@ -654,7 +654,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  	/*
>  	 * ditto for block numbers
>  	 */
> -	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before))  {
> +	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
>  #ifdef XR_DIR_TRACE
>  		fprintf(stderr, "bad directory btree pointer, child bno "
>  			"should be %d, block bno is %d, hashval is %u\n",
> @@ -670,8 +670,8 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  	 * block against the hashval in the current entry
>  	 */
>  	if (cursor->level[p_level].hashval !=
> -				be32_to_cpu(btree[entry].hashval))  {
> -		if (!no_modify)  {
> +				be32_to_cpu(btree[entry].hashval)) {
> +		if (!no_modify) {
>  			do_warn(_("correcting bad hashval in interior "
>  				  "dir/attr block\n\tin (level %d) in "
>  				  "inode %" PRIu64 ".\n"),
> @@ -679,7 +679,7 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  			btree[entry].hashval = cpu_to_be32(
>  						cursor->level[p_level].hashval);
>  			cursor->level[this_level].dirty++;
> -		} else  {
> +		} else {
>  			do_warn(_("would correct bad hashval in interior "
>  				  "dir/attr block\n\tin (level %d) in "
>  				  "inode %" PRIu64 ".\n"),
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 8cf981f..7b47a9e 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -183,7 +183,7 @@ _("can't read block %u for directory inode %" PRIu64 "\n"),
>  		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
>  
>  		if (nodehdr.magic == XFS_DIR2_LEAFN_MAGIC ||
> -		    nodehdr.magic == XFS_DIR3_LEAFN_MAGIC)  {
> +		    nodehdr.magic == XFS_DIR3_LEAFN_MAGIC) {
>  			if ( i != -1 ) {
>  				do_warn(
>  _("found non-root LEAFN node in inode %" PRIu64 " bno = %u\n"),
> @@ -195,7 +195,7 @@ _("found non-root LEAFN node in inode %" PRIu64 " bno = %u\n"),
>  		}
>  
>  		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
> -		    nodehdr.magic != XFS_DA3_NODE_MAGIC)  {
> +		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
>  			libxfs_putbuf(bp);
>  			do_warn(
>  _("bad dir magic number 0x%x in inode %" PRIu64 " bno = %u\n"),
> @@ -212,7 +212,7 @@ _("corrupt tree block %u for directory inode %" PRIu64 "\n"),
>  			goto error_out;
>  		}
>  		btree = M_DIROPS(mp)->node_tree_p(node);
> -		if (nodehdr.count > geo->node_ents)  {
> +		if (nodehdr.count > geo->node_ents) {
>  			do_warn(
>  _("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
>  				da_cursor->ino, nodehdr.count, geo->node_ents);
> @@ -233,9 +233,9 @@ _("bad header depth for directory inode %" PRIu64 "\n"),
>  				goto error_out;
>  			}
>  		} else {
> -			if (nodehdr.level == i - 1)  {
> +			if (nodehdr.level == i - 1) {
>  				i--;
> -			} else  {
> +			} else {
>  				do_warn(
>  _("bad directory btree for directory inode %" PRIu64 "\n"),
>  					da_cursor->ino);
> @@ -262,7 +262,7 @@ _("bad directory btree for directory inode %" PRIu64 "\n"),
>  	return(1);
>  
>  error_out:
> -	while (i > 1 && i <= da_cursor->active)  {
> +	while (i > 1 && i <= da_cursor->active) {
>  		libxfs_putbuf(da_cursor->level[i].bp);
>  		i++;
>  	}
> @@ -358,7 +358,7 @@ verify_final_dir2_path(xfs_mount_t	*mp,
>  	 * that all entries are used, encountered and expected hashvals
>  	 * match, etc.
>  	 */
> -	if (entry != nodehdr.count - 1)  {
> +	if (entry != nodehdr.count - 1) {
>  		do_warn(
>  		_("directory block used/count inconsistency - %d / %hu\n"),
>  			entry, nodehdr.count);
> @@ -368,20 +368,20 @@ verify_final_dir2_path(xfs_mount_t	*mp,
>  	 * hash values monotonically increasing ???
>  	 */
>  	if (cursor->level[this_level].hashval >=
> -				be32_to_cpu(btree[entry].hashval))  {
> +				be32_to_cpu(btree[entry].hashval)) {
>  		do_warn(_("directory/attribute block hashvalue inconsistency, "
>  			  "expected > %u / saw %u\n"),
>  			cursor->level[this_level].hashval,
>  			be32_to_cpu(btree[entry].hashval));
>  		bad++;
>  	}
> -	if (nodehdr.forw != 0)  {
> +	if (nodehdr.forw != 0) {
>  		do_warn(_("bad directory/attribute forward block pointer, "
>  			  "expected 0, saw %u\n"),
>  			nodehdr.forw);
>  		bad++;
>  	}
> -	if (bad)  {
> +	if (bad) {
>  		do_warn(_("bad directory block in inode %" PRIu64 "\n"), cursor->ino);
>  		return(1);
>  	}
> @@ -408,8 +408,8 @@ verify_final_dir2_path(xfs_mount_t	*mp,
>  	}
>  
>  	if (cursor->level[p_level].hashval !=
> -				be32_to_cpu(btree[entry].hashval))  {
> -		if (!no_modify)  {
> +				be32_to_cpu(btree[entry].hashval)) {
> +		if (!no_modify) {
>  			do_warn(
>  _("correcting bad hashval in non-leaf dir block\n"
>    "\tin (level %d) in inode %" PRIu64 ".\n"),
> @@ -417,7 +417,7 @@ _("correcting bad hashval in non-leaf dir block\n"
>  			btree[entry].hashval = cpu_to_be32(
>  						cursor->level[p_level].hashval);
>  			cursor->level[this_level].dirty++;
> -		} else  {
> +		} else {
>  			do_warn(
>  _("would correct bad hashval in non-leaf dir block\n"
>    "\tin (level %d) in inode %" PRIu64 ".\n"),
> @@ -454,7 +454,7 @@ _("would correct bad hashval in non-leaf dir block\n"
>  		return(0);
>  	}
>  	/*
> -	 * set hashvalue to correctl reflect the now-validated
> +	 * set hashvalue to correctly reflect the now-validated
>  	 * last entry in this block and continue upwards validation
>  	 */
>  	cursor->level[this_level].hashval = hashval;
> @@ -536,7 +536,7 @@ verify_dir2_path(xfs_mount_t	*mp,
>  	 * block and move on to the next block.
>  	 * and update cursor value for said level
>  	 */
> -	if (entry >= nodehdr.count)  {
> +	if (entry >= nodehdr.count) {
>  		/*
>  		 * update the hash value for this level before
>  		 * validating it.  bno value should be ok since
> @@ -599,27 +599,27 @@ _("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
>  				dabno, cursor->ino);
>  			bad++;
>  		}
> -		if (nodehdr.back != cursor->level[this_level].bno)  {
> +		if (nodehdr.back != cursor->level[this_level].bno) {
>  			do_warn(
>  _("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
>  				dabno, cursor->ino);
>  			bad++;
>  		}
> -		if (nodehdr.count > geo->node_ents)  {
> +		if (nodehdr.count > geo->node_ents) {
>  			do_warn(
>  _("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
>  				nodehdr.count,
>  				dabno, cursor->ino);
>  			bad++;
>  		}
> -		if (nodehdr.level != this_level)  {
> +		if (nodehdr.level != this_level) {
>  			do_warn(
>  _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>  				nodehdr.level,
>  				dabno, cursor->ino);
>  			bad++;
>  		}
> -		if (bad)  {
> +		if (bad) {
>  #ifdef XR_DIR_TRACE
>  			fprintf(stderr, "verify_dir2_path returns 1 (bad) #4\n");
>  #endif
> @@ -643,6 +643,8 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>  			libxfs_writebuf(cursor->level[this_level].bp, 0);
>  		else
>  			libxfs_putbuf(cursor->level[this_level].bp);
> +
> +		/* switch cursor to point at the new buffer we just read */
>  		cursor->level[this_level].bp = bp;
>  		cursor->level[this_level].dirty = 0;
>  		cursor->level[this_level].bno = dabno;
> @@ -670,8 +672,8 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>  	 * block against the hashval in the current entry
>  	 */
>  	if (cursor->level[p_level].hashval !=
> -				be32_to_cpu(btree[entry].hashval))  {
> -		if (!no_modify)  {
> +				be32_to_cpu(btree[entry].hashval)) {
> +		if (!no_modify) {
>  			do_warn(
>  _("correcting bad hashval in interior dir block\n"
>    "\tin (level %d) in inode %" PRIu64 ".\n"),
> @@ -679,7 +681,7 @@ _("correcting bad hashval in interior dir block\n"
>  			btree[entry].hashval = cpu_to_be32(
>  					cursor->level[p_level].hashval);
>  			cursor->level[this_level].dirty++;
> -		} else  {
> +		} else {
>  			do_warn(
>  _("would correct bad hashval in interior dir block\n"
>    "\tin (level %d) in inode %" PRIu64 ".\n"),
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 12/13] xfs_repair: move common dir2 and attr_repair code to da_util.c

[Eric Sandeen]

Now that dir2.c and attr_repair.c are functionally identical,
move the duplicate code into a new file da_util.c, with da_util.h
as a header file for the common functions.

Last step will be to fix up comments and printfs' to be appropriate
for code that checks both dirs and attrs.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/Makefile      |    6 +-
 repair/attr_repair.c |  617 +-------------------------------------------
 repair/da_util.c     |  701 ++++++++++++++++++++++++++++++++++++++++++++++++++
 repair/da_util.h     |   82 ++++++
 repair/dir2.c        |  649 +---------------------------------------------
 repair/dir2.h        |   49 ----
 6 files changed, 801 insertions(+), 1303 deletions(-)
 create mode 100644 repair/da_util.c
 create mode 100644 repair/da_util.h

diff --git a/repair/Makefile b/repair/Makefile
index 6d84ade..251722b 100644
--- a/repair/Makefile
+++ b/repair/Makefile
@@ -10,11 +10,11 @@ LSRCFILES = README
 LTCOMMAND = xfs_repair
 
 HFILES = agheader.h attr_repair.h avl.h avl64.h bmap.h btree.h \
-	dinode.h dir2.h err_protos.h globals.h incore.h protos.h rt.h \
-	progress.h scan.h versions.h prefetch.h threads.h
+	da_util.h dinode.h dir2.h err_protos.h globals.h incore.h protos.h \
+	rt.h progress.h scan.h versions.h prefetch.h threads.h
 
 CFILES = agheader.c attr_repair.c avl.c avl64.c bmap.c btree.c \
-	dino_chunks.c dinode.c dir2.c globals.c incore.c \
+	da_util.c dino_chunks.c dinode.c dir2.c globals.c incore.c \
 	incore_bmc.c init.c incore_ext.c incore_ino.c phase1.c \
 	phase2.c phase3.c phase4.c phase5.c phase6.c phase7.c \
 	progress.c prefetch.c rt.c sb.c scan.c threads.c \
diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 0804a22..0d3b7a5 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -24,6 +24,7 @@
 #include "bmap.h"
 #include "protos.h"
 #include "dir2.h"
+#include "da_util.h"
 
 static int xfs_acl_valid(struct xfs_mount *mp, struct xfs_acl *daclp);
 static int xfs_mac_valid(xfs_mac_label_t *lp);
@@ -39,43 +40,6 @@ static int xfs_mac_valid(xfs_mac_label_t *lp);
 typedef unsigned char	da_freemap_t;
 
 /*
- * the cursor gets passed up and down the da btree processing
- * routines.  The interior block processing routines use the
- * cursor to determine if the pointers to and from the preceding
- * and succeeding sibling blocks are ok and whether the values in
- * the current block are consistent with the entries in the parent
- * nodes.  When a block is traversed, a parent-verification routine
- * is called to verify if the next logical entry in the next level up
- * is consistent with the greatest hashval in the next block of the
- * current level.  The verification routine is itself recursive and
- * calls itself if it has to traverse an interior block to get
- * the next logical entry.  The routine recurses upwards through
- * the tree until it finds a block where it can simply step to
- * the next entry.  The hashval in that entry should be equal to
- * the hashval being passed to it (the greatest hashval in the block
- * that the entry points to).  If that isn't true, then the tree
- * is blown and we need to trash it, salvage and trash it, or fix it.
- * Currently, we just trash it.
- */
-typedef struct da_level_state  {
-	xfs_buf_t	*bp;		/* block bp */
-	xfs_dablk_t	bno;		/* file block number */
-	xfs_dahash_t	hashval;	/* last verified hashval */
-	int		index;		/* current index in block */
-	int		dirty;		/* is buffer dirty ? (1 == yes) */
-} da_level_state_t;
-
-typedef struct da_bt_cursor  {
-	int			active;	/* highest level in tree (# levels-1) */
-	xfs_ino_t		ino;
-	xfs_dablk_t		greatest_bno;
-	xfs_dinode_t		*dip;
-	da_level_state_t	level[XFS_DA_NODE_MAXDEPTH];
-	struct blkmap		*blkmap;
-} da_bt_cursor_t;
-
-
-/*
  * Allocate a freespace map for directory or attr leaf blocks (1 bit per byte)
  * 1 == used, 0 == free.
  */
@@ -127,577 +91,6 @@ set_da_freemap(xfs_mount_t *mp, da_freemap_t *map, int start, int stop)
 }
 
 /*
- * walk tree from root to the left-most leaf block reading in
- * blocks and setting up cursor.  passes back file block number of the
- * left-most leaf block if successful (bno).  returns 1 if successful,
- * 0 if unsuccessful.
- */
-static int
-traverse_int_dablock(xfs_mount_t	*mp,
-		da_bt_cursor_t		*da_cursor,
-		xfs_dablk_t		*rbno,
-		int			whichfork)
-{
-	bmap_ext_t		*bmp;
-	xfs_dablk_t		bno;
-	int			i;
-	int			nex;
-	xfs_da_intnode_t	*node;
-	bmap_ext_t		lbmp;
-	xfs_fsblock_t		fsbno;
-	xfs_buf_t		*bp;
-	struct xfs_da_geometry	*geo;
-	struct xfs_da_node_entry *btree;
-	struct xfs_da3_icnode_hdr nodehdr;
-
-	geo = mp->m_attr_geo;
-
-	/*
-	 * traverse down left-side of tree until we hit the
-	 * left-most leaf block setting up the btree cursor along
-	 * the way.
-	 */
-	bno = 0;
-	i = -1;
-	node = NULL;
-	da_cursor->active = 0;
-
-	do {
-		/*
-		 * read in each block along the way and set up cursor
-		 */
-		nex = blkmap_getn(da_cursor->blkmap, bno,
-				geo->fsbcount, &bmp, &lbmp);
-
-		if (nex == 0)
-			goto error_out;
-
-		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
-		if (bmp != &lbmp)
-			free(bmp);
-
-		if (!bp) {
-			do_warn(
-	_("can't read block %u (fsbno %" PRIu64 ") for attrbute fork of inode %" PRIu64 "\n"),
-					bno, fsbno, da_cursor->ino);
-			goto error_out;
-		}
-
-		node = bp->b_addr;
-		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
-
-		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
-		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
-			do_warn(_("bad dir/attr magic number in inode %" PRIu64 ", "
-				  "file bno = %u, fsbno = %" PRIu64 "\n"),
-				da_cursor->ino, bno, fsbno);
-			libxfs_putbuf(bp);
-			goto error_out;
-		}
-
-		/* corrupt node; rebuild the dir. */
-		if (bp->b_error == -EFSBADCRC || bp->b_error == -EFSCORRUPTED) {
-			libxfs_putbuf(bp);
-			do_warn(
-_("corrupt tree block %u for directory inode %" PRIu64 "\n"),
-				bno, da_cursor->ino);
-			goto error_out;
-		}
-
-		btree = M_DIROPS(mp)->node_tree_p(node);
-		if (nodehdr.count > geo->node_ents) {
-			do_warn(_("bad record count in inode %" PRIu64 ", "
-				  "count = %d, max = %d\n"),
-				da_cursor->ino, nodehdr.count, geo->node_ents);
-			libxfs_putbuf(bp);
-			goto error_out;
-		}
-
-		/*
-		 * maintain level counter
-		 */
-		if (i == -1) {
-			i = da_cursor->active = nodehdr.level;
-			if (i < 1 || i >= XFS_DA_NODE_MAXDEPTH) {
-				do_warn(
-_("bad header depth for directory inode %" PRIu64 "\n"),
-					da_cursor->ino);
-				libxfs_putbuf(bp);
-				i = -1;
-				goto error_out;
-			}
-		} else {
-			if (nodehdr.level == i - 1) {
-				i--;
-			} else {
-				do_warn(_("bad attribute fork btree "
-					  "for inode %" PRIu64 "\n"),
-					da_cursor->ino);
-				libxfs_putbuf(bp);
-				goto error_out;
-			}
-		}
-
-		da_cursor->level[i].hashval = be32_to_cpu(btree[0].hashval);
-		da_cursor->level[i].bp = bp;
-		da_cursor->level[i].bno = bno;
-		da_cursor->level[i].index = 0;
-
-		/*
-		 * set up new bno for next level down
-		 */
-		bno = be32_to_cpu(btree[0].before);
-	} while (node != NULL && i > 1);
-
-	/*
-	 * now return block number and get out
-	 */
-	*rbno = da_cursor->level[0].bno = bno;
-	return(1);
-
-error_out:
-	while (i > 1 && i <= da_cursor->active) {
-		libxfs_putbuf(da_cursor->level[i].bp);
-		i++;
-	}
-
-	return(0);
-}
-
-/*
- * blow out buffer for this level and all the rest above as well
- * if error == 0, we are not expecting to encounter any unreleased
- * buffers (e.g. if we do, it's a mistake).  if error == 1, we're
- * in an error-handling case so unreleased buffers may exist.
- */
-static void
-release_da_cursor_int(xfs_mount_t	*mp,
-			da_bt_cursor_t	*cursor,
-			int		prev_level,
-			int		error)
-{
-	int	level = prev_level + 1;
-
-	if (cursor->level[level].bp != NULL)  {
-		if (!error)  {
-			do_warn(_("release_da_cursor_int got unexpected "
-				  "non-null bp, dabno = %u\n"),
-				cursor->level[level].bno);
-		}
-		ASSERT(error != 0);
-
-		libxfs_putbuf(cursor->level[level].bp);
-		cursor->level[level].bp = NULL;
-	}
-
-	if (level < cursor->active)
-		release_da_cursor_int(mp, cursor, level, error);
-
-	return;
-}
-
-static void
-release_da_cursor(xfs_mount_t	*mp,
-		da_bt_cursor_t	*cursor,
-		int		prev_level)
-{
-	release_da_cursor_int(mp, cursor, prev_level, 0);
-}
-
-static void
-err_release_da_cursor(xfs_mount_t	*mp,
-			da_bt_cursor_t	*cursor,
-			int		prev_level)
-{
-	release_da_cursor_int(mp, cursor, prev_level, 1);
-}
-
-/*
- * make sure that all entries in all blocks along the right side of
- * of the tree are used and hashval's are consistent.  level is the
- * level of the descendent block.  returns 0 if good (even if it had
- * to be fixed up), and 1 if bad.  The right edge of the tree is
- * technically a block boundary.  this routine should be used then
- * instead of verify_da_path().
- */
-static int
-verify_final_da_path(xfs_mount_t	*mp,
-		da_bt_cursor_t		*cursor,
-		const int		p_level)
-{
-	xfs_da_intnode_t	*node;
-	xfs_dahash_t		hashval;
-	int			bad = 0;
-	int			entry;
-	int			this_level = p_level + 1;
-	struct xfs_da_node_entry *btree;
-	struct xfs_da3_icnode_hdr nodehdr;
-
-#ifdef XR_DIR_TRACE
-	fprintf(stderr, "in verify_final_da_path, this_level = %d\n",
-		this_level);
-#endif
-	/*
-	 * the index should point to the next "unprocessed" entry
-	 * in the block which should be the final (rightmost) entry
-	 */
-	entry = cursor->level[this_level].index;
-	node = cursor->level[this_level].bp->b_addr;
-	btree = M_DIROPS(mp)->node_tree_p(node);
-	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
-
-	/*
-	 * check internal block consistency on this level -- ensure
-	 * that all entries are used, encountered and expected hashvals
-	 * match, etc.
-	 */
-	if (entry != nodehdr.count - 1) {
-		do_warn(_("directory/attribute block used/count "
-			  "inconsistency - %d/%hu\n"),
-			entry, nodehdr.count);
-		bad++;
-	}
-	/*
-	 * hash values monotonically increasing ???
-	 */
-	if (cursor->level[this_level].hashval >= 
-				be32_to_cpu(btree[entry].hashval)) {
-		do_warn(_("directory/attribute block hashvalue inconsistency, "
-			  "expected > %u / saw %u\n"),
-			cursor->level[this_level].hashval,
-			be32_to_cpu(btree[entry].hashval));
-		bad++;
-	}
-	if (nodehdr.forw != 0) {
-		do_warn(_("bad directory/attribute forward block pointer, "
-			  "expected 0, saw %u\n"),
-			nodehdr.forw);
-		bad++;
-	}
-	if (bad) {
-		do_warn(_("bad directory block in dir ino %" PRIu64 "\n"),
-			cursor->ino);
-		return(1);
-	}
-	/*
-	 * keep track of greatest block # -- that gets
-	 * us the length of the directory
-	 */
-	if (cursor->level[this_level].bno > cursor->greatest_bno)
-		cursor->greatest_bno = cursor->level[this_level].bno;
-
-	/*
-	 * ok, now check descendant block number against this level
-	 */
-	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
-#ifdef XR_DIR_TRACE
-		fprintf(stderr, "bad directory btree pointer, child bno should "
-				"be %d, block bno is %d, hashval is %u\n",
-			be16_to_cpu(btree[entry].before),
-			cursor->level[p_level].bno,
-			cursor->level[p_level].hashval);
-		fprintf(stderr, "verify_final_da_path returns 1 (bad) #1a\n");
-#endif
-		return(1);
-	}
-
-	if (cursor->level[p_level].hashval != be32_to_cpu(btree[entry].hashval)) {
-		if (!no_modify) {
-			do_warn(_("correcting bad hashval in non-leaf "
-				  "dir/attr block\n\tin (level %d) in "
-				  "inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
-			btree[entry].hashval = cpu_to_be32(
-						cursor->level[p_level].hashval);
-			cursor->level[this_level].dirty++;
-		} else {
-			do_warn(_("would correct bad hashval in non-leaf "
-				  "dir/attr block\n\tin (level %d) in "
-				  "inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
-		}
-	}
-
-	/*
-	 * Note: squirrel hashval away _before_ releasing the
-	 * buffer, preventing a use-after-free problem.
-	 */
-	hashval = be32_to_cpu(btree[entry].hashval);
-
-	/*
-	 * release/write buffer
-	 */
-	ASSERT(cursor->level[this_level].dirty == 0 ||
-		(cursor->level[this_level].dirty && !no_modify));
-
-	if (cursor->level[this_level].dirty && !no_modify)
-		libxfs_writebuf(cursor->level[this_level].bp, 0);
-	else
-		libxfs_putbuf(cursor->level[this_level].bp);
-
-	cursor->level[this_level].bp = NULL;
-
-	/*
-	 * bail out if this is the root block (top of tree)
-	 */
-	if (this_level >= cursor->active) {
-#ifdef XR_DIR_TRACE
-		fprintf(stderr, "verify_final_da_path returns 0 (ok)\n");
-#endif
-		return(0);
-	}
-	/*
-	 * set hashvalue to correctly reflect the now-validated
-	 * last entry in this block and continue upwards validation
-	 */
-	cursor->level[this_level].hashval = hashval;
-	return(verify_final_da_path(mp, cursor, this_level));
-}
-
-/*
- * Verifies the path from a descendant block up to the root.
- * Should be called when the descendant level traversal hits
- * a block boundary before crossing the boundary (reading in a new
- * block).
- *
- * the directory/attr btrees work differently to the other fs btrees.
- * each interior block contains records that are <hashval, bno>
- * pairs.  The bno is a file bno, not a filesystem bno.  The last
- * hashvalue in the block <bno> will be <hashval>.  BUT unlike
- * the freespace btrees, the *last* value in each block gets
- * propagated up the tree instead of the first value in each block.
- * that is, the interior records point to child blocks and the *greatest*
- * hash value contained by the child block is the one the block above
- * uses as the key for the child block.
- *
- * level is the level of the descendent block.  returns 0 if good,
- * and 1 if bad.  The descendant block may be a leaf block.
- *
- * the invariant here is that the values in the cursor for the
- * levels beneath this level (this_level) and the cursor index
- * for this level *must* be valid.
- *
- * that is, the hashval/bno info is accurate for all
- * DESCENDANTS and match what the node[index] information
- * for the current index in the cursor for this level.
- *
- * the index values in the cursor for the descendant level
- * are allowed to be off by one as they will reflect the
- * next entry at those levels to be processed.
- *
- * the hashvalue for the current level can't be set until
- * we hit the last entry in the block so, it's garbage
- * until set by this routine.
- *
- * bno and bp for the current block/level are always valid
- * since they have to be set so we can get a buffer for the
- * block.
- */
-static int
-verify_da_path(xfs_mount_t	*mp,
-	da_bt_cursor_t		*cursor,
-	const int		p_level)
-{
-	xfs_da_intnode_t	*node;
-	xfs_da_intnode_t	*newnode;
-	xfs_fsblock_t		fsbno;
-	xfs_dablk_t		dabno;
-	xfs_buf_t		*bp;
-	int			bad;
-	int			entry;
-	int			this_level = p_level + 1;
-	bmap_ext_t		*bmp;
-	int			nex;
-	bmap_ext_t		lbmp;
-	struct xfs_da_geometry	*geo;
-	struct xfs_da_node_entry *btree;
-	struct xfs_da3_icnode_hdr nodehdr;
-
-	geo = mp->m_attr_geo;
-
-	/*
-	 * index is currently set to point to the entry that
-	 * should be processed now in this level.
-	 */
-	entry = cursor->level[this_level].index;
-	node = cursor->level[this_level].bp->b_addr;
-	btree = M_DIROPS(mp)->node_tree_p(node);
-	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
-
-	/*
-	 * if this block is out of entries, validate this
-	 * block and move on to the next block.
-	 * and update cursor value for said level
-	 */
-	if (entry >= nodehdr.count) {
-		/*
-		 * update the hash value for this level before
-		 * validating it.  bno value should be ok since
-		 * it was set when the block was first read in.
-		 */
-		cursor->level[this_level].hashval =
-				be32_to_cpu(btree[entry - 1].hashval);
-
-		/*
-		 * keep track of greatest block # -- that gets
-		 * us the length of the directory
-		 */
-		if (cursor->level[this_level].bno > cursor->greatest_bno)
-			cursor->greatest_bno = cursor->level[this_level].bno;
-
-		/*
-		 * validate the path for the current used-up block
-		 * before we trash it
-		 */
-		if (verify_da_path(mp, cursor, this_level))
-			return(1);
-		/*
-		 * ok, now get the next buffer and check sibling pointers
-		 */
-		dabno = nodehdr.forw;
-		ASSERT(dabno != 0);
-		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
-			&bmp, &lbmp);
-		if (nex == 0) {
-			do_warn(
-_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
-				dabno, cursor->ino);
-			return(1);
-		}
-
-		fsbno = bmp[0].startblock;
-
-		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
-		if (bmp != &lbmp)
-			free(bmp);
-
-		if (!bp) {
-			do_warn(
-	_("can't read block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
-				dabno, fsbno, cursor->ino);
-			return(1);
-		}
-
-		newnode = bp->b_addr;
-		btree = M_DIROPS(mp)->node_tree_p(newnode);
-		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, newnode);
-
-		/*
-		 * verify magic number and back pointer, sanity-check
-		 * entry count, verify level
-		 */
-		bad = 0;
-		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
-		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
-			do_warn(
-	_("bad magic number %x in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
-				nodehdr.magic,
-				dabno, fsbno, cursor->ino);
-			bad++;
-		}
-		if (nodehdr.back != cursor->level[this_level].bno) {
-			do_warn(
-	_("bad back pointer in block %u (%"PRIu64 ") for directory inode %" PRIu64 "\n"),
-				dabno, fsbno, cursor->ino);
-			bad++;
-		}
-		if (nodehdr.count > geo->node_ents) {
-			do_warn(
-	_("entry count %d too large in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
-				nodehdr.count,
-				dabno, fsbno, cursor->ino);
-			bad++;
-		}
-		if (nodehdr.level != this_level) {
-			do_warn(
-	_("bad level %d in block %u (%" PRIu64 ") for directory inode %" PRIu64 "\n"),
-				nodehdr.level,
-				dabno, fsbno, cursor->ino);
-			bad++;
-		}
-		if (bad) {
-#ifdef XR_DIR_TRACE
-			fprintf(stderr, "verify_da_path returns 1 (bad) #4\n");
-#endif
-			libxfs_putbuf(bp);
-			return(1);
-		}
-
-		/*
-		 * update cursor, write out the *current* level if
-		 * required.  don't write out the descendant level
-		 */
-		ASSERT(cursor->level[this_level].dirty == 0 ||
-			(cursor->level[this_level].dirty && !no_modify));
-
-		/*
-		 * If block looks ok but CRC didn't match, make sure to
-		 * recompute it.
-		 */
-		if (!no_modify &&
-		    cursor->level[this_level].bp->b_error == -EFSBADCRC)
-			cursor->level[this_level].dirty = 1;
-
-		if (cursor->level[this_level].dirty && !no_modify)
-			libxfs_writebuf(cursor->level[this_level].bp, 0);
-		else
-			libxfs_putbuf(cursor->level[this_level].bp);
-
-		/* switch cursor to point at the new buffer we just read */
-		cursor->level[this_level].bp = bp;
-		cursor->level[this_level].dirty = 0;
-		cursor->level[this_level].bno = dabno;
-		cursor->level[this_level].hashval =
-					be32_to_cpu(btree[0].hashval);
-		entry = cursor->level[this_level].index = 0;
-	}
-	/*
-	 * ditto for block numbers
-	 */
-	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
-#ifdef XR_DIR_TRACE
-		fprintf(stderr, "bad directory btree pointer, child bno "
-			"should be %d, block bno is %d, hashval is %u\n",
-			be32_to_cpu(btree[entry].before),
-			cursor->level[p_level].bno,
-			cursor->level[p_level].hashval);
-		fprintf(stderr, "verify_da_path returns 1 (bad) #1a\n");
-#endif
-		return(1);
-	}
-	/*
-	 * ok, now validate last hashvalue in the descendant
-	 * block against the hashval in the current entry
-	 */
-	if (cursor->level[p_level].hashval !=
-				be32_to_cpu(btree[entry].hashval)) {
-		if (!no_modify) {
-			do_warn(_("correcting bad hashval in interior "
-				  "dir/attr block\n\tin (level %d) in "
-				  "inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
-			btree[entry].hashval = cpu_to_be32(
-						cursor->level[p_level].hashval);
-			cursor->level[this_level].dirty++;
-		} else {
-			do_warn(_("would correct bad hashval in interior "
-				  "dir/attr block\n\tin (level %d) in "
-				  "inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
-		}
-	}
-	/*
-	 * increment index for this level to point to next entry
-	 * (which should point to the next descendant block)
-	 */
-	cursor->level[this_level].index++;
-#ifdef XR_DIR_TRACE
-	fprintf(stderr, "verify_da_path returns 0 (ok)\n");
-#endif
-	return(0);
-}
-
-/*
  * For attribute repair, there are 3 formats to worry about. First, is
  * shortform attributes which reside in the inode. Second is the leaf
  * form, and lastly the btree. Much of this models after the directory
@@ -1435,9 +828,11 @@ process_leaf_attr_level(xfs_mount_t	*mp,
 		prev_bno = da_bno;
 		da_bno = leafhdr.forw;
 
-		if (da_bno != 0 && verify_da_path(mp, da_cursor, 0))  {
-			libxfs_putbuf(bp);
-			goto error_out;
+		if (da_bno != 0) {
+			if (verify_da_path(mp, da_cursor, 0, XFS_ATTR_FORK)) {
+				libxfs_putbuf(bp);
+				goto error_out;
+			}
 		}
 
 		current_hashval = greatest_hashval;
diff --git a/repair/da_util.c b/repair/da_util.c
new file mode 100644
index 0000000..e5d5535
--- /dev/null
+++ b/repair/da_util.c
@@ -0,0 +1,701 @@
+/*
+ * Copyright (c) 2015 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
+ */
+
+/* Various utilities for repair of directory and attribute metadata */
+
+#include "libxfs.h"
+#include "globals.h"
+#include "err_protos.h"
+#include "bmap.h"
+#include "da_util.h"
+
+/*
+ * takes a name and length (name need not be null-terminated)
+ * and returns 1 if the name contains a '/' or a \0, returns 0
+ * otherwise
+ */
+int
+namecheck(char *name, int length)
+{
+	char *c;
+	int i;
+
+	ASSERT(length < MAXNAMELEN);
+
+	for (c = name, i = 0; i < length; i++, c++) {
+		if (*c == '/' || *c == '\0')
+			return 1;
+	}
+
+	return 0;
+}
+
+/*
+ * the cursor gets passed up and down the da btree processing
+ * routines.  The interior block processing routines use the
+ * cursor to determine if the pointers to and from the preceding
+ * and succeeding sibling blocks are ok and whether the values in
+ * the current block are consistent with the entries in the parent
+ * nodes.  When a block is traversed, a parent-verification routine
+ * is called to verify if the next logical entry in the next level up
+ * is consistent with the greatest hashval in the next block of the
+ * current level.  The verification routine is itself recursive and
+ * calls itself if it has to traverse an interior block to get
+ * the next logical entry.  The routine recurses upwards through
+ * the tree until it finds a block where it can simply step to
+ * the next entry.  The hashval in that entry should be equal to
+ * the hashval being passed to it (the greatest hashval in the block
+ * that the entry points to).  If that isn't true, then the tree
+ * is blown and we need to trash it, salvage and trash it, or fix it.
+ * Currently, we just trash it.
+ */
+
+/*
+ * Multibuffer handling.
+ * V2 directory blocks can be noncontiguous, needing multiple buffers.
+ */
+struct xfs_buf *
+da_read_buf(
+	xfs_mount_t	*mp,
+	int		nex,
+	bmap_ext_t	*bmp,
+	const struct xfs_buf_ops *ops)
+{
+#define MAP_ARRAY_SZ 4
+	struct xfs_buf_map map_array[MAP_ARRAY_SZ];
+	struct xfs_buf_map *map;
+	struct xfs_buf	*bp;
+	int		i;
+
+	if (nex > MAP_ARRAY_SZ) {
+		map = calloc(nex, sizeof(*map));
+		if (map == NULL) {
+			do_error(_("couldn't malloc dir2 buffer list\n"));
+			exit(1);
+		}
+	} else {
+		/* common case avoids calloc/free */
+		map = map_array;
+	}
+	for (i = 0; i < nex; i++) {
+		map[i].bm_bn = XFS_FSB_TO_DADDR(mp, bmp[i].startblock);
+		map[i].bm_len = XFS_FSB_TO_BB(mp, bmp[i].blockcount);
+	}
+	bp = libxfs_readbuf_map(mp->m_dev, map, nex, 0, ops);
+	if (map != map_array)
+		free(map);
+	return bp;
+}
+
+/*
+ * walk tree from root to the left-most leaf block reading in
+ * blocks and setting up cursor.  passes back file block number of the
+ * left-most leaf block if successful (bno).  returns 1 if successful,
+ * 0 if unsuccessful.
+ */
+int
+traverse_int_dablock(
+	xfs_mount_t		*mp,
+	da_bt_cursor_t		*da_cursor,
+	xfs_dablk_t		*rbno,
+	int			whichfork)
+{
+	bmap_ext_t		*bmp;
+	xfs_dablk_t		bno;
+	struct xfs_buf		*bp;
+	int			i;
+	int			nex;
+	xfs_da_intnode_t	*node;
+	bmap_ext_t		lbmp;
+	struct xfs_da_geometry	*geo;
+	struct xfs_da_node_entry *btree;
+	struct xfs_da3_icnode_hdr nodehdr;
+
+	if (whichfork == XFS_DATA_FORK) {
+		geo = mp->m_dir_geo;
+		bno = geo->leafblk;
+	} else {
+		geo = mp->m_attr_geo;
+		bno = 0;
+	}
+
+	/*
+	 * traverse down left-side of tree until we hit the
+	 * left-most leaf block setting up the btree cursor along
+	 * the way.
+	 */
+	i = -1;
+	node = NULL;
+	da_cursor->active = 0;
+
+	do {
+		/*
+		 * read in each block along the way and set up cursor
+		 */
+		nex = blkmap_getn(da_cursor->blkmap, bno,
+				geo->fsbcount, &bmp, &lbmp);
+
+		if (nex == 0)
+			goto error_out;
+
+		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
+		if (bmp != &lbmp)
+			free(bmp);
+
+		if (!bp) {
+			do_warn(
+_("can't read block %u for directory inode %" PRIu64 "\n"),
+				bno, da_cursor->ino);
+			goto error_out;
+		}
+
+		node = bp->b_addr;
+		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
+
+		if (whichfork == XFS_DATA_FORK &&
+		    (nodehdr.magic == XFS_DIR2_LEAFN_MAGIC ||
+		    nodehdr.magic == XFS_DIR3_LEAFN_MAGIC)) {
+			if (i != -1) {
+				do_warn(
+_("found non-root LEAFN node in inode %" PRIu64 " bno = %u\n"),
+					da_cursor->ino, bno);
+			}
+			*rbno = 0;
+			libxfs_putbuf(bp);
+			return 1;
+		}
+
+		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
+		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
+			do_warn(
+_("bad dir magic number 0x%x in inode %" PRIu64 " bno = %u\n"),
+					nodehdr.magic,
+					da_cursor->ino, bno);
+			libxfs_putbuf(bp);
+			goto error_out;
+		}
+
+		/* corrupt node; rebuild the dir. */
+		if (bp->b_error == -EFSBADCRC || bp->b_error == -EFSCORRUPTED) {
+			libxfs_putbuf(bp);
+			do_warn(
+_("corrupt tree block %u for directory inode %" PRIu64 "\n"),
+				bno, da_cursor->ino);
+			goto error_out;
+		}
+
+		btree = M_DIROPS(mp)->node_tree_p(node);
+		if (nodehdr.count > geo->node_ents) {
+			do_warn(
+_("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
+				da_cursor->ino, nodehdr.count, geo->node_ents);
+			libxfs_putbuf(bp);
+			goto error_out;
+		}
+
+		/*
+		 * maintain level counter
+		 */
+		if (i == -1) {
+			i = da_cursor->active = nodehdr.level;
+			if (i < 1 || i >= XFS_DA_NODE_MAXDEPTH) {
+				do_warn(
+_("bad header depth for directory inode %" PRIu64 "\n"),
+					da_cursor->ino);
+				libxfs_putbuf(bp);
+				i = -1;
+				goto error_out;
+			}
+		} else {
+			if (nodehdr.level == i - 1) {
+				i--;
+			} else {
+				do_warn(
+_("bad directory btree for directory inode %" PRIu64 "\n"),
+					da_cursor->ino);
+				libxfs_putbuf(bp);
+				goto error_out;
+			}
+		}
+
+		da_cursor->level[i].hashval = be32_to_cpu(btree[0].hashval);
+		da_cursor->level[i].bp = bp;
+		da_cursor->level[i].bno = bno;
+		da_cursor->level[i].index = 0;
+
+		/*
+		 * set up new bno for next level down
+		 */
+		bno = be32_to_cpu(btree[0].before);
+	} while (node != NULL && i > 1);
+
+	/*
+	 * now return block number and get out
+	 */
+	*rbno = da_cursor->level[0].bno = bno;
+	return 1;
+
+error_out:
+	while (i > 1 && i <= da_cursor->active) {
+		libxfs_putbuf(da_cursor->level[i].bp);
+		i++;
+	}
+
+	return 0;
+}
+
+/*
+ * blow out buffer for this level and all the rest above as well
+ * if error == 0, we are not expecting to encounter any unreleased
+ * buffers (e.g. if we do, it's a mistake).  if error == 1, we're
+ * in an error-handling case so unreleased buffers may exist.
+ */
+static void
+release_da_cursor_int(
+	xfs_mount_t	*mp,
+	da_bt_cursor_t	*cursor,
+	int		prev_level,
+	int		error)
+{
+	int		level = prev_level + 1;
+
+	if (cursor->level[level].bp != NULL)  {
+		if (!error)  {
+			do_warn(_("release_da_cursor_int got unexpected "
+				  "non-null bp, dabno = %u\n"),
+				cursor->level[level].bno);
+		}
+		ASSERT(error != 0);
+
+		libxfs_putbuf(cursor->level[level].bp);
+		cursor->level[level].bp = NULL;
+	}
+
+	if (level < cursor->active)
+		release_da_cursor_int(mp, cursor, level, error);
+
+	return;
+}
+
+void
+release_da_cursor(
+	xfs_mount_t	*mp,
+	da_bt_cursor_t	*cursor,
+	int		prev_level)
+{
+	release_da_cursor_int(mp, cursor, prev_level, 0);
+}
+
+void
+err_release_da_cursor(
+	xfs_mount_t	*mp,
+	da_bt_cursor_t	*cursor,
+	int		prev_level)
+{
+	release_da_cursor_int(mp, cursor, prev_level, 1);
+}
+
+/*
+ * make sure that all entries in all blocks along the right side of
+ * of the tree are used and hashval's are consistent.  level is the
+ * level of the descendent block.  returns 0 if good (even if it had
+ * to be fixed up), and 1 if bad.  The right edge of the tree is
+ * technically a block boundary.  This routine should be used then
+ * instead of verify_da_path().
+ */
+int
+verify_final_da_path(
+	xfs_mount_t		*mp,
+	da_bt_cursor_t		*cursor,
+	const int		p_level)
+{
+	xfs_da_intnode_t	*node;
+	xfs_dahash_t		hashval;
+	int			bad = 0;
+	int			entry;
+	int			this_level = p_level + 1;
+	struct xfs_da_node_entry *btree;
+	struct xfs_da3_icnode_hdr nodehdr;
+
+#ifdef XR_DIR_TRACE
+	fprintf(stderr, "in verify_final_da_path, this_level = %d\n",
+		this_level);
+#endif
+
+	/*
+	 * the index should point to the next "unprocessed" entry
+	 * in the block which should be the final (rightmost) entry
+	 */
+	entry = cursor->level[this_level].index;
+	node = cursor->level[this_level].bp->b_addr;
+	btree = M_DIROPS(mp)->node_tree_p(node);
+	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
+
+	/*
+	 * check internal block consistency on this level -- ensure
+	 * that all entries are used, encountered and expected hashvals
+	 * match, etc.
+	 */
+	if (entry != nodehdr.count - 1) {
+		do_warn(
+		_("directory block used/count inconsistency - %d/%hu\n"),
+			entry, nodehdr.count);
+		bad++;
+	}
+	/*
+	 * hash values monotonically increasing ???
+	 */
+	if (cursor->level[this_level].hashval >=
+				be32_to_cpu(btree[entry].hashval)) {
+		do_warn(_("directory/attribute block hashvalue inconsistency, "
+			  "expected > %u / saw %u\n"),
+			cursor->level[this_level].hashval,
+			be32_to_cpu(btree[entry].hashval));
+		bad++;
+	}
+	if (nodehdr.forw != 0) {
+		do_warn(_("bad directory/attribute forward block pointer, "
+			  "expected 0, saw %u\n"),
+			nodehdr.forw);
+		bad++;
+	}
+	if (bad) {
+		do_warn(_("bad directory block in inode %" PRIu64 "\n"), cursor->ino);
+		return 1;
+	}
+	/*
+	 * keep track of greatest block # -- that gets
+	 * us the length of the directory
+	 */
+	if (cursor->level[this_level].bno > cursor->greatest_bno)
+		cursor->greatest_bno = cursor->level[this_level].bno;
+
+	/*
+	 * ok, now check descendant block number against this level
+	 */
+	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
+#ifdef XR_DIR_TRACE
+		fprintf(stderr, "bad directory btree pointer, child bno should "
+				"be %d, block bno is %d, hashval is %u\n",
+			be16_to_cpu(btree[entry].before),
+			cursor->level[p_level].bno,
+			cursor->level[p_level].hashval);
+		fprintf(stderr, "verify_final_da_path returns 1 (bad) #1a\n");
+#endif
+		return 1;
+	}
+
+	if (cursor->level[p_level].hashval !=
+				be32_to_cpu(btree[entry].hashval)) {
+		if (!no_modify) {
+			do_warn(
+_("correcting bad hashval in non-leaf dir block\n"
+ "\tin (level %d) in inode %" PRIu64 ".\n"),
+				this_level, cursor->ino);
+			btree[entry].hashval = cpu_to_be32(
+						cursor->level[p_level].hashval);
+			cursor->level[this_level].dirty++;
+		} else {
+			do_warn(
+_("would correct bad hashval in non-leaf dir block\n"
+ "\tin (level %d) in inode %" PRIu64 ".\n"),
+				this_level, cursor->ino);
+		}
+	}
+
+	/*
+	 * Note: squirrel hashval away _before_ releasing the
+	 * buffer, preventing a use-after-free problem.
+	 */
+	hashval = be32_to_cpu(btree[entry].hashval);
+
+	/*
+	 * release/write buffer
+	 */
+	ASSERT(cursor->level[this_level].dirty == 0 ||
+		(cursor->level[this_level].dirty && !no_modify));
+
+	if (cursor->level[this_level].dirty && !no_modify)
+		libxfs_writebuf(cursor->level[this_level].bp, 0);
+	else
+		libxfs_putbuf(cursor->level[this_level].bp);
+
+	cursor->level[this_level].bp = NULL;
+
+	/*
+	 * bail out if this is the root block (top of tree)
+	 */
+	if (this_level >= cursor->active) {
+#ifdef XR_DIR_TRACE
+		fprintf(stderr, "verify_final_da_path returns 0 (ok)\n");
+#endif
+		return 0;
+	}
+	/*
+	 * set hashvalue to correctly reflect the now-validated
+	 * last entry in this block and continue upwards validation
+	 */
+	cursor->level[this_level].hashval = hashval;
+
+	return verify_final_da_path(mp, cursor, this_level);
+}
+
+/*
+ * Verifies the path from a descendant block up to the root.
+ * Should be called when the descendant level traversal hits
+ * a block boundary before crossing the boundary (reading in a new
+ * block).
+ *
+ * the directory/attr btrees work differently to the other fs btrees.
+ * each interior block contains records that are <hashval, bno>
+ * pairs.  The bno is a file bno, not a filesystem bno.  The last
+ * hashvalue in the block <bno> will be <hashval>.  BUT unlike
+ * the freespace btrees, the *last* value in each block gets
+ * propagated up the tree instead of the first value in each block.
+ * that is, the interior records point to child blocks and the *greatest*
+ * hash value contained by the child block is the one the block above
+ * uses as the key for the child block.
+ *
+ * level is the level of the descendent block.  returns 0 if good,
+ * and 1 if bad.  The descendant block may be a leaf block.
+ *
+ * the invariant here is that the values in the cursor for the
+ * levels beneath this level (this_level) and the cursor index
+ * for this level *must* be valid.
+ *
+ * that is, the hashval/bno info is accurate for all
+ * DESCENDANTS and match what the node[index] information
+ * for the current index in the cursor for this level.
+ *
+ * the index values in the cursor for the descendant level
+ * are allowed to be off by one as they will reflect the
+ * next entry at those levels to be processed.
+ *
+ * the hashvalue for the current level can't be set until
+ * we hit the last entry in the block so, it's garbage
+ * until set by this routine.
+ *
+ * bno and bp for the current block/level are always valid
+ * since they have to be set so we can get a buffer for the
+ * block.
+ */
+int
+verify_da_path(
+	xfs_mount_t		*mp,
+	da_bt_cursor_t		*cursor,
+	const int		p_level,
+	int			whichfork)
+{
+	xfs_da_intnode_t	*node;
+	xfs_da_intnode_t	*newnode;
+	xfs_dablk_t		dabno;
+	struct xfs_buf		*bp;
+	int			bad;
+	int			entry;
+	int			this_level = p_level + 1;
+	bmap_ext_t		*bmp;
+	int			nex;
+	bmap_ext_t		lbmp;
+	struct xfs_da_geometry	*geo;
+	struct xfs_da_node_entry *btree;
+	struct xfs_da3_icnode_hdr nodehdr;
+
+	if (whichfork == XFS_DATA_FORK)
+		geo = mp->m_dir_geo;
+	else
+		geo = mp->m_attr_geo;
+
+	/*
+	 * index is currently set to point to the entry that
+	 * should be processed now in this level.
+	 */
+	entry = cursor->level[this_level].index;
+	node = cursor->level[this_level].bp->b_addr;
+	btree = M_DIROPS(mp)->node_tree_p(node);
+	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
+
+	/*
+	 * if this block is out of entries, validate this
+	 * block and move on to the next block.
+	 * and update cursor value for said level
+	 */
+	if (entry >= nodehdr.count) {
+		/*
+		 * update the hash value for this level before
+		 * validating it.  bno value should be ok since
+		 * it was set when the block was first read in.
+		 */
+		cursor->level[this_level].hashval =
+				be32_to_cpu(btree[entry - 1].hashval);
+
+		/*
+		 * keep track of greatest block # -- that gets
+		 * us the length of the directory
+		 */
+		if (cursor->level[this_level].bno > cursor->greatest_bno)
+			cursor->greatest_bno = cursor->level[this_level].bno;
+
+		/*
+		 * validate the path for the current used-up block
+		 * before we trash it
+		 */
+		if (verify_da_path(mp, cursor, this_level, whichfork))
+			return 1;
+		/*
+		 * ok, now get the next buffer and check sibling pointers
+		 */
+		dabno = nodehdr.forw;
+		ASSERT(dabno != 0);
+		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
+			&bmp, &lbmp);
+		if (nex == 0) {
+			do_warn(
+_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
+				dabno, cursor->ino);
+			return 1;
+		}
+
+		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
+		if (bmp != &lbmp)
+			free(bmp);
+
+		if (!bp) {
+			do_warn(
+_("can't read block %u for directory inode %" PRIu64 "\n"),
+				dabno, cursor->ino);
+			return 1;
+		}
+
+		newnode = bp->b_addr;
+		btree = M_DIROPS(mp)->node_tree_p(newnode);
+		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, newnode);
+
+		/*
+		 * verify magic number and back pointer, sanity-check
+		 * entry count, verify level
+		 */
+		bad = 0;
+		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
+		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
+			do_warn(
+_("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
+				nodehdr.magic,
+				dabno, cursor->ino);
+			bad++;
+		}
+		if (nodehdr.back != cursor->level[this_level].bno) {
+			do_warn(
+_("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
+				dabno, cursor->ino);
+			bad++;
+		}
+		if (nodehdr.count > geo->node_ents) {
+			do_warn(
+_("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
+				nodehdr.count,
+				dabno, cursor->ino);
+			bad++;
+		}
+		if (nodehdr.level != this_level) {
+			do_warn(
+_("bad level %d in block %u for directory inode %" PRIu64 "\n"),
+				nodehdr.level,
+				dabno, cursor->ino);
+			bad++;
+		}
+		if (bad) {
+#ifdef XR_DIR_TRACE
+			fprintf(stderr, "verify_da_path returns 1 (bad) #4\n");
+#endif
+			libxfs_putbuf(bp);
+			return 1;
+		}
+
+		/*
+		 * update cursor, write out the *current* level if
+		 * required.  don't write out the descendant level
+		 */
+		ASSERT(cursor->level[this_level].dirty == 0 ||
+			(cursor->level[this_level].dirty && !no_modify));
+
+		/*
+		 * If block looks ok but CRC didn't match, make sure to
+		 * recompute it.
+		 */
+		if (!no_modify &&
+		    cursor->level[this_level].bp->b_error == -EFSBADCRC)
+			cursor->level[this_level].dirty = 1;
+
+		if (cursor->level[this_level].dirty && !no_modify)
+			libxfs_writebuf(cursor->level[this_level].bp, 0);
+		else
+			libxfs_putbuf(cursor->level[this_level].bp);
+
+		/* switch cursor to point at the new buffer we just read */
+		cursor->level[this_level].bp = bp;
+		cursor->level[this_level].dirty = 0;
+		cursor->level[this_level].bno = dabno;
+		cursor->level[this_level].hashval =
+					be32_to_cpu(btree[0].hashval);
+
+		entry = cursor->level[this_level].index = 0;
+	}
+	/*
+	 * ditto for block numbers
+	 */
+	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
+#ifdef XR_DIR_TRACE
+		fprintf(stderr, "bad directory btree pointer, child bno "
+			"should be %d, block bno is %d, hashval is %u\n",
+			be32_to_cpu(btree[entry].before),
+			cursor->level[p_level].bno,
+			cursor->level[p_level].hashval);
+		fprintf(stderr, "verify_da_path returns 1 (bad) #1a\n");
+#endif
+		return 1;
+	}
+	/*
+	 * ok, now validate last hashvalue in the descendant
+	 * block against the hashval in the current entry
+	 */
+	if (cursor->level[p_level].hashval !=
+				be32_to_cpu(btree[entry].hashval)) {
+		if (!no_modify) {
+			do_warn(
+_("correcting bad hashval in interior dir block\n"
+  "\tin (level %d) in inode %" PRIu64 ".\n"),
+				this_level, cursor->ino);
+			btree[entry].hashval = cpu_to_be32(
+						cursor->level[p_level].hashval);
+			cursor->level[this_level].dirty++;
+		} else {
+			do_warn(
+_("would correct bad hashval in interior dir block\n"
+  "\tin (level %d) in inode %" PRIu64 ".\n"),
+				this_level, cursor->ino);
+		}
+	}
+	/*
+	 * increment index for this level to point to next entry
+	 * (which should point to the next descendant block)
+	 */
+	cursor->level[this_level].index++;
+#ifdef XR_DIR_TRACE
+	fprintf(stderr, "verify_da_path returns 0 (ok)\n");
+#endif
+	return 0;
+}
diff --git a/repair/da_util.h b/repair/da_util.h
new file mode 100644
index 0000000..7971d63
--- /dev/null
+++ b/repair/da_util.h
@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2015 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef _XR_DA_UTIL_H
+#define	_XR_DA_UTIL_H
+
+struct da_level_state  {
+	xfs_buf_t	*bp;		/* block bp */
+	xfs_dablk_t	bno;		/* file block number */
+	xfs_dahash_t	hashval;	/* last verified hashval */
+	int		index;		/* current index in block */
+	int		dirty;		/* is buffer dirty ? (1 == yes) */
+};
+
+typedef struct da_bt_cursor {
+	int			active;	/* highest level in tree (# levels-1) */
+	xfs_ino_t		ino;
+	xfs_dablk_t		greatest_bno;
+	xfs_dinode_t		*dip;
+	struct da_level_state	level[XFS_DA_NODE_MAXDEPTH];
+	struct blkmap		*blkmap;
+} da_bt_cursor_t;
+
+int
+namecheck(
+	char		*name,
+	int		length);
+
+struct xfs_buf *
+da_read_buf(
+	xfs_mount_t	*mp,
+	int		nex,
+	bmap_ext_t	*bmp,
+	const struct xfs_buf_ops *ops);
+
+void
+release_da_cursor(
+	xfs_mount_t	*mp,
+	da_bt_cursor_t	*cursor,
+	int		prev_level);
+
+void
+err_release_da_cursor(
+	xfs_mount_t	*mp,
+	da_bt_cursor_t	*cursor,
+	int		prev_level);
+
+int
+traverse_int_dablock(
+	xfs_mount_t	*mp,
+	da_bt_cursor_t	*da_cursor,
+	xfs_dablk_t	*rbno,
+	int		whichfork);
+
+int
+verify_da_path(
+	xfs_mount_t	*mp,
+	da_bt_cursor_t	*cursor,
+	const int	p_level,
+	int		whichfork);
+
+int
+verify_final_da_path(
+	xfs_mount_t	*mp,
+	da_bt_cursor_t	*cursor,
+	const int	p_level);
+#endif	/* _XR_DA_UTIL_H */
diff --git a/repair/dir2.c b/repair/dir2.c
index 7b47a9e..492b3e7 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -24,6 +24,7 @@
 #include "dinode.h"
 #include "dir2.h"
 #include "bmap.h"
+#include "da_util.h"
 #include "prefetch.h"
 #include "progress.h"
 
@@ -68,638 +69,6 @@ dir2_is_badino(
 }
 
 /*
- * takes a name and length (name need not be null-terminated)
- * and returns 1 if the name contains a '/' or a \0, returns 0
- * otherwise
- */
-int
-namecheck(char *name, int length)
-{
-	char *c;
-	int i;
-
-	ASSERT(length < MAXNAMELEN);
-
-	for (c = name, i = 0; i < length; i++, c++)  {
-		if (*c == '/' || *c == '\0')
-			return(1);
-	}
-
-	return(0);
-}
-
-/*
- * Multibuffer handling.
- * V2 directory blocks can be noncontiguous, needing multiple buffers.
- */
-struct xfs_buf *
-da_read_buf(
-	xfs_mount_t	*mp,
-	int		nex,
-	bmap_ext_t	*bmp,
-	const struct xfs_buf_ops *ops)
-{
-#define MAP_ARRAY_SZ 4
-	struct xfs_buf_map map_array[MAP_ARRAY_SZ];
-	struct xfs_buf_map *map;
-	struct xfs_buf	*bp;
-	int		i;
-
-	if (nex > MAP_ARRAY_SZ) {
-		map = calloc(nex, sizeof(*map));
-		if (map == NULL) {
-			do_error(_("couldn't malloc dir2 buffer list\n"));
-			exit(1);
-		}
-	} else {
-		/* common case avoids calloc/free */
-		map = map_array;
-	}
-	for (i = 0; i < nex; i++) {
-		map[i].bm_bn = XFS_FSB_TO_DADDR(mp, bmp[i].startblock);
-		map[i].bm_len = XFS_FSB_TO_BB(mp, bmp[i].blockcount);
-	}
-	bp = libxfs_readbuf_map(mp->m_dev, map, nex, 0, ops);
-	if (map != map_array)
-		free(map);
-	return bp;
-}
-
-/*
- * walk tree from root to the left-most leaf block reading in
- * blocks and setting up cursor.  passes back file block number of the
- * left-most leaf block if successful (bno).  returns 1 if successful,
- * 0 if unsuccessful.
- */
-static int
-traverse_int_dir2block(xfs_mount_t	*mp,
-		dir2_bt_cursor_t	*da_cursor,
-		xfs_dablk_t		*rbno)
-{
-	bmap_ext_t		*bmp;
-	xfs_dablk_t		bno;
-	struct xfs_buf		*bp;
-	int			i;
-	int			nex;
-	xfs_da_intnode_t	*node;
-	bmap_ext_t		lbmp;
-	struct xfs_da_geometry	*geo;
-	struct xfs_da_node_entry *btree;
-	struct xfs_da3_icnode_hdr nodehdr;
-
-	geo = mp->m_dir_geo;
-
-	/*
-	 * traverse down left-side of tree until we hit the
-	 * left-most leaf block setting up the btree cursor along
-	 * the way.
-	 */
-	bno = mp->m_dir_geo->leafblk;
-	i = -1;
-	node = NULL;
-	da_cursor->active = 0;
-
-	do {
-		/*
-		 * read in each block along the way and set up cursor
-		 */
-		nex = blkmap_getn(da_cursor->blkmap, bno,
-				geo->fsbcount, &bmp, &lbmp);
-
-		if (nex == 0)
-			goto error_out;
-
-		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
-		if (bmp != &lbmp)
-			free(bmp);
-		if (!bp) {
-			do_warn(
-_("can't read block %u for directory inode %" PRIu64 "\n"),
-				bno, da_cursor->ino);
-			goto error_out;
-		}
-
-		node = bp->b_addr;
-		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
-
-		if (nodehdr.magic == XFS_DIR2_LEAFN_MAGIC ||
-		    nodehdr.magic == XFS_DIR3_LEAFN_MAGIC) {
-			if ( i != -1 ) {
-				do_warn(
-_("found non-root LEAFN node in inode %" PRIu64 " bno = %u\n"),
-					da_cursor->ino, bno);
-			}
-			*rbno = 0;
-			libxfs_putbuf(bp);
-			return(1);
-		}
-
-		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
-		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
-			libxfs_putbuf(bp);
-			do_warn(
-_("bad dir magic number 0x%x in inode %" PRIu64 " bno = %u\n"),
-					nodehdr.magic,
-					da_cursor->ino, bno);
-			goto error_out;
-		}
-		/* corrupt node; rebuild the dir. */
-		if (bp->b_error == -EFSBADCRC || bp->b_error == -EFSCORRUPTED) {
-			libxfs_putbuf(bp);
-			do_warn(
-_("corrupt tree block %u for directory inode %" PRIu64 "\n"),
-				bno, da_cursor->ino);
-			goto error_out;
-		}
-		btree = M_DIROPS(mp)->node_tree_p(node);
-		if (nodehdr.count > geo->node_ents) {
-			do_warn(
-_("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
-				da_cursor->ino, nodehdr.count, geo->node_ents);
-			libxfs_putbuf(bp);
-			goto error_out;
-		}
-		/*
-		 * maintain level counter
-		 */
-		if (i == -1) {
-			i = da_cursor->active = nodehdr.level;
-			if (i < 1 || i >= XFS_DA_NODE_MAXDEPTH) {
-				do_warn(
-_("bad header depth for directory inode %" PRIu64 "\n"),
-					da_cursor->ino);
-				libxfs_putbuf(bp);
-				i = -1;
-				goto error_out;
-			}
-		} else {
-			if (nodehdr.level == i - 1) {
-				i--;
-			} else {
-				do_warn(
-_("bad directory btree for directory inode %" PRIu64 "\n"),
-					da_cursor->ino);
-				libxfs_putbuf(bp);
-				goto error_out;
-			}
-		}
-
-		da_cursor->level[i].hashval = be32_to_cpu(btree[0].hashval);
-		da_cursor->level[i].bp = bp;
-		da_cursor->level[i].bno = bno;
-		da_cursor->level[i].index = 0;
-
-		/*
-		 * set up new bno for next level down
-		 */
-		bno = be32_to_cpu(btree[0].before);
-	} while (node != NULL && i > 1);
-
-	/*
-	 * now return block number and get out
-	 */
-	*rbno = da_cursor->level[0].bno = bno;
-	return(1);
-
-error_out:
-	while (i > 1 && i <= da_cursor->active) {
-		libxfs_putbuf(da_cursor->level[i].bp);
-		i++;
-	}
-
-	return(0);
-}
-
-/*
- * blow out buffer for this level and all the rest above as well
- * if error == 0, we are not expecting to encounter any unreleased
- * buffers (e.g. if we do, it's a mistake).  if error == 1, we're
- * in an error-handling case so unreleased buffers may exist.
- */
-static void
-release_dir2_cursor_int(xfs_mount_t		*mp,
-			dir2_bt_cursor_t	*cursor,
-			int			prev_level,
-			int			error)
-{
-	int	level = prev_level + 1;
-
-	if (cursor->level[level].bp != NULL)  {
-		if (!error)  {
-			do_warn(_("release_dir2_cursor_int got unexpected "
-				  "non-null bp, dabno = %u\n"),
-				cursor->level[level].bno);
-		}
-		ASSERT(error != 0);
-
-		libxfs_putbuf(cursor->level[level].bp);
-		cursor->level[level].bp = NULL;
-	}
-
-	if (level < cursor->active)
-		release_dir2_cursor_int(mp, cursor, level, error);
-
-	return;
-}
-
-static void
-release_dir2_cursor(xfs_mount_t		*mp,
-		dir2_bt_cursor_t	*cursor,
-		int			prev_level)
-{
-	release_dir2_cursor_int(mp, cursor, prev_level, 0);
-}
-
-static void
-err_release_dir2_cursor(xfs_mount_t		*mp,
-			dir2_bt_cursor_t	*cursor,
-			int			prev_level)
-{
-	release_dir2_cursor_int(mp, cursor, prev_level, 1);
-}
-
-/*
- * make sure that all entries in all blocks along the right side of
- * of the tree are used and hashval's are consistent.  level is the
- * level of the descendent block.  returns 0 if good (even if it had
- * to be fixed up), and 1 if bad.  The right edge of the tree is
- * technically a block boundary.  This routine should be used then
- * instead of verify_dir2_path().
- */
-static int
-verify_final_dir2_path(xfs_mount_t	*mp,
-		dir2_bt_cursor_t	*cursor,
-		const int		p_level)
-{
-	xfs_da_intnode_t	*node;
-	xfs_dahash_t		hashval;
-	int			bad = 0;
-	int			entry;
-	int			this_level = p_level + 1;
-	struct xfs_da_node_entry *btree;
-	struct xfs_da3_icnode_hdr nodehdr;
-
-#ifdef XR_DIR_TRACE
-	fprintf(stderr, "in verify_final_dir2_path, this_level = %d\n",
-		this_level);
-#endif
-
-	/*
-	 * the index should point to the next "unprocessed" entry
-	 * in the block which should be the final (rightmost) entry
-	 */
-	entry = cursor->level[this_level].index;
-	node = cursor->level[this_level].bp->b_addr;
-	btree = M_DIROPS(mp)->node_tree_p(node);
-	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
-
-	/*
-	 * check internal block consistency on this level -- ensure
-	 * that all entries are used, encountered and expected hashvals
-	 * match, etc.
-	 */
-	if (entry != nodehdr.count - 1) {
-		do_warn(
-		_("directory block used/count inconsistency - %d / %hu\n"),
-			entry, nodehdr.count);
-		bad++;
-	}
-	/*
-	 * hash values monotonically increasing ???
-	 */
-	if (cursor->level[this_level].hashval >=
-				be32_to_cpu(btree[entry].hashval)) {
-		do_warn(_("directory/attribute block hashvalue inconsistency, "
-			  "expected > %u / saw %u\n"),
-			cursor->level[this_level].hashval,
-			be32_to_cpu(btree[entry].hashval));
-		bad++;
-	}
-	if (nodehdr.forw != 0) {
-		do_warn(_("bad directory/attribute forward block pointer, "
-			  "expected 0, saw %u\n"),
-			nodehdr.forw);
-		bad++;
-	}
-	if (bad) {
-		do_warn(_("bad directory block in inode %" PRIu64 "\n"), cursor->ino);
-		return(1);
-	}
-	/*
-	 * keep track of greatest block # -- that gets
-	 * us the length of the directory
-	 */
-	if (cursor->level[this_level].bno > cursor->greatest_bno)
-		cursor->greatest_bno = cursor->level[this_level].bno;
-
-	/*
-	 * ok, now check descendant block number against this level
-	 */
-	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
-#ifdef XR_DIR_TRACE
-		fprintf(stderr, "bad directory btree pointer, child bno should "
-				"be %d, block bno is %d, hashval is %u\n",
-			be16_to_cpu(btree[entry].before),
-			cursor->level[p_level].bno,
-			cursor->level[p_level].hashval);
-		fprintf(stderr, "verify_final_dir2_path returns 1 (bad) #1a\n");
-#endif
-		return(1);
-	}
-
-	if (cursor->level[p_level].hashval !=
-				be32_to_cpu(btree[entry].hashval)) {
-		if (!no_modify) {
-			do_warn(
-_("correcting bad hashval in non-leaf dir block\n"
-  "\tin (level %d) in inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
-			btree[entry].hashval = cpu_to_be32(
-						cursor->level[p_level].hashval);
-			cursor->level[this_level].dirty++;
-		} else {
-			do_warn(
-_("would correct bad hashval in non-leaf dir block\n"
-  "\tin (level %d) in inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
-		}
-	}
-
-	/*
-	 * Note: squirrel hashval away _before_ releasing the
-	 * buffer, preventing a use-after-free problem.
-	 */
-	hashval = be32_to_cpu(btree[entry].hashval);
-
-	/*
-	 * release/write buffer
-	 */
-	ASSERT(cursor->level[this_level].dirty == 0 ||
-		(cursor->level[this_level].dirty && !no_modify));
-
-	if (cursor->level[this_level].dirty && !no_modify)
-		libxfs_writebuf(cursor->level[this_level].bp, 0);
-	else
-		libxfs_putbuf(cursor->level[this_level].bp);
-
-	cursor->level[this_level].bp = NULL;
-
-	/*
-	 * bail out if this is the root block (top of tree)
-	 */
-	if (this_level >= cursor->active) {
-#ifdef XR_DIR_TRACE
-		fprintf(stderr, "verify_final_dir2_path returns 0 (ok)\n");
-#endif
-		return(0);
-	}
-	/*
-	 * set hashvalue to correctly reflect the now-validated
-	 * last entry in this block and continue upwards validation
-	 */
-	cursor->level[this_level].hashval = hashval;
-
-	return(verify_final_dir2_path(mp, cursor, this_level));
-}
-
-/*
- * Verifies the path from a descendant block up to the root.
- * Should be called when the descendant level traversal hits
- * a block boundary before crossing the boundary (reading in a new
- * block).
- *
- * the directory/attr btrees work differently to the other fs btrees.
- * each interior block contains records that are <hashval, bno>
- * pairs.  The bno is a file bno, not a filesystem bno.  The last
- * hashvalue in the block <bno> will be <hashval>.  BUT unlike
- * the freespace btrees, the *last* value in each block gets
- * propagated up the tree instead of the first value in each block.
- * that is, the interior records point to child blocks and the *greatest*
- * hash value contained by the child block is the one the block above
- * uses as the key for the child block.
- *
- * level is the level of the descendent block.  returns 0 if good,
- * and 1 if bad.  The descendant block may be a leaf block.
- *
- * the invariant here is that the values in the cursor for the
- * levels beneath this level (this_level) and the cursor index
- * for this level *must* be valid.
- *
- * that is, the hashval/bno info is accurate for all
- * DESCENDANTS and match what the node[index] information
- * for the current index in the cursor for this level.
- *
- * the index values in the cursor for the descendant level
- * are allowed to be off by one as they will reflect the
- * next entry at those levels to be processed.
- *
- * the hashvalue for the current level can't be set until
- * we hit the last entry in the block so, it's garbage
- * until set by this routine.
- *
- * bno and bp for the current block/level are always valid
- * since they have to be set so we can get a buffer for the
- * block.
- */
-static int
-verify_dir2_path(xfs_mount_t	*mp,
-	dir2_bt_cursor_t	*cursor,
-	const int		p_level)
-{
-	xfs_da_intnode_t	*node;
-	xfs_da_intnode_t	*newnode;
-	xfs_dablk_t		dabno;
-	struct xfs_buf		*bp;
-	int			bad;
-	int			entry;
-	int			this_level = p_level + 1;
-	bmap_ext_t		*bmp;
-	int			nex;
-	bmap_ext_t		lbmp;
-	struct xfs_da_geometry	*geo;
-	struct xfs_da_node_entry *btree;
-	struct xfs_da3_icnode_hdr nodehdr;
-
-	geo = mp->m_dir_geo;
-
-	/*
-	 * index is currently set to point to the entry that
-	 * should be processed now in this level.
-	 */
-	entry = cursor->level[this_level].index;
-	node = cursor->level[this_level].bp->b_addr;
-	btree = M_DIROPS(mp)->node_tree_p(node);
-	M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, node);
-
-	/*
-	 * if this block is out of entries, validate this
-	 * block and move on to the next block.
-	 * and update cursor value for said level
-	 */
-	if (entry >= nodehdr.count) {
-		/*
-		 * update the hash value for this level before
-		 * validating it.  bno value should be ok since
-		 * it was set when the block was first read in.
-		 */
-		cursor->level[this_level].hashval =
-			be32_to_cpu(btree[entry - 1].hashval);
-
-		/*
-		 * keep track of greatest block # -- that gets
-		 * us the length of the directory
-		 */
-		if (cursor->level[this_level].bno > cursor->greatest_bno)
-			cursor->greatest_bno = cursor->level[this_level].bno;
-
-		/*
-		 * validate the path for the current used-up block
-		 * before we trash it
-		 */
-		if (verify_dir2_path(mp, cursor, this_level))
-			return(1);
-		/*
-		 * ok, now get the next buffer and check sibling pointers
-		 */
-		dabno = nodehdr.forw;
-		ASSERT(dabno != 0);
-		nex = blkmap_getn(cursor->blkmap, dabno, geo->fsbcount,
-			&bmp, &lbmp);
-		if (nex == 0) {
-			do_warn(
-_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
-				dabno, cursor->ino);
-			return(1);
-		}
-
-		bp = da_read_buf(mp, nex, bmp, &xfs_da3_node_buf_ops);
-		if (bmp != &lbmp)
-			free(bmp);
-
-		if (!bp) {
-			do_warn(
-_("can't read block %u for directory inode %" PRIu64 "\n"),
-				dabno, cursor->ino);
-			return(1);
-		}
-
-		newnode = bp->b_addr;
-		btree = M_DIROPS(mp)->node_tree_p(newnode);
-		M_DIROPS(mp)->node_hdr_from_disk(&nodehdr, newnode);
-		/*
-		 * verify magic number and back pointer, sanity-check
-		 * entry count, verify level
-		 */
-		bad = 0;
-		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
-		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
-			do_warn(
-_("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
-				nodehdr.magic,
-				dabno, cursor->ino);
-			bad++;
-		}
-		if (nodehdr.back != cursor->level[this_level].bno) {
-			do_warn(
-_("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
-				dabno, cursor->ino);
-			bad++;
-		}
-		if (nodehdr.count > geo->node_ents) {
-			do_warn(
-_("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
-				nodehdr.count,
-				dabno, cursor->ino);
-			bad++;
-		}
-		if (nodehdr.level != this_level) {
-			do_warn(
-_("bad level %d in block %u for directory inode %" PRIu64 "\n"),
-				nodehdr.level,
-				dabno, cursor->ino);
-			bad++;
-		}
-		if (bad) {
-#ifdef XR_DIR_TRACE
-			fprintf(stderr, "verify_dir2_path returns 1 (bad) #4\n");
-#endif
-			libxfs_putbuf(bp);
-			return(1);
-		}
-		/*
-		 * update cursor, write out the *current* level if
-		 * required.  don't write out the descendant level
-		 */
-		ASSERT(cursor->level[this_level].dirty == 0 ||
-			(cursor->level[this_level].dirty && !no_modify));
-		/*
-		 * If block looks ok but CRC didn't match, make sure to
-		 * recompute it.
-		 */
-		if (!no_modify &&
-		    cursor->level[this_level].bp->b_error == -EFSBADCRC)
-			cursor->level[this_level].dirty = 1;
-		if (cursor->level[this_level].dirty && !no_modify)
-			libxfs_writebuf(cursor->level[this_level].bp, 0);
-		else
-			libxfs_putbuf(cursor->level[this_level].bp);
-
-		/* switch cursor to point at the new buffer we just read */
-		cursor->level[this_level].bp = bp;
-		cursor->level[this_level].dirty = 0;
-		cursor->level[this_level].bno = dabno;
-		cursor->level[this_level].hashval =
-			be32_to_cpu(btree[0].hashval);
-
-		entry = cursor->level[this_level].index = 0;
-	}
-	/*
-	 * ditto for block numbers
-	 */
-	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
-#ifdef XR_DIR_TRACE
-		fprintf(stderr, "bad directory btree pointer, child bno "
-			"should be %d, block bno is %d, hashval is %u\n",
-			be32_to_cpu(btree[entry].before),
-			cursor->level[p_level].bno,
-			cursor->level[p_level].hashval);
-		fprintf(stderr, "verify_dir2_path returns 1 (bad) #1a\n");
-#endif
-		return(1);
-	}
-	/*
-	 * ok, now validate last hashvalue in the descendant
-	 * block against the hashval in the current entry
-	 */
-	if (cursor->level[p_level].hashval !=
-				be32_to_cpu(btree[entry].hashval)) {
-		if (!no_modify) {
-			do_warn(
-_("correcting bad hashval in interior dir block\n"
-  "\tin (level %d) in inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
-			btree[entry].hashval = cpu_to_be32(
-					cursor->level[p_level].hashval);
-			cursor->level[this_level].dirty++;
-		} else {
-			do_warn(
-_("would correct bad hashval in interior dir block\n"
-  "\tin (level %d) in inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
-		}
-	}
-	/*
-	 * increment index for this level to point to next entry
-	 * (which should point to the next descendant block)
-	 */
-	cursor->level[this_level].index++;
-#ifdef XR_DIR_TRACE
-       fprintf(stderr, "verify_dir2_path returns 0 (ok)\n");
-#endif
-	return(0);
-}
-
-/*
  * Fix up a shortform directory which was in long form (i8count set)
  * and is now in short form (i8count clear).
  * Return pointer to the end of the data when done.
@@ -1697,7 +1066,7 @@ _("bad stale count in block %u of directory inode %" PRIu64 "\n"),
 static int
 process_leaf_level_dir2(
 	xfs_mount_t		*mp,
-	dir2_bt_cursor_t	*da_cursor,
+	da_bt_cursor_t		*da_cursor,
 	int			*repair)
 {
 	bmap_ext_t		*bmp;
@@ -1791,7 +1160,7 @@ _("bad sibling back pointer for block %u in directory inode %" PRIu64 "\n"),
 		prev_bno = da_bno;
 		da_bno = leafhdr.forw;
 		if (da_bno != 0) {
-			if (verify_dir2_path(mp, da_cursor, 0)) {
+			if (verify_da_path(mp, da_cursor, 0, XFS_DATA_FORK)) {
 				libxfs_putbuf(bp);
 				goto error_out;
 			}
@@ -1810,7 +1179,7 @@ _("bad sibling back pointer for block %u in directory inode %" PRIu64 "\n"),
 		} else
 			libxfs_putbuf(bp);
 	} while (da_bno != 0);
-	if (verify_final_dir2_path(mp, da_cursor, 0)) {
+	if (verify_final_da_path(mp, da_cursor, 0)) {
 		/*
 		 * Verify the final path up (right-hand-side) if still ok.
 		 */
@@ -1820,14 +1189,14 @@ _("bad sibling back pointer for block %u in directory inode %" PRIu64 "\n"),
 	/*
 	 * Redundant but just for testing.
 	 */
-	release_dir2_cursor(mp, da_cursor, 0);
+	release_da_cursor(mp, da_cursor, 0);
 	return 0;
 
 error_out:
 	/*
 	 * Release all buffers holding interior btree blocks.
 	 */
-	err_release_dir2_cursor(mp, da_cursor, 0);
+	err_release_da_cursor(mp, da_cursor, 0);
 	if (bmp && (bmp != &lbmp))
 		free(bmp);
 	return 1;
@@ -1846,7 +1215,7 @@ process_node_dir2(
 	int		*repair)
 {
 	xfs_dablk_t		bno;
-	dir2_bt_cursor_t	da_cursor;
+	da_bt_cursor_t		da_cursor;
 
 	/*
 	 * Try again -- traverse down left-side of tree until we hit the
@@ -1862,14 +1231,14 @@ process_node_dir2(
 	/*
 	 * Now process interior node.
 	 */
-	if (traverse_int_dir2block(mp, &da_cursor, &bno) == 0)
+	if (traverse_int_dablock(mp, &da_cursor, &bno, XFS_DATA_FORK) == 0)
 		return 1;
 
 	/*
 	 * Skip directories with a root marked XFS_DIR2_LEAFN_MAGIC
 	 */
 	if (bno == 0) {
-		release_dir2_cursor(mp, &da_cursor, 0);
+		release_da_cursor(mp, &da_cursor, 0);
 		return 0;
 	} else {
 		/*
diff --git a/repair/dir2.h b/repair/dir2.h
index 186633f..e4d4eeb 100644
--- a/repair/dir2.h
+++ b/repair/dir2.h
@@ -22,50 +22,6 @@
 struct blkmap;
 struct bmap_ext;
 
-/*
- * the cursor gets passed up and down the da btree processing
- * routines.  The interior block processing routines use the
- * cursor to determine if the pointers to and from the preceding
- * and succeeding sibling blocks are ok and whether the values in
- * the current block are consistent with the entries in the parent
- * nodes.  When a block is traversed, a parent-verification routine
- * is called to verify if the next logical entry in the next level up
- * is consistent with the greatest hashval in the next block of the
- * current level.  The verification routine is itself recursive and
- * calls itself if it has to traverse an interior block to get
- * the next logical entry.  The routine recurses upwards through
- * the tree until it finds a block where it can simply step to
- * the next entry.  The hashval in that entry should be equal to
- * the hashval being passed to it (the greatest hashval in the block
- * that the entry points to).  If that isn't true, then the tree
- * is blown and we need to trash it, salvage and trash it, or fix it.
- * Currently, we just trash it.
- */
-typedef struct dir2_level_state  {
-	xfs_buf_t	*bp;		/* block bp */
-	xfs_dablk_t	bno;		/* file block number */
-	xfs_dahash_t	hashval;	/* last verified hashval */
-	int		index;		/* current index in block */
-	int		dirty;		/* is buffer dirty ? (1 == yes) */
-} dir2_level_state_t;
-
-typedef struct dir2_bt_cursor  {
-	int			active;	/* highest level in tree (# levels-1) */
-	xfs_ino_t		ino;
-	xfs_dablk_t		greatest_bno;
-	xfs_dinode_t		*dip;
-	dir2_level_state_t	level[XFS_DA_NODE_MAXDEPTH];
-	struct blkmap		*blkmap;
-} dir2_bt_cursor_t;
-
-#include "bmap.h"	/* Goes away in later refactoring */
-struct xfs_buf *
-da_read_buf(
-	xfs_mount_t	*mp,
-	int		nex,
-	bmap_ext_t	*bmp,
-	const struct xfs_buf_ops *ops);
-
 int
 process_dir2(
 	xfs_mount_t	*mp,
@@ -87,9 +43,4 @@ int
 dir2_is_badino(
 	xfs_ino_t	ino);
 
-int
-namecheck(
-	char		*name,
-	int		length);
-
 #endif	/* _XR_DIR2_H */
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 13/13] xfs_repair: Fix up warning strings in da_util.c

[Eric Sandeen]

Switch the warning messages based on which fork has
encountered the problem.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
 repair/attr_repair.c |    2 +-
 repair/da_util.c     |   97 +++++++++++++++++++++++++++-----------------------
 repair/da_util.h     |    3 +-
 repair/dir2.c        |    2 +-
 4 files changed, 56 insertions(+), 48 deletions(-)

diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 0d3b7a5..da2800d 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -849,7 +849,7 @@ process_leaf_attr_level(xfs_mount_t	*mp,
 			libxfs_putbuf(bp);
 	} while (da_bno != 0);
 
-	if (verify_final_da_path(mp, da_cursor, 0))  {
+	if (verify_final_da_path(mp, da_cursor, 0, XFS_ATTR_FORK))  {
 		/*
 		 * verify the final path up (right-hand-side) if still ok
 		 */
diff --git a/repair/da_util.c b/repair/da_util.c
index e5d5535..89d41cc 100644
--- a/repair/da_util.c
+++ b/repair/da_util.c
@@ -67,6 +67,7 @@ namecheck(char *name, int length)
 /*
  * Multibuffer handling.
  * V2 directory blocks can be noncontiguous, needing multiple buffers.
+ * attr blocks are single blocks; this code handles that as well.
  */
 struct xfs_buf *
 da_read_buf(
@@ -101,6 +102,8 @@ da_read_buf(
 	return bp;
 }
 
+#define FORKNAME(type) (type == XFS_DATA_FORK ? _("directory") : _("attribute"))
+
 /*
  * walk tree from root to the left-most leaf block reading in
  * blocks and setting up cursor.  passes back file block number of the
@@ -158,8 +161,8 @@ traverse_int_dablock(
 
 		if (!bp) {
 			do_warn(
-_("can't read block %u for directory inode %" PRIu64 "\n"),
-				bno, da_cursor->ino);
+_("can't read %s block %u for inode %" PRIu64 "\n"),
+				FORKNAME(whichfork), bno, da_cursor->ino);
 			goto error_out;
 		}
 
@@ -182,8 +185,8 @@ _("found non-root LEAFN node in inode %" PRIu64 " bno = %u\n"),
 		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
 		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
 			do_warn(
-_("bad dir magic number 0x%x in inode %" PRIu64 " bno = %u\n"),
-					nodehdr.magic,
+_("bad %s magic number 0x%x in inode %" PRIu64 " bno = %u\n"),
+					FORKNAME(whichfork), nodehdr.magic,
 					da_cursor->ino, bno);
 			libxfs_putbuf(bp);
 			goto error_out;
@@ -193,16 +196,17 @@ _("bad dir magic number 0x%x in inode %" PRIu64 " bno = %u\n"),
 		if (bp->b_error == -EFSBADCRC || bp->b_error == -EFSCORRUPTED) {
 			libxfs_putbuf(bp);
 			do_warn(
-_("corrupt tree block %u for directory inode %" PRIu64 "\n"),
-				bno, da_cursor->ino);
+_("corrupt %s tree block %u for inode %" PRIu64 "\n"),
+				FORKNAME(whichfork), bno, da_cursor->ino);
 			goto error_out;
 		}
 
 		btree = M_DIROPS(mp)->node_tree_p(node);
 		if (nodehdr.count > geo->node_ents) {
 			do_warn(
-_("bad record count in inode %" PRIu64 ", count = %d, max = %d\n"),
-				da_cursor->ino, nodehdr.count, geo->node_ents);
+_("bad %s record count in inode %" PRIu64 ", count = %d, max = %d\n"),
+				FORKNAME(whichfork), da_cursor->ino,
+				nodehdr.count, geo->node_ents);
 			libxfs_putbuf(bp);
 			goto error_out;
 		}
@@ -225,8 +229,8 @@ _("bad header depth for directory inode %" PRIu64 "\n"),
 				i--;
 			} else {
 				do_warn(
-_("bad directory btree for directory inode %" PRIu64 "\n"),
-					da_cursor->ino);
+_("bad %s btree for inode %" PRIu64 "\n"),
+					FORKNAME(whichfork), da_cursor->ino);
 				libxfs_putbuf(bp);
 				goto error_out;
 			}
@@ -321,7 +325,8 @@ int
 verify_final_da_path(
 	xfs_mount_t		*mp,
 	da_bt_cursor_t		*cursor,
-	const int		p_level)
+	const int		p_level,
+	int			whichfork)
 {
 	xfs_da_intnode_t	*node;
 	xfs_dahash_t		hashval;
@@ -352,8 +357,8 @@ verify_final_da_path(
 	 */
 	if (entry != nodehdr.count - 1) {
 		do_warn(
-		_("directory block used/count inconsistency - %d/%hu\n"),
-			entry, nodehdr.count);
+_("%s block used/count inconsistency - %d/%hu\n"),
+			FORKNAME(whichfork), entry, nodehdr.count);
 		bad++;
 	}
 	/*
@@ -361,25 +366,27 @@ verify_final_da_path(
 	 */
 	if (cursor->level[this_level].hashval >=
 				be32_to_cpu(btree[entry].hashval)) {
-		do_warn(_("directory/attribute block hashvalue inconsistency, "
-			  "expected > %u / saw %u\n"),
+		do_warn(
+_("%s block hashvalue inconsistency, expected > %u / saw %u\n"),
+			FORKNAME(whichfork),
 			cursor->level[this_level].hashval,
 			be32_to_cpu(btree[entry].hashval));
 		bad++;
 	}
 	if (nodehdr.forw != 0) {
-		do_warn(_("bad directory/attribute forward block pointer, "
-			  "expected 0, saw %u\n"),
-			nodehdr.forw);
+		do_warn(
+_("bad %s forward block pointer, expected 0, saw %u\n"),
+			FORKNAME(whichfork), nodehdr.forw);
 		bad++;
 	}
 	if (bad) {
-		do_warn(_("bad directory block in inode %" PRIu64 "\n"), cursor->ino);
+		do_warn(_("bad %s block in inode %" PRIu64 "\n"),
+			FORKNAME(whichfork), cursor->ino);
 		return 1;
 	}
 	/*
 	 * keep track of greatest block # -- that gets
-	 * us the length of the directory
+	 * us the length of the directory/attribute 
 	 */
 	if (cursor->level[this_level].bno > cursor->greatest_bno)
 		cursor->greatest_bno = cursor->level[this_level].bno;
@@ -389,9 +396,9 @@ verify_final_da_path(
 	 */
 	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
 #ifdef XR_DIR_TRACE
-		fprintf(stderr, "bad directory btree pointer, child bno should "
+		fprintf(stderr, "bad %s btree pointer, child bno should "
 				"be %d, block bno is %d, hashval is %u\n",
-			be16_to_cpu(btree[entry].before),
+			FORKNAME(whichfork), be16_to_cpu(btree[entry].before),
 			cursor->level[p_level].bno,
 			cursor->level[p_level].hashval);
 		fprintf(stderr, "verify_final_da_path returns 1 (bad) #1a\n");
@@ -403,17 +410,17 @@ verify_final_da_path(
 				be32_to_cpu(btree[entry].hashval)) {
 		if (!no_modify) {
 			do_warn(
-_("correcting bad hashval in non-leaf dir block\n"
+_("correcting bad hashval in non-leaf %s block\n"
  "\tin (level %d) in inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
+				FORKNAME(whichfork), this_level, cursor->ino);
 			btree[entry].hashval = cpu_to_be32(
 						cursor->level[p_level].hashval);
 			cursor->level[this_level].dirty++;
 		} else {
 			do_warn(
-_("would correct bad hashval in non-leaf dir block\n"
+_("would correct bad hashval in non-leaf %s block\n"
  "\tin (level %d) in inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
+				FORKNAME(whichfork), this_level, cursor->ino);
 		}
 	}
 
@@ -451,7 +458,7 @@ _("would correct bad hashval in non-leaf dir block\n"
 	 */
 	cursor->level[this_level].hashval = hashval;
 
-	return verify_final_da_path(mp, cursor, this_level);
+	return verify_final_da_path(mp, cursor, this_level, whichfork);
 }
 
 /*
@@ -564,8 +571,8 @@ verify_da_path(
 			&bmp, &lbmp);
 		if (nex == 0) {
 			do_warn(
-_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
-				dabno, cursor->ino);
+_("can't get map info for %s block %u of inode %" PRIu64 "\n"),
+				FORKNAME(whichfork), dabno, cursor->ino);
 			return 1;
 		}
 
@@ -575,8 +582,8 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
 
 		if (!bp) {
 			do_warn(
-_("can't read block %u for directory inode %" PRIu64 "\n"),
-				dabno, cursor->ino);
+_("can't read %s block %u for inode %" PRIu64 "\n"),
+				FORKNAME(whichfork), dabno, cursor->ino);
 			return 1;
 		}
 
@@ -592,28 +599,28 @@ _("can't read block %u for directory inode %" PRIu64 "\n"),
 		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
 		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
 			do_warn(
-_("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
-				nodehdr.magic,
+_("bad magic number %x in %s block %u for inode %" PRIu64 "\n"),
+				nodehdr.magic, FORKNAME(whichfork),
 				dabno, cursor->ino);
 			bad++;
 		}
 		if (nodehdr.back != cursor->level[this_level].bno) {
 			do_warn(
-_("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
-				dabno, cursor->ino);
+_("bad back pointer in %s block %u for inode %" PRIu64 "\n"),
+				FORKNAME(whichfork), dabno, cursor->ino);
 			bad++;
 		}
 		if (nodehdr.count > geo->node_ents) {
 			do_warn(
-_("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
-				nodehdr.count,
+_("entry count %d too large in %s block %u for inode %" PRIu64 "\n"),
+				nodehdr.count, FORKNAME(whichfork),
 				dabno, cursor->ino);
 			bad++;
 		}
 		if (nodehdr.level != this_level) {
 			do_warn(
-_("bad level %d in block %u for directory inode %" PRIu64 "\n"),
-				nodehdr.level,
+_("bad level %d in %s block %u for inode %" PRIu64 "\n"),
+				nodehdr.level, FORKNAME(whichfork),
 				dabno, cursor->ino);
 			bad++;
 		}
@@ -659,9 +666,9 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
 	 */
 	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
 #ifdef XR_DIR_TRACE
-		fprintf(stderr, "bad directory btree pointer, child bno "
+		fprintf(stderr, "bad %s btree pointer, child bno "
 			"should be %d, block bno is %d, hashval is %u\n",
-			be32_to_cpu(btree[entry].before),
+			FORKNAME(whichfork), be32_to_cpu(btree[entry].before),
 			cursor->level[p_level].bno,
 			cursor->level[p_level].hashval);
 		fprintf(stderr, "verify_da_path returns 1 (bad) #1a\n");
@@ -676,17 +683,17 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
 				be32_to_cpu(btree[entry].hashval)) {
 		if (!no_modify) {
 			do_warn(
-_("correcting bad hashval in interior dir block\n"
+_("correcting bad hashval in interior %s block\n"
   "\tin (level %d) in inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
+				FORKNAME(whichfork), this_level, cursor->ino);
 			btree[entry].hashval = cpu_to_be32(
 						cursor->level[p_level].hashval);
 			cursor->level[this_level].dirty++;
 		} else {
 			do_warn(
-_("would correct bad hashval in interior dir block\n"
+_("would correct bad hashval in interior %s block\n"
   "\tin (level %d) in inode %" PRIu64 ".\n"),
-				this_level, cursor->ino);
+				FORKNAME(whichfork), this_level, cursor->ino);
 		}
 	}
 	/*
diff --git a/repair/da_util.h b/repair/da_util.h
index 7971d63..442f9f1 100644
--- a/repair/da_util.h
+++ b/repair/da_util.h
@@ -78,5 +78,6 @@ int
 verify_final_da_path(
 	xfs_mount_t	*mp,
 	da_bt_cursor_t	*cursor,
-	const int	p_level);
+	const int	p_level,
+	int		whichfork);
 #endif	/* _XR_DA_UTIL_H */
diff --git a/repair/dir2.c b/repair/dir2.c
index 492b3e7..61912d1 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -1179,7 +1179,7 @@ _("bad sibling back pointer for block %u in directory inode %" PRIu64 "\n"),
 		} else
 			libxfs_putbuf(bp);
 	} while (da_bno != 0);
-	if (verify_final_da_path(mp, da_cursor, 0)) {
+	if (verify_final_da_path(mp, da_cursor, 0, XFS_DATA_FORK)) {
 		/*
 		 * Verify the final path up (right-hand-side) if still ok.
 		 */
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 13/13] xfs_repair: Fix up warning strings in da_util.c

[Brian Foster]

On Wed, Sep 09, 2015 at 02:34:11PM -0500, Eric Sandeen wrote:
> Switch the warning messages based on which fork has
> encountered the problem.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> ---

I assume this is being reworked based on the discussion with Carlos (and
I'm really not familiar with the localization stuff). That aside,
shouldn't this be ordered at some point before these functions are
called from both directory and attribute contexts so the messages aren't
ever incorrect?

Also, one nit below...

>  repair/attr_repair.c |    2 +-
>  repair/da_util.c     |   97 +++++++++++++++++++++++++++-----------------------
>  repair/da_util.h     |    3 +-
>  repair/dir2.c        |    2 +-
>  4 files changed, 56 insertions(+), 48 deletions(-)
> 
...
> diff --git a/repair/da_util.c b/repair/da_util.c
> index e5d5535..89d41cc 100644
> --- a/repair/da_util.c
> +++ b/repair/da_util.c
...
> @@ -361,25 +366,27 @@ verify_final_da_path(
>  	 */
>  	if (cursor->level[this_level].hashval >=
>  				be32_to_cpu(btree[entry].hashval)) {
> -		do_warn(_("directory/attribute block hashvalue inconsistency, "
> -			  "expected > %u / saw %u\n"),
> +		do_warn(
> +_("%s block hashvalue inconsistency, expected > %u / saw %u\n"),
> +			FORKNAME(whichfork),
>  			cursor->level[this_level].hashval,
>  			be32_to_cpu(btree[entry].hashval));
>  		bad++;
>  	}
>  	if (nodehdr.forw != 0) {
> -		do_warn(_("bad directory/attribute forward block pointer, "
> -			  "expected 0, saw %u\n"),
> -			nodehdr.forw);
> +		do_warn(
> +_("bad %s forward block pointer, expected 0, saw %u\n"),
> +			FORKNAME(whichfork), nodehdr.forw);
>  		bad++;
>  	}
>  	if (bad) {
> -		do_warn(_("bad directory block in inode %" PRIu64 "\n"), cursor->ino);
> +		do_warn(_("bad %s block in inode %" PRIu64 "\n"),
> +			FORKNAME(whichfork), cursor->ino);
>  		return 1;
>  	}
>  	/*
>  	 * keep track of greatest block # -- that gets
> -	 * us the length of the directory
> +	 * us the length of the directory/attribute 

Trailing whitespace above.

Brian

>  	 */
>  	if (cursor->level[this_level].bno > cursor->greatest_bno)
>  		cursor->greatest_bno = cursor->level[this_level].bno;
> @@ -389,9 +396,9 @@ verify_final_da_path(
>  	 */
>  	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
>  #ifdef XR_DIR_TRACE
> -		fprintf(stderr, "bad directory btree pointer, child bno should "
> +		fprintf(stderr, "bad %s btree pointer, child bno should "
>  				"be %d, block bno is %d, hashval is %u\n",
> -			be16_to_cpu(btree[entry].before),
> +			FORKNAME(whichfork), be16_to_cpu(btree[entry].before),
>  			cursor->level[p_level].bno,
>  			cursor->level[p_level].hashval);
>  		fprintf(stderr, "verify_final_da_path returns 1 (bad) #1a\n");
> @@ -403,17 +410,17 @@ verify_final_da_path(
>  				be32_to_cpu(btree[entry].hashval)) {
>  		if (!no_modify) {
>  			do_warn(
> -_("correcting bad hashval in non-leaf dir block\n"
> +_("correcting bad hashval in non-leaf %s block\n"
>   "\tin (level %d) in inode %" PRIu64 ".\n"),
> -				this_level, cursor->ino);
> +				FORKNAME(whichfork), this_level, cursor->ino);
>  			btree[entry].hashval = cpu_to_be32(
>  						cursor->level[p_level].hashval);
>  			cursor->level[this_level].dirty++;
>  		} else {
>  			do_warn(
> -_("would correct bad hashval in non-leaf dir block\n"
> +_("would correct bad hashval in non-leaf %s block\n"
>   "\tin (level %d) in inode %" PRIu64 ".\n"),
> -				this_level, cursor->ino);
> +				FORKNAME(whichfork), this_level, cursor->ino);
>  		}
>  	}
>  
> @@ -451,7 +458,7 @@ _("would correct bad hashval in non-leaf dir block\n"
>  	 */
>  	cursor->level[this_level].hashval = hashval;
>  
> -	return verify_final_da_path(mp, cursor, this_level);
> +	return verify_final_da_path(mp, cursor, this_level, whichfork);
>  }
>  
>  /*
> @@ -564,8 +571,8 @@ verify_da_path(
>  			&bmp, &lbmp);
>  		if (nex == 0) {
>  			do_warn(
> -_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
> -				dabno, cursor->ino);
> +_("can't get map info for %s block %u of inode %" PRIu64 "\n"),
> +				FORKNAME(whichfork), dabno, cursor->ino);
>  			return 1;
>  		}
>  
> @@ -575,8 +582,8 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>  
>  		if (!bp) {
>  			do_warn(
> -_("can't read block %u for directory inode %" PRIu64 "\n"),
> -				dabno, cursor->ino);
> +_("can't read %s block %u for inode %" PRIu64 "\n"),
> +				FORKNAME(whichfork), dabno, cursor->ino);
>  			return 1;
>  		}
>  
> @@ -592,28 +599,28 @@ _("can't read block %u for directory inode %" PRIu64 "\n"),
>  		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
>  		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
>  			do_warn(
> -_("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
> -				nodehdr.magic,
> +_("bad magic number %x in %s block %u for inode %" PRIu64 "\n"),
> +				nodehdr.magic, FORKNAME(whichfork),
>  				dabno, cursor->ino);
>  			bad++;
>  		}
>  		if (nodehdr.back != cursor->level[this_level].bno) {
>  			do_warn(
> -_("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
> -				dabno, cursor->ino);
> +_("bad back pointer in %s block %u for inode %" PRIu64 "\n"),
> +				FORKNAME(whichfork), dabno, cursor->ino);
>  			bad++;
>  		}
>  		if (nodehdr.count > geo->node_ents) {
>  			do_warn(
> -_("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
> -				nodehdr.count,
> +_("entry count %d too large in %s block %u for inode %" PRIu64 "\n"),
> +				nodehdr.count, FORKNAME(whichfork),
>  				dabno, cursor->ino);
>  			bad++;
>  		}
>  		if (nodehdr.level != this_level) {
>  			do_warn(
> -_("bad level %d in block %u for directory inode %" PRIu64 "\n"),
> -				nodehdr.level,
> +_("bad level %d in %s block %u for inode %" PRIu64 "\n"),
> +				nodehdr.level, FORKNAME(whichfork),
>  				dabno, cursor->ino);
>  			bad++;
>  		}
> @@ -659,9 +666,9 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>  	 */
>  	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
>  #ifdef XR_DIR_TRACE
> -		fprintf(stderr, "bad directory btree pointer, child bno "
> +		fprintf(stderr, "bad %s btree pointer, child bno "
>  			"should be %d, block bno is %d, hashval is %u\n",
> -			be32_to_cpu(btree[entry].before),
> +			FORKNAME(whichfork), be32_to_cpu(btree[entry].before),
>  			cursor->level[p_level].bno,
>  			cursor->level[p_level].hashval);
>  		fprintf(stderr, "verify_da_path returns 1 (bad) #1a\n");
> @@ -676,17 +683,17 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>  				be32_to_cpu(btree[entry].hashval)) {
>  		if (!no_modify) {
>  			do_warn(
> -_("correcting bad hashval in interior dir block\n"
> +_("correcting bad hashval in interior %s block\n"
>    "\tin (level %d) in inode %" PRIu64 ".\n"),
> -				this_level, cursor->ino);
> +				FORKNAME(whichfork), this_level, cursor->ino);
>  			btree[entry].hashval = cpu_to_be32(
>  						cursor->level[p_level].hashval);
>  			cursor->level[this_level].dirty++;
>  		} else {
>  			do_warn(
> -_("would correct bad hashval in interior dir block\n"
> +_("would correct bad hashval in interior %s block\n"
>    "\tin (level %d) in inode %" PRIu64 ".\n"),
> -				this_level, cursor->ino);
> +				FORKNAME(whichfork), this_level, cursor->ino);
>  		}
>  	}
>  	/*
> diff --git a/repair/da_util.h b/repair/da_util.h
> index 7971d63..442f9f1 100644
> --- a/repair/da_util.h
> +++ b/repair/da_util.h
> @@ -78,5 +78,6 @@ int
>  verify_final_da_path(
>  	xfs_mount_t	*mp,
>  	da_bt_cursor_t	*cursor,
> -	const int	p_level);
> +	const int	p_level,
> +	int		whichfork);
>  #endif	/* _XR_DA_UTIL_H */
> diff --git a/repair/dir2.c b/repair/dir2.c
> index 492b3e7..61912d1 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -1179,7 +1179,7 @@ _("bad sibling back pointer for block %u in directory inode %" PRIu64 "\n"),
>  		} else
>  			libxfs_putbuf(bp);
>  	} while (da_bno != 0);
> -	if (verify_final_da_path(mp, da_cursor, 0)) {
> +	if (verify_final_da_path(mp, da_cursor, 0, XFS_DATA_FORK)) {
>  		/*
>  		 * Verify the final path up (right-hand-side) if still ok.
>  		 */
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 13/13] xfs_repair: Fix up warning strings in da_util.c

[Eric Sandeen]

On 9/14/15 3:06 PM, Brian Foster wrote:
> On Wed, Sep 09, 2015 at 02:34:11PM -0500, Eric Sandeen wrote:
>> Switch the warning messages based on which fork has
>> encountered the problem.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>> ---
> 
> I assume this is being reworked based on the discussion with Carlos (and
> I'm really not familiar with the localization stuff). That aside,
> shouldn't this be ordered at some point before these functions are
> called from both directory and attribute contexts so the messages aren't
> ever incorrect?

Well, tradeoffs I guess.  I ordered it to make review easy.  If we think
message correctness is a bisectability issue, I could rework it so that
it's always correct, but it didn't seem like that big a deal to me.

> Also, one nit below...
> 
>>  repair/attr_repair.c |    2 +-
>>  repair/da_util.c     |   97 +++++++++++++++++++++++++++-----------------------
>>  repair/da_util.h     |    3 +-
>>  repair/dir2.c        |    2 +-
>>  4 files changed, 56 insertions(+), 48 deletions(-)
>>
> ...
>> diff --git a/repair/da_util.c b/repair/da_util.c
>> index e5d5535..89d41cc 100644
>> --- a/repair/da_util.c
>> +++ b/repair/da_util.c
> ...
>> @@ -361,25 +366,27 @@ verify_final_da_path(
>>  	 */
>>  	if (cursor->level[this_level].hashval >=
>>  				be32_to_cpu(btree[entry].hashval)) {
>> -		do_warn(_("directory/attribute block hashvalue inconsistency, "
>> -			  "expected > %u / saw %u\n"),
>> +		do_warn(
>> +_("%s block hashvalue inconsistency, expected > %u / saw %u\n"),
>> +			FORKNAME(whichfork),
>>  			cursor->level[this_level].hashval,
>>  			be32_to_cpu(btree[entry].hashval));
>>  		bad++;
>>  	}
>>  	if (nodehdr.forw != 0) {
>> -		do_warn(_("bad directory/attribute forward block pointer, "
>> -			  "expected 0, saw %u\n"),
>> -			nodehdr.forw);
>> +		do_warn(
>> +_("bad %s forward block pointer, expected 0, saw %u\n"),
>> +			FORKNAME(whichfork), nodehdr.forw);
>>  		bad++;
>>  	}
>>  	if (bad) {
>> -		do_warn(_("bad directory block in inode %" PRIu64 "\n"), cursor->ino);
>> +		do_warn(_("bad %s block in inode %" PRIu64 "\n"),
>> +			FORKNAME(whichfork), cursor->ino);
>>  		return 1;
>>  	}
>>  	/*
>>  	 * keep track of greatest block # -- that gets
>> -	 * us the length of the directory
>> +	 * us the length of the directory/attribute 
> 
> Trailing whitespace above.

ok :)

Thanks,
-Eric

> Brian
> 
>>  	 */
>>  	if (cursor->level[this_level].bno > cursor->greatest_bno)
>>  		cursor->greatest_bno = cursor->level[this_level].bno;
>> @@ -389,9 +396,9 @@ verify_final_da_path(
>>  	 */
>>  	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
>>  #ifdef XR_DIR_TRACE
>> -		fprintf(stderr, "bad directory btree pointer, child bno should "
>> +		fprintf(stderr, "bad %s btree pointer, child bno should "
>>  				"be %d, block bno is %d, hashval is %u\n",
>> -			be16_to_cpu(btree[entry].before),
>> +			FORKNAME(whichfork), be16_to_cpu(btree[entry].before),
>>  			cursor->level[p_level].bno,
>>  			cursor->level[p_level].hashval);
>>  		fprintf(stderr, "verify_final_da_path returns 1 (bad) #1a\n");
>> @@ -403,17 +410,17 @@ verify_final_da_path(
>>  				be32_to_cpu(btree[entry].hashval)) {
>>  		if (!no_modify) {
>>  			do_warn(
>> -_("correcting bad hashval in non-leaf dir block\n"
>> +_("correcting bad hashval in non-leaf %s block\n"
>>   "\tin (level %d) in inode %" PRIu64 ".\n"),
>> -				this_level, cursor->ino);
>> +				FORKNAME(whichfork), this_level, cursor->ino);
>>  			btree[entry].hashval = cpu_to_be32(
>>  						cursor->level[p_level].hashval);
>>  			cursor->level[this_level].dirty++;
>>  		} else {
>>  			do_warn(
>> -_("would correct bad hashval in non-leaf dir block\n"
>> +_("would correct bad hashval in non-leaf %s block\n"
>>   "\tin (level %d) in inode %" PRIu64 ".\n"),
>> -				this_level, cursor->ino);
>> +				FORKNAME(whichfork), this_level, cursor->ino);
>>  		}
>>  	}
>>  
>> @@ -451,7 +458,7 @@ _("would correct bad hashval in non-leaf dir block\n"
>>  	 */
>>  	cursor->level[this_level].hashval = hashval;
>>  
>> -	return verify_final_da_path(mp, cursor, this_level);
>> +	return verify_final_da_path(mp, cursor, this_level, whichfork);
>>  }
>>  
>>  /*
>> @@ -564,8 +571,8 @@ verify_da_path(
>>  			&bmp, &lbmp);
>>  		if (nex == 0) {
>>  			do_warn(
>> -_("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>> -				dabno, cursor->ino);
>> +_("can't get map info for %s block %u of inode %" PRIu64 "\n"),
>> +				FORKNAME(whichfork), dabno, cursor->ino);
>>  			return 1;
>>  		}
>>  
>> @@ -575,8 +582,8 @@ _("can't get map info for block %u of directory inode %" PRIu64 "\n"),
>>  
>>  		if (!bp) {
>>  			do_warn(
>> -_("can't read block %u for directory inode %" PRIu64 "\n"),
>> -				dabno, cursor->ino);
>> +_("can't read %s block %u for inode %" PRIu64 "\n"),
>> +				FORKNAME(whichfork), dabno, cursor->ino);
>>  			return 1;
>>  		}
>>  
>> @@ -592,28 +599,28 @@ _("can't read block %u for directory inode %" PRIu64 "\n"),
>>  		if (nodehdr.magic != XFS_DA_NODE_MAGIC &&
>>  		    nodehdr.magic != XFS_DA3_NODE_MAGIC) {
>>  			do_warn(
>> -_("bad magic number %x in block %u for directory inode %" PRIu64 "\n"),
>> -				nodehdr.magic,
>> +_("bad magic number %x in %s block %u for inode %" PRIu64 "\n"),
>> +				nodehdr.magic, FORKNAME(whichfork),
>>  				dabno, cursor->ino);
>>  			bad++;
>>  		}
>>  		if (nodehdr.back != cursor->level[this_level].bno) {
>>  			do_warn(
>> -_("bad back pointer in block %u for directory inode %" PRIu64 "\n"),
>> -				dabno, cursor->ino);
>> +_("bad back pointer in %s block %u for inode %" PRIu64 "\n"),
>> +				FORKNAME(whichfork), dabno, cursor->ino);
>>  			bad++;
>>  		}
>>  		if (nodehdr.count > geo->node_ents) {
>>  			do_warn(
>> -_("entry count %d too large in block %u for directory inode %" PRIu64 "\n"),
>> -				nodehdr.count,
>> +_("entry count %d too large in %s block %u for inode %" PRIu64 "\n"),
>> +				nodehdr.count, FORKNAME(whichfork),
>>  				dabno, cursor->ino);
>>  			bad++;
>>  		}
>>  		if (nodehdr.level != this_level) {
>>  			do_warn(
>> -_("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>> -				nodehdr.level,
>> +_("bad level %d in %s block %u for inode %" PRIu64 "\n"),
>> +				nodehdr.level, FORKNAME(whichfork),
>>  				dabno, cursor->ino);
>>  			bad++;
>>  		}
>> @@ -659,9 +666,9 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>>  	 */
>>  	if (cursor->level[p_level].bno != be32_to_cpu(btree[entry].before)) {
>>  #ifdef XR_DIR_TRACE
>> -		fprintf(stderr, "bad directory btree pointer, child bno "
>> +		fprintf(stderr, "bad %s btree pointer, child bno "
>>  			"should be %d, block bno is %d, hashval is %u\n",
>> -			be32_to_cpu(btree[entry].before),
>> +			FORKNAME(whichfork), be32_to_cpu(btree[entry].before),
>>  			cursor->level[p_level].bno,
>>  			cursor->level[p_level].hashval);
>>  		fprintf(stderr, "verify_da_path returns 1 (bad) #1a\n");
>> @@ -676,17 +683,17 @@ _("bad level %d in block %u for directory inode %" PRIu64 "\n"),
>>  				be32_to_cpu(btree[entry].hashval)) {
>>  		if (!no_modify) {
>>  			do_warn(
>> -_("correcting bad hashval in interior dir block\n"
>> +_("correcting bad hashval in interior %s block\n"
>>    "\tin (level %d) in inode %" PRIu64 ".\n"),
>> -				this_level, cursor->ino);
>> +				FORKNAME(whichfork), this_level, cursor->ino);
>>  			btree[entry].hashval = cpu_to_be32(
>>  						cursor->level[p_level].hashval);
>>  			cursor->level[this_level].dirty++;
>>  		} else {
>>  			do_warn(
>> -_("would correct bad hashval in interior dir block\n"
>> +_("would correct bad hashval in interior %s block\n"
>>    "\tin (level %d) in inode %" PRIu64 ".\n"),
>> -				this_level, cursor->ino);
>> +				FORKNAME(whichfork), this_level, cursor->ino);
>>  		}
>>  	}
>>  	/*
>> diff --git a/repair/da_util.h b/repair/da_util.h
>> index 7971d63..442f9f1 100644
>> --- a/repair/da_util.h
>> +++ b/repair/da_util.h
>> @@ -78,5 +78,6 @@ int
>>  verify_final_da_path(
>>  	xfs_mount_t	*mp,
>>  	da_bt_cursor_t	*cursor,
>> -	const int	p_level);
>> +	const int	p_level,
>> +	int		whichfork);
>>  #endif	/* _XR_DA_UTIL_H */
>> diff --git a/repair/dir2.c b/repair/dir2.c
>> index 492b3e7..61912d1 100644
>> --- a/repair/dir2.c
>> +++ b/repair/dir2.c
>> @@ -1179,7 +1179,7 @@ _("bad sibling back pointer for block %u in directory inode %" PRIu64 "\n"),
>>  		} else
>>  			libxfs_putbuf(bp);
>>  	} while (da_bno != 0);
>> -	if (verify_final_da_path(mp, da_cursor, 0)) {
>> +	if (verify_final_da_path(mp, da_cursor, 0, XFS_DATA_FORK)) {
>>  		/*
>>  		 * Verify the final path up (right-hand-side) if still ok.
>>  		 */
>> -- 
>> 1.7.1
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/13] xfs_repair: recombine cut&waste code in dir2.c/attr_repair.c

[Carlos Maiolino]

On Wed, Sep 09, 2015 at 02:33:58PM -0500, Eric Sandeen wrote:
> repair/dir2.c and repair/attr_repair.c got cut and pasted
> long, long ago and have diverged since.
> 
> This series brings them back together, presumably fixes some
> bugs, and loses a few hundred lines in the process.
> 
> o/~ reunited, and it feels so so good ... o/~
> 
> The series accomplishes this through a combination of trivial changes
> (removing unused structure members, whitespace, etc) as well
> as by "cross-porting" changes & fixes which happened to one
> file but not the other over the past many years.
> 
> Along the way, a graphical diff of dir2 vs. attr_repair should
> show the convergence.
> 
> Up until the last patch, I don't worry about dir vs. attr naming
> in comments or error messages; the goal is to make these chunks
> of the two files sufficiently similar so that by patch 11, the
> reviewer can do a diff and say "yeah, ok, those really are
> substantially the same now."
> 
> Also instructive is to apply up to patch 11, copy dir2.c and
> attr_repair.c to /tmp, apply patch 12, and do a 3-way graphical
> diff of the 3 files to see that the move really is OK and
> didn't play any significant tricks.
> 
> The last patch fixes up the dir vs. attr text in error messages
> and comments.  I do have a question about whether this is ok
> for i8n:
> 
> 	printf(_("This string is %s"), _("awesome"));

This should be fine for i18n, I had used it a lot when I added i18n support in
gfs2-utils, and _() is the default macro that should embrace every string that
needs to be translated. It will be replaced by gettext("awesome"), and there is
no problem in using it as printf() argument for format specifiers.

What you should be careful though, is that how these strings will 'look' to the
person translating it, which, in most of cases, they are not going to look at
the code to get a better meaning of the string. So, the sentences to be
translated, should make sense by itself.


I particularly, don't like much the idea of split strings as you did in the
example, exactly because how it might look to the translators, both strings
makes the same sentence, but they will show to the translators as completely
different strings, and the translator might not be able to find the proper
grammatical construction. So, I'd do something like:

printf(funny ? _("This string is awesome") : _("This string is boring")) 


I know that I might sound picky here, but, this is the best way to avoid weird
and non-sense string translations.

>
> because that's essentially the trick I used...
> 
> Thanks,
> -Eric
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/13] xfs_repair: recombine cut&waste code in dir2.c/attr_repair.c

[Eric Sandeen]

On 9/10/15 4:22 AM, Carlos Maiolino wrote:
> On Wed, Sep 09, 2015 at 02:33:58PM -0500, Eric Sandeen wrote:

...

>> The last patch fixes up the dir vs. attr text in error messages
>> and comments.  I do have a question about whether this is ok
>> for i8n:
>>
>> 	printf(_("This string is %s"), _("awesome"));
> 
> This should be fine for i18n, I had used it a lot when I added i18n support in
> gfs2-utils, and _() is the default macro that should embrace every string that
> needs to be translated. It will be replaced by gettext("awesome"), and there is
> no problem in using it as printf() argument for format specifiers.
> 
> What you should be careful though, is that how these strings will 'look' to the
> person translating it, which, in most of cases, they are not going to look at
> the code to get a better meaning of the string. So, the sentences to be
> translated, should make sense by itself.
> 
> 
> I particularly, don't like much the idea of split strings as you did in the
> example, exactly because how it might look to the translators, both strings
> makes the same sentence, but they will show to the translators as completely
> different strings, and the translator might not be able to find the proper
> grammatical construction. So, I'd do something like:
> 
> printf(funny ? _("This string is awesome") : _("This string is boring")) 
> 
> 
> I know that I might sound picky here, but, this is the best way to avoid weird
> and non-sense string translations.

No, that makes sense, but it kind of sucks, too - writing the same string
twice everywhere, once for attr & once for dir, is a bit bleah.

Maybe I can restructure it such that it's more easily translatable,
something like using a prefix, i.e.

%s: block %d is unreadable for inode %lld

-> turns into ->

dir: block %d is unreadable for inode %lld
 - or -
attr: block %d is unreadable for inode %lld

and then it's not cutting a sentence in half, to interfere with grammar
from other languages ...

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 0/13] xfs_repair: recombine cut&waste code in dir2.c/attr_repair.c

[Carlos Maiolino]

On Thu, Sep 10, 2015 at 11:51:08AM -0500, Eric Sandeen wrote:
> On 9/10/15 4:22 AM, Carlos Maiolino wrote:
> > On Wed, Sep 09, 2015 at 02:33:58PM -0500, Eric Sandeen wrote:
> 
> ...
> 
> >> The last patch fixes up the dir vs. attr text in error messages
> >> and comments.  I do have a question about whether this is ok
> >> for i8n:
> >>
> >> 	printf(_("This string is %s"), _("awesome"));
> > 
> > This should be fine for i18n, I had used it a lot when I added i18n support in
> > gfs2-utils, and _() is the default macro that should embrace every string that
> > needs to be translated. It will be replaced by gettext("awesome"), and there is
> > no problem in using it as printf() argument for format specifiers.
> > 
> > What you should be careful though, is that how these strings will 'look' to the
> > person translating it, which, in most of cases, they are not going to look at
> > the code to get a better meaning of the string. So, the sentences to be
> > translated, should make sense by itself.
> > 
> > 
> > I particularly, don't like much the idea of split strings as you did in the
> > example, exactly because how it might look to the translators, both strings
> > makes the same sentence, but they will show to the translators as completely
> > different strings, and the translator might not be able to find the proper
> > grammatical construction. So, I'd do something like:
> > 
> > printf(funny ? _("This string is awesome") : _("This string is boring")) 
> > 
> > 
> > I know that I might sound picky here, but, this is the best way to avoid weird
> > and non-sense string translations.
> 
> No, that makes sense, but it kind of sucks, too - writing the same string

> twice everywhere, once for attr & once for dir, is a bit bleah.
> 
> Maybe I can restructure it such that it's more easily translatable,
> something like using a prefix, i.e.
> 
> %s: block %d is unreadable for inode %lld
> 
> -> turns into ->
> 
> dir: block %d is unreadable for inode %lld
>  - or -
> attr: block %d is unreadable for inode %lld
> 
> and then it's not cutting a sentence in half, to interfere with grammar
> from other languages ...
> 

Yep, that makes sense. Or, you can play with variables, but, that would make the
code only a huge spaghetti of strings and probably nobody would want to see
things like that.


> -Eric
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs