On Feb 17, 2021, at 1:08 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> You are missing my point.
> Never mind which server. The server does not *need* to rely on
> vfs_copy_file_range() to copy files from XFS to ext4.
> The server is very capable of implementing the fallback generic copy
> in case source/target fs do not support native {copy,remap}_file_range().
>
> w.r.t semantics of copy_file_range() syscall vs. the fallback to userespace
> 'cp' tool (check source file size before copy or not), please note that the
> semantics of CIFS_IOC_COPYCHUNK_FILE are that of the former:
>
> rc = cifs_file_copychunk_range(xid, src_file.file, 0, dst_file, 0,
> src_inode->i_size, 0);
>
> It will copy zero bytes if advertised source file size if zero.
>
> NFS server side copy semantics are currently de-facto the same
> because both the client and the server will have to pass through this
> line in vfs_copy_file_range():
>
> if (len == 0)
> return 0;
>
> IMO, and this opinion was voiced by several other filesystem developers,
> the shortend copy semantics are the correct semantics for copy_file_range()
> syscall as well as for vfs_copy_file_range() for internal kernel users.
>
> I guess what this means is that if the 'cp' tool ever tries an opportunistic
> copy_file_range() syscall (e.g. --cfr=auto), it may result in zero size copy.
Having a syscall that does the "wrong thing" when called on two files
doesn't make sense. Expecting userspace to check whether source/target
files supports CFR is also not practical. This is trivial for the
kernel to determine and return -EOPNOTSUPP to the caller if the source
file (procfs/sysfs/etc) does not work with CFR properly.
Applications must already handle -EOPNOTSUPP with a fallback, but
expecting all applications that may call copy_file_range() to be
properly coded to handle corner cases is just asking for trouble.
That is doubly true given that an existing widely-used tool like
cp and mv are using this syscall if it is available in the kernel.
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
