Patch to add support for advertising FULLSYNC to Mac OSX Clients

[Ralph Böhme]

On Fri, Apr 07, 2017 at 11:04:04AM -0700, Jeremy Allison wrote:
> 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.

Generally a good idea, but as Uri is pointing out, we might just move the aapl
state variable to a proper global context.

> 
> > +	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.

It says:

  In response, the server must first process the SMB2 FLUSH command in the usual
  manner, and then must flush the data to stable storage (that is, flush the
  physical disk’s track cache) before responding to the client.
  
So we must use the sync sync.

Cheerio!
-slow