xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 6/9] xfs: disable swap extents ioctl on CRC enabled filesystems
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Wed, 29 May 2013 17:06:08 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1369636707-15150-7-git-send-email-david@xxxxxxxxxxxxx>
References: <1369636707-15150-1-git-send-email-david@xxxxxxxxxxxxx> <1369636707-15150-7-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6
On 05/27/2013 02:38 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---

Pretty straightforward...

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  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);
> 

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