linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] xfsprogs: initial EFSBADCRC support
@ 2014-03-04  8:51 Dave Chinner
  2014-03-04  8:51 ` [PATCH 01/13] libxfs: be more forgiving of a v4 secondary sb w/ junk in v5 fields Dave Chinner
                   ` (14 more replies)
  0 siblings, 15 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH 01/13] libxfs: be more forgiving of a v4 secondary sb w/ junk in v5 fields
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 02/13] libxfs: sanitize sb_inopblock in xfs_mount_validate_sb Dave Chinner
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 libxfs/xfs_sb.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index 48b1a97..16088a6 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -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);
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 02/13] libxfs: sanitize sb_inopblock in xfs_mount_validate_sb
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
  2014-03-04  8:51 ` [PATCH 01/13] libxfs: be more forgiving of a v4 secondary sb w/ junk in v5 fields Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 03/13] libxfs: xfs_sb_read_verify() doesn't flag bad crcs on primary sb Dave Chinner
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 libxfs/xfs_sb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index 16088a6..4be7366 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -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)	||
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 03/13] libxfs: xfs_sb_read_verify() doesn't flag bad crcs on primary sb
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
  2014-03-04  8:51 ` [PATCH 01/13] libxfs: be more forgiving of a v4 secondary sb w/ junk in v5 fields Dave Chinner
  2014-03-04  8:51 ` [PATCH 02/13] libxfs: sanitize sb_inopblock in xfs_mount_validate_sb Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 04/13] libxfs: skip verification on initial "guess" superblock read Dave Chinner
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 libxfs/xfs_sb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index 4be7366..5c10c97 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -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;
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 04/13] libxfs: skip verification on initial "guess" superblock read
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (2 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 03/13] libxfs: xfs_sb_read_verify() doesn't flag bad crcs on primary sb Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 05/13] libxfs: limit superblock corruption errors to actual corruption Dave Chinner
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 libxfs/xfs_sb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index 5c10c97..6ad99a5 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -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);
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 05/13] libxfs: limit superblock corruption errors to actual corruption
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (3 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 04/13] libxfs: skip verification on initial "guess" superblock read Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 06/13] libxfs: skip pointless CRC updates after verifier failures Dave Chinner
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 libxfs/xfs_sb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index 6ad99a5..05bc860 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -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);
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 06/13] libxfs: skip pointless CRC updates after verifier failures
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (4 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 05/13] libxfs: limit superblock corruption errors to actual corruption Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 07/13] libxfs: Use defines for CRC offsets in all cases Dave Chinner
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 libxfs/xfs_alloc_btree.c  | 1 +
 libxfs/xfs_ialloc_btree.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c
index 282a320..5b38c24 100644
--- a/libxfs/xfs_alloc_btree.c
+++ b/libxfs/xfs_alloc_btree.c
@@ -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);
 
diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
index 27a5dd9..d9589b7 100644
--- a/libxfs/xfs_ialloc_btree.c
+++ b/libxfs/xfs_ialloc_btree.c
@@ -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);
 
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 07/13] libxfs: Use defines for CRC offsets in all cases
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (5 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 06/13] libxfs: skip pointless CRC updates after verifier failures Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 08/13] libxfs: add helper for verifying checksums on xfs_bufs Dave Chinner
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 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(-)

diff --git a/include/xfs_ag.h b/include/xfs_ag.h
index 3fc1098..0fdd410 100644
--- a/include/xfs_ag.h
+++ b/include/xfs_ag.h
@@ -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
  */
diff --git a/include/xfs_dinode.h b/include/xfs_dinode.h
index e5869b5..623bbe8 100644
--- a/include/xfs_dinode.h
+++ b/include/xfs_dinode.h
@@ -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
 
 /*
diff --git a/include/xfs_format.h b/include/xfs_format.h
index 997c770..77f6b8b 100644
--- a/include/xfs_format.h
+++ b/include/xfs_format.h
@@ -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
diff --git a/include/xfs_sb.h b/include/xfs_sb.h
index 35061d4..f7b2fe7 100644
--- a/include/xfs_sb.h
+++ b/include/xfs_sb.h
@@ -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.
diff --git a/libxfs/xfs_alloc.c b/libxfs/xfs_alloc.c
index e4fb1ad..8a6725e 100644
--- a/libxfs/xfs_alloc.c
+++ b/libxfs/xfs_alloc.c
@@ -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 = {
diff --git a/libxfs/xfs_ialloc.c b/libxfs/xfs_ialloc.c
index afe1a82..b83ad98 100644
--- a/libxfs/xfs_ialloc.c
+++ b/libxfs/xfs_ialloc.c
@@ -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 = {
diff --git a/libxfs/xfs_inode_buf.c b/libxfs/xfs_inode_buf.c
index d245d72..4f29033 100644
--- a/libxfs/xfs_inode_buf.c
+++ b/libxfs/xfs_inode_buf.c
@@ -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);
 }
 
diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index 05bc860..bbcf886 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -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 = {
diff --git a/libxfs/xfs_symlink_remote.c b/libxfs/xfs_symlink_remote.c
index 539db0c..09ee9d4 100644
--- a/libxfs/xfs_symlink_remote.c
+++ b/libxfs/xfs_symlink_remote.c
@@ -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 = {
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 08/13] libxfs: add helper for verifying checksums on xfs_bufs
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (6 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 07/13] libxfs: Use defines for CRC offsets in all cases Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 09/13] libxfs: add helper for updating " Dave Chinner
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 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(-)

diff --git a/include/libxfs.h b/include/libxfs.h
index f688598..610e7d5 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -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)
diff --git a/libxfs/xfs_alloc.c b/libxfs/xfs_alloc.c
index 8a6725e..2f6a241 100644
--- a/libxfs/xfs_alloc.c
+++ b/libxfs/xfs_alloc.c
@@ -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);
 
diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
index fd52397..0bd3169 100644
--- a/libxfs/xfs_attr_leaf.c
+++ b/libxfs/xfs_attr_leaf.c
@@ -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);
diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
index 2dd6fb7..fffed0a 100644
--- a/libxfs/xfs_btree.c
+++ b/libxfs/xfs_btree.c
@@ -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;
 }
 
diff --git a/libxfs/xfs_da_btree.c b/libxfs/xfs_da_btree.c
index 53414f5..4e8b41a 100644
--- a/libxfs/xfs_da_btree.c
+++ b/libxfs/xfs_da_btree.c
@@ -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:
diff --git a/libxfs/xfs_dir2_block.c b/libxfs/xfs_dir2_block.c
index 1d8f598..13561c3 100644
--- a/libxfs/xfs_dir2_block.c
+++ b/libxfs/xfs_dir2_block.c
@@ -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);
diff --git a/libxfs/xfs_dir2_data.c b/libxfs/xfs_dir2_data.c
index 189699f..2a0de28 100644
--- a/libxfs/xfs_dir2_data.c
+++ b/libxfs/xfs_dir2_data.c
@@ -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);
diff --git a/libxfs/xfs_dir2_leaf.c b/libxfs/xfs_dir2_leaf.c
index 683536e..0fa147f 100644
--- a/libxfs/xfs_dir2_leaf.c
+++ b/libxfs/xfs_dir2_leaf.c
@@ -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);
diff --git a/libxfs/xfs_dir2_node.c b/libxfs/xfs_dir2_node.c
index ced8c58..78c77ed 100644
--- a/libxfs/xfs_dir2_node.c
+++ b/libxfs/xfs_dir2_node.c
@@ -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);
diff --git a/libxfs/xfs_ialloc.c b/libxfs/xfs_ialloc.c
index b83ad98..21683b5 100644
--- a/libxfs/xfs_ialloc.c
+++ b/libxfs/xfs_ialloc.c
@@ -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,
diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index bbcf886..43fdb59 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -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)) {
diff --git a/libxfs/xfs_symlink_remote.c b/libxfs/xfs_symlink_remote.c
index 09ee9d4..716f8a3 100644
--- a/libxfs/xfs_symlink_remote.c
+++ b/libxfs/xfs_symlink_remote.c
@@ -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);
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 09/13] libxfs: add helper for updating checksums on xfs_bufs
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (7 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 08/13] libxfs: add helper for verifying checksums on xfs_bufs Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 10/13] libxfs: add xfs_verifier_error() Dave Chinner
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 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(-)

diff --git a/include/libxfs.h b/include/libxfs.h
index 610e7d5..7500903 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -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)
diff --git a/libxfs/xfs_alloc.c b/libxfs/xfs_alloc.c
index 2f6a241..dca612b 100644
--- a/libxfs/xfs_alloc.c
+++ b/libxfs/xfs_alloc.c
@@ -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 = {
diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
index 0bd3169..90937d6 100644
--- a/libxfs/xfs_attr_leaf.c
+++ b/libxfs/xfs_attr_leaf.c
@@ -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);
 }
 
 /*
diff --git a/libxfs/xfs_btree.c b/libxfs/xfs_btree.c
index fffed0a..9be4abd 100644
--- a/libxfs/xfs_btree.c
+++ b/libxfs/xfs_btree.c
@@ -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
diff --git a/libxfs/xfs_da_btree.c b/libxfs/xfs_da_btree.c
index 4e8b41a..0c0c5e0 100644
--- a/libxfs/xfs_da_btree.c
+++ b/libxfs/xfs_da_btree.c
@@ -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);
 }
 
 /*
diff --git a/libxfs/xfs_dir2_block.c b/libxfs/xfs_dir2_block.c
index 13561c3..3fe10ea 100644
--- a/libxfs/xfs_dir2_block.c
+++ b/libxfs/xfs_dir2_block.c
@@ -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 = {
diff --git a/libxfs/xfs_dir2_data.c b/libxfs/xfs_dir2_data.c
index 2a0de28..bcb4c7f 100644
--- a/libxfs/xfs_dir2_data.c
+++ b/libxfs/xfs_dir2_data.c
@@ -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 = {
diff --git a/libxfs/xfs_dir2_leaf.c b/libxfs/xfs_dir2_leaf.c
index 0fa147f..710f005 100644
--- a/libxfs/xfs_dir2_leaf.c
+++ b/libxfs/xfs_dir2_leaf.c
@@ -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
diff --git a/libxfs/xfs_dir2_node.c b/libxfs/xfs_dir2_node.c
index 78c77ed..f4260ea 100644
--- a/libxfs/xfs_dir2_node.c
+++ b/libxfs/xfs_dir2_node.c
@@ -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 = {
diff --git a/libxfs/xfs_ialloc.c b/libxfs/xfs_ialloc.c
index 21683b5..b1382d6 100644
--- a/libxfs/xfs_ialloc.c
+++ b/libxfs/xfs_ialloc.c
@@ -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 = {
diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index 43fdb59..db267c2 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -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 = {
diff --git a/libxfs/xfs_symlink_remote.c b/libxfs/xfs_symlink_remote.c
index 716f8a3..b59bf14 100644
--- a/libxfs/xfs_symlink_remote.c
+++ b/libxfs/xfs_symlink_remote.c
@@ -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 = {
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 10/13] libxfs: add xfs_verifier_error()
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (8 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 09/13] libxfs: add helper for updating " Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 11/13] libxfs: modify verifiers to differentiate CRC from other errors Dave Chinner
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 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(+)

diff --git a/include/darwin.h b/include/darwin.h
index 97b8990..95f865b 100644
--- a/include/darwin.h
+++ b/include/darwin.h
@@ -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
diff --git a/include/freebsd.h b/include/freebsd.h
index 2e1ae49..b51688b 100644
--- a/include/freebsd.h
+++ b/include/freebsd.h
@@ -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;
diff --git a/include/gnukfreebsd.h b/include/gnukfreebsd.h
index 1ec291f..2140acd 100644
--- a/include/gnukfreebsd.h
+++ b/include/gnukfreebsd.h
@@ -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;
diff --git a/include/irix.h b/include/irix.h
index a450684..5040451 100644
--- a/include/irix.h
+++ b/include/irix.h
@@ -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;
diff --git a/include/linux.h b/include/linux.h
index 502fd1f..5586290 100644
--- a/include/linux.h
+++ b/include/linux.h
@@ -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;
diff --git a/libxfs/util.c b/libxfs/util.c
index 8109ab3..1b05540 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -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));
+}
diff --git a/libxfs/xfs.h b/libxfs/xfs.h
index 364fd83..5a21590 100644
--- a/libxfs/xfs.h
+++ b/libxfs/xfs.h
@@ -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);
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 11/13] libxfs: modify verifiers to differentiate CRC from other errors
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (9 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 10/13] libxfs: add xfs_verifier_error() Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-04  8:51 ` [PATCH 12/13] xfs_db: Use EFSBADCRC for CRC validity indication Dave Chinner
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

[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 <dchinner@redhat.com>
---
 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(-)

diff --git a/include/libxfs.h b/include/libxfs.h
index 7500903..6bc6c94 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -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))
diff --git a/libxfs/xfs_alloc.c b/libxfs/xfs_alloc.c
index dca612b..6c82be0 100644
--- a/libxfs/xfs_alloc.c
+++ b/libxfs/xfs_alloc.c
@@ -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;
 	}
 
diff --git a/libxfs/xfs_alloc_btree.c b/libxfs/xfs_alloc_btree.c
index 5b38c24..215be7e 100644
--- a/libxfs/xfs_alloc_btree.c
+++ b/libxfs/xfs_alloc_btree.c
@@ -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);
diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
index 90937d6..f7f02ae 100644
--- a/libxfs/xfs_attr_leaf.c
+++ b/libxfs/xfs_attr_leaf.c
@@ -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 = {
diff --git a/libxfs/xfs_attr_remote.c b/libxfs/xfs_attr_remote.c
index 59bb12d..5cf5c73 100644
--- a/libxfs/xfs_attr_remote.c
+++ b/libxfs/xfs_attr_remote.c
@@ -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) {
diff --git a/libxfs/xfs_bmap_btree.c b/libxfs/xfs_bmap_btree.c
index 3136e4f..a4bd69d 100644
--- a/libxfs/xfs_bmap_btree.c
+++ b/libxfs/xfs_bmap_btree.c
@@ -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);
diff --git a/libxfs/xfs_da_btree.c b/libxfs/xfs_da_btree.c
index 0c0c5e0..154adb1 100644
--- a/libxfs/xfs_da_btree.c
+++ b/libxfs/xfs_da_btree.c
@@ -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 = {
diff --git a/libxfs/xfs_dir2_block.c b/libxfs/xfs_dir2_block.c
index 3fe10ea..cede01f 100644
--- a/libxfs/xfs_dir2_block.c
+++ b/libxfs/xfs_dir2_block.c
@@ -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;
 	}
 
diff --git a/libxfs/xfs_dir2_data.c b/libxfs/xfs_dir2_data.c
index bcb4c7f..dc9df4d 100644
--- a/libxfs/xfs_dir2_data.c
+++ b/libxfs/xfs_dir2_data.c
@@ -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;
 	}
 
diff --git a/libxfs/xfs_dir2_leaf.c b/libxfs/xfs_dir2_leaf.c
index 710f005..8e0cbc9 100644
--- a/libxfs/xfs_dir2_leaf.c
+++ b/libxfs/xfs_dir2_leaf.c
@@ -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;
 	}
 
diff --git a/libxfs/xfs_dir2_node.c b/libxfs/xfs_dir2_node.c
index f4260ea..3256756 100644
--- a/libxfs/xfs_dir2_node.c
+++ b/libxfs/xfs_dir2_node.c
@@ -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;
 	}
 
diff --git a/libxfs/xfs_dquot_buf.c b/libxfs/xfs_dquot_buf.c
index 6bbb0ff..e089ec8 100644
--- a/libxfs/xfs_dquot_buf.c
+++ b/libxfs/xfs_dquot_buf.c
@@ -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;
 	}
 }
diff --git a/libxfs/xfs_ialloc.c b/libxfs/xfs_ialloc.c
index b1382d6..c19d84a 100644
--- a/libxfs/xfs_ialloc.c
+++ b/libxfs/xfs_ialloc.c
@@ -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;
 	}
 
diff --git a/libxfs/xfs_ialloc_btree.c b/libxfs/xfs_ialloc_btree.c
index d9589b7..0a29d73 100644
--- a/libxfs/xfs_ialloc_btree.c
+++ b/libxfs/xfs_ialloc_btree.c
@@ -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);
diff --git a/libxfs/xfs_inode_buf.c b/libxfs/xfs_inode_buf.c
index 4f29033..de16ed9 100644
--- a/libxfs/xfs_inode_buf.c
+++ b/libxfs/xfs_inode_buf.c
@@ -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)",
diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index db267c2..7ee4612 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -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;
 	}
 
diff --git a/libxfs/xfs_symlink_remote.c b/libxfs/xfs_symlink_remote.c
index b59bf14..ebf60ac 100644
--- a/libxfs/xfs_symlink_remote.c
+++ b/libxfs/xfs_symlink_remote.c
@@ -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;
 	}
 
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 12/13] xfs_db: Use EFSBADCRC for CRC validity indication
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (10 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 11/13] libxfs: modify verifiers to differentiate CRC from other errors Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-05  0:12   ` Eric Sandeen
  2014-03-04  8:51 ` [PATCH 13/13] repair: phase 1 does not handle superblock CRCs Dave Chinner
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

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 <dchinner@redhat.com>
---
 db/io.c | 5 +++--
 db/io.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/db/io.c b/db/io.c
index d29816c..9a787c8 100644
--- a/db/io.c
+++ b/db/io.c
@@ -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;
diff --git a/db/io.h b/db/io.h
index d8cf383..ad39bee 100644
--- a/db/io.h
+++ b/db/io.h
@@ -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));
 }
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 13/13] repair: phase 1 does not handle superblock CRCs
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (11 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 12/13] xfs_db: Use EFSBADCRC for CRC validity indication Dave Chinner
@ 2014-03-04  8:51 ` Dave Chinner
  2014-03-06 16:01   ` Brian Foster
  2014-03-05  0:04 ` [PATCH 00/13] xfsprogs: initial EFSBADCRC support Eric Sandeen
  2014-03-07 10:46 ` Christoph Hellwig
  14 siblings, 1 reply; 20+ messages in thread
From: Dave Chinner @ 2014-03-04  8:51 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

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 <dchinner@redhat.com>
---
 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(-)

diff --git a/repair/agheader.c b/repair/agheader.c
index 53e47b6..fc5dac9 100644
--- a/repair/agheader.c
+++ b/repair/agheader.c
@@ -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"),
diff --git a/repair/globals.h b/repair/globals.h
index cbb2ce7..f6e0a22 100644
--- a/repair/globals.h
+++ b/repair/globals.h
@@ -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 */
 
diff --git a/repair/phase1.c b/repair/phase1.c
index 62de211..ec75ada 100644
--- a/repair/phase1.c
+++ b/repair/phase1.c
@@ -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))
diff --git a/repair/protos.h b/repair/protos.h
index 601f2a9..ff42fa7 100644
--- a/repair/protos.h
+++ b/repair/protos.h
@@ -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,
diff --git a/repair/sb.c b/repair/sb.c
index c54d89b..b111aca 100644
--- a/repair/sb.c
+++ b/repair/sb.c
@@ -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
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index 6327076..08b25f0 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -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
-- 
1.9.0

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH 00/13] xfsprogs: initial EFSBADCRC support
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (12 preceding siblings ...)
  2014-03-04  8:51 ` [PATCH 13/13] repair: phase 1 does not handle superblock CRCs Dave Chinner
@ 2014-03-05  0:04 ` Eric Sandeen
  2014-03-07 10:46 ` Christoph Hellwig
  14 siblings, 0 replies; 20+ messages in thread
From: Eric Sandeen @ 2014-03-05  0:04 UTC (permalink / raw)
  To: Dave Chinner, xfs

On 3/4/14, 2:51 AM, Dave Chinner wrote:
> 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.

You can slap:

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 12/13] xfs_db: Use EFSBADCRC for CRC validity indication
  2014-03-04  8:51 ` [PATCH 12/13] xfs_db: Use EFSBADCRC for CRC validity indication Dave Chinner
@ 2014-03-05  0:12   ` Eric Sandeen
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Sandeen @ 2014-03-05  0:12 UTC (permalink / raw)
  To: Dave Chinner, xfs

On 3/4/14, 2:51 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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...

> 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 <dchinner@redhat.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  db/io.c | 5 +++--
>  db/io.h | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/db/io.c b/db/io.c
> index d29816c..9a787c8 100644
> --- a/db/io.c
> +++ b/db/io.c
> @@ -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;
> diff --git a/db/io.h b/db/io.h
> index d8cf383..ad39bee 100644
> --- a/db/io.h
> +++ b/db/io.h
> @@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 13/13] repair: phase 1 does not handle superblock CRCs
  2014-03-04  8:51 ` [PATCH 13/13] repair: phase 1 does not handle superblock CRCs Dave Chinner
@ 2014-03-06 16:01   ` Brian Foster
  2014-03-06 23:22     ` Dave Chinner
  0 siblings, 1 reply; 20+ messages in thread
From: Brian Foster @ 2014-03-06 16:01 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On Tue, Mar 04, 2014 at 07:51:57PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> 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 <dchinner@redhat.com>
> ---
>  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(-)
> 
> diff --git a/repair/agheader.c b/repair/agheader.c
> index 53e47b6..fc5dac9 100644
> --- a/repair/agheader.c
> +++ b/repair/agheader.c
> @@ -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"),
> diff --git a/repair/globals.h b/repair/globals.h
> index cbb2ce7..f6e0a22 100644
> --- a/repair/globals.h
> +++ b/repair/globals.h
> @@ -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 */
>  
> diff --git a/repair/phase1.c b/repair/phase1.c
> index 62de211..ec75ada 100644
> --- a/repair/phase1.c
> +++ b/repair/phase1.c
> @@ -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
>  	 */

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

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

> -	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))
> diff --git a/repair/protos.h b/repair/protos.h
> index 601f2a9..ff42fa7 100644
> --- a/repair/protos.h
> +++ b/repair/protos.h
> @@ -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,
> diff --git a/repair/sb.c b/repair/sb.c
> index c54d89b..b111aca 100644
> --- a/repair/sb.c
> +++ b/repair/sb.c
> @@ -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
> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> index 6327076..08b25f0 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -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
> -- 
> 1.9.0
> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 13/13] repair: phase 1 does not handle superblock CRCs
  2014-03-06 16:01   ` Brian Foster
@ 2014-03-06 23:22     ` Dave Chinner
  0 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-06 23:22 UTC (permalink / raw)
  To: Brian Foster; +Cc: xfs

On Thu, Mar 06, 2014 at 11:01:55AM -0500, Brian Foster wrote:
> On Tue, Mar 04, 2014 at 07:51:57PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > 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 <dchinner@redhat.com>
> > ---
> >  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(-)
> > 
> > diff --git a/repair/agheader.c b/repair/agheader.c
> > index 53e47b6..fc5dac9 100644
> > --- a/repair/agheader.c
> > +++ b/repair/agheader.c
> > @@ -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"),
> > diff --git a/repair/globals.h b/repair/globals.h
> > index cbb2ce7..f6e0a22 100644
> > --- a/repair/globals.h
> > +++ b/repair/globals.h
> > @@ -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 */
> >  
> > diff --git a/repair/phase1.c b/repair/phase1.c
> > index 62de211..ec75ada 100644
> > --- a/repair/phase1.c
> > +++ b/repair/phase1.c
> > @@ -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
> >  	 */
> 
> This comment can probably go away now. Otherwise, looks good...
> 
> Reviewed-by: Brian Foster <bfoster@redhat.com>

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

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 00/13] xfsprogs: initial EFSBADCRC support
  2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
                   ` (13 preceding siblings ...)
  2014-03-05  0:04 ` [PATCH 00/13] xfsprogs: initial EFSBADCRC support Eric Sandeen
@ 2014-03-07 10:46 ` Christoph Hellwig
  2014-03-07 22:50   ` Dave Chinner
  14 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2014-03-07 10:46 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

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

Metadata corruption detected at block XXX/XXX

lines from repair in various tests.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 00/13] xfsprogs: initial EFSBADCRC support
  2014-03-07 10:46 ` Christoph Hellwig
@ 2014-03-07 22:50   ` Dave Chinner
  0 siblings, 0 replies; 20+ messages in thread
From: Dave Chinner @ 2014-03-07 22:50 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Fri, Mar 07, 2014 at 02:46:04AM -0800, Christoph Hellwig wrote:
> 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.
-- 
Dave Chinner
david@fromorbit.com

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

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2014-03-07 22:50 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-04  8:51 [PATCH 00/13] xfsprogs: initial EFSBADCRC support Dave Chinner
2014-03-04  8:51 ` [PATCH 01/13] libxfs: be more forgiving of a v4 secondary sb w/ junk in v5 fields Dave Chinner
2014-03-04  8:51 ` [PATCH 02/13] libxfs: sanitize sb_inopblock in xfs_mount_validate_sb Dave Chinner
2014-03-04  8:51 ` [PATCH 03/13] libxfs: xfs_sb_read_verify() doesn't flag bad crcs on primary sb Dave Chinner
2014-03-04  8:51 ` [PATCH 04/13] libxfs: skip verification on initial "guess" superblock read Dave Chinner
2014-03-04  8:51 ` [PATCH 05/13] libxfs: limit superblock corruption errors to actual corruption Dave Chinner
2014-03-04  8:51 ` [PATCH 06/13] libxfs: skip pointless CRC updates after verifier failures Dave Chinner
2014-03-04  8:51 ` [PATCH 07/13] libxfs: Use defines for CRC offsets in all cases Dave Chinner
2014-03-04  8:51 ` [PATCH 08/13] libxfs: add helper for verifying checksums on xfs_bufs Dave Chinner
2014-03-04  8:51 ` [PATCH 09/13] libxfs: add helper for updating " Dave Chinner
2014-03-04  8:51 ` [PATCH 10/13] libxfs: add xfs_verifier_error() Dave Chinner
2014-03-04  8:51 ` [PATCH 11/13] libxfs: modify verifiers to differentiate CRC from other errors Dave Chinner
2014-03-04  8:51 ` [PATCH 12/13] xfs_db: Use EFSBADCRC for CRC validity indication Dave Chinner
2014-03-05  0:12   ` Eric Sandeen
2014-03-04  8:51 ` [PATCH 13/13] repair: phase 1 does not handle superblock CRCs Dave Chinner
2014-03-06 16:01   ` Brian Foster
2014-03-06 23:22     ` Dave Chinner
2014-03-05  0:04 ` [PATCH 00/13] xfsprogs: initial EFSBADCRC support Eric Sandeen
2014-03-07 10:46 ` Christoph Hellwig
2014-03-07 22:50   ` Dave Chinner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).