On Wed, Feb 5, 2020 at 11:28 AM Luis Henriques <lhenriques@xxxxxxxx> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Luis Henriques <lhenriques@xxxxxxxx>
> ---
> 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
