On Thu, Dec 22, 2022 at 9:25 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> Thanks for the analysis.
> Would you be interested in trying to fix the bug and writing a test?
> I can help if you would like.
I can give it a try unless it turns out that some deep VFS changes are
required, but let's try to narrow down the reasonable API semantics
first.
> It's hard to follow all the changes since
> 54dbc1517237 ("vfs: hoist the btrfs deduplication ioctl to the vfs")
> in v4.5, but it *looks* like this behavior might have been in btrfs,
> before the ioctl was promoted to vfs.. not sure.
>
> We have fstests coverage for the "good" case of same size src/dst
> (generic/136), but I didn't find a test for the non-same size src/dst.
>
> In any case, vfs_dedupe_file_range_one() and ->remap_file_range()
> do not even have an interface to return the actual bytes_deduped,
> so I do not see how any of the REMAP_FILE_CAN_SHORTEN cases
> are valid, regardless of EOF.
Not sure about this, it looks to me that they are actually returning
the number of bytes deduped, but the value is not used, but maybe I'm
missing something.
Anyway I think there are valid cases when REMAP_FILE_CAN_SHORTEN makes sense.
For example if a source file content is a prefix of a destination file
content and we want to dedup the whole range of the source file
without REMAP_FILE_CAN_SHORTEN,
then the ioctl will only succeed when the end of the source file is at
the block boundary, otherwise it will just fail. This will render the
API very inconsistent.
Cheers,
Zbigniew
