[PATCH 0/13] add SMB2 server-side copy support - V2
George K Colley
gcolley at apple.com
Thu Nov 29 09:08:22 MST 2012
Jeremy,
Can you let me know when this gets into a build that we can test against? Do you know which version of samba this is going to make?
Thanks,
George
On Nov 27, 2012, at 2:04 PM, Jeremy Allison <jra at samba.org> wrote:
> On Tue, Nov 13, 2012 at 04:16:24PM +0100, David Disseldorp wrote:
>> This change-set adds support for the FSCTL_SRV_COPYCHUNK and
>> FSCTL_SRV_REQUEST_RESUME_KEY server-side copy SMB2 ioctls, allowing
>> for network round-trip avoidance during a file copy, i.e.:
>> http://www.samba.org/~ddiss/slides/snappery_samba/img17.html
>>
>> Changes since V1:
>> - Replace failure returns with torture_fail()/_skip()
>> - Free tevent subreq after receive, rather than waiting for parent recv
>> - Remove unneeded subreq from vfs_time_audit.c copychunk state
>> - Alloc tevent reqs for copychunk tracking in skel VFS example code
>> - Minor white space and debug output cleanups
>>
>> Feedback appreciated.
>
> Hi David,
>
> Sorry it's taken a while to get to this review. But there
> are a couple of changes I'd still like to see (sorry).
>
> In this patch:
>
> smb2_ioctl: add support for FSCTL_SRV_COPYCHUNK
>
> SMB2 clients can issue FSCTL_SRV_COPYCHUNK requests in order to copy
> data between files on the server side only, rather than reading data
> then writing back from the client. FSCTL_SRV_COPYCHUNK is used by
> default for Explorer SMB2 file copies on Windows Server 2012.
>
> 2.2.32.1 SRV_COPYCHUNK_RESPONSE in [MS-SMB2] describes the requirement
> for the server to provide maximum copychunk request size limits in ioctl
> responses carrying STATUS_INVALID_PARAMETER.
>
> We're actually doing a write operation (via the COPYCHUNK request).
> However, there is no CHECK_WRITE(fsp) call on the destination
> fsp. This function checks that the fsp is open for writing and
> writing is allowed on this share - without it we could be writing
> to a file opened without FILE_WRITE access.
>
> I think inside fsctl_srv_copychunk_send() you should have a check
> similar to the one inside source3/smbd/smb2_write. Look for
> the CHECK_WRITE macro inside that file for details.
>
> Secondly in this patch:
>
> s3-vfs: add copy_chunk vfs hooks
>
> copy_chunk copies n bytes from a source file at a specific offset to a
> destination file at a given offset. This interface will be used in
> handling smb2 FSCTL_SRV_COPYCHUNK ioctl requests.
>
> Provide send and receive hooks for copy chunk VFS interface, allowing
> asynchronous behaviour.
>
> There's a segment:
>
> --- a/source3/smbd/vfs.c
> +++ b/source3/smbd/vfs.c
> @@ -2157,11 +2157,33 @@ NTSTATUS smb_vfs_call_fsctl(struct vfs_handle_struct *handle,
> uint32_t *out_len)
> {
> VFS_FIND(fsctl);
> - return handle->fns->fsctl_fn(handle, fsp, ctx, function, req_flags,
> - in_data, in_len, out_data, max_out_len,
> + return handle->fns->fsctl_fn(handle, fsp, ctx, function, req_flags,
> + in_data, in_len, out_data, max_out_len,
> out_len);
>
> Looks like just a whitespace fix to me. Can you make that a separate
> change please.
>
> Hope this is ok - shouldn't be too many more goes
> around before we get this into master.
>
> Cheers,
>
> Jeremy.
More information about the samba-technical
mailing list