[PATCH][CIFS] SMB2/SMB3 Copy offload support (refcopy) finishup

[David Disseldorp]

On Sat, 16 Nov 2013 01:14:19 -0600
Steve French <smfrench at gmail.com> wrote:

>  fs/cifs/smb2ops.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>  fs/cifs/smb2pdu.c |  2 +-
>  2 files changed, 79 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 11dde4b..7a21447 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -532,7 +532,10 @@ smb2_clone_range(const unsigned int xid,
>  	int rc;
>  	unsigned int ret_data_len;
>  	struct copychunk_ioctl *pcchunk;
> -	char *retbuf = NULL;
> +	struct copychunk_ioctl_rsp *retbuf = NULL;
> +	struct cifs_tcon *tcon;
> +	int chunks_copied = 0;
> +	bool chunk_sizes_updated = false;
>  
>  	pcchunk = kmalloc(sizeof(struct copychunk_ioctl), GFP_KERNEL);
>  
> @@ -552,22 +555,86 @@ smb2_clone_range(const unsigned int xid,

The SMB2_request_res_key() error path above here should be fixed to free
pcchunk.

>  	/* For now array only one chunk long, will make more flexible later */
>  	pcchunk->ChunkCount = __constant_cpu_to_le32(1);
>  	pcchunk->Reserved = 0;
> -	pcchunk->SourceOffset = cpu_to_le64(src_off);
> -	pcchunk->TargetOffset = cpu_to_le64(dest_off);
> -	pcchunk->Length = cpu_to_le32(len);
>  	pcchunk->Reserved2 = 0;
>  
> +	tcon = tlink_tcon(trgtfile->tlink);
> +
> +	while (len > 0) {
> +		pcchunk->SourceOffset = cpu_to_le64(src_off);
> +		pcchunk->TargetOffset = cpu_to_le64(dest_off);
> +		pcchunk->Length =
> +			cpu_to_le32(min_t(u32, len, tcon->max_bytes_chunk));
> +
>  	/* Request that server copy to target from src file identified by key */

Comment should be indented here too.

> -	rc = SMB2_ioctl(xid, tlink_tcon(trgtfile->tlink),
> -			trgtfile->fid.persistent_fid,
> +		rc = SMB2_ioctl(xid, tcon, trgtfile->fid.persistent_fid,
>  			trgtfile->fid.volatile_fid, FSCTL_SRV_COPYCHUNK_WRITE,
>  			true /* is_fsctl */, (char *)pcchunk,
> -			sizeof(struct copychunk_ioctl),	&retbuf, &ret_data_len);
> -
> -	/* BB need to special case rc = EINVAL to alter chunk size */
> -
> -	cifs_dbg(FYI, "rc %d data length out %d\n", rc, ret_data_len);
> +			sizeof(struct copychunk_ioctl),	(char **)&retbuf,
> +			&ret_data_len);
> +		if (rc == 0) {
> +			if (ret_data_len != sizeof(struct copychunk_ioctl_rsp))
> +				goto cchunk_out;
> +			if (retbuf->TotalBytesWritten == 0) {
> +				cifs_dbg(FYI, "no bytes copied\n");
> +				goto cchunk_out;
> +			}

These two error cases should set rc non-zero, it may not be the last
copy-chunk for this clone.
...

> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -1214,7 +1214,7 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
>  	rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0);
>  	rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
>  
> -	if (rc != 0) {
> +	if ((rc != 0) && (rc != -EINVAL)) {

This should probably be FSCTL_SRV_COPYCHUNK[_WRITE] specific. See MS-SMB2
  3.3.4.4   Sending an Error Response
  When the server is responding with a failure to any command sent by the
  client, the response message MUST be constructed as described here. An
  error code other than one of the following indicates a failure:
  ... 

  STATUS_INVALID_PARAMETER in an FSCTL_SRV_COPYCHUNK or
  FSCTL_SRV_COPYCHUNK_WRITE response, when returning an
  SRV_COPYCHUNK_RESPONSE as described in section 3.3.5.15.6.2.


>  		if (tcon)
>  			cifs_stats_fail_inc(tcon, SMB2_IOCTL_HE);
>  		goto ioctl_exit;

Cheers, David