Skip to contentSkip to conversation

xfsprogs: initial EFSBADCRC support

PATCH In review Dave Chinner 4 participants 20 messages Mar 4, 2014 – Mar 7, 2014

lore
Share & tools
lore

Dave Chinner

author
#0 lore raw

cover xfsprogs: initial EFSBADCRC support

Hi Folks,

This series is a userspace port of Eric's EFSBADCRC verifier error discrimination patch set. It brings across all the relevant pathes form the kernel code and shoe-horns them into the libxfs codebase. There are two extra patches on the end of the series - one to make xfs_db use EFSBADCRC for it's CRC validity indication, and the other to make xfs_repair detect and handle primary superblock CRC failures and recalculation when the primary superblock is rebuilt and rewritten.

This is the first step in bring full awareness of CRC errors into xfs_repair. More patches will follow as I make more repair code aware that EFSBADCRC means that the object that was just read needs repair.

Cheers,

Dave.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#1 lore raw

PATCH 1/13 libxfs: be more forgiving of a v4 secondary sb w/ junk in v5 fields

From: Dave Chinner <[email protected]>

[userspace port]

Today, if xfs_sb_read_verify encounters a v4 superblock with junk past v4 fields which includes data in sb_crc, it will be treated as a failing checksum and a significant corruption.

There are known prior bugs which leave junk at the end of the V4 superblock; we don't need to actually fail the verification in this case if other checks pan out ok.

So if this is a secondary superblock, and the primary superblock doesn't indicate that this is a V5 filesystem, don't treat this as an actual checksum failure.

We should probably check the garbage condition as we do in xfs_repair, and possibly warn about it or self-heal, but that's a different scope of work.

Signed-off-by Dave Chinner <[email protected]>
---
 libxfs/xfs_sb.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
libxfs/xfs_sb.c +11 -2
index 48b1a97..16088a6 100644
@@ -542,6 +542,11 @@ xfs_sb_verify(
  * single bit error could clear the feature bit and unused parts of the
  * superblock are supposed to be zero. Hence a non-null crc field indicates that
  * we've potentially lost a feature bit and we should check it anyway.
+ *
+ * However, past bugs (i.e. in growfs) left non-zeroed regions beyond the
+ * last field in V4 secondary superblocks.  So for secondary superblocks,
+ * we are more forgiving, and ignore CRC failures if the primary doesn't
+ * indicate that the fs version is V5.
  */
 static void
 xfs_sb_read_verify(

@@ -562,8 +567,12 @@ xfs_sb_read_verify(
 
 		if (!xfs_verify_cksum(bp->b_addr, be16_to_cpu(dsb->sb_sectsize),
 				      offsetof(struct xfs_sb, sb_crc))) {
-			error = EFSCORRUPTED;
-			goto out_error;
+			/* Only fail bad secondaries on a known V5 filesystem */
+			if (bp->b_bn != XFS_SB_DADDR &&
+			    xfs_sb_version_hascrc(&mp->m_sb)) {
+				error = EFSCORRUPTED;
+				goto out_error;
+			}
 		}
 	}
 	error = xfs_sb_verify(bp, true);
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#2 lore raw

PATCH 2/13 libxfs: sanitize sb_inopblock in xfs_mount_validate_sb

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

xfs_mount_validate_sb doesn't check sb_inopblock for sanity (as does its xfs_repair counterpart, FWIW).

If it's out of bounds, we can go off the rails in i.e. xfs_inode_buf_verify(), which uses sb_inopblock as a loop limit when stepping through a metadata buffer.

The problem can be demonstrated easily by corrupting sb_inopblock with xfs_db and trying to mount the result:

# mkfs.xfs -dfile,name=fsfile,size=1g
# xfs_db -x fsfile
xfs_db> sb 0
xfs_db> write inopblock 512
inopblock = 512
xfs_db> quit
# mount -o loop fsfile  mnt
and we blow up in xfs_inode_buf_verify().

With this patch, we get a (very noisy) corruption error, and fail the mount as we should.

Signed-off-by Dave Chinner <[email protected]>
---
 libxfs/xfs_sb.c | 1 +
 1 file changed, 1 insertion(+)
libxfs/xfs_sb.c +1 -0
index 16088a6..4be7366 100644
@@ -258,6 +258,7 @@ xfs_mount_validate_sb(
 	    sbp->sb_inodelog < XFS_DINODE_MIN_LOG			||
 	    sbp->sb_inodelog > XFS_DINODE_MAX_LOG			||
 	    sbp->sb_inodesize != (1 << sbp->sb_inodelog)		||
+	    sbp->sb_inopblock != howmany(sbp->sb_blocksize,sbp->sb_inodesize) ||
 	    (sbp->sb_blocklog - sbp->sb_inodelog != sbp->sb_inopblog)	||
 	    (sbp->sb_rextsize * sbp->sb_blocksize > XFS_MAX_RTEXTSIZE)	||
 	    (sbp->sb_rextsize * sbp->sb_blocksize < XFS_MIN_RTEXTSIZE)	||
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#3 lore raw

PATCH 3/13 libxfs: xfs_sb_read_verify() doesn't flag bad crcs on primary sb

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

My earlier commit 10e6e65 deserves a layer or two of brown paper bags. The logic in that commit means that a CRC failure on the primary superblock will *never* result in an error return.

Hopefully this fixes it, so that we always return the error if it's a primary superblock, otherwise only if the filesystem has CRCs enabled.

Signed-off-by Dave Chinner <[email protected]>
---
 libxfs/xfs_sb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
libxfs/xfs_sb.c +1 -1
index 4be7366..5c10c97 100644
@@ -569,7 +569,7 @@ xfs_sb_read_verify(
 		if (!xfs_verify_cksum(bp->b_addr, be16_to_cpu(dsb->sb_sectsize),
 				      offsetof(struct xfs_sb, sb_crc))) {
 			/* Only fail bad secondaries on a known V5 filesystem */
-			if (bp->b_bn != XFS_SB_DADDR &&
+			if (bp->b_bn == XFS_SB_DADDR ||
 			    xfs_sb_version_hascrc(&mp->m_sb)) {
 				error = EFSCORRUPTED;
 				goto out_error;
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#4 lore raw

PATCH 4/13 libxfs: skip verification on initial "guess" superblock read

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

When xfs_readsb() does the very first read of the superblock, it makes a guess at the length of the buffer, based on the sector size of the underlying storage. This may or may not match the filesystem sector size in sb_sectsize, so we can't i.e. do a CRC check on it; it might be too short.

In fact, mounting a filesystem with sb_sectsize larger than the device sector size will cause a mount failure if CRCs are enabled, because we are checksumming a length which exceeds the buffer passed to it.

So always read twice; the first time we read with NULL buffer ops to skip verification; then set the proper read length, hook up the proper verifier, and give it another go.

Once we are sure that we've got the right buffer length, we can also use bp->b_length in the xfs_sb_read_verify, rather than the less-trusted on-disk sectorsize for secondary superblocks. Before this we ran the risk of passing junk to the crc32c routines, which didn't always handle extreme values.

Signed-off-by Dave Chinner <[email protected]>
---
 libxfs/xfs_sb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
libxfs/xfs_sb.c +1 -2
index 5c10c97..6ad99a5 100644
@@ -566,7 +566,7 @@ xfs_sb_read_verify(
 						XFS_SB_VERSION_5) ||
 	     dsb->sb_crc != 0)) {
 
-		if (!xfs_verify_cksum(bp->b_addr, be16_to_cpu(dsb->sb_sectsize),
+		if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
 				      offsetof(struct xfs_sb, sb_crc))) {
 			/* Only fail bad secondaries on a known V5 filesystem */
 			if (bp->b_bn == XFS_SB_DADDR ||

@@ -599,7 +599,6 @@ xfs_sb_quiet_read_verify(
 {
 	struct xfs_dsb	*dsb = XFS_BUF_TO_SBP(bp);
 
-
 	if (dsb->sb_magicnum == cpu_to_be32(XFS_SB_MAGIC)) {
 		/* XFS filesystem, verify noisily! */
 		xfs_sb_read_verify(bp);
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#5 lore raw

PATCH 5/13 libxfs: limit superblock corruption errors to actual corruption

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

Today, if

xfs_sb_read_verify
xfs_sb_verify
xfs_mount_validate_sb

detects superblock corruption, it'll be extremely noisy, dumping 2 stacks, 2 hexdumps, etc.

This is because we call XFS_CORRUPTION_ERROR in xfs_mount_validate_sb as well as in xfs_sb_read_verify.

Also, *any* errors in xfs_mount_validate_sb which are not corruption per se; things like too-big-blocksize, bad version, bad magic, v1 dirs, rw-incompat etc - things which do not return EFSCORRUPTED - will still do the whole XFS_CORRUPTION_ERROR spew when xfs_sb_read_verify sees any error at all. And it suggests to the user that they should run xfs_repair, even if the root cause of the mount failure is a simple incompatibility.

I'll submit that the probably-not-corrupted errors don't warrant this much noise, so this patch removes the warning for anything other than EFSCORRUPTED returns, and replaces the lower-level XFS_CORRUPTION_ERROR with an xfs_notice().

Signed-off-by Dave Chinner <[email protected]>
---
 libxfs/xfs_sb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
libxfs/xfs_sb.c +2 -3
index 6ad99a5..05bc860 100644
@@ -266,8 +266,7 @@ xfs_mount_validate_sb(
 	    sbp->sb_dblocks == 0					||
 	    sbp->sb_dblocks > XFS_MAX_DBLOCKS(sbp)			||
 	    sbp->sb_dblocks < XFS_MIN_DBLOCKS(sbp))) {
-		XFS_CORRUPTION_ERROR("SB sanity check failed",
-				XFS_ERRLEVEL_LOW, mp, sbp);
+		xfs_notice(mp, "SB sanity check failed");
 		return XFS_ERROR(EFSCORRUPTED);
 	}
 

@@ -580,7 +579,7 @@ xfs_sb_read_verify(
 
 out_error:
 	if (error) {
-		if (error != EWRONGFS)
+		if (error == EFSCORRUPTED)
 			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
 					     mp, bp->b_addr);
 		xfs_buf_ioerror(bp, error);
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#6 lore raw

PATCH 6/13 libxfs: skip pointless CRC updates after verifier failures

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

Most write verifiers don't update CRCs after the verifier has failed and the buffer has been marked in error. These two didn't, but should.

Add returns to the verifier failure block, since the buffer won't be written anyway.

Signed-off-by Dave Chinner <[email protected]>
---
 libxfs/xfs_alloc_btree.c  | 1 +
 libxfs/xfs_ialloc_btree.c | 1 +
 2 files changed, 2 insertions(+)
libxfs/xfs_alloc_btree.c +1 -0
index 282a320..5b38c24 100644
@@ -355,6 +355,7 @@ xfs_allocbt_write_verify(
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
 				     bp->b_target->bt_mount, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		return;
 	}
 	xfs_btree_sblock_calc_crc(bp);
libxfs/xfs_ialloc_btree.c +1 -0
index 27a5dd9..d9589b7 100644
@@ -242,6 +242,7 @@ xfs_inobt_write_verify(
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
 				     bp->b_target->bt_mount, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		return;
 	}
 	xfs_btree_sblock_calc_crc(bp);
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#7 lore raw

PATCH 7/13 libxfs: Use defines for CRC offsets in all cases

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

Some calls to crc functions used useful #defines,
others used awkward offsetof() constructs.

Switch them all to #define to make things a bit cleaner.

Signed-off-by Dave Chinner <[email protected]>
---
 include/xfs_ag.h            |  6 ++++++
 include/xfs_dinode.h        |  2 ++
 include/xfs_format.h        |  2 ++
 include/xfs_sb.h            |  2 ++
 libxfs/xfs_alloc.c          | 10 ++++------
 libxfs/xfs_ialloc.c         |  5 ++---
 libxfs/xfs_inode_buf.c      |  4 ++--
 libxfs/xfs_sb.c             |  5 ++---
 libxfs/xfs_symlink_remote.c |  5 ++---
 9 files changed, 24 insertions(+), 17 deletions(-)
include/xfs_ag.h +6 -0
index 3fc1098..0fdd410 100644
@@ -89,6 +89,8 @@ typedef struct xfs_agf {
 	/* structure must be padded to 64 bit alignment */
 } xfs_agf_t;
 
+#define XFS_AGF_CRC_OFF		offsetof(struct xfs_agf, agf_crc)
+
 #define	XFS_AGF_MAGICNUM	0x00000001
 #define	XFS_AGF_VERSIONNUM	0x00000002
 #define	XFS_AGF_SEQNO		0x00000004

@@ -167,6 +169,8 @@ typedef struct xfs_agi {
 	/* structure must be padded to 64 bit alignment */
 } xfs_agi_t;
 
+#define XFS_AGI_CRC_OFF		offsetof(struct xfs_agi, agi_crc)
+
 #define	XFS_AGI_MAGICNUM	0x00000001
 #define	XFS_AGI_VERSIONNUM	0x00000002
 #define	XFS_AGI_SEQNO		0x00000004

@@ -222,6 +226,8 @@ typedef struct xfs_agfl {
 	__be32		agfl_bno[];	/* actually XFS_AGFL_SIZE(mp) */
 } xfs_agfl_t;
 
+#define XFS_AGFL_CRC_OFF	offsetof(struct xfs_agfl, agfl_crc)
+
 /*
  * tags for inode radix tree
  */
include/xfs_dinode.h +2 -0
index e5869b5..623bbe8 100644
@@ -89,6 +89,8 @@ typedef struct xfs_dinode {
 	/* structure must be padded to 64 bit alignment */
 } xfs_dinode_t;
 
+#define XFS_DINODE_CRC_OFF	offsetof(struct xfs_dinode, di_crc)
+
 #define DI_MAX_FLUSH 0xffff
 
 /*
include/xfs_format.h +2 -0
index 997c770..77f6b8b 100644
@@ -145,6 +145,8 @@ struct xfs_dsymlink_hdr {
 	__be64	sl_lsn;
 };
 
+#define XFS_SYMLINK_CRC_OFF	offsetof(struct xfs_dsymlink_hdr, sl_crc)
+
 /*
  * The maximum pathlen is 1024 bytes. Since the minimum file system
  * blocksize is 512 bytes, we can get a max of 3 extents back from
include/xfs_sb.h +2 -0
index 35061d4..f7b2fe7 100644
@@ -182,6 +182,8 @@ typedef struct xfs_sb {
 	/* must be padded to 64 bit alignment */
 } xfs_sb_t;
 
+#define XFS_SB_CRC_OFF		offsetof(struct xfs_sb, sb_crc)
+
 /*
  * Superblock - on disk version.  Must match the in core version above.
  * Must be padded to 64 bit alignment.
libxfs/xfs_alloc.c +4 -6
index e4fb1ad..8a6725e 100644
@@ -464,7 +464,7 @@ xfs_agfl_read_verify(
 		return;
 
 	agfl_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-				   offsetof(struct xfs_agfl, agfl_crc));
+				   XFS_AGFL_CRC_OFF);
 
 	agfl_ok = agfl_ok && xfs_agfl_verify(bp);
 

@@ -494,8 +494,7 @@ xfs_agfl_write_verify(
 	if (bip)
 		XFS_BUF_TO_AGFL(bp)->agfl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
-			 offsetof(struct xfs_agfl, agfl_crc));
+	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGFL_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_agfl_buf_ops = {

@@ -2222,7 +2221,7 @@ xfs_agf_read_verify(
 
 	if (xfs_sb_version_hascrc(&mp->m_sb))
 		agf_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					  offsetof(struct xfs_agf, agf_crc));
+					  XFS_AGF_CRC_OFF);
 
 	agf_ok = agf_ok && xfs_agf_verify(mp, bp);
 

@@ -2252,8 +2251,7 @@ xfs_agf_write_verify(
 	if (bip)
 		XFS_BUF_TO_AGF(bp)->agf_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
-			 offsetof(struct xfs_agf, agf_crc));
+	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGF_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_agf_buf_ops = {
libxfs/xfs_ialloc.c +2 -3
index afe1a82..b83ad98 100644
@@ -1555,7 +1555,7 @@ xfs_agi_read_verify(
 
 	if (xfs_sb_version_hascrc(&mp->m_sb))
 		agi_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					  offsetof(struct xfs_agi, agi_crc));
+					  XFS_AGI_CRC_OFF);
 	agi_ok = agi_ok && xfs_agi_verify(bp);
 
 	if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,

@@ -1583,8 +1583,7 @@ xfs_agi_write_verify(
 
 	if (bip)
 		XFS_BUF_TO_AGI(bp)->agi_lsn = cpu_to_be64(bip->bli_item.li_lsn);
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
-			 offsetof(struct xfs_agi, agi_crc));
+	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGI_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_agi_buf_ops = {
libxfs/xfs_inode_buf.c +2 -2
index d245d72..4f29033 100644
@@ -292,7 +292,7 @@ xfs_dinode_verify(
 	if (!xfs_sb_version_hascrc(&mp->m_sb))
 		return false;
 	if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
-			      offsetof(struct xfs_dinode, di_crc)))
+			      XFS_DINODE_CRC_OFF))
 		return false;
 	if (be64_to_cpu(dip->di_ino) != ino)
 		return false;

@@ -313,7 +313,7 @@ xfs_dinode_calc_crc(
 
 	ASSERT(xfs_sb_version_hascrc(&mp->m_sb));
 	crc = xfs_start_cksum((char *)dip, mp->m_sb.sb_inodesize,
-			      offsetof(struct xfs_dinode, di_crc));
+			      XFS_DINODE_CRC_OFF);
 	dip->di_crc = xfs_end_cksum(crc);
 }
libxfs/xfs_sb.c +2 -3
index 05bc860..bbcf886 100644
@@ -566,7 +566,7 @@ xfs_sb_read_verify(
 	     dsb->sb_crc != 0)) {
 
 		if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-				      offsetof(struct xfs_sb, sb_crc))) {
+				      XFS_SB_CRC_OFF)) {
 			/* Only fail bad secondaries on a known V5 filesystem */
 			if (bp->b_bn == XFS_SB_DADDR ||
 			    xfs_sb_version_hascrc(&mp->m_sb)) {

@@ -629,8 +629,7 @@ xfs_sb_write_verify(
 	if (bip)
 		XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
-			 offsetof(struct xfs_sb, sb_crc));
+	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SB_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_sb_buf_ops = {
libxfs/xfs_symlink_remote.c +2 -3
index 539db0c..09ee9d4 100644
@@ -117,7 +117,7 @@ xfs_symlink_read_verify(
 		return;
 
 	if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-				  offsetof(struct xfs_dsymlink_hdr, sl_crc)) ||
+				  XFS_SYMLINK_CRC_OFF) ||
 	    !xfs_symlink_verify(bp)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);

@@ -145,8 +145,7 @@ xfs_symlink_write_verify(
 		struct xfs_dsymlink_hdr *dsl = bp->b_addr;
 		dsl->sl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 	}
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
-			 offsetof(struct xfs_dsymlink_hdr, sl_crc));
+	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SYMLINK_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_symlink_buf_ops = {
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#8 lore raw

PATCH 8/13 libxfs: add helper for verifying checksums on xfs_bufs

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

Many/most callers of xfs_verify_cksum() pass bp->b_addr and BBTOB(bp->b_length) as the first 2 args. Add a helper which can just accept the bp and the crc offset, and work it out on its own, for brevity.

Signed-off-by Dave Chinner <[email protected]>
---
 include/libxfs.h            | 7 +++++++
 libxfs/xfs_alloc.c          | 6 ++----
 libxfs/xfs_attr_leaf.c      | 3 +--
 libxfs/xfs_btree.c          | 8 ++++----
 libxfs/xfs_da_btree.c       | 3 +--
 libxfs/xfs_dir2_block.c     | 3 +--
 libxfs/xfs_dir2_data.c      | 3 +--
 libxfs/xfs_dir2_leaf.c      | 3 +--
 libxfs/xfs_dir2_node.c      | 3 +--
 libxfs/xfs_ialloc.c         | 4 ++--
 libxfs/xfs_sb.c             | 3 +--
 libxfs/xfs_symlink_remote.c | 3 +--
 12 files changed, 23 insertions(+), 26 deletions(-)
include/libxfs.h +7 -0
index f688598..610e7d5 100644
@@ -779,6 +779,13 @@ extern uint32_t crc32c_le(uint32_t crc, unsigned char const *p, size_t len);
 
 #include <xfs/xfs_cksum.h>
 
+static inline int
+xfs_buf_verify_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
+{
+	return xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
+				cksum_offset);
+}
+
 #define xfs_notice(mp,fmt,args...)		cmn_err(CE_NOTE,fmt, ## args)
 #define xfs_warn(mp,fmt,args...)		cmn_err(CE_WARN,fmt, ## args)
 #define xfs_alert(mp,fmt,args...)		cmn_err(CE_ALERT,fmt, ## args)
libxfs/xfs_alloc.c +2 -4
index 8a6725e..2f6a241 100644
@@ -463,8 +463,7 @@ xfs_agfl_read_verify(
 	if (!xfs_sb_version_hascrc(&mp->m_sb))
 		return;
 
-	agfl_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-				   XFS_AGFL_CRC_OFF);
+	agfl_ok = xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF);
 
 	agfl_ok = agfl_ok && xfs_agfl_verify(bp);
 

@@ -2220,8 +2219,7 @@ xfs_agf_read_verify(
 	int		agf_ok = 1;
 
 	if (xfs_sb_version_hascrc(&mp->m_sb))
-		agf_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					  XFS_AGF_CRC_OFF);
+		agf_ok = xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF);
 
 	agf_ok = agf_ok && xfs_agf_verify(mp, bp);
libxfs/xfs_attr_leaf.c +1 -2
index fd52397..0bd3169 100644
@@ -214,8 +214,7 @@ xfs_attr3_leaf_read_verify(
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
 	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					  XFS_ATTR3_LEAF_CRC_OFF)) ||
+	     !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF)) ||
 	    !xfs_attr3_leaf_verify(bp)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
libxfs/xfs_btree.c +4 -4
index 2dd6fb7..fffed0a 100644
@@ -227,8 +227,8 @@ xfs_btree_lblock_verify_crc(
 	struct xfs_buf		*bp)
 {
 	if (xfs_sb_version_hascrc(&bp->b_target->bt_mount->m_sb))
-		return xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					XFS_BTREE_LBLOCK_CRC_OFF);
+		return xfs_buf_verify_cksum(bp, XFS_BTREE_LBLOCK_CRC_OFF);
+
 	return true;
 }
 

@@ -260,8 +260,8 @@ xfs_btree_sblock_verify_crc(
 	struct xfs_buf		*bp)
 {
 	if (xfs_sb_version_hascrc(&bp->b_target->bt_mount->m_sb))
-		return xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					XFS_BTREE_SBLOCK_CRC_OFF);
+		return xfs_buf_verify_cksum(bp, XFS_BTREE_SBLOCK_CRC_OFF);
+
 	return true;
 }
libxfs/xfs_da_btree.c +1 -2
index 53414f5..4e8b41a 100644
@@ -238,8 +238,7 @@ xfs_da3_node_read_verify(
 
 	switch (be16_to_cpu(info->magic)) {
 		case XFS_DA3_NODE_MAGIC:
-			if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					      XFS_DA3_NODE_CRC_OFF))
+			if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF))
 				break;
 			/* fall through */
 		case XFS_DA_NODE_MAGIC:
libxfs/xfs_dir2_block.c +1 -2
index 1d8f598..13561c3 100644
@@ -71,8 +71,7 @@ xfs_dir3_block_read_verify(
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
 	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					  XFS_DIR3_DATA_CRC_OFF)) ||
+	     !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
 	    !xfs_dir3_block_verify(bp)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
libxfs/xfs_dir2_data.c +1 -2
index 189699f..2a0de28 100644
@@ -235,8 +235,7 @@ xfs_dir3_data_read_verify(
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
 	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					  XFS_DIR3_DATA_CRC_OFF)) ||
+	     !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
 	    !xfs_dir3_data_verify(bp)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
libxfs/xfs_dir2_leaf.c +1 -2
index 683536e..0fa147f 100644
@@ -207,8 +207,7 @@ __read_verify(
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
 	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					  XFS_DIR3_LEAF_CRC_OFF)) ||
+	     !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF)) ||
 	    !xfs_dir3_leaf_verify(bp, magic)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
libxfs/xfs_dir2_node.c +1 -2
index ced8c58..78c77ed 100644
@@ -99,8 +99,7 @@ xfs_dir3_free_read_verify(
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
 	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					  XFS_DIR3_FREE_CRC_OFF)) ||
+	     !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF)) ||
 	    !xfs_dir3_free_verify(bp)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
libxfs/xfs_ialloc.c +2 -2
index b83ad98..21683b5 100644
@@ -1554,8 +1554,8 @@ xfs_agi_read_verify(
 	int		agi_ok = 1;
 
 	if (xfs_sb_version_hascrc(&mp->m_sb))
-		agi_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-					  XFS_AGI_CRC_OFF);
+		agi_ok = xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF);
+
 	agi_ok = agi_ok && xfs_agi_verify(bp);
 
 	if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
libxfs/xfs_sb.c +1 -2
index bbcf886..43fdb59 100644
@@ -565,8 +565,7 @@ xfs_sb_read_verify(
 						XFS_SB_VERSION_5) ||
 	     dsb->sb_crc != 0)) {
 
-		if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-				      XFS_SB_CRC_OFF)) {
+		if (!xfs_buf_verify_cksum(bp, XFS_SB_CRC_OFF)) {
 			/* Only fail bad secondaries on a known V5 filesystem */
 			if (bp->b_bn == XFS_SB_DADDR ||
 			    xfs_sb_version_hascrc(&mp->m_sb)) {
libxfs/xfs_symlink_remote.c +1 -2
index 09ee9d4..716f8a3 100644
@@ -116,8 +116,7 @@ xfs_symlink_read_verify(
 	if (!xfs_sb_version_hascrc(&mp->m_sb))
 		return;
 
-	if (!xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
-				  XFS_SYMLINK_CRC_OFF) ||
+	if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF) ||
 	    !xfs_symlink_verify(bp)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#9 lore raw

PATCH 9/13 libxfs: add helper for updating checksums on xfs_bufs

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

Many/most callers of xfs_update_cksum() pass bp->b_addr and BBTOB(bp->b_length) as the first 2 args. Add a helper which can just accept the bp and the crc offset, and work it out on its own, for brevity.

Signed-off-by Dave Chinner <[email protected]>
---
 include/libxfs.h            | 7 +++++++
 libxfs/xfs_alloc.c          | 4 ++--
 libxfs/xfs_attr_leaf.c      | 2 +-
 libxfs/xfs_btree.c          | 6 ++----
 libxfs/xfs_da_btree.c       | 2 +-
 libxfs/xfs_dir2_block.c     | 2 +-
 libxfs/xfs_dir2_data.c      | 2 +-
 libxfs/xfs_dir2_leaf.c      | 2 +-
 libxfs/xfs_dir2_node.c      | 2 +-
 libxfs/xfs_ialloc.c         | 2 +-
 libxfs/xfs_sb.c             | 2 +-
 libxfs/xfs_symlink_remote.c | 2 +-
 12 files changed, 20 insertions(+), 15 deletions(-)
include/libxfs.h +7 -0
index 610e7d5..7500903 100644
@@ -786,6 +786,13 @@ xfs_buf_verify_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
 				cksum_offset);
 }
 
+static inline void
+xfs_buf_update_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
+{
+	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
+			 cksum_offset);
+}
+
 #define xfs_notice(mp,fmt,args...)		cmn_err(CE_NOTE,fmt, ## args)
 #define xfs_warn(mp,fmt,args...)		cmn_err(CE_WARN,fmt, ## args)
 #define xfs_alert(mp,fmt,args...)		cmn_err(CE_ALERT,fmt, ## args)
libxfs/xfs_alloc.c +2 -2
index 2f6a241..dca612b 100644
@@ -493,7 +493,7 @@ xfs_agfl_write_verify(
 	if (bip)
 		XFS_BUF_TO_AGFL(bp)->agfl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGFL_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_AGFL_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_agfl_buf_ops = {

@@ -2249,7 +2249,7 @@ xfs_agf_write_verify(
 	if (bip)
 		XFS_BUF_TO_AGF(bp)->agf_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGF_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_AGF_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_agf_buf_ops = {
libxfs/xfs_attr_leaf.c +1 -1
index 0bd3169..90937d6 100644
@@ -198,7 +198,7 @@ xfs_attr3_leaf_write_verify(
 	if (bip)
 		hdr3->info.lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_ATTR3_LEAF_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF);
 }
 
 /*
libxfs/xfs_btree.c +2 -4
index fffed0a..9be4abd 100644
@@ -218,8 +218,7 @@ xfs_btree_lblock_calc_crc(
 		return;
 	if (bip)
 		block->bb_u.l.bb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
-			 XFS_BTREE_LBLOCK_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_BTREE_LBLOCK_CRC_OFF);
 }
 
 bool

@@ -251,8 +250,7 @@ xfs_btree_sblock_calc_crc(
 		return;
 	if (bip)
 		block->bb_u.s.bb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length),
-			 XFS_BTREE_SBLOCK_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_BTREE_SBLOCK_CRC_OFF);
 }
 
 bool
libxfs/xfs_da_btree.c +1 -1
index 4e8b41a..0c0c5e0 100644
@@ -220,7 +220,7 @@ xfs_da3_node_write_verify(
 	if (bip)
 		hdr3->info.lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DA3_NODE_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_DA3_NODE_CRC_OFF);
 }
 
 /*
libxfs/xfs_dir2_block.c +1 -1
index 13561c3..3fe10ea 100644
@@ -98,7 +98,7 @@ xfs_dir3_block_write_verify(
 	if (bip)
 		hdr3->lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DIR3_DATA_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_DIR3_DATA_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_dir3_block_buf_ops = {
libxfs/xfs_dir2_data.c +1 -1
index 2a0de28..bcb4c7f 100644
@@ -262,7 +262,7 @@ xfs_dir3_data_write_verify(
 	if (bip)
 		hdr3->lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DIR3_DATA_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_DIR3_DATA_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_dir3_data_buf_ops = {
libxfs/xfs_dir2_leaf.c +1 -1
index 0fa147f..710f005 100644
@@ -235,7 +235,7 @@ __write_verify(
 	if (bip)
 		hdr3->info.lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DIR3_LEAF_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_DIR3_LEAF_CRC_OFF);
 }
 
 static void
libxfs/xfs_dir2_node.c +1 -1
index 78c77ed..f4260ea 100644
@@ -126,7 +126,7 @@ xfs_dir3_free_write_verify(
 	if (bip)
 		hdr3->lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_DIR3_FREE_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_DIR3_FREE_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_dir3_free_buf_ops = {
libxfs/xfs_ialloc.c +1 -1
index 21683b5..b1382d6 100644
@@ -1583,7 +1583,7 @@ xfs_agi_write_verify(
 
 	if (bip)
 		XFS_BUF_TO_AGI(bp)->agi_lsn = cpu_to_be64(bip->bli_item.li_lsn);
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_AGI_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_AGI_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_agi_buf_ops = {
libxfs/xfs_sb.c +1 -1
index 43fdb59..db267c2 100644
@@ -628,7 +628,7 @@ xfs_sb_write_verify(
 	if (bip)
 		XFS_BUF_TO_SBP(bp)->sb_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SB_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_SB_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_sb_buf_ops = {
libxfs/xfs_symlink_remote.c +1 -1
index 716f8a3..b59bf14 100644
@@ -144,7 +144,7 @@ xfs_symlink_write_verify(
 		struct xfs_dsymlink_hdr *dsl = bp->b_addr;
 		dsl->sl_lsn = cpu_to_be64(bip->bli_item.li_lsn);
 	}
-	xfs_update_cksum(bp->b_addr, BBTOB(bp->b_length), XFS_SYMLINK_CRC_OFF);
+	xfs_buf_update_cksum(bp, XFS_SYMLINK_CRC_OFF);
 }
 
 const struct xfs_buf_ops xfs_symlink_buf_ops = {
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#10 lore raw

PATCH 10/13 libxfs: add xfs_verifier_error()

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

We want to distinguish between corruption, CRC errors, etc. In addition, the full stack trace on verifier errors seems less than helpful; it looks more like an oops than corruption.

Create a new function to specifically alert the user to verifier errors, which can differentiate between EFSCORRUPTED and CRC mismatches. It doesn't dump stack unless the xfs error level is turned up high.

Define a new error message (EFSBADCRC) to clearly identify CRC errors. (Defined to EBADMSG, bad message)

Signed-off-by Dave Chinner <[email protected]>
---
 include/darwin.h      |  1 +
 include/freebsd.h     |  1 +
 include/gnukfreebsd.h |  1 +
 include/irix.h        |  2 ++
 include/linux.h       |  1 +
 libxfs/util.c         | 13 +++++++++++++
 libxfs/xfs.h          |  1 +
 7 files changed, 20 insertions(+)
include/darwin.h +1 -0
index 97b8990..95f865b 100644
@@ -150,6 +150,7 @@ typedef unsigned char	uchar_t;
 
 #define ENOATTR		989     /* Attribute not found */
 #define EFSCORRUPTED	990	/* Filesystem is corrupted */
+#define EFSBADCRC	991	/* Bad CRC detected */
 #define constpp		char * const *
 
 #define HAVE_FID	1
include/freebsd.h +1 -0
index 2e1ae49..b51688b 100644
@@ -45,6 +45,7 @@
 #define constpp	char * const *
 
 #define EFSCORRUPTED	990	/* Filesystem is corrupted */
+#define EFSBADCRC	991	/* Bad CRC detected */
 
 typedef off_t		xfs_off_t;
 typedef off_t		off64_t;
include/gnukfreebsd.h +1 -0
index 1ec291f..2140acd 100644
@@ -36,6 +36,7 @@
 #define constpp	char * const *
 
 #define EFSCORRUPTED	990	/* Filesystem is corrupted */
+#define EFSBADCRC	991	/* Bad CRC detected */
 
 typedef off_t		xfs_off_t;
 typedef __uint64_t	xfs_ino_t;
include/irix.h +2 -0
index a450684..5040451 100644
@@ -52,6 +52,8 @@ typedef char*		xfs_caddr_t;
 #define xfs_flock64	flock64
 #define xfs_flock64_t	struct flock64
 
+#define EFSBADCRC	991	/* Bad CRC detected */
+
 typedef struct xfs_error_injection {
 	__int32_t	fd;
 	__int32_t	errtag;
include/linux.h +1 -0
index 502fd1f..5586290 100644
@@ -136,6 +136,7 @@ platform_discard_blocks(int fd, uint64_t start, uint64_t len)
 
 #define ENOATTR		ENODATA	/* Attribute not found */
 #define EFSCORRUPTED	EUCLEAN	/* Filesystem is corrupted */
+#define EFSBADCRC	EBADMSG	/* Bad CRC detected */
 
 typedef loff_t		xfs_off_t;
 typedef __uint64_t	xfs_ino_t;
libxfs/util.c +13 -0
index 8109ab3..1b05540 100644
@@ -730,3 +730,16 @@ cmn_err(int level, char *fmt, ...)
 	fputs("\n", stderr);
 	va_end(ap);
 }
+
+/*
+ * Warnings specifically for verifier errors.  Differentiate CRC vs. invalid
+ * values, and omit the stack trace unless the error level is tuned high.
+ */
+void
+xfs_verifier_error(
+	struct xfs_buf		*bp)
+{
+	xfs_alert(NULL, "Metadata %s detected at block 0x%llx/0x%x",
+		  bp->b_error == EFSBADCRC ? "CRC error" : "corruption",
+		  bp->b_bn, BBTOB(bp->b_length));
+}
libxfs/xfs.h +1 -0
index 364fd83..5a21590 100644
@@ -449,3 +449,4 @@ int  xfs_mod_incore_sb(xfs_mount_t *, xfs_sb_field_t, int64_t, int);
 void xfs_trans_mod_sb(xfs_trans_t *, uint, long);
 void xfs_trans_init(struct xfs_mount *);
 int  xfs_trans_roll(struct xfs_trans **, struct xfs_inode *);
+void xfs_verifier_error(struct xfs_buf *bp);
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#11 lore raw

PATCH 11/13 libxfs: modify verifiers to differentiate CRC from other errors

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

[userspace port]

Modify all read & write verifiers to differentiate between CRC errors and other inconsistencies.

This sets the appropriate error number on bp->b_error, and then calls xfs_verifier_error() if something went wrong. That function will issue the appropriate message to the user.

Also, fix the silly bug in xfs_buf_ioerror() that this patch exposes.

Signed-off-by Dave Chinner <[email protected]>
---
 include/libxfs.h            |  2 +-
 libxfs/xfs_alloc.c          | 37 +++++++++++++++++--------------------
 libxfs/xfs_alloc_btree.c    | 15 ++++++++-------
 libxfs/xfs_attr_leaf.c      | 14 ++++++++------
 libxfs/xfs_attr_remote.c    | 15 ++++++---------
 libxfs/xfs_bmap_btree.c     | 16 ++++++++--------
 libxfs/xfs_da_btree.c       | 14 ++++++++------
 libxfs/xfs_dir2_block.c     | 14 ++++++++------
 libxfs/xfs_dir2_data.c      | 17 +++++++++--------
 libxfs/xfs_dir2_leaf.c      | 14 ++++++++------
 libxfs/xfs_dir2_node.c      | 14 ++++++++------
 libxfs/xfs_dquot_buf.c      | 11 +++++++----
 libxfs/xfs_ialloc.c         | 21 ++++++++++-----------
 libxfs/xfs_ialloc_btree.c   | 15 ++++++++-------
 libxfs/xfs_inode_buf.c      |  3 +--
 libxfs/xfs_sb.c             | 10 ++++------
 libxfs/xfs_symlink_remote.c | 12 +++++++-----
 17 files changed, 126 insertions(+), 118 deletions(-)
include/libxfs.h +1 -1
index 7500903..6bc6c94 100644
@@ -365,7 +365,7 @@ enum xfs_buf_flags_t {	/* b_flags bits */
 #define XFS_BUF_PRIORITY(bp)		(cache_node_get_priority( \
 						(struct cache_node *)(bp)))
 #define xfs_buf_set_ref(bp,ref)		((void) 0)
-#define xfs_buf_ioerror(bp,err)		(bp)->b_error = (err);
+#define xfs_buf_ioerror(bp,err)		((bp)->b_error = (err))
 
 #define xfs_daddr_to_agno(mp,d) \
 	((xfs_agnumber_t)(XFS_BB_TO_FSBT(mp, d) / (mp)->m_sb.sb_agblocks))
libxfs/xfs_alloc.c +17 -20
index dca612b..6c82be0 100644
@@ -452,7 +452,6 @@ xfs_agfl_read_verify(
 	struct xfs_buf	*bp)
 {
 	struct xfs_mount *mp = bp->b_target->bt_mount;
-	int		agfl_ok = 1;
 
 	/*
 	 * There is no verification of non-crc AGFLs because mkfs does not

@@ -463,14 +462,13 @@ xfs_agfl_read_verify(
 	if (!xfs_sb_version_hascrc(&mp->m_sb))
 		return;
 
-	agfl_ok = xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF);
-
-	agfl_ok = agfl_ok && xfs_agfl_verify(bp);
-
-	if (!agfl_ok) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (!xfs_buf_verify_cksum(bp, XFS_AGFL_CRC_OFF))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_agfl_verify(bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 static void

@@ -485,8 +483,8 @@ xfs_agfl_write_verify(
 		return;
 
 	if (!xfs_agfl_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
 

@@ -2216,18 +2214,17 @@ xfs_agf_read_verify(
 	struct xfs_buf	*bp)
 {
 	struct xfs_mount *mp = bp->b_target->bt_mount;
-	int		agf_ok = 1;
-
-	if (xfs_sb_version_hascrc(&mp->m_sb))
-		agf_ok = xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF);
 
-	agf_ok = agf_ok && xfs_agf_verify(mp, bp);
-
-	if (unlikely(XFS_TEST_ERROR(!agf_ok, mp, XFS_ERRTAG_ALLOC_READ_AGF,
-			XFS_RANDOM_ALLOC_READ_AGF))) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (xfs_sb_version_hascrc(&mp->m_sb) &&
+	    !xfs_buf_verify_cksum(bp, XFS_AGF_CRC_OFF))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (XFS_TEST_ERROR(!xfs_agf_verify(mp, bp), mp,
+				XFS_ERRTAG_ALLOC_READ_AGF,
+				XFS_RANDOM_ALLOC_READ_AGF))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 static void

@@ -2238,8 +2235,8 @@ xfs_agf_write_verify(
 	struct xfs_buf_log_item	*bip = bp->b_fspriv;
 
 	if (!xfs_agf_verify(mp, bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
libxfs/xfs_alloc_btree.c +8 -7
index 5b38c24..215be7e 100644
@@ -337,12 +337,14 @@ static void
 xfs_allocbt_read_verify(
 	struct xfs_buf	*bp)
 {
-	if (!(xfs_btree_sblock_verify_crc(bp) &&
-	      xfs_allocbt_verify(bp))) {
-		trace_xfs_btree_corrupt(bp, _RET_IP_);
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
-				     bp->b_target->bt_mount, bp->b_addr);
+	if (!xfs_btree_sblock_verify_crc(bp))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_allocbt_verify(bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+
+	if (bp->b_error) {
+		trace_xfs_btree_corrupt(bp, _RET_IP_);
+		xfs_verifier_error(bp);
 	}
 }
 

@@ -352,9 +354,8 @@ xfs_allocbt_write_verify(
 {
 	if (!xfs_allocbt_verify(bp)) {
 		trace_xfs_btree_corrupt(bp, _RET_IP_);
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
-				     bp->b_target->bt_mount, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
 	xfs_btree_sblock_calc_crc(bp);
libxfs/xfs_attr_leaf.c +8 -6
index 90937d6..f7f02ae 100644
@@ -187,8 +187,8 @@ xfs_attr3_leaf_write_verify(
 	struct xfs_attr3_leaf_hdr *hdr3 = bp->b_addr;
 
 	if (!xfs_attr3_leaf_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
 

@@ -213,12 +213,14 @@ xfs_attr3_leaf_read_verify(
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF)) ||
-	    !xfs_attr3_leaf_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (xfs_sb_version_hascrc(&mp->m_sb) &&
+	     !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_attr3_leaf_verify(bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 const struct xfs_buf_ops xfs_attr3_leaf_buf_ops = {
libxfs/xfs_attr_remote.c +6 -9
index 59bb12d..5cf5c73 100644
@@ -100,7 +100,6 @@ xfs_attr3_rmt_read_verify(
 	struct xfs_mount *mp = bp->b_target->bt_mount;
 	char		*ptr;
 	int		len;
-	bool		corrupt = false;
 	xfs_daddr_t	bno;
 
 	/* no verification of non-crc buffers */

@@ -115,11 +114,11 @@ xfs_attr3_rmt_read_verify(
 	while (len > 0) {
 		if (!xfs_verify_cksum(ptr, XFS_LBSIZE(mp),
 				      XFS_ATTR3_RMT_CRC_OFF)) {
-			corrupt = true;
+			xfs_buf_ioerror(bp, EFSBADCRC);
 			break;
 		}
 		if (!xfs_attr3_rmt_verify(mp, ptr, XFS_LBSIZE(mp), bno)) {
-			corrupt = true;
+			xfs_buf_ioerror(bp, EFSCORRUPTED);
 			break;
 		}
 		len -= XFS_LBSIZE(mp);

@@ -127,10 +126,9 @@ xfs_attr3_rmt_read_verify(
 		bno += mp->m_bsize;
 	}
 
-	if (corrupt) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
-		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	} else
+	if (bp->b_error)
+		xfs_verifier_error(bp);
+	else
 		ASSERT(len == 0);
 }
 

@@ -155,9 +153,8 @@ xfs_attr3_rmt_write_verify(
 
 	while (len > 0) {
 		if (!xfs_attr3_rmt_verify(mp, ptr, XFS_LBSIZE(mp), bno)) {
-			XFS_CORRUPTION_ERROR(__func__,
-					    XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 			xfs_buf_ioerror(bp, EFSCORRUPTED);
+			xfs_verifier_error(bp);
 			return;
 		}
 		if (bip) {
libxfs/xfs_bmap_btree.c +8 -8
index 3136e4f..a4bd69d 100644
@@ -759,12 +759,14 @@ static void
 xfs_bmbt_read_verify(
 	struct xfs_buf	*bp)
 {
-	if (!(xfs_btree_lblock_verify_crc(bp) &&
-	      xfs_bmbt_verify(bp))) {
-		trace_xfs_btree_corrupt(bp, _RET_IP_);
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
-				     bp->b_target->bt_mount, bp->b_addr);
+	if (!xfs_btree_lblock_verify_crc(bp))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_bmbt_verify(bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+
+	if (bp->b_error) {
+		trace_xfs_btree_corrupt(bp, _RET_IP_);
+		xfs_verifier_error(bp);
 	}
 }
 

@@ -773,11 +775,9 @@ xfs_bmbt_write_verify(
 	struct xfs_buf	*bp)
 {
 	if (!xfs_bmbt_verify(bp)) {
-		xfs_warn(bp->b_target->bt_mount, "bmbt daddr 0x%llx failed", bp->b_bn);
 		trace_xfs_btree_corrupt(bp, _RET_IP_);
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
-				     bp->b_target->bt_mount, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
 	xfs_btree_lblock_calc_crc(bp);
libxfs/xfs_da_btree.c +8 -6
index 0c0c5e0..154adb1 100644
@@ -209,8 +209,8 @@ xfs_da3_node_write_verify(
 	struct xfs_da3_node_hdr *hdr3 = bp->b_addr;
 
 	if (!xfs_da3_node_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
 

@@ -233,17 +233,20 @@ static void
 xfs_da3_node_read_verify(
 	struct xfs_buf		*bp)
 {
-	struct xfs_mount	*mp = bp->b_target->bt_mount;
 	struct xfs_da_blkinfo	*info = bp->b_addr;
 
 	switch (be16_to_cpu(info->magic)) {
 		case XFS_DA3_NODE_MAGIC:
-			if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF))
+			if (!xfs_buf_verify_cksum(bp, XFS_DA3_NODE_CRC_OFF)) {
+				xfs_buf_ioerror(bp, EFSBADCRC);
 				break;
+			}
 			/* fall through */
 		case XFS_DA_NODE_MAGIC:
-			if (!xfs_da3_node_verify(bp))
+			if (!xfs_da3_node_verify(bp)) {
+				xfs_buf_ioerror(bp, EFSCORRUPTED);
 				break;
+			}
 			return;
 		case XFS_ATTR_LEAF_MAGIC:
 		case XFS_ATTR3_LEAF_MAGIC:

@@ -260,8 +263,7 @@ xfs_da3_node_read_verify(
 	}
 
 	/* corrupt block */
-	XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
-	xfs_buf_ioerror(bp, EFSCORRUPTED);
+	xfs_verifier_error(bp);
 }
 
 const struct xfs_buf_ops xfs_da3_node_buf_ops = {
libxfs/xfs_dir2_block.c +8 -6
index 3fe10ea..cede01f 100644
@@ -70,12 +70,14 @@ xfs_dir3_block_read_verify(
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
-	    !xfs_dir3_block_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (xfs_sb_version_hascrc(&mp->m_sb) &&
+	     !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_dir3_block_verify(bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 static void

@@ -87,8 +89,8 @@ xfs_dir3_block_write_verify(
 	struct xfs_dir3_blk_hdr	*hdr3 = bp->b_addr;
 
 	if (!xfs_dir3_block_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
libxfs/xfs_dir2_data.c +9 -8
index bcb4c7f..dc9df4d 100644
@@ -208,7 +208,6 @@ static void
 xfs_dir3_data_reada_verify(
 	struct xfs_buf		*bp)
 {
-	struct xfs_mount	*mp = bp->b_target->bt_mount;
 	struct xfs_dir2_data_hdr *hdr = bp->b_addr;
 
 	switch (hdr->magic) {

@@ -222,8 +221,8 @@ xfs_dir3_data_reada_verify(
 		xfs_dir3_data_verify(bp);
 		return;
 	default:
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, hdr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		break;
 	}
 }

@@ -234,12 +233,14 @@ xfs_dir3_data_read_verify(
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF)) ||
-	    !xfs_dir3_data_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (xfs_sb_version_hascrc(&mp->m_sb) &&
+	     !xfs_buf_verify_cksum(bp, XFS_DIR3_DATA_CRC_OFF))
+		 xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_dir3_data_verify(bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 static void

@@ -251,8 +252,8 @@ xfs_dir3_data_write_verify(
 	struct xfs_dir3_blk_hdr	*hdr3 = bp->b_addr;
 
 	if (!xfs_dir3_data_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
libxfs/xfs_dir2_leaf.c +8 -6
index 710f005..8e0cbc9 100644
@@ -206,12 +206,14 @@ __read_verify(
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF)) ||
-	    !xfs_dir3_leaf_verify(bp, magic)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (xfs_sb_version_hascrc(&mp->m_sb) &&
+	     !xfs_buf_verify_cksum(bp, XFS_DIR3_LEAF_CRC_OFF))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_dir3_leaf_verify(bp, magic))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 static void

@@ -224,8 +226,8 @@ __write_verify(
 	struct xfs_dir3_leaf_hdr *hdr3 = bp->b_addr;
 
 	if (!xfs_dir3_leaf_verify(bp, magic)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
libxfs/xfs_dir2_node.c +8 -6
index f4260ea..3256756 100644
@@ -98,12 +98,14 @@ xfs_dir3_free_read_verify(
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	if ((xfs_sb_version_hascrc(&mp->m_sb) &&
-	     !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF)) ||
-	    !xfs_dir3_free_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (xfs_sb_version_hascrc(&mp->m_sb) &&
+	    !xfs_buf_verify_cksum(bp, XFS_DIR3_FREE_CRC_OFF))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_dir3_free_verify(bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 static void

@@ -115,8 +117,8 @@ xfs_dir3_free_write_verify(
 	struct xfs_dir3_blk_hdr	*hdr3 = bp->b_addr;
 
 	if (!xfs_dir3_free_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
libxfs/xfs_dquot_buf.c +7 -4
index 6bbb0ff..e089ec8 100644
@@ -237,10 +237,13 @@ xfs_dquot_buf_read_verify(
 {
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
-	if (!xfs_dquot_buf_verify_crc(mp, bp) || !xfs_dquot_buf_verify(mp, bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (!xfs_dquot_buf_verify_crc(mp, bp))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_dquot_buf_verify(mp, bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 /*

@@ -255,8 +258,8 @@ xfs_dquot_buf_write_verify(
 	struct xfs_mount	*mp = bp->b_target->bt_mount;
 
 	if (!xfs_dquot_buf_verify(mp, bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
 }
libxfs/xfs_ialloc.c +10 -11
index b1382d6..c19d84a 100644
@@ -1551,18 +1551,17 @@ xfs_agi_read_verify(
 	struct xfs_buf	*bp)
 {
 	struct xfs_mount *mp = bp->b_target->bt_mount;
-	int		agi_ok = 1;
 
-	if (xfs_sb_version_hascrc(&mp->m_sb))
-		agi_ok = xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF);
-
-	agi_ok = agi_ok && xfs_agi_verify(bp);
-
-	if (unlikely(XFS_TEST_ERROR(!agi_ok, mp, XFS_ERRTAG_IALLOC_READ_AGI,
-			XFS_RANDOM_IALLOC_READ_AGI))) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (xfs_sb_version_hascrc(&mp->m_sb) &&
+	    !xfs_buf_verify_cksum(bp, XFS_AGI_CRC_OFF))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (XFS_TEST_ERROR(!xfs_agi_verify(bp), mp,
+				XFS_ERRTAG_IALLOC_READ_AGI,
+				XFS_RANDOM_IALLOC_READ_AGI))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 static void

@@ -1573,8 +1572,8 @@ xfs_agi_write_verify(
 	struct xfs_buf_log_item	*bip = bp->b_fspriv;
 
 	if (!xfs_agi_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
libxfs/xfs_ialloc_btree.c +8 -7
index d9589b7..0a29d73 100644
@@ -224,12 +224,14 @@ static void
 xfs_inobt_read_verify(
 	struct xfs_buf	*bp)
 {
-	if (!(xfs_btree_sblock_verify_crc(bp) &&
-	      xfs_inobt_verify(bp))) {
-		trace_xfs_btree_corrupt(bp, _RET_IP_);
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
-				     bp->b_target->bt_mount, bp->b_addr);
+	if (!xfs_btree_sblock_verify_crc(bp))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_inobt_verify(bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+
+	if (bp->b_error) {
+		trace_xfs_btree_corrupt(bp, _RET_IP_);
+		xfs_verifier_error(bp);
 	}
 }
 

@@ -239,9 +241,8 @@ xfs_inobt_write_verify(
 {
 	if (!xfs_inobt_verify(bp)) {
 		trace_xfs_btree_corrupt(bp, _RET_IP_);
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
-				     bp->b_target->bt_mount, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
 	xfs_btree_sblock_calc_crc(bp);
libxfs/xfs_inode_buf.c +1 -2
index 4f29033..de16ed9 100644
@@ -88,8 +88,7 @@ xfs_inode_buf_verify(
 			}
 
 			xfs_buf_ioerror(bp, EFSCORRUPTED);
-			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_HIGH,
-					     mp, dip);
+			xfs_verifier_error(bp);
 #ifdef DEBUG
 			xfs_alert(mp,
 				"bad inode magic/vsn daddr %lld #%d (magic=%x)",
libxfs/xfs_sb.c +4 -6
index db267c2..7ee4612 100644
@@ -569,7 +569,7 @@ xfs_sb_read_verify(
 			/* Only fail bad secondaries on a known V5 filesystem */
 			if (bp->b_bn == XFS_SB_DADDR ||
 			    xfs_sb_version_hascrc(&mp->m_sb)) {
-				error = EFSCORRUPTED;
+				error = EFSBADCRC;
 				goto out_error;
 			}
 		}

@@ -578,10 +578,9 @@ xfs_sb_read_verify(
 
 out_error:
 	if (error) {
-		if (error == EFSCORRUPTED)
-			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
-					     mp, bp->b_addr);
 		xfs_buf_ioerror(bp, error);
+		if (error == EFSCORRUPTED || error == EFSBADCRC)
+			xfs_verifier_error(bp);
 	}
 }
 

@@ -616,9 +615,8 @@ xfs_sb_write_verify(
 
 	error = xfs_sb_verify(bp, false);
 	if (error) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
-				     mp, bp->b_addr);
 		xfs_buf_ioerror(bp, error);
+		xfs_verifier_error(bp);
 		return;
 	}
libxfs/xfs_symlink_remote.c +7 -5
index b59bf14..ebf60ac 100644
@@ -116,11 +116,13 @@ xfs_symlink_read_verify(
 	if (!xfs_sb_version_hascrc(&mp->m_sb))
 		return;
 
-	if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF) ||
-	    !xfs_symlink_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
+	if (!xfs_buf_verify_cksum(bp, XFS_SYMLINK_CRC_OFF))
+		xfs_buf_ioerror(bp, EFSBADCRC);
+	else if (!xfs_symlink_verify(bp))
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
-	}
+
+	if (bp->b_error)
+		xfs_verifier_error(bp);
 }
 
 static void

@@ -135,8 +137,8 @@ xfs_symlink_write_verify(
 		return;
 
 	if (!xfs_symlink_verify(bp)) {
-		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, bp->b_addr);
 		xfs_buf_ioerror(bp, EFSCORRUPTED);
+		xfs_verifier_error(bp);
 		return;
 	}
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#12 lore raw

PATCH 12/13 xfs_db: Use EFSBADCRC for CRC validity indication

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

xfs_db currently gives indication as to whether a buffer CRC is ok or not. Currently it does this by checking for EFSCORRUPTED in the b_error field of the buffer. Not that we have EFSBADCRC to indicate a bad CRC independently of structure corruption, use that instead to drive the CRC correct/incorrect indication in the structured output.

Signed-off-by Dave Chinner <[email protected]>
---
 db/io.c | 5 +++--
 db/io.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)
db/io.c +3 -2
index d29816c..9a787c8 100644
@@ -521,10 +521,11 @@ set_cur(
 	}
 
 	/*
-	 * keep the buffer even if the verifier says it is corrupted.
+	 * Keep the buffer even if the verifier says it is corrupted.
 	 * We're a diagnostic tool, after all.
 	 */
-	if (!bp || (bp->b_error && bp->b_error != EFSCORRUPTED))
+	if (!bp || (bp->b_error && bp->b_error != EFSCORRUPTED &&
+				   bp->b_error != EFSBADCRC))
 		return;
 	iocur_top->buf = bp->b_addr;
 	iocur_top->bp = bp;
db/io.h +1 -1
index d8cf383..ad39bee 100644
@@ -67,6 +67,6 @@ static inline bool
 iocur_crc_valid()
 {
 	return (iocur_top->bp &&
-		iocur_top->bp->b_error != EFSCORRUPTED &&
+		iocur_top->bp->b_error != EFSBADCRC &&
 		(!iocur_top->ino_buf || iocur_top->ino_crc_ok));
 }
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 12/13
#13 lore raw

Re: [PATCH 12/13] xfs_db: Use EFSBADCRC for CRC validity indication

Quoted 5 lines · Dave Chinner

From: Dave Chinner <[email protected]>

xfs_db currently gives indication as to whether a buffer CRC is ok or not. Currently it does this by checking for EFSCORRUPTED in the b_error field of the buffer. Not that we have EFSBADCRC to indicate

Now that...

Quoted 4 lines

a bad CRC independently of structure corruption, use that instead to drive the CRC correct/incorrect indication in the structured output.

Signed-off-by Dave Chinner <[email protected]>
Reviewed-by Eric Sandeen <[email protected]>
Quoted 29 lines
---
 db/io.c | 5 +++--
 db/io.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)
db/io.c +3 -2
index d29816c..9a787c8 100644
@@ -521,10 +521,11 @@ set_cur(
 	}
 
 	/*
-	 * keep the buffer even if the verifier says it is corrupted.
+	 * Keep the buffer even if the verifier says it is corrupted.
 	 * We're a diagnostic tool, after all.
 	 */
-	if (!bp || (bp->b_error && bp->b_error != EFSCORRUPTED))
+	if (!bp || (bp->b_error && bp->b_error != EFSCORRUPTED &&
+				   bp->b_error != EFSBADCRC))
 		return;
 	iocur_top->buf = bp->b_addr;
 	iocur_top->bp = bp;
db/io.h +0 -0
index d8cf383..ad39bee 100644

db/io.h static inline bool · line 72

Quoted 7 lines
db/io.h +1 -1
@@ -67,6 +67,6 @@ static inline bool
 iocur_crc_valid()
 {
 	return (iocur_top->bp &&
-		iocur_top->bp->b_error != EFSCORRUPTED &&
+		iocur_top->bp->b_error != EFSBADCRC &&
 		(!iocur_top->ino_buf || iocur_top->ino_crc_ok));
 }

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#14 lore raw

PATCH 13/13 repair: phase 1 does not handle superblock CRCs

In reply to Dave Chinner

From: Dave Chinner <[email protected]>

Phase 1 of xfs_repair verifies and corrects the primary superblock of the filesystem. It does not verify that the CRC of the superblock that is found is correct, nor does it recalculate the CRC of the superblock it rewrites.

This happens because phase1 does not use the libxfs buffer cache - it just uses pread/pwrite on a memory buffer, and works directly from that buffer. Hence we need to add CRC verification to verify_sb(), and CRC recalculation to write_primary_sb() so that it works correctly.

This also enables us to use get_sb() as the method of fetching the superblock from disk after phase 1 without needing to use the libxfs buffer cache and guessing at the sector size. This prevents a verifier error because it attempts to CRC a superblock buffer that is much longer than the usual sector sizes.

Signed-off-by Dave Chinner <[email protected]>
---
 repair/agheader.c   |  2 +-
 repair/globals.h    |  3 ++-
 repair/phase1.c     |  5 ++--
 repair/protos.h     |  3 ++-
 repair/sb.c         | 71 +++++++++++++++++++++++++++++------------------------
 repair/xfs_repair.c | 26 +++++++++++---------
 6 files changed, 62 insertions(+), 48 deletions(-)
repair/agheader.c +1 -1
index 53e47b6..fc5dac9 100644
@@ -472,7 +472,7 @@ verify_set_agheader(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
 	int status = XR_OK;
 	int status_sb = XR_OK;
 
-	status = verify_sb(sb, (i == 0));
+	status = verify_sb(sbuf->b_addr, sb, (i == 0));
 
 	if (status != XR_OK)  {
 		do_warn(_("bad on-disk superblock %d - %s\n"),
repair/globals.h +2 -1
index cbb2ce7..f6e0a22 100644
@@ -49,7 +49,8 @@
 #define XR_BAD_SB_UNIT		17	/* bad stripe unit */
 #define XR_BAD_SB_WIDTH		18	/* bad stripe width */
 #define XR_BAD_SVN		19	/* bad shared version number */
-#define XR_BAD_ERR_CODE		20	/* Bad error code */
+#define XR_BAD_CRC		20	/* Bad CRC */
+#define XR_BAD_ERR_CODE		21	/* Bad error code */
 
 /* XFS filesystem (il)legal values */
repair/phase1.c +3 -2
index 62de211..ec75ada 100644
@@ -70,13 +70,14 @@ phase1(xfs_mount_t *mp)
 	ag_bp = alloc_ag_buf(MAX_SECTSIZE);
 	sb = (xfs_sb_t *) ag_bp;
 
-	if (get_sb(sb, 0LL, MAX_SECTSIZE, 0) == XR_EOF)
+	rval = get_sb(sb, 0LL, MAX_SECTSIZE, 0);
+	if (rval == XR_EOF)
 		do_error(_("error reading primary superblock\n"));
 
 	/*
 	 * is this really an sb, verify internal consistency
 	 */
-	if ((rval = verify_sb(sb, 1)) != XR_OK)  {
+	if (rval != XR_OK)  {
 		do_warn(_("bad primary superblock - %s !!!\n"),
 			err_string(rval));
 		if (!find_secondary_sb(sb))
repair/protos.h +2 -1
index 601f2a9..ff42fa7 100644
@@ -18,7 +18,8 @@
 
 void	xfs_init(libxfs_init_t *args);
 
-int	verify_sb(xfs_sb_t		*sb,
+int	verify_sb(char			*sb_buf,
+		xfs_sb_t		*sb,
 		int			is_primary_sb);
 int	verify_set_primary_sb(xfs_sb_t	*root_sb,
 			int		sb_index,
repair/sb.c +39 -32

125 lines not on this page · raw message

repair/xfs_repair.c +15 -11
index 6327076..08b25f0 100644
@@ -137,6 +137,8 @@ err_string(int err_code)
 			_("bad stripe width in superblock");
 		err_message[XR_BAD_SVN] =
 			_("bad shared version number in superblock");
+		err_message[XR_BAD_CRC] =
+			_("bad CRC in superblock");
 		done = 1;
 	}
 

@@ -529,6 +531,8 @@ main(int argc, char **argv)
 	xfs_buf_t	*sbp;
 	xfs_mount_t	xfs_m;
 	char		*msgbuf;
+	struct xfs_sb	psb;
+	int		rval;
 
 	progname = basename(argv[0]);
 	setlocale(LC_ALL, "");

@@ -558,13 +562,12 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
-	/* prepare the mount structure */
-	memset(&xfs_m, 0, sizeof(xfs_mount_t));
-	libxfs_buftarg_init(&xfs_m, x.ddev, x.logdev, x.rtdev);
-	sbp = libxfs_readbuf(xfs_m.m_ddev_targp, XFS_SB_DADDR,
-				1 << (XFS_MAX_SECTORSIZE_LOG - BBSHIFT), 0,
-				&xfs_sb_buf_ops);
-	libxfs_sb_from_disk(&xfs_m.m_sb, XFS_BUF_TO_SBP(sbp));
+	rval = get_sb(&psb, 0, XFS_MAX_SECTORSIZE, 0);
+	if (rval != XR_OK) {
+		do_warn(_("Primary superblock bad after phase 1!\n"
+			  "Exiting now.\n"));
+		exit(1);
+	}
 
 	/*
 	 * if the sector size of the filesystem we are trying to repair is

@@ -583,7 +586,7 @@ main(int argc, char **argv)
 			geom.sectsize = BBSIZE;
 		}
 
-		if (xfs_m.m_sb.sb_sectsize < geom.sectsize) {
+		if (psb.sb_sectsize < geom.sectsize) {
 			long	old_flags;
 
 			old_flags = fcntl(fd, F_GETFL, 0);

@@ -595,7 +598,10 @@ main(int argc, char **argv)
 			}
 		}
 	}
-	mp = libxfs_mount(&xfs_m, &xfs_m.m_sb, x.ddev, x.logdev, x.rtdev, 0);
+
+	/* prepare the mount structure */
+	memset(&xfs_m, 0, sizeof(xfs_mount_t));
+	mp = libxfs_mount(&xfs_m, &psb, x.ddev, x.logdev, x.rtdev, 0);
 
 	if (!mp)  {
 		fprintf(stderr,

@@ -603,8 +609,6 @@ main(int argc, char **argv)
 			progname);
 		exit(1);
 	}
-	libxfs_putbuf(sbp);
-	libxfs_purgebuf(sbp);
 
 	/*
 	 * set XFS-independent status vars from the mount/sb structure
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 13/13
#15 lore raw

Re: [PATCH 13/13] repair: phase 1 does not handle superblock CRCs

Quoted 62 lines · Dave Chinner

From: Dave Chinner <[email protected]>

Phase 1 of xfs_repair verifies and corrects the primary superblock of the filesystem. It does not verify that the CRC of the superblock that is found is correct, nor does it recalculate the CRC of the superblock it rewrites.

This happens because phase1 does not use the libxfs buffer cache - it just uses pread/pwrite on a memory buffer, and works directly from that buffer. Hence we need to add CRC verification to verify_sb(), and CRC recalculation to write_primary_sb() so that it works correctly.

This also enables us to use get_sb() as the method of fetching the superblock from disk after phase 1 without needing to use the libxfs buffer cache and guessing at the sector size. This prevents a verifier error because it attempts to CRC a superblock buffer that is much longer than the usual sector sizes.

Signed-off-by Dave Chinner <[email protected]>
---
 repair/agheader.c   |  2 +-
 repair/globals.h    |  3 ++-
 repair/phase1.c     |  5 ++--
 repair/protos.h     |  3 ++-
 repair/sb.c         | 71 +++++++++++++++++++++++++++++------------------------
 repair/xfs_repair.c | 26 +++++++++++---------
 6 files changed, 62 insertions(+), 48 deletions(-)
repair/agheader.c +1 -1
index 53e47b6..fc5dac9 100644
@@ -472,7 +472,7 @@ verify_set_agheader(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
 	int status = XR_OK;
 	int status_sb = XR_OK;
 
-	status = verify_sb(sb, (i == 0));
+	status = verify_sb(sbuf->b_addr, sb, (i == 0));
 
 	if (status != XR_OK)  {
 		do_warn(_("bad on-disk superblock %d - %s\n"),
repair/globals.h +2 -1
index cbb2ce7..f6e0a22 100644
@@ -49,7 +49,8 @@
 #define XR_BAD_SB_UNIT		17	/* bad stripe unit */
 #define XR_BAD_SB_WIDTH		18	/* bad stripe width */
 #define XR_BAD_SVN		19	/* bad shared version number */
-#define XR_BAD_ERR_CODE		20	/* Bad error code */
+#define XR_BAD_CRC		20	/* Bad CRC */
+#define XR_BAD_ERR_CODE		21	/* Bad error code */
 
 /* XFS filesystem (il)legal values */
repair/phase1.c +0 -0
index 62de211..ec75ada 100644
@@ -70,13 +70,14 @@ phase1(xfs_mount_t *mp)
 	ag_bp = alloc_ag_buf(MAX_SECTSIZE);

repair/phase1.c phase1(xfs_mount_t *mp) · line 79

Quoted 10 lines
repair/phase1.c +2 -1
 	sb = (xfs_sb_t *) ag_bp;
 
-	if (get_sb(sb, 0LL, MAX_SECTSIZE, 0) == XR_EOF)
+	rval = get_sb(sb, 0LL, MAX_SECTSIZE, 0);
+	if (rval == XR_EOF)
 		do_error(_("error reading primary superblock\n"));
 
 	/*
 	 * is this really an sb, verify internal consistency
 	 */

This comment can probably go away now. Otherwise, looks good...

Reviewed-by Brian Foster <[email protected]>
Quoted 235 lines
repair/phase1.c +1 -1
-	if ((rval = verify_sb(sb, 1)) != XR_OK)  {
+	if (rval != XR_OK)  {
 		do_warn(_("bad primary superblock - %s !!!\n"),
 			err_string(rval));
 		if (!find_secondary_sb(sb))
repair/protos.h +2 -1
index 601f2a9..ff42fa7 100644
@@ -18,7 +18,8 @@
 
 void	xfs_init(libxfs_init_t *args);
 
-int	verify_sb(xfs_sb_t		*sb,
+int	verify_sb(char			*sb_buf,
+		xfs_sb_t		*sb,
 		int			is_primary_sb);
 int	verify_set_primary_sb(xfs_sb_t	*root_sb,
 			int		sb_index,
repair/sb.c +39 -32
index c54d89b..b111aca 100644
@@ -139,7 +139,7 @@ find_secondary_sb(xfs_sb_t *rsb)
 			c_bufsb = (char *)sb + i;
 			libxfs_sb_from_disk(&bufsb, (xfs_dsb_t *)c_bufsb);
 
-			if (verify_sb(&bufsb, 0) != XR_OK)
+			if (verify_sb(c_bufsb, &bufsb, 0) != XR_OK)
 				continue;
 
 			do_warn(_("found candidate secondary superblock...\n"));

@@ -245,7 +245,7 @@ sb_validate_ino_align(struct xfs_sb *sb)
  */
 
 int
-verify_sb(xfs_sb_t *sb, int is_primary_sb)
+verify_sb(char *sb_buf, xfs_sb_t *sb, int is_primary_sb)
 {
 	__uint32_t	bsize;
 	int		i;

@@ -263,8 +263,34 @@ verify_sb(xfs_sb_t *sb, int is_primary_sb)
 	if (is_primary_sb && sb->sb_inprogress == 1)
 		return(XR_BAD_INPROGRESS);
 
-	/* check to make sure blocksize is legal 2^N, 9 <= N <= 16 */
+	/*
+	 * before going *any further*, validate the sector size and if the
+	 * version says we should have CRCs enabled, validate that.
+	 */
+
+	/* check to make sure sectorsize is legal 2^N, 9 <= N <= 15 */
+	if (sb->sb_sectsize == 0)
+		return(XR_BAD_SECT_SIZE_DATA);
+
+	bsize = 1;
+	for (i = 0; bsize < sb->sb_sectsize &&
+		i < sizeof(sb->sb_sectsize) * NBBY; i++)  {
+		bsize <<= 1;
+	}
+
+	if (i < XFS_MIN_SECTORSIZE_LOG || i > XFS_MAX_SECTORSIZE_LOG)
+		return(XR_BAD_SECT_SIZE_DATA);
+
+	/* check sb sectorsize field against sb sectlog field */
+	if (i != sb->sb_sectlog)
+		return(XR_BAD_SECT_SIZE_DATA);
+
+	/* sector size in range - CRC check time */
+	if (xfs_sb_version_hascrc(sb) &&
+	    !xfs_verify_cksum(sb_buf, sb->sb_sectsize, XFS_SB_CRC_OFF))
+		return XR_BAD_CRC;
 
+	/* check to make sure blocksize is legal 2^N, 9 <= N <= 16 */
 	if (sb->sb_blocksize == 0)
 		return(XR_BAD_BLOCKSIZE);
 

@@ -300,26 +326,6 @@ verify_sb(xfs_sb_t *sb, int is_primary_sb)
 		sb->sb_inopblock != howmany(sb->sb_blocksize,sb->sb_inodesize))
 		return(XR_BAD_INO_SIZE_DATA);
 
-	/* check to make sure sectorsize is legal 2^N, 9 <= N <= 15 */
-
-	if (sb->sb_sectsize == 0)
-		return(XR_BAD_SECT_SIZE_DATA);
-
-	bsize = 1;
-
-	for (i = 0; bsize < sb->sb_sectsize &&
-		i < sizeof(sb->sb_sectsize) * NBBY; i++)  {
-		bsize <<= 1;
-	}
-
-	if (i < XFS_MIN_SECTORSIZE_LOG || i > XFS_MAX_SECTORSIZE_LOG)
-		return(XR_BAD_SECT_SIZE_DATA);
-
-	/* check sb sectorsize field against sb sectlog field */
-
-	if (i != sb->sb_sectlog)
-		return(XR_BAD_SECT_SIZE_DATA);
-
 	if (xfs_sb_version_hassector(sb))  {
 
 		/* check to make sure log sector is legal 2^N, 9 <= N <= 15 */

@@ -482,9 +488,11 @@ write_primary_sb(xfs_sb_t *sbp, int size)
 		do_error(_("couldn't seek to offset 0 in filesystem\n"));
 	}
 
-	
 	libxfs_sb_to_disk(buf, sbp, XFS_SB_ALL_BITS);
 
+	if (xfs_sb_version_hascrc(sbp))
+		xfs_update_cksum((char *)buf, size, XFS_SB_CRC_OFF);
+
 	if (write(x.dfd, buf, size) != size) {
 		free(buf);
 		do_error(_("primary superblock write failed!\n"));

@@ -494,7 +502,7 @@ write_primary_sb(xfs_sb_t *sbp, int size)
 }
 
 /*
- * get a possible superblock -- don't check for internal consistency
+ * get a possible superblock -- checks for internal consistency
  */
 int
 get_sb(xfs_sb_t *sbp, xfs_off_t off, int size, xfs_agnumber_t agno)

@@ -529,9 +537,10 @@ get_sb(xfs_sb_t *sbp, xfs_off_t off, int size, xfs_agnumber_t agno)
 		do_error("%s\n", strerror(error));
 	}
 	libxfs_sb_from_disk(sbp, buf);
-	free(buf);
 
-	return (verify_sb(sbp, 0));
+	rval = verify_sb((char *)buf, sbp, agno == 0);
+	free(buf);
+	return rval;
 }
 
 /* returns element on list with highest reference count */

@@ -745,13 +754,11 @@ verify_set_primary_sb(xfs_sb_t		*rsb,
 			off = (xfs_off_t)agno * rsb->sb_agblocks << rsb->sb_blocklog;
 
 			checked[agno] = 1;
-
-			if (get_sb(sb, off, size, agno) == XR_EOF)  {
-				retval = XR_EOF;
+			retval = get_sb(sb, off, size, agno);
+			if (retval == XR_EOF)
 				goto out_free_list;
-			}
 
-			if (verify_sb(sb, 0) == XR_OK)  {
+			if (retval == XR_OK) {
 				/*
 				 * save away geometry info.
 				 * don't bother checking the sb
repair/xfs_repair.c +15 -11
index 6327076..08b25f0 100644
@@ -137,6 +137,8 @@ err_string(int err_code)
 			_("bad stripe width in superblock");
 		err_message[XR_BAD_SVN] =
 			_("bad shared version number in superblock");
+		err_message[XR_BAD_CRC] =
+			_("bad CRC in superblock");
 		done = 1;
 	}
 

@@ -529,6 +531,8 @@ main(int argc, char **argv)
 	xfs_buf_t	*sbp;
 	xfs_mount_t	xfs_m;
 	char		*msgbuf;
+	struct xfs_sb	psb;
+	int		rval;
 
 	progname = basename(argv[0]);
 	setlocale(LC_ALL, "");

@@ -558,13 +562,12 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
-	/* prepare the mount structure */
-	memset(&xfs_m, 0, sizeof(xfs_mount_t));
-	libxfs_buftarg_init(&xfs_m, x.ddev, x.logdev, x.rtdev);
-	sbp = libxfs_readbuf(xfs_m.m_ddev_targp, XFS_SB_DADDR,
-				1 << (XFS_MAX_SECTORSIZE_LOG - BBSHIFT), 0,
-				&xfs_sb_buf_ops);
-	libxfs_sb_from_disk(&xfs_m.m_sb, XFS_BUF_TO_SBP(sbp));
+	rval = get_sb(&psb, 0, XFS_MAX_SECTORSIZE, 0);
+	if (rval != XR_OK) {
+		do_warn(_("Primary superblock bad after phase 1!\n"
+			  "Exiting now.\n"));
+		exit(1);
+	}
 
 	/*
 	 * if the sector size of the filesystem we are trying to repair is

@@ -583,7 +586,7 @@ main(int argc, char **argv)
 			geom.sectsize = BBSIZE;
 		}
 
-		if (xfs_m.m_sb.sb_sectsize < geom.sectsize) {
+		if (psb.sb_sectsize < geom.sectsize) {
 			long	old_flags;
 
 			old_flags = fcntl(fd, F_GETFL, 0);

@@ -595,7 +598,10 @@ main(int argc, char **argv)
 			}
 		}
 	}
-	mp = libxfs_mount(&xfs_m, &xfs_m.m_sb, x.ddev, x.logdev, x.rtdev, 0);
+
+	/* prepare the mount structure */
+	memset(&xfs_m, 0, sizeof(xfs_mount_t));
+	mp = libxfs_mount(&xfs_m, &psb, x.ddev, x.logdev, x.rtdev, 0);
 
 	if (!mp)  {
 		fprintf(stderr,

@@ -603,8 +609,6 @@ main(int argc, char **argv)
 			progname);
 		exit(1);
 	}
-	libxfs_putbuf(sbp);
-	libxfs_purgebuf(sbp);
 
 	/*
 	 * set XFS-independent status vars from the mount/sb structure
Signature · 6 lines
1.9.0

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#16 lore raw

Re: [PATCH 13/13] repair: phase 1 does not handle superblock CRCs

Quoted 70 lines · Brian Foster
Quoted 69 lines · Dave Chinner

From: Dave Chinner <[email protected]>

Phase 1 of xfs_repair verifies and corrects the primary superblock of the filesystem. It does not verify that the CRC of the superblock that is found is correct, nor does it recalculate the CRC of the superblock it rewrites.

This happens because phase1 does not use the libxfs buffer cache - it just uses pread/pwrite on a memory buffer, and works directly from that buffer. Hence we need to add CRC verification to verify_sb(), and CRC recalculation to write_primary_sb() so that it works correctly.

This also enables us to use get_sb() as the method of fetching the superblock from disk after phase 1 without needing to use the libxfs buffer cache and guessing at the sector size. This prevents a verifier error because it attempts to CRC a superblock buffer that is much longer than the usual sector sizes.

Signed-off-by Dave Chinner <[email protected]>
---
 repair/agheader.c   |  2 +-
 repair/globals.h    |  3 ++-
 repair/phase1.c     |  5 ++--
 repair/protos.h     |  3 ++-
 repair/sb.c         | 71 +++++++++++++++++++++++++++++------------------------
 repair/xfs_repair.c | 26 +++++++++++---------
 6 files changed, 62 insertions(+), 48 deletions(-)
repair/agheader.c +1 -1
index 53e47b6..fc5dac9 100644
@@ -472,7 +472,7 @@ verify_set_agheader(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
 	int status = XR_OK;
 	int status_sb = XR_OK;
 
-	status = verify_sb(sb, (i == 0));
+	status = verify_sb(sbuf->b_addr, sb, (i == 0));
 
 	if (status != XR_OK)  {
 		do_warn(_("bad on-disk superblock %d - %s\n"),
repair/globals.h +2 -1
index cbb2ce7..f6e0a22 100644
@@ -49,7 +49,8 @@
 #define XR_BAD_SB_UNIT		17	/* bad stripe unit */
 #define XR_BAD_SB_WIDTH		18	/* bad stripe width */
 #define XR_BAD_SVN		19	/* bad shared version number */
-#define XR_BAD_ERR_CODE		20	/* Bad error code */
+#define XR_BAD_CRC		20	/* Bad CRC */
+#define XR_BAD_ERR_CODE		21	/* Bad error code */
 
 /* XFS filesystem (il)legal values */
repair/phase1.c +2 -1
index 62de211..ec75ada 100644
@@ -70,13 +70,14 @@ phase1(xfs_mount_t *mp)
 	ag_bp = alloc_ag_buf(MAX_SECTSIZE);
 	sb = (xfs_sb_t *) ag_bp;
 
-	if (get_sb(sb, 0LL, MAX_SECTSIZE, 0) == XR_EOF)
+	rval = get_sb(sb, 0LL, MAX_SECTSIZE, 0);
+	if (rval == XR_EOF)
 		do_error(_("error reading primary superblock\n"));
Quoted 7 lines

repair/phase1.c phase1(xfs_mount_t *mp) · line 79

Quoted 3 lines
repair/phase1.c +0 -0
 	/*
 	 * is this really an sb, verify internal consistency
 	 */

This comment can probably go away now. Otherwise, looks good...

Reviewed-by Brian Foster <[email protected]>

Ok, thanks. I'll kill the comment in a followup patch that touches this code again...

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on the v1 series
#17 lore raw
In reply to Dave Chinner
Quoted 11 lines · Dave Chinner

Hi Folks,

This series is a userspace port of Eric's EFSBADCRC verifier error discrimination patch set. It brings across all the relevant pathes form the kernel code and shoe-horns them into the libxfs codebase. There are two extra patches on the end of the series - one to make xfs_db use EFSBADCRC for it's CRC validity indication, and the other to make xfs_repair detect and handle primary superblock CRC failures and recalculation when the primary superblock is rebuilt and rewritten.

Quoted 4 lines

This is the first step in bring full awareness of CRC errors into xfs_repair. More patches will follow as I make more repair code aware that EFSBADCRC means that the object that was just read needs repair.

You can slap:

Reviewed-by Eric Sandeen <[email protected]>

on the first 11 of these anyway.

(or maybe we should swap SOB/RB, and add a new Userspace-ported-by ;))

-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#19 lore raw
Quoted 4 lines · Christoph Hellwig

Seems like xfstests will need some updates after this series to ignore

Metadata corruption detected at block XXX/XXX

Yes, I'm seeing that from 2 tests - xfs/030 and xfs/033. I've got a patch in my local xfstests tree that handles this.

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs