Amir Goldstein <amir73il@xxxxxxxxx> writes:
> Because ceph doesn't hold destination inode lock throughout the copy,
> strip setuid bits before and after copy.
>
> The destination inode mtime is updated before and after the copy and the
> source inode atime is updated after the copy, similar to the filesystem
> ->read_iter() implementation.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
>
> Hi Ilya,
>
> Please consider applying this patch to ceph branch after merging
> Darrick's copy-file-range-fixes branch from:
> git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git
>
> The series (including this patch) was tested on ceph by
> Luis Henriques using new copy_range xfstests.
>
> AFAIK, only fallback from ceph to generic_copy_file_range()
> implementation was tested and not the actual ceph clustered
> copy_file_range.
JFYI I've also run tests that exercise the ceph implementation,
i.e. that actually do the copy offload. It's the xfstests that (AFAIR)
only exercise the generic VFS copy_file_range as they never meet the
requirements for this copy offload to happen (for example, the copy must
be at least the same length as the files object size which is 4M by
default).
Cheers,
--
Luis
>
> Thanks,
> Amir.
>
> fs/ceph/file.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index c5517ffeb11c..b04c97c7d393 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -1949,6 +1949,15 @@ static ssize_t __ceph_copy_file_range(struct file *src_file, loff_t src_off,
> goto out;
> }
>
> + /* Should dst_inode lock be held throughout the copy operation? */
> + inode_lock(dst_inode);
> + ret = file_modified(dst_file);
> + inode_unlock(dst_inode);
> + if (ret < 0) {
> + dout("failed to modify dst file before copy (%zd)\n", ret);
> + goto out;
> + }
> +
> /*
> * We need FILE_WR caps for dst_ci and FILE_RD for src_ci as other
> * clients may have dirty data in their caches. And OSDs know nothing
> @@ -2099,6 +2108,14 @@ static ssize_t __ceph_copy_file_range(struct file *src_file, loff_t src_off,
> out:
> ceph_free_cap_flush(prealloc_cf);
>
> + file_accessed(src_file);
> + /* To be on the safe side, try to remove privs also after copy */
> + inode_lock(dst_inode);
> + err = file_modified(dst_file);
> + inode_unlock(dst_inode);
> + if (err < 0)
> + dout("failed to modify dst file after copy (%d)\n", err);
> +
> return ret;
> }
