All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: xfs@oss.sgi.com
Subject: [PATCH] metadump: don't verify metadata being dumped
Date: Fri, 28 Feb 2014 11:53:45 +1100	[thread overview]
Message-ID: <1393548825-16499-1-git-send-email-david@fromorbit.com> (raw)

From: Dave Chinner <dchinner@redhat.com>

The discontiguous buffer support series added a verifier check on
the metadata buffers before they go written to the metadump image.
If this failed, it returned an error, and the restul woul dbe that
we stopped processing the metadata and exited, resulting in a
truncated dump.

xfs_metadump is supposed to dump the metadata in the filesystem
for forensic analysis purposes, which means we actually want it to
retain any corruptions it finds in the filesystem. Hence running the
verifier - even to recalculate CRCs - is the wrong thing to be
doing. And stopping the dum pwhen we come across an error is even
worse.

Therefore remove the code tha truns the verifier and causes all
these problems and replace it with a comment explaining why we don't
want to run verifiers in the metadump process.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 db/metadump.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/db/metadump.c b/db/metadump.c
index 5baf83d..c10e76a 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -190,6 +190,13 @@ write_buf_segment(
 	return 0;
 }
 
+/*
+ * we want to preserve the state of the metadata in the dump - whether it is
+ * intact or corrupt, so even if the buffer has a verifier attached to it we
+ * don't want to run it prior to writing the buffer to the metadump image.
+ * Even just recalculating the CRCs is the wrong thing to do here as it can
+ * hide errors that only the CRCs were detecting.
+ */
 static int
 write_buf(
 	iocur_t		*buf)
@@ -197,22 +204,6 @@ write_buf(
 	int		i;
 	int		ret;
 
-	/*
-	 * Run the write verifier to recalculate the buffer CRCs and check
-	 * we are writing something valid to disk
-	 */
-	if (buf->bp && buf->bp->b_ops) {
-		buf->bp->b_error = 0;
-		buf->bp->b_ops->verify_write(buf->bp);
-		if (buf->bp->b_error) {
-			fprintf(stderr,
-	_("%s: write verifer failed on bno 0x%llx/0x%x\n"),
-				__func__, (long long)buf->bp->b_bn,
-				buf->bp->b_bcount);
-			return -buf->bp->b_error;
-		}
-	}
-
 	/* handle discontiguous buffers */
 	if (!buf->bbmap) {
 		ret = write_buf_segment(buf->data, buf->bb, buf->blen);
-- 
1.9.0

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

             reply	other threads:[~2014-02-28  0:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-28  0:53 Dave Chinner [this message]
2014-02-28  1:43 ` [PATCH] metadump: don't verify metadata being dumped Eric Sandeen
2014-02-28  1:57   ` Dave Chinner
2014-02-28  2:51 ` [PATCH, V2] " Dave Chinner
2014-02-28  4:06   ` Eric Sandeen
2014-02-28  4:46     ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1393548825-16499-1-git-send-email-david@fromorbit.com \
    --to=david@fromorbit.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.