Patch to add support for advertising FULLSYNC to Mac OSX Clients

[Jeremy Allison]

On Tue, Apr 04, 2017 at 01:21:06PM +0200, Ralph Böhme wrote:
> Hi Kevin,
> 
> On Sun, Apr 02, 2017 at 10:40:06PM -0400, Kevin Anderson wrote:
> > Hi Ralph,
> >    I started an attempt to implement the remaining piece of this patch
> > and I have parsed out the reserved1 field in the
> > smbd_smb2_request_process_flush function. Does that seem appropriate
> > to you? If so, I only have to determine the best way to pass the fsync
> > to the actual file. Can/should the fsync be handled in the fruit
> > module? I would be open to any advice that you have as I have tried a
> > number of things already but they were unsuccessful.
> 
> maybe something like the attached. Just cobbled together, completely untested.
> 
> Jeremy, can you take a look as well please?
> 
> Cheerio!
> -slow

> From 2c3ec983cfec3dceda77e979784e2ea34a0ef03e Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 4 Apr 2017 13:13:30 +0200
> Subject: [PATCH 1/2] s3/smbd: add aapl_enabled flag to fsp
> 
> This will be needed in the next commit to implement macOS fullsync extension.
> ---
>  source3/include/vfs.h       | 1 +
>  source3/modules/vfs_fruit.c | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/source3/include/vfs.h b/source3/include/vfs.h
> index 0810fc2..fa964c0 100644
> --- a/source3/include/vfs.h
> +++ b/source3/include/vfs.h
> @@ -289,6 +289,7 @@ typedef struct files_struct {
>  	bool is_sparse;
>  	bool backup_intent; /* Handle was successfully opened with backup intent
>  				and opener has privilege to do so. */
> +	bool aapl_enabled; /* macOS client with extensions enabled */

I *hate* adding these aapl_XXX flags into 'struct files_struct'.

I know aapl_copyfile_supported is used in fsctl_srv_copychunk_loop(),
but that's a layering violation we haven't yet solved IMHO. I don't
want to make this worse for the 4.7.0 VFS.

Can't we change this to a generic:

bool VFS_QUERY_FEATURE(fsp, const char *vfs_module_name, const char *feature_name);

or

void *VFS_QUERY_FEATURE(fsp, const char *vfs_module_name, const char *feature_name);

that can sweep aapl-specific flags/structs out of the main interfaces
and back onto the module-specific private data struct where they
belong ?

That way the upper-level code has a module-agnostic way of asking
for module features that change our protocol-level behavior.

>  	bool aapl_copyfile_supported;
>  	bool use_ofd_locks; /* Are we using open file description locks ? */
>  	struct smb_filename *fsp_name;
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index 0b5558a..bf149ab 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -4945,6 +4945,8 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle,
>  	fsp = *result;
>  
>  	if (global_fruit_config.nego_aapl) {
> +		fsp->aapl_enabled = true;
> +
>  		if (config->copyfile_enabled) {
>  			/*
>  			 * Set a flag in the fsp. Gets used in
> -- 
> 2.9.3
> 
> 
> From 5c49e46489fc3496acc4c6653f036a8ec11d9bd9 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 4 Apr 2017 13:10:00 +0200
> Subject: [PATCH 2/2] s3/smbd: macOS fullsync
> 
> ---
>  source3/smbd/smb2_flush.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/source3/smbd/smb2_flush.c b/source3/smbd/smb2_flush.c
> index d077c62..e4ce0a0 100644
> --- a/source3/smbd/smb2_flush.c
> +++ b/source3/smbd/smb2_flush.c
> @@ -27,7 +27,8 @@
>  static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
>  					       struct tevent_context *ev,
>  					       struct smbd_smb2_request *smb2req,
> -					       struct files_struct *fsp);
> +					       struct files_struct *fsp,
> +					       uint16_t flush_reserved1);
>  static NTSTATUS smbd_smb2_flush_recv(struct tevent_req *req);
>  
>  static void smbd_smb2_request_flush_done(struct tevent_req *subreq);
> @@ -37,6 +38,7 @@ NTSTATUS smbd_smb2_request_process_flush(struct smbd_smb2_request *req)
>  	const uint8_t *inbody;
>  	uint64_t in_file_id_persistent;
>  	uint64_t in_file_id_volatile;
> +	uint16_t in_flush_reserved1;
>  	struct files_struct *in_fsp;
>  	struct tevent_req *subreq;
>  
> @@ -46,6 +48,7 @@ NTSTATUS smbd_smb2_request_process_flush(struct smbd_smb2_request *req)
>  	}
>  	inbody = SMBD_SMB2_IN_BODY_PTR(req);
>  
> +	in_flush_reserved1	= SVAL(inbody, 0x2);
>  	in_file_id_persistent	= BVAL(inbody, 0x08);
>  	in_file_id_volatile	= BVAL(inbody, 0x10);
>  
> @@ -55,7 +58,7 @@ NTSTATUS smbd_smb2_request_process_flush(struct smbd_smb2_request *req)
>  	}
>  
>  	subreq = smbd_smb2_flush_send(req, req->sconn->ev_ctx,
> -				      req, in_fsp);
> +				      req, in_fsp, in_flush_reserved1);
>  	if (subreq == NULL) {
>  		return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY);
>  	}
> @@ -115,7 +118,8 @@ static void smbd_smb2_flush_done(struct tevent_req *subreq);
>  static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
>  					       struct tevent_context *ev,
>  					       struct smbd_smb2_request *smb2req,
> -					       struct files_struct *fsp)
> +					       struct files_struct *fsp,
> +					       uint16_t flush_reserved1)
>  {
>  	struct tevent_req *req;
>  	struct tevent_req *subreq;
> @@ -153,6 +157,22 @@ static struct tevent_req *smbd_smb2_flush_send(TALLOC_CTX *mem_ctx,
>  		return tevent_req_post(req, ev);
>  	}
>  
> +	if (fsp->aapl_enabled && (flush_reserved1 == 0xffff)) {
> +		/*
> +		 * macOS specific fullsync, cf https://goo.gl/Lw5bdz
> +		 */
> +		ret = SMB_VFS_FSYNC(fsp);

Doesn't this need to be SMB_VFS_FSYNC_SEND() and
the whole thing use the standard async callback
mechanisms ?

Looking at https://goo.gl/Lw5bdz it doesn't say we can't
return an intermediate response followed by a completed
response. I don't want to add in another synchronous-sync
call (if you'll pardon the pun :-).

Jeremy.