[PATCHSET] support FSCTL_DUPLICATE_EXTENTS_TO_FILE
ddiss at suse.de
Mon May 8 00:22:11 UTC 2017
Thanks a lot for the review, Ralph!
Please find a version 2 patchset attached. Further comments below...
On Sun, 7 May 2017 13:30:48 +0200, Ralph Böhme wrote:
> Hi David,
> nice work! Some nitpicks below...
> On Thu, May 04, 2017 at 05:30:35PM +0200, David Disseldorp via samba-technical wrote:
> > Please find attached a patchset from Aurélien and myself, which adds
> > support for FSCTL_DUPLICATE_EXTENTS_TO_FILE - a new reflink/clone FSCTL
> > supported on Windows Server 2016 shares backed by ReFS.
> > A couple of things that I'm still a little unsure about:
> > - It might be cleaner to move the locking calls out of the copy-chunk
> > VFS function (drop VFS_COPY_CHUNK_FL_IGNORE_LOCKS), and move them
> > into fsctl_srv_copychunk_loop(). Thoughts?
> Yeah, probably makes sense, but I'd have to take a closer look. As I'm going to
> do some larger changes here in the near future for offload read/write as, I can
> move the locking stuff around as part of that.
I've left it as is for now (with the flag).
> > - The server behaviour for success on truncated clone (where the
> > dest file's allocation size is less than the size of the clone) seems
> > prone to races, especially given that the operation ignores locks.
> > + See test_ioctl_dup_extents_len_beyond_dest(). Dochelp are aware of
> > this inconsistency.
> > + There's not much we can do here, as we need to match Windows Server
> > 2016 + ReFS behaviour.
> [PATCH 03/10] vfs: add parameter to copy chunk VFS function to handle dup_extents
> TODO: please move "[ddiss at samba.org: split VFS API change from ioctl code]" to subject body
For the kernel this is a pretty common way of flagging changes which
have been made after an author (aaptel) has added his sign off:
If Aurélien says that he's fine with the change as it is now, then I'll
drop the line completely.
> otherwise rb: me.
> [PATCH 4/10] smbd/smb2_ioctl: add support for FSCTL_DUPLICATE_EXTENTS_TO_FILE
> - initialize all pointer vars to NULL
> - use DEBUG helper macros DBG_ERR, DBG_INFO, ...
> - fsctl_dup_extents_send/recv() must create a tevent_req and return that
> and not just return the SMB_VFS_COPY_CHUNK_SEND subreq
> - empty line between var declaration and subsequent first statement
I've added a SQUASH commit with these changes. If you and Aurélien are
okay with it then I'll squash it in.
> Everything else: reviewed-by: me.
> Fwiw, I tested copy behaviour of a Windows 10 client against Windows 2016
> sharing a refs filesystem, as discussed last week. What I see is:
> - server returns FILE_SUPPORTS_BLOCK_REFCOUNTING in volume attributes
> - copying a 100 MB file with c-c -> c-v in the same share
> - client issues offload read ioctl
> - server returns invalid device request
> - client falls back to copy-chunk
> I don't see clone-range on the wire, trace attached. Am I missing something?
Interesting, thanks for checking this. I've been using the cifs.ko
client until now. I don't know whether Explorer makes use of it at this
stage. Will try with robocopy.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 49064 bytes
Desc: not available
More information about the samba-technical