Hi Dave,
On Wed, Apr 03, 2013 at 04:11:29PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Add buffer types to the buffer log items so that log recovery can
> validate the buffers and calculate CRCs correctly after the buffers
> are recovered.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Comments below.
> diff --git a/fs/xfs/xfs_da_btree.c b/fs/xfs/xfs_da_btree.c
> index 2b7c9f6..a78865e 100644
> --- a/fs/xfs/xfs_da_btree.c
> +++ b/fs/xfs/xfs_da_btree.c
> @@ -292,7 +292,6 @@ const struct xfs_buf_ops xfs_da3_node_buf_ops = {
> .verify_write = xfs_da3_node_write_verify,
> };
>
> -
> int
> xfs_da3_node_read(
> struct xfs_trans *tp,
> @@ -302,8 +301,35 @@ xfs_da3_node_read(
> struct xfs_buf **bpp,
> int which_fork)
> {
> - return xfs_da_read_buf(tp, dp, bno, mappedbno, bpp,
> + int err;
> +
> + err = xfs_da_read_buf(tp, dp, bno, mappedbno, bpp,
> which_fork, &xfs_da3_node_buf_ops);
> + if (!err && tp) {
> + struct xfs_da_blkinfo *info = (*bpp)->b_addr;
> + int type;
> +
> + switch (be16_to_cpu(info->magic)) {
> + case XFS_DA3_NODE_MAGIC:
> + case XFS_DA_NODE_MAGIC:
Nit:
case XFS_DA_NODE_MAGIC:
case XFS_DA3_NODE_MAGIC:
> + type = XFS_BLF_DA_NODE_BUF;
> + break;
> + case XFS_ATTR_LEAF_MAGIC:
> + case XFS_ATTR3_LEAF_MAGIC:
> + type = XFS_BLF_ATTR_LEAF_BUF;
> + break;
> + case XFS_DIR2_LEAFN_MAGIC:
> + case XFS_DIR3_LEAFN_MAGIC:
> + type = XFS_BLF_DIR_LEAFN_BUF;
> + break;
> + default:
> + type = 0;
> + ASSERT(0);
> + break;
Would some kind of shutdown be desireable here? Maybe not.
> @@ -2004,19 +1960,169 @@ xlog_recover_do_reg_buffer(
> bp->b_ops = &xfs_inode_buf_ops;
> break;
> case XFS_BLF_SYMLINK_BUF:
> - if (*(__be32 *)bp->b_addr != cpu_to_be32(XFS_SYMLINK_MAGIC)) {
> + if (magic32 != XFS_SYMLINK_MAGIC) {
> xfs_warn(mp, "Bad symlink block magic!");
> ASSERT(0);
> break;
> }
> bp->b_ops = &xfs_symlink_buf_ops;
> break;
...
> + case XFS_BLF_ATTR_RMT_BUF:
> + if (!xfs_sb_version_hascrc(&mp->m_sb))
> + break;
> + if (magicda != XFS_ATTR3_RMT_MAGIC) {
magic32
The new remote attribute header has magic at offset 0.
> +STATIC void
> +xlog_recover_do_reg_buffer(
> + struct xfs_mount *mp,
> + xlog_recover_item_t *item,
> + struct xfs_buf *bp,
> + xfs_buf_log_format_t *buf_f)
> +{
...
> + /* Shouldn't be any more regions */
> + ASSERT(i == item->ri_total);
> +
> + xlog_recovery_validate_buf_type(mp, bp, buf_f);
> +
Extra line.
Looks good.
-Ben
|