Re: Samba server bug: CopyChunk from one share to a different share on same server

[David Disseldorp <ddiss-l3A5Bk7waGM@public.gmane.org>]

On Sun, 8 Nov 2015 20:52:16 -0600, Steve French wrote:

> Tried a quick experiment using cloner (David's tool - xfstests/src/cloner)
> to copy a file from share1 to share2 on the same target server (using
> CopyChunk and SMB3).  Works fine for Windows (tried Windows 8.1) but failed
> for Samba 4.2.
> 
> This is one of the more important cases to support (copying a file from one
> share to another)
> 
> Samba returns STATUS_OBJECT_NAME_NOT_FOUND on FSCTL_SRV_COPYCHUNK_WRITE
> unless source and target are on the same share.  I didn't try it with
> vfs_btrfs (the test system was running ext4, and both exports are on the
> same volume on the server).

It's no different across Samba VFS modules - this behaviour is intended.
The persistent/volatile handle is used to generate the copy-chunk resume
key. file_fsp_get() is currently used to lookup the source fsp using the
destination file connection context.

> Was testing the change below where I relax the copy offload check as
> follows (to allow cross share copy chunk as Windows does)
> 
> diff --git a/fs/cifs/ioctl.c b/fs/cifs/ioctl.c
> index 28a77bf..35cf990 100644
> --- a/fs/cifs/ioctl.c
> +++ b/fs/cifs/ioctl.c
> @@ -85,9 +85,14 @@ static long cifs_ioctl_clone(unsigned int xid, struct
> file *dst_file,
>      src_tcon = tlink_tcon(smb_file_src->tlink);
>      target_tcon = tlink_tcon(smb_file_target->tlink);
> 
> -    /* check if source and target are on same tree connection */
> -    if (src_tcon != target_tcon) {
> -        cifs_dbg(VFS, "file copy src and target on different volume\n");
> +    /* check source and target on same server (or volume if dup_extents) */
> +    if (dup_extents && (src_tcon != target_tcon)) {
> +        cifs_dbg(VFS, "source and target of copy not on same share\n");
> +        goto out_fput;
> +    }
> +
> +    if (!dup_extents && (src_tcon->ses != target_tcon->ses)) {
> +        cifs_dbg(VFS, "source and target of copy not on same server\n");
>          goto out_fput;
>      }
> 
> 

This change looks fine to me.
Reviewed-by: David Disseldorp <ddiss-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Cheers, David