* [PATCH] metadump: don't verify metadata being dumped @ 2014-02-28 0:53 Dave Chinner 2014-02-28 1:43 ` Eric Sandeen 2014-02-28 2:51 ` [PATCH, V2] " Dave Chinner 0 siblings, 2 replies; 6+ messages in thread From: Dave Chinner @ 2014-02-28 0:53 UTC (permalink / raw) To: xfs 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] metadump: don't verify metadata being dumped 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 2014-02-28 2:51 ` [PATCH, V2] " Dave Chinner 1 sibling, 1 reply; 6+ messages in thread From: Eric Sandeen @ 2014-02-28 1:43 UTC (permalink / raw) To: Dave Chinner, xfs 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? Maybe this patch stands ok on its own but it seems like there's more work to do. :) -Eric > 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); > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] metadump: don't verify metadata being dumped 2014-02-28 1:43 ` Eric Sandeen @ 2014-02-28 1:57 ` Dave Chinner 0 siblings, 0 replies; 6+ messages in thread From: Dave Chinner @ 2014-02-28 1:57 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH, V2] metadump: don't verify metadata being dumped 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 2:51 ` Dave Chinner 2014-02-28 4:06 ` Eric Sandeen 1 sibling, 1 reply; 6+ messages in thread From: Dave Chinner @ 2014-02-28 2:51 UTC (permalink / raw) To: xfs metadump: don't verify metadata being dumped 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 - when the metadata is unmodified is the wrong thing to be doing. And stopping the dump when we come across an error is even worse. We still need to do CRC recalculation when obfuscating names and attributes. Hence we need to make running the verifier conditional on the buffer or inode: a) being uncorrupted when read, and b) modified by the obfuscation code. If either of these conditions is not true, then we don't run the verifier or recalculate the CRCs. Signed-off-by: Dave Chinner <dchinner@redhat.com> --- V2: run verifiers on buffers and inodes modified by obfuscation, but only if they are not corrupt before obfuscation. Thanks, Eric! db/io.h | 1 + db/metadump.c | 57 ++++++++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/db/io.h b/db/io.h index 4f24c83..d8cf383 100644 --- a/db/io.h +++ b/db/io.h @@ -41,6 +41,7 @@ typedef struct iocur { int ino_crc_ok:1; int ino_buf:1; int dquot_buf:1; + int need_crc:1; } iocur_t; #define DB_RING_ADD 1 /* add to ring on set_cur */ diff --git a/db/metadump.c b/db/metadump.c index 5baf83d..c829726 100644 --- a/db/metadump.c +++ b/db/metadump.c @@ -190,26 +190,36 @@ 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. + * + * The only reason for running the verifier is to recalculate the CRCs on a + * buffer that has been obfuscated. i.e. a buffer than metadump modified itself. + * In this case, we only run the verifier if the buffer was not corrupt to begin + * with so that we don't accidentally correct buffers with CRC or errors in them + * when we are obfuscating them. + */ static int write_buf( iocur_t *buf) { + struct xfs_buf *bp = buf->bp; int i; int ret; /* * Run the write verifier to recalculate the buffer CRCs and check - * we are writing something valid to disk + * metadump didn't introduce a new corruption. Warn if the verifier + * failed, but still continue to dump it into the output file. */ - 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; + if (buf->need_crc && bp && bp->b_ops && !bp->b_error) { + bp->b_ops->verify_write(bp); + if (bp->b_error) { + print_warning( + "obfuscation corrupted block at bno 0x%llx/0x%x", + (long long)bp->b_bn, bp->b_bcount); } } @@ -1374,6 +1384,7 @@ process_single_fsb_objects( o++; dp += mp->m_sb.sb_blocksize; } + iocur_top->need_crc = 1; ret = write_buf(iocur_top); out_pop: @@ -1442,6 +1453,7 @@ process_multi_fsb_objects( obfuscate_dir_data_block(iocur_top->data, o, last == mp->m_dirblkfsbs); + iocur_top->need_crc = 1; ret = write_buf(iocur_top); out_pop: pop_cur(); @@ -1722,6 +1734,13 @@ process_inode_data( return 1; } +/* + * when we process the inode, we may change the data in the data and/or + * attribute fork if they are in short form and we are obfuscating names. + * In this case we need to recalculate the CRC of the inode, but we should + * only do that if the CRC in the inode is good to begin with. If the crc + * is not ok, we just leave it alone. + */ static int process_inode( xfs_agnumber_t agno, @@ -1729,17 +1748,28 @@ process_inode( xfs_dinode_t *dip) { int success; + bool crc_ok = false; /* don't recalc by default */ + bool need_crc = false; success = 1; cur_ino = XFS_AGINO_TO_INO(mp, agno, agino); + /* we only care about crc recalculation if we are obfuscating names. */ + if (!dont_obfuscate) + crc_ok = xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize, + offsetof(struct xfs_dinode, di_crc)); + /* copy appropriate data fork metadata */ switch (be16_to_cpu(dip->di_mode) & S_IFMT) { case S_IFDIR: success = process_inode_data(dip, TYP_DIR2); + if (dip->di_format == XFS_DINODE_FMT_LOCAL) + need_crc = 1; break; case S_IFLNK: success = process_inode_data(dip, TYP_SYMLINK); + if (dip->di_format == XFS_DINODE_FMT_LOCAL) + need_crc = 1; break; case S_IFREG: success = process_inode_data(dip, TYP_DATA); @@ -1754,6 +1784,7 @@ process_inode( attr_data.remote_val_count = 0; switch (dip->di_aformat) { case XFS_DINODE_FMT_LOCAL: + need_crc = 1; if (!dont_obfuscate) obfuscate_sf_attr(dip); break; @@ -1768,6 +1799,9 @@ process_inode( } nametable_clear(); } + + if (crc_ok && need_crc) + xfs_dinode_calc_crc(mp, dip); return success; } @@ -1838,9 +1872,6 @@ 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)) _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH, V2] metadump: don't verify metadata being dumped 2014-02-28 2:51 ` [PATCH, V2] " Dave Chinner @ 2014-02-28 4:06 ` Eric Sandeen 2014-02-28 4:46 ` Dave Chinner 0 siblings, 1 reply; 6+ messages in thread From: Eric Sandeen @ 2014-02-28 4:06 UTC (permalink / raw) To: Dave Chinner, xfs On 2/27/14, 8:51 PM, Dave Chinner wrote: > metadump: don't verify metadata being dumped Not a complete summary anymore... > 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 wheny ou type too fasty ou end up with thes ame sort of typosa sIdo. ;) > 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 - when the metadata is > unmodified is the wrong thing to be doing. And stopping the dump > when we come across an error is even worse. > > We still need to do CRC recalculation when obfuscating names and > attributes. Hence we need to make running the verifier conditional > on the buffer or inode: > a) being uncorrupted when read, and > b) modified by the obfuscation code. > > If either of these conditions is not true, then we don't run the > verifier or recalculate the CRCs. I think this looks mostly ok; small questions & nitpicks below. > Signed-off-by: Dave Chinner <dchinner@redhat.com> > --- > V2: run verifiers on buffers and inodes modified by obfuscation, but > only if they are not corrupt before obfuscation. Thanks, Eric! > > db/io.h | 1 + > db/metadump.c | 57 ++++++++++++++++++++++++++++++++++++++++++++------------- > 2 files changed, 45 insertions(+), 13 deletions(-) > > diff --git a/db/io.h b/db/io.h > index 4f24c83..d8cf383 100644 > --- a/db/io.h > +++ b/db/io.h > @@ -41,6 +41,7 @@ typedef struct iocur { > int ino_crc_ok:1; > int ino_buf:1; > int dquot_buf:1; > + int need_crc:1; > } iocur_t; > > #define DB_RING_ADD 1 /* add to ring on set_cur */ > diff --git a/db/metadump.c b/db/metadump.c > index 5baf83d..c829726 100644 > --- a/db/metadump.c > +++ b/db/metadump.c > @@ -190,26 +190,36 @@ 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. > + * > + * The only reason for running the verifier is to recalculate the CRCs on a > + * buffer that has been obfuscated. i.e. a buffer than metadump modified itself. > + * In this case, we only run the verifier if the buffer was not corrupt to begin > + * with so that we don't accidentally correct buffers with CRC or errors in them > + * when we are obfuscating them. > + */ > static int > write_buf( > iocur_t *buf) > { > + struct xfs_buf *bp = buf->bp; > int i; > int ret; > > /* > * Run the write verifier to recalculate the buffer CRCs and check > - * we are writing something valid to disk > + * metadump didn't introduce a new corruption. Warn if the verifier > + * failed, but still continue to dump it into the output file. > */ > - 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; > + if (buf->need_crc && bp && bp->b_ops && !bp->b_error) { > + bp->b_ops->verify_write(bp); > + if (bp->b_error) { > + print_warning( > + "obfuscation corrupted block at bno 0x%llx/0x%x", > + (long long)bp->b_bn, bp->b_bcount); > } > } > > @@ -1374,6 +1384,7 @@ process_single_fsb_objects( > o++; > dp += mp->m_sb.sb_blocksize; > } > + iocur_top->need_crc = 1; in the default: case we don't obfuscate. Should it still get need_crc? > ret = write_buf(iocur_top); > > out_pop: > @@ -1442,6 +1453,7 @@ process_multi_fsb_objects( > > obfuscate_dir_data_block(iocur_top->data, o, > last == mp->m_dirblkfsbs); > + iocur_top->need_crc = 1; > ret = write_buf(iocur_top); > out_pop: > pop_cur(); > @@ -1722,6 +1734,13 @@ process_inode_data( > return 1; > } > > +/* > + * when we process the inode, we may change the data in the data and/or > + * attribute fork if they are in short form and we are obfuscating names. > + * In this case we need to recalculate the CRC of the inode, but we should > + * only do that if the CRC in the inode is good to begin with. If the crc > + * is not ok, we just leave it alone. > + */ > static int > process_inode( > xfs_agnumber_t agno, > @@ -1729,17 +1748,28 @@ process_inode( > xfs_dinode_t *dip) > { > int success; > + bool crc_ok = false; /* don't recalc by default */ > + bool need_crc = false; I might do + bool crc_was_ok = false; /* don't recalc by default */ + bool need_new_crc = false; for clarity...? > success = 1; > cur_ino = XFS_AGINO_TO_INO(mp, agno, agino); > > + /* we only care about crc recalculation if we are obfuscating names. */ > + if (!dont_obfuscate) > + crc_ok = xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize, > + offsetof(struct xfs_dinode, di_crc)); > + > /* copy appropriate data fork metadata */ > switch (be16_to_cpu(dip->di_mode) & S_IFMT) { > case S_IFDIR: > success = process_inode_data(dip, TYP_DIR2); > + if (dip->di_format == XFS_DINODE_FMT_LOCAL) > + need_crc = 1; I wish this were closer to the point of obfuscation, but oh well. > break; > case S_IFLNK: > success = process_inode_data(dip, TYP_SYMLINK); > + if (dip->di_format == XFS_DINODE_FMT_LOCAL) > + need_crc = 1; > break; > case S_IFREG: > success = process_inode_data(dip, TYP_DATA); > @@ -1754,6 +1784,7 @@ process_inode( > attr_data.remote_val_count = 0; > switch (dip->di_aformat) { > case XFS_DINODE_FMT_LOCAL: > + need_crc = 1; > if (!dont_obfuscate) > obfuscate_sf_attr(dip); > break; > @@ -1768,6 +1799,9 @@ process_inode( > } > nametable_clear(); > } > + > + if (crc_ok && need_crc) > + xfs_dinode_calc_crc(mp, dip); > return success; > } > > @@ -1838,9 +1872,6 @@ 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)) > > _______________________________________________ > 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] 6+ messages in thread
* Re: [PATCH, V2] metadump: don't verify metadata being dumped 2014-02-28 4:06 ` Eric Sandeen @ 2014-02-28 4:46 ` Dave Chinner 0 siblings, 0 replies; 6+ messages in thread From: Dave Chinner @ 2014-02-28 4:46 UTC (permalink / raw) To: Eric Sandeen; +Cc: xfs On Thu, Feb 27, 2014 at 10:06:05PM -0600, Eric Sandeen wrote: > On 2/27/14, 8:51 PM, Dave Chinner wrote: > > @@ -1374,6 +1384,7 @@ process_single_fsb_objects( > > o++; > > dp += mp->m_sb.sb_blocksize; > > } > > + iocur_top->need_crc = 1; > > in the > > default: > > case we don't obfuscate. Should it still get need_crc? Ah, right. I looked at the "dont_obfuscate" jump, not the switch statement. Will fix. > > +/* > > + * when we process the inode, we may change the data in the data and/or > > + * attribute fork if they are in short form and we are obfuscating names. > > + * In this case we need to recalculate the CRC of the inode, but we should > > + * only do that if the CRC in the inode is good to begin with. If the crc > > + * is not ok, we just leave it alone. > > + */ > > static int > > process_inode( > > xfs_agnumber_t agno, > > @@ -1729,17 +1748,28 @@ process_inode( > > xfs_dinode_t *dip) > > { > > int success; > > + bool crc_ok = false; /* don't recalc by default */ > > + bool need_crc = false; > > I might do > > + bool crc_was_ok = false; /* don't recalc by default */ > + bool need_new_crc = false; > > for clarity...? *nod* > > success = 1; > > cur_ino = XFS_AGINO_TO_INO(mp, agno, agino); > > > > + /* we only care about crc recalculation if we are obfuscating names. */ > > + if (!dont_obfuscate) > > + crc_ok = xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize, > > + offsetof(struct xfs_dinode, di_crc)); > > + > > /* copy appropriate data fork metadata */ > > switch (be16_to_cpu(dip->di_mode) & S_IFMT) { > > case S_IFDIR: > > success = process_inode_data(dip, TYP_DIR2); > > + if (dip->di_format == XFS_DINODE_FMT_LOCAL) > > + need_crc = 1; > > I wish this were closer to the point of obfuscation, but oh well. It means carrying a tristate or extra varaible through the stack, which doesn't make it any simpler to understand because we have to do the CRC here once we know we've finished all the modifications to the inode.... > > > break; > > case S_IFLNK: > > success = process_inode_data(dip, TYP_SYMLINK); > > + if (dip->di_format == XFS_DINODE_FMT_LOCAL) > > + need_crc = 1; > > break; > > case S_IFREG: > > success = process_inode_data(dip, TYP_DATA); > > @@ -1754,6 +1784,7 @@ process_inode( > > attr_data.remote_val_count = 0; > > switch (dip->di_aformat) { > > case XFS_DINODE_FMT_LOCAL: > > + need_crc = 1; > > if (!dont_obfuscate) > > obfuscate_sf_attr(dip); And this one kind of forces us to do it here if we want to only have one place where we recalc CRCs... 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] 6+ messages in thread
end of thread, other threads:[~2014-02-28 4:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 2014-02-28 2:51 ` [PATCH, V2] " Dave Chinner 2014-02-28 4:06 ` Eric Sandeen 2014-02-28 4:46 ` Dave Chinner
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.