Re: [PATCH 1/2] xfs_db: fix inode CRC validity state, and warn on read if invalid

[Dave Chinner <david@fromorbit.com>]

On Sat, Sep 06, 2014 at 12:46:07PM -0500, Eric Sandeen wrote:
> Currently, the "ino_crc_ok" field on the io cursor
> reflects overall inode validity, not CRC correctness.
> Because it is only used when printing CRC validity,
> change it to reflect only that.
> 
> In addition, when reading an inode, print the current
> CRC state.
> 
> Note, if specifying an inode which doesn't actually
> exist, this will claim corruption; I'm not sure if
> that's good or bad.  Today, it already issues corruption
> errors on the way; this adds a new message as well ;)
> 
> xfs_db> inode 129
> Metadata corruption detected at block 0x80/0x2000
> Metadata corruption detected at block 0x80/0x2000
> ...
> Metadata CRC error detected for ino 129
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/db/inode.c b/db/inode.c
> index 24170ba..244d03b 100644
> --- a/db/inode.c
> +++ b/db/inode.c
> @@ -684,13 +684,18 @@ set_cur_inode(
>  		numblks, DB_RING_IGN, NULL);
>  	off_cur(offset << mp->m_sb.sb_inodelog, mp->m_sb.sb_inodesize);
>  	dip = iocur_top->data;
> -	iocur_top->ino_crc_ok = libxfs_dinode_verify(mp, ino, dip);
> +	iocur_top->ino_crc_ok = xfs_verify_cksum((char *)dip,
> +						 mp->m_sb.sb_inodesize,
> +						 XFS_DINODE_CRC_OFF);

that needs to be "libxfs_verify_cksum".

>  	iocur_top->ino_buf = 1;
>  	iocur_top->ino = ino;
>  	iocur_top->mode = be16_to_cpu(dip->di_mode);
>  	if ((iocur_top->mode & S_IFMT) == S_IFDIR)
>  		iocur_top->dirino = ino;
> +	if (xfs_sb_version_hascrc(&mp->m_sb) && !iocur_top->ino_crc_ok)
> +		dbprintf(_("Metadata CRC error detected for ino %lld\n"), ino);
> +

and we probably shoul dbe looking at making all those sb version
checks "libxfs_sb_version_has...." as well.

>  	/* track updated info in ring */
>  	ring_add();
>  }
> diff --git a/db/io.c b/db/io.c
> index 7f1b76a..93ebf5c 100644
> --- a/db/io.c
> +++ b/db/io.c
> @@ -473,6 +473,17 @@ write_cur(void)
>  		write_cur_bbs();
>  	else
>  		write_cur_buf();
> +
> +	if (iocur_top->ino_buf) {
> +		xfs_dinode_t    *dip;
> +		xfs_ino_t       ino;
> +
> +		dip = iocur_top->data;
> +		ino = iocur_top->ino;
> +		iocur_top->ino_crc_ok = xfs_verify_cksum((char *)dip,
> +						 mp->m_sb.sb_inodesize,
> +						 XFS_DINODE_CRC_OFF);
> +	}

Needs a comment explaining why we are reverifying the inode crc.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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