From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] metadump: don't verify metadata being dumped
Date: Fri, 28 Feb 2014 12:57:01 +1100 [thread overview]
Message-ID: <20140228015701.GI30131@dastard> (raw)
In-Reply-To: <530FE9CD.4000701@sandeen.net>
On Thu, Feb 27, 2014 at 07:43:41PM -0600, Eric Sandeen wrote:
> On 2/27/14, 6:53 PM, Dave Chinner wrote:
> > 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.
>
> This leaves the net functional change from
> 8ab75c db: enable metadump on CRC filesystems
> as:
>
> @@ -1727,6 +1743,9 @@ copy_inode_chunk(
>
> if (!process_inode(agno, agino + i, dip))
> goto pop_out;
> +
> + /* calculate the new CRC for the inode */
> + xfs_dinode_calc_crc(mp, dip);
> }
> skip_processing:
> if (!write_buf(iocur_top))
>
> which seems a) minimal, but also b) like we shouldn't be recalculating
> CRCs if the point is to copy out existing fs state...?
>
> OTOH if we're obfuscating, we would HAVE to recalculate CRCs,
> but then would lose the info that the CRC was bad before.
>
> So probably should skip CRC recalculating if the original CRC
> was bad, in the obfuscating case?
Yeah, you are right - I wasn't thinking of the obfuscation case
changing the metadata....
> Maybe this patch stands ok on its own but it seems like there's
> more work to do. :)
I'll rework it.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2014-02-28 1:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-28 0:53 [PATCH] metadump: don't verify metadata being dumped Dave Chinner
2014-02-28 1:43 ` Eric Sandeen
2014-02-28 1:57 ` Dave Chinner [this message]
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=20140228015701.GI30131@dastard \
--to=david@fromorbit.com \
--cc=sandeen@sandeen.net \
--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.