Re: [PATCH, V2] metadump: don't verify metadata being dumped

[Dave Chinner <david@fromorbit.com>]

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