Re: reflink support and Samba running over XFS

[David Disseldorp <ddiss@samba.org>]

Hi Jeremy,

On Wed, 9 Nov 2022 11:24:26 -0800, Jeremy Allison via samba-technical wrote:

> On Wed, Nov 09, 2022 at 10:47:41AM -0800, Jeremy Allison wrote:
> >
> >So it *looks* like the copy_file_range() syscall will internally
> >call the equivalent of FICLONERANGE if the underlying file
> >system supports it.
> >
> >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 ?  
> 
> So right now Steve what is preventing FSCTL_DUP_EXTENTS_TO_FILE
> from working against anything other then btrfs on Samba is the
> following code:
> 
> source3/smbd/smb2_ioctl_filesys.c:fsctl_dup_extents_send()
> 
> 180         if ((dst_fsp->conn->fs_capabilities
> 181                                 & FILE_SUPPORTS_BLOCK_REFCOUNTING) == 0) {
> 182                 DBG_INFO("FS does not advertise block refcounting support\n");
> 183                 tevent_req_nterror(req, NT_STATUS_INVALID_DEVICE_REQUEST);
> 184                 return tevent_req_post(req, ev);
> 185         }
> 
> because currently only the vfs_btrfs module reports FILE_SUPPORTS_BLOCK_REFCOUNTING,
> not vfs_default.
> 
> and also in:
> 
> source3/modules/vfs_default.c:vfswrap_offload_write_send()
> 
> 2194         case FSCTL_DUP_EXTENTS_TO_FILE:
> 2195                 DBG_DEBUG("COW clones not supported by vfs_default\n");
> 2196                 tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
> 2197                 return tevent_req_post(req, ev);
> 
> but looking at vfs_btrfs it looks like that code should
> probably also be in vfswrap_offload_read_send() as well
> and the error code should be NT_STATUS_INVALID_DEVICE_REQUEST.
> 
> We also need to duplicate the logic in vfs_btrfs for
> handling FSCTL_DUP_EXTENTS_TO_FILE into VFS default,
> gated on support for the copy_file_range() system
> call (which would set FILE_SUPPORTS_BLOCK_REFCOUNTING
> in the fs_capabilities return from vfs_default).
> 
> I think this is doable with some work...

I think it's doable too :-). As indicated in my other mail, I think
FICLONERANGE will need to be used for FSCTL_DUP_EXTENTS_TO_FILE instead
of copy_file_range().
I'm not sure how to best handle FILE_SUPPORTS_BLOCK_REFCOUNTING -
ideally we'd set it for Btrfs and XFS(reflink=1) backed shares only.
Another option might be to advertise FILE_SUPPORTS_BLOCK_REFCOUNTING and
then propagate errors up to the client if FICLONERANGE fails for
FSCTL_DUP_EXTENTS_TO_FILE. Client copy fallback would work, but the
extra request overhead would be ugly.

Cheers, David