[Ocfs2-devel] [PATCH 6/6] ocfs2: implement the VFS clone_range, copy_range, and dedupe_range features

[Darrick J. Wong]

On Fri, Nov 11, 2016 at 02:45:54PM +0800, Eric Ren wrote:
> On 11/11/2016 02:20 PM, Darrick J. Wong wrote:
> >On Fri, Nov 11, 2016 at 01:49:48PM +0800, Eric Ren wrote:
> >>Hi,
> >>
> >>A few issues obvious to me:
> >>
> >>On 11/10/2016 06:51 AM, Darrick J. Wong wrote:
> >>>Connect the new VFS clone_range, copy_range, and dedupe_range features
> >>>to the existing reflink capability of ocfs2.  Compared to the existing
> >>>ocfs2 reflink ioctl We have to do things a little differently to support
> >>>the VFS semantics (we can clone subranges of a file but we don't clone
> >>>xattrs), but the VFS ioctls are more broadly supported.
> >>How can I test the new ocfs2 reflink (with this patch) manually? What
> >>commands should I use to do xxx_range things?
> >See the 'reflink', 'dedupe', and 'copy_range' commands in xfs_io.
> >
> >The first two were added in xfsprogs 4.3, and copy_range in 4.7.
> 
> OK, thanks. I think you are missing the following two inline comments:
> 
> >>>+	spin_lock(&OCFS2_I(dest)->ip_lock);
> >>>+	if (newlen > i_size_read(dest)) {
> >>>+		i_size_write(dest, newlen);
> >>>+		di->i_size = newlen;
> >>di->i_size = cpu_to_le64(newlen);

Good catch!

> >>>+	}
> >>>+	spin_unlock(&OCFS2_I(dest)->ip_lock);
> >>>+
> >>Add ocfs2_update_inode_fsync_trans() here? Looks this function was
> >>introduced by you to improve efficiency.
> >>Just want to awake your memory about this, though I don't know about the
> >>details why it should be.

D'oh!  Yes, I did miss that.

The function updates the destination inode's information.  Specifically,
it updates i_size if we reflinked blocks into the file past EOF.
Looking at it some more, I also need to update i_blocks or the stat(2) info
will be wrong, and I also need to convert inline data to extents prior
to reflinking.

--D

> >>
> >>Eric
> Thanks,
> Eric