xfs
[Top] [All Lists]

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

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH 1/4] xfs_repair: validate & fix inode CRCs
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Fri, 13 Mar 2015 09:20:06 -0400
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <54F771DE.40107@xxxxxxxxxxx>
References: <54F77199.8030708@xxxxxxxxxx> <54F771DE.40107@xxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
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@xxxxxxxxxx>
> ---
> 
> 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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>