reflink support and Samba running over XFS

[Amir Goldstein]

On Wed, Nov 9, 2022 at 8:47 PM Jeremy Allison <jra at samba.org> wrote:
>
> On Wed, Nov 09, 2022 at 10:38:02AM -0800, Jeremy Allison via samba-technical wrote:
> >On Wed, Nov 09, 2022 at 11:32:30AM +0200, Amir Goldstein wrote:
> >>On Tue, Nov 8, 2022 at 7:53 PM Jeremy Allison via samba-technical
> >><samba-technical at lists.samba.org> wrote:
> >>>
> >>>On Mon, Nov 07, 2022 at 10:47:48PM -0800, Christoph Hellwig wrote:
> >>>>On Mon, Nov 07, 2022 at 04:53:42PM -0800, Jeremy Allison via samba-technical wrote:
> >>>>> ret = ioctl(fsp_get_io_fd(dest_fsp), BTRFS_IOC_CLONE_RANGE, &cr_args);
> >>>>>
> >>>>> what ioctls are used for this in XFS ?
> >>>>>
> >>>>> We'd need a VFS module that implements them for XFS.
> >>>>
> >>>>That ioctl is now implemented in the Linux VFS and supported by btrfs,
> >>>>ocfs2, xfs, nfs (v4.2), cifs and overlayfs.
> >>>
> >>>I'm assuming it's this:
> >>>
> >>>https://man7.org/linux/man-pages/man2/ioctl_ficlonerange.2.html
> >>>
> >>>Yeah ? I'll write some test code and see if I can get it
> >>>into the vfs_default code.
> >>>
> >>
> >>Looks like this was already discussed during the work on generic
> >>implementation of FSCTL_SRV_COPYCHUNK:
> >>https://bugzilla.samba.org/show_bug.cgi?id=12033#c3
> >>
> >>Forgotten?
> >
> >Yep :-).
> >
> >>Left for later?
> >
> >So looks like we do copy_file_range(), but not CLONE_RANGE,
> >or rather CLONE_RANGE only in btrfs.
> >
> >So the code change needed is to move the logic in vfs_btrfs.c
> >into vfs_default.c, and change the call in vfs_btrfs.c:btrfs_offload_write_send()
> >to SMB_VFS_NEXT_OFFLOAD_WRITE_SEND() to call the old fallback code
> >inside vfs_default.c (vfswrap_offload_write_send()).
>
> Although looking at the current Linux kernel I find inside:
>
> ssize_t vfs_copy_file_range(struct file *file_in, loff_t pos_in,
>                             struct file *file_out, loff_t pos_out,
>                             size_t len, unsigned int flags)
> {
>
> https://github.com/torvalds/linux/blob/0adc313c4f20639f7e235b8d6719d96a2024cf91/fs/read_write.c#L1506
>
>         /*
>          * Try cloning first, this is supported by more file systems, and
>          * more efficient if both clone and copy are supported (e.g. NFS).
>          */
>         if (file_in->f_op->remap_file_range &&
>             file_inode(file_in)->i_sb == file_inode(file_out)->i_sb) {
>                 loff_t cloned;
>
>                 cloned = file_in->f_op->remap_file_range(file_in, pos_in,
>                                 file_out, pos_out,
>                                 min_t(loff_t, MAX_RW_COUNT, len),
>                                 REMAP_FILE_CAN_SHORTEN);
>                 if (cloned > 0) {
>                         ret = cloned;
>
> and looking at the code supporting int ioctl(int dest_fd, FICLONERANGE, struct file_clone_range *arg);
> we have:
>
> loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,
>                            struct file *file_out, loff_t pos_out,
>                            loff_t len, unsigned int remap_flags)
> ...
>         ret = file_in->f_op->remap_file_range(file_in, pos_in,
>                         file_out, pos_out, len, remap_flags);
>
> So it *looks* like the copy_file_range() syscall will internally
> call the equivalent of FICLONERANGE if the underlying file
> system supports it.
>

It's true.
Unless you have some SMB command that requires the clone to be
a clone (what is VFS_COPY_CHUNK_FL_MUST_CLONE in the
referred comment?)
because currently there is no flag that can be passed to
copy_file_range() to request only clone.

Thanks,
Amir.

> So maybe the right fix is to remove the FICLONERANGE specific
> code from our vfs_btrfs.c and just always use copy_file_range().
>
> Any comments from other Samba Team members ?
>
> Jeremy.