xfs
[Top] [All Lists]

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

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH 1/2] xfs_db: fix inode CRC validity state, and warn on read if invalid
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sun, 7 Sep 2014 09:00:08 +1000
Cc: xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <540B485F.8070502@xxxxxxxxxxx>
References: <540B4399.4020804@xxxxxxxxxxx> <540B485F.8070502@xxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxx>
> ---
> 
> 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@xxxxxxxxxxxxx

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