On Tue, Sep 13, 2016 at 3:08 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> On Mon, Sep 12, 2016 at 06:06:42PM +0300, Amir Goldstein wrote:
>> copy_file_range syscall returns -EXDEV if src and dest
>> file are not on the same file system.
>> The vfs_copy_file_range() helper, however, knows how to copy
>> across file systems with do_splice_direct().
>>
>> Move the enforcement of same file system from the vfs helper
>> to the syscall code.
>>
>> A following patch is going to use the vfs_copy_file_range()
>> helper in overlayfs to copy up between lower and upper
>> not on the same file system.
>>
>> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>> ---
>> fs/read_write.c | 15 ++++++++++-----
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/read_write.c b/fs/read_write.c
>> index 9dc6e52..c4675c6 100644
>> --- a/fs/read_write.c
>> +++ b/fs/read_write.c
>> @@ -1502,10 +1502,6 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
>> (file_out->f_flags & O_APPEND))
>> return -EBADF;
>>
>> - /* this could be relaxed once a method supports cross-fs copies */
>> - if (inode_in->i_sb != inode_out->i_sb)
>> - return -EXDEV;
>> -
>> if (len == 0)
>> return 0;
>>
>> @@ -1514,7 +1510,8 @@ ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
>> return ret;
>>
>> ret = -EOPNOTSUPP;
>> - if (file_out->f_op->copy_file_range)
>> + if (inode_in->i_sb == inode_out->i_sb &&
>> + file_out->f_op->copy_file_range)
>> ret = file_out->f_op->copy_file_range(file_in, pos_in, file_out,
>> pos_out, len, flags);
>
> Indenting is wrong, and you dropped an important comment. i.e
> copy_file_range() still doesn't support cross fs copies.....
OK. moved the comment down this this same i_sb test
>
>> if (ret == -EOPNOTSUPP)
>> @@ -1569,6 +1566,14 @@ SYSCALL_DEFINE6(copy_file_range, int, fd_in, loff_t __user *, off_in,
>> pos_out = f_out.file->f_pos;
>> }
>>
>> + /*
>> + * FIXME: should copy_file_range syscall enforce that src and
>> + * dest files are on the same mount point or only on the same
>> + * file system? none of the above?
>> + */
>> + if (file_inode(f_in.file)->i_sb != file_inode(f_out.file)->i_sb)
>> + return -EXDEV;
>
> For the purposes of this patch, it should simply do what it already
> does. If there's a API semantic change that needs to be made, then
> get that sorted out now rather than adding a "fixme" comment that
> will simply be ignored....
How about:
/*
* vfs_copy_file_range() can do cross-fs copy, but we want to
* fulfill the guaranty to userland that copy_file_range syscall
* does not allow cross-fs copy
*/
Cheers,
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
