ceph: have copy op fall back when src_inode == dst_inode
diff mbox series

Message ID 20190724120542.26391-1-jlayton@kernel.org
State New
Headers show
Series
  • ceph: have copy op fall back when src_inode == dst_inode
Related show

Commit Message

Jeff Layton July 24, 2019, 12:05 p.m. UTC
Currently this just fails, but the fallback implementation can handle
this case. Change it to return -EOPNOTSUPP instead of -EINVAL when
copying data to a different spot in the same inode.

Cc: Luis Henriques <lhenriques@suse.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/file.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

NB: with this patch, xfstest generic/075 now passes

Comments

Luis Henriques July 24, 2019, 1:19 p.m. UTC | #1
"Jeff Layton" <jlayton@kernel.org> writes:

> Currently this just fails, but the fallback implementation can handle
> this case. Change it to return -EOPNOTSUPP instead of -EINVAL when
> copying data to a different spot in the same inode.

Thanks, Jeff!

So, just FTR (we had a quick chat on IRC already): I have a slightly
different patch sitting on my tree for a while.  The difference is that
my patch still allows to use the 'copy-from' operation in some cases,
even when src == dst.

I'll run a few more tests on it and send it out soon.

Cheers,
Luis Henriques July 24, 2019, 3:31 p.m. UTC | #2
Luis Henriques <lhenriques@suse.com> writes:

> "Jeff Layton" <jlayton@kernel.org> writes:
>
>> Currently this just fails, but the fallback implementation can handle
>> this case. Change it to return -EOPNOTSUPP instead of -EINVAL when
>> copying data to a different spot in the same inode.
>
> Thanks, Jeff!
>
> So, just FTR (we had a quick chat on IRC already): I have a slightly
> different patch sitting on my tree for a while.  The difference is that
> my patch still allows to use the 'copy-from' operation in some cases,
> even when src == dst.
>
> I'll run a few more tests on it and send it out soon.

So, after looking at my local patch I realised it was a bit outdated.
After commit 96e6e8f4a68d ("vfs: add missing checks to copy_file_range")
we only need to remove the 'if' statement -- the only other thing I had
locally was to return -EOPNOTSUPP if there were overlaps, which is now
handled in the VFS generic code.

I've tested this (i.e. dropping the 2nd hunk in your patch), and it
seems to be working fine.  Feel free to add my Acked-by to this
(modified) patch.

My next move was going to allow cross-filesystem copies, but I won't be
looking into that while https://github.com/ceph/ceph/pull/25374 isn't
merged (my initial pull-request is from December 2018!!!).

Cheers,

Patch
diff mbox series

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 82af4a3c714d..1b25df9d5853 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -1915,8 +1915,6 @@  static ssize_t ceph_copy_file_range(struct file *src_file, loff_t src_off,
 
 	if (src_inode->i_sb != dst_inode->i_sb)
 		return -EXDEV;
-	if (src_inode == dst_inode)
-		return -EINVAL;
 	if (ceph_snap(dst_inode) != CEPH_NOSNAP)
 		return -EROFS;
 
@@ -1928,6 +1926,10 @@  static ssize_t ceph_copy_file_range(struct file *src_file, loff_t src_off,
 	 * efficient).
 	 */
 
+	/* Can't do OSD copy op to same object */
+	if (src_inode == dst_inode)
+		return -EOPNOTSUPP;
+
 	if (ceph_test_mount_opt(ceph_inode_to_client(src_inode), NOCOPYFROM))
 		return -EOPNOTSUPP;