Re: [PATCH 6/9] xfs: disable swap extents ioctl on CRC enabled filesystems

Brian Foster
2013-05-29T21:09:06Z

On 05/27/2013 02:38 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Currently, swapping extents from one inode to another is a simple
> act of switching data and attribute forks from one inode to another.
> This, unfortunately in no longer so simple with CRC enabled
> filesystems as there is owner information embedded into the BMBT
> blocks that are swapped between inodes. Hence swapping the forks
> between inodes results in the inodes having mapping blocks that
> point to the wrong owner and hence are considered corrupt.
>
> To fix this we need an extent tree block or record based swap
> algorithm so that the BMBT block owner information can be updated
> atomically in the swap transaction. This is a significant piece of
> new work, so for the moment simply don't allow swap extent
> operations to succeed on CRC enabled filesystems.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Pretty straightforward...

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

> fs/xfs/xfs_dfrag.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
> index f852b08..c407e1c 100644
> --- a/fs/xfs/xfs_dfrag.c
> +++ b/fs/xfs/xfs_dfrag.c
> @@ -219,6 +219,14 @@ xfs_swap_extents(
> int taforkblks = 0;
> __uint64_t tmp;
>
> + /*
> + * We have no way of updating owner information in the BMBT blocks for
> + * each inode on CRC enabled filesystems, so to avoid corrupting the
> + * this metadata we simply don't allow extent swaps to occur.
> + */
> + if (xfs_sb_version_hascrc(&mp->m_sb))
> + return XFS_ERROR(EINVAL);
> +
> tempifp = kmem_alloc(sizeof(xfs_ifork_t), KM_MAYFAIL);
> if (!tempifp) {
> error = XFS_ERROR(ENOMEM);