Re: [PATCH v2] btrfs: send: fix invalid clone operation for file that got its size decreased

[Josef Bacik <josef@toxicpanda.com>]

On Fri, Sep 27, 2024 at 12:03:55PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> During an incremental send we may end up sending an invalid clone
> operation, for the last extent of a file which ends at an unaligned offset
> that matches the final i_size of the file in the send snapshot, in case
> the file had its initial size (the size in the parent snapshot) decreased
> in the send snapshot. In this case the destination will fail to apply the
> clone operation because its end offset is not sector size aligned and it
> ends before the current size of the file.
> 
> Sending the truncate operation always happens when we finish processing an
> inode, after we process all its extents (and xattrs, names, etc). So fix
> this by ensuring the file has a valid size before we send a clone
> operation for an unaligned extent that ends at the final i_size of the
> file. The size we truncate to matches the start offset of the clone range
> but it could be any value between that start offset and the final size of
> the file since the clone operation will expand the i_size if the current
> size is smaller than the end offset. The start offset of the range was
> chosen because it's always sector size aligned and avoids a truncation
> into the middle of a page, which results in dirtying the page due to
> filling part of it with zeroes and then making the clone operation at the
> receiver trigger IO.
> 
> The following test reproduces the issue:
> 
>   $ cat test.sh
>   #!/bin/bash
> 
>   DEV=/dev/sdi
>   MNT=/mnt/sdi
> 
>   mkfs.btrfs -f $DEV
>   mount $DEV $MNT
> 
>   # Create a file with a size of 256K + 5 bytes, having two extents, one
>   # with a size of 128K and another one with a size of 128K + 5 bytes.
>   last_ext_size=$((128 * 1024 + 5))
>   xfs_io -f -d -c "pwrite -S 0xab -b 128K 0 128K" \
>          -c "pwrite -S 0xcd -b $last_ext_size 128K $last_ext_size" \
>          $MNT/foo
> 
>   # Another file which we will later clone foo into, but initially with
>   # a larger size than foo.
>   xfs_io -f -c "pwrite -S 0xef 0 1M" $MNT/bar
> 
>   btrfs subvolume snapshot -r $MNT/ $MNT/snap1
> 
>   # Now resize bar and clone foo into it.
>   xfs_io -c "truncate 0" \
>          -c "reflink $MNT/foo" $MNT/bar
> 
>   btrfs subvolume snapshot -r $MNT/ $MNT/snap2
> 
>   rm -f /tmp/send-full /tmp/send-inc
>   btrfs send -f /tmp/send-full $MNT/snap1
>   btrfs send -p $MNT/snap1 -f /tmp/send-inc $MNT/snap2
> 
>   umount $MNT
>   mkfs.btrfs -f $DEV
>   mount $DEV $MNT
> 
>   btrfs receive -f /tmp/send-full $MNT
>   btrfs receive -f /tmp/send-inc $MNT
> 
>   umount $MNT
> 
> Running it before this patch:
> 
>   $ ./test.sh
>   (...)
>   At subvol snap1
>   At snapshot snap2
>   ERROR: failed to clone extents to bar: Invalid argument
> 
> A test case for fstests will be sent soon.
> 
> Reported-by: Ben Millwood <thebenmachine@gmail.com>
> Link: https://lore.kernel.org/linux-btrfs/CAJhrHS2z+WViO2h=ojYvBPDLsATwLbg+7JaNCyYomv0fUxEpQQ@mail.gmail.com/
> Fixes: 46a6e10a1ab1 ("btrfs: send: allow cloning non-aligned extent if it ends at i_size")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef