Re: [PATCH 1/3] xfs: move kernel-specific superblock validation out of libxfs

[Brian Foster <bfoster@redhat.com>]

On Mon, Nov 30, 2020 at 07:37:25PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> A couple of the superblock validation checks apply only to the kernel,
> so move them to xfs_mount.c before we start changing sb_inprogress.
> This also reduces the diff between kernel and userspace libxfs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_sb.c |   23 ++++-------------------
>  fs/xfs/libxfs/xfs_sb.h |    3 +++
>  fs/xfs/xfs_mount.c     |   31 +++++++++++++++++++++++++++++++
>  3 files changed, 38 insertions(+), 19 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 5aeafa59ed27..a2c43fe38f64 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -223,6 +223,7 @@ xfs_validate_sb_common(
>  	struct xfs_dsb		*dsb = bp->b_addr;
>  	uint32_t		agcount = 0;
>  	uint32_t		rem;
> +	int			error;
>  
>  	if (!xfs_verify_magic(bp, dsb->sb_magicnum)) {
>  		xfs_warn(mp, "bad magic number");
> @@ -382,16 +383,9 @@ xfs_validate_sb_common(
>  		return -EFSCORRUPTED;
>  	}
>  
> -	/*
> -	 * Until this is fixed only page-sized or smaller data blocks work.
> -	 */
> -	if (unlikely(sbp->sb_blocksize > PAGE_SIZE)) {
> -		xfs_warn(mp,
> -		"File system with blocksize %d bytes. "
> -		"Only pagesize (%ld) or less will currently work.",
> -				sbp->sb_blocksize, PAGE_SIZE);
> -		return -ENOSYS;
> -	}
> +	error = xfs_sb_validate_mount(mp, bp, sbp);
> +	if (error)
> +		return error;
>  
>  	/*
>  	 * Currently only very few inode sizes are supported.
> @@ -415,15 +409,6 @@ xfs_validate_sb_common(
>  		return -EFBIG;
>  	}
>  
> -	/*
> -	 * Don't touch the filesystem if a user tool thinks it owns the primary
> -	 * superblock.  mkfs doesn't clear the flag from secondary supers, so
> -	 * we don't check them at all.
> -	 */
> -	if (XFS_BUF_ADDR(bp) == XFS_SB_DADDR && sbp->sb_inprogress) {
> -		xfs_warn(mp, "Offline file system operation in progress!");
> -		return -EFSCORRUPTED;
> -	}
>  	return 0;
>  }
>  
> diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
> index 92465a9a5162..ee0a5858dd47 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -42,4 +42,7 @@ extern int	xfs_sb_get_secondary(struct xfs_mount *mp,
>  				struct xfs_trans *tp, xfs_agnumber_t agno,
>  				struct xfs_buf **bpp);
>  
> +int xfs_sb_validate_mount(struct xfs_mount *mp, struct xfs_buf *bp,
> +		struct xfs_sb *sbp);
> +
>  #endif	/* __XFS_SB_H__ */
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 7110507a2b6b..7bc7901d648d 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -259,6 +259,37 @@ xfs_initialize_perag(
>  	return error;
>  }
>  
> +/* Validate the superblock is compatible with this mount. */
> +int
> +xfs_sb_validate_mount(
> +	struct xfs_mount	*mp,
> +	struct xfs_buf		*bp,
> +	struct xfs_sb		*sbp)
> +{
> +	/*
> +	 * Don't touch the filesystem if a user tool thinks it owns the primary
> +	 * superblock.  mkfs doesn't clear the flag from secondary supers, so
> +	 * we don't check them at all.
> +	 */
> +	if (XFS_BUF_ADDR(bp) == XFS_SB_DADDR && sbp->sb_inprogress) {
> +		xfs_warn(mp, "Offline file system operation in progress!");
> +		return -EFSCORRUPTED;
> +	}
> +
> +	/*
> +	 * Until this is fixed only page-sized or smaller data blocks work.
> +	 */
> +	if (unlikely(sbp->sb_blocksize > PAGE_SIZE)) {
> +		xfs_warn(mp,
> +		"File system with blocksize %d bytes. "
> +		"Only pagesize (%ld) or less will currently work.",
> +				sbp->sb_blocksize, PAGE_SIZE);
> +		return -ENOSYS;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * xfs_readsb
>   *
>