Messages in this thread | | | From | Ilya Dryomov <> | Date | Wed, 5 Feb 2020 12:16:02 +0100 | Subject | Re: [PATCH] ceph: fix copy_file_range error path in short copies |
| |
On Wed, Feb 5, 2020 at 11:28 AM Luis Henriques <lhenriques@suse.com> wrote: > > When there's an error in the copying loop but some bytes have already been > copied into the destination file, it is necessary to dirty the caps and > eventually update the MDS with the file metadata (timestamps, size). This > patch fixes this error path. > > Cc: stable@vger.kernel.org > Signed-off-by: Luis Henriques <lhenriques@suse.com> > --- > fs/ceph/file.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index 11929d2bb594..7be47d24edb1 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -2104,9 +2104,16 @@ static ssize_t __ceph_copy_file_range(struct file *src_file, loff_t src_off, > CEPH_OSD_OP_FLAG_FADVISE_DONTNEED, 0); > if (err) { > dout("ceph_osdc_copy_from returned %d\n", err); > - if (!ret) > + /* > + * If we haven't done any copy yet, just exit with the > + * error code; otherwise, return the number of bytes > + * already copied, update metadata and dirty caps. > + */ > + if (!ret) { > ret = err; > - goto out_caps; > + goto out_caps; > + } > + goto out_early; > } > len -= object_size; > src_off += object_size; > @@ -2118,6 +2125,7 @@ static ssize_t __ceph_copy_file_range(struct file *src_file, loff_t src_off, > /* We still need one final local copy */ > do_final_copy = true; > > +out_early:
out_early is misleading, especially given that there already is out_caps, which just puts caps. I suggest something like update_dst_inode.
> file_update_time(dst_file); > inode_inc_iversion_raw(dst_inode); >
I think this is still buggy. What follows is this:
if (endoff > size) { int caps_flags = 0;
/* Let the MDS know about dst file size change */ if (ceph_quota_is_max_bytes_approaching(dst_inode, endoff)) caps_flags |= CHECK_CAPS_NODELAY; if (ceph_inode_set_size(dst_inode, endoff)) caps_flags |= CHECK_CAPS_AUTHONLY; if (caps_flags) ceph_check_caps(dst_ci, caps_flags, NULL); }
with endoff being:
size = i_size_read(dst_inode); endoff = dst_off + len;
So a short copy effectively zero-fills the destination file...
Thanks,
Ilya
| |