Re: [PATCH 1/4] xfs_repair: validate & fix inode CRCs

[Brian Foster <bfoster@redhat.com>]

On Wed, Mar 04, 2015 at 02:58:06PM -0600, Eric Sandeen wrote:
> xfs_repair doesn't ever check an inode's CRC, so it never repairs
> them.  If the root inode or realtime inodes have bad crcs, the
> fs won't even mount and can't be fixed (without using xfs_db).
> 
> It's fairly straightforward to just test the inode CRC before
> we do any other checking or modification of the inode, once we
> get past the "verify only" phase of process_dinode_int();
> just mark it dirty if it's wrong and needs to be re-written.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> Resend of patch from a while ago, but with CRC verification
> now earlier in the function, prior to any changes are made.
> 
> diff --git a/repair/dinode.c b/repair/dinode.c
> index 5d9094b..384e2dd 100644
> --- a/repair/dinode.c
> +++ b/repair/dinode.c
> @@ -2291,6 +2291,27 @@ process_dinode_int(xfs_mount_t *mp,
>  	 */
>  	ASSERT(uncertain == 0 || verify_mode != 0);
>  
> +	/*
> +	 * We'd really like to know if the CRC is bad before we
> +	 * go fixing anything; that way we have some hint about
> +	 * bit-rot vs bugs.  Also, any changes will invalidate the
> +	 * existing CRC, so this is the only valid point to test it.
> +	 *
> +	 * Of course if we make any modifications after this, the
> +	 * inode gets rewritten, and CRC is updated automagically.
> +	 */
> +	if (!verify_mode && xfs_sb_version_hascrc(&mp->m_sb)) {

What about verify_mode? It should probably report the inode as bunk if
the crc is bad. Some of the subsequent checks handle this with some
logic to just return 1 if something is bogus and we're in verify mode.

> +		if(!xfs_verify_cksum((char *)dino, mp->m_sb.sb_inodesize,
> +                		XFS_DINODE_CRC_OFF)) {

I got a whitespace damage warning here:

/.../xfsprogs-dev/.git/rebase-apply/patch:25: space before tab in indent.
                		XFS_DINODE_CRC_OFF)) {
warning: 1 line adds whitespace errors.

Brian

> +			do_warn(_("bad CRC for inode %" PRIu64), lino);
> +			if (!no_modify) {
> +				do_warn(_(", will rewrite\n"));
> +				*dirty = 1;
> +			} else
> +				do_warn(_(", would rewrite\n"));
> +		}
> +	}
> +
>  	if (be16_to_cpu(dino->di_magic) != XFS_DINODE_MAGIC)  {
>  		retval = 1;
>  		if (!uncertain)
> 
> _______________________________________________
> 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