Re: [PATCH] Btrfs: incremental send, fix emission of invalid clone operations

From: Liu Bo <bo.li.liu@oracle.com>
To: fdmanana@kernel.org
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] Btrfs: incremental send, fix emission of invalid clone operations
Date: Tue, 15 Aug 2017 10:45:26 -0600	[thread overview]
Message-ID: <20170815164526.GA18477@dhcp-10-211-47-181.usdhcp.oraclecorp.com> (raw)
In-Reply-To: <20170810215451.4240-1-fdmanana@kernel.org>

On Thu, Aug 10, 2017 at 10:54:51PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When doing an incremental send it's possible that the computed send stream
> contains clone operations that will fail on the receiver if the receiver
> has compression enabled and the clone operations target a sector sized
> extent that starts at a zero file offset, is not compressed on the source
> filesystem but ends up being compressed and inlined at the destination
> filesystem.
> 
> Example scenario:
> 
>   $ mkfs.btrfs -f /dev/sdb
>   $ mount -o compress /dev/sdb /mnt
> 
>   # By doing a direct IO write, the data is not compressed.
>   $ xfs_io -f -d -c "pwrite -S 0xab 0 4K" /mnt/foobar
>   $ btrfs subvolume snapshot -r /mnt /mnt/mysnap1
> 
>   $ xfs_io -c "reflink /mnt/foobar 0 8K 4K" /mnt/foobar
>   $ btrfs subvolume snapshot -r /mnt /mnt/mysnap2
> 
>   $ btrfs send -f /tmp/1.snap /mnt/mysnap1
>   $ btrfs send -f /tmp/2.snap -p /mnt/mysnap1 /mnt/mysnap2
>   $ umount /mnt
> 
>   $ mkfs.btrfs -f /dev/sdc
>   $ mount -o compress /dev/sdc /mnt
>   $ btrfs receive -f /tmp/1.snap /mnt
>   $ btrfs receive -f /tmp/2.snap /mnt
>   ERROR: failed to clone extents to foobar
>   Operation not supported
> 
> The same could be achieved by mounting the source filesystem without
> compression and doing a buffered IO write instead of a direct IO one,
> and mounting the destination filesystem with compression enabled.
> 
> So fix this by issuing regular write operations in the send stream
> instead of clone operations when the source offset is zero and the
> range has a length matching the sector size.

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

Thanks,

-liubo
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  fs/btrfs/send.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index b082210df9c8..460be72ab78b 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -4992,6 +4992,25 @@ static int clone_range(struct send_ctx *sctx,
>  	struct btrfs_key key;
>  	int ret;
>  
> +	/*
> +	 * Prevent cloning from a zero offset with a length matching the sector
> +	 * size because in some scenarios this will make the receiver fail.
> +	 *
> +	 * For example, if in the source filesystem the extent at offset 0
> +	 * has a length of sectorsize and it was written using direct IO, then
> +	 * it can never be an inline extent (even if compression is enabled).
> +	 * Then this extent can be cloned in the original filesystem to a non
> +	 * zero file offset, but it may not be possible to clone in the
> +	 * destination filesystem because it can be inlined due to compression
> +	 * on the destination filesystem (as the receiver's write operations are
> +	 * always done using buffered IO). The same happens when the original
> +	 * filesystem does not have compression enabled but the destination
> +	 * filesystem has.
> +	 */
> +	if (clone_root->offset == 0 &&
> +	    len == sctx->send_root->fs_info->sectorsize)
> +		return send_extent_data(sctx, offset, len);
> +
>  	path = alloc_path_for_send();
>  	if (!path)
>  		return -ENOMEM;
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html