Patch to add support for advertising FULLSYNC to Mac OSX Clients

Ralph Böhme slow at samba.org
Fri Jun 23 11:21:37 UTC 2017


Hi Kevin,

sorry for the delay.

On Fri, Apr 07, 2017 at 05:27:02PM -0400, Kevin Anderson wrote:
> >> > We should ask for a clarification to:
> >> >
> >> > "flush the physical disk’s track cache) before making a final
> >> > response to the client".
> >>
> >> There's no mention of *final* response in the doc. It just says
> >>
> >>   before responding to the client.
> >>
> >> I'm reading this as "sync and don't cheat!".
> >
> > But making that single call forced-sync makes no sense.
> >
> > Enforcing that client side would mean adding extra code
> > to prevent any other pending non-related SMB2 requests
> > from being processed whilst this code is awaiting response.
> >
> > I really looks like an ambiguous document statement than
> > anything like a design decision.
> >
> > I really don't want us to force sync on this until I know
> > this is really mandatory in the spec, and even then I
> > want the spec ammended to say so :-).
> >
> > Allowing clients to force sync like this can *kill* any
> > asyncio performance from us.
> 
> I understand that you stated it *can* kill performance rather than it
> will but I wanted to add that not every flush generates a sync. In the
> last backup set that I ran, 1500 FLUSH requests were sent and of those
> requests, only 76 sync flushes were sent. I am not sure if that helps
> at all but I can try to quantify impact more if necessary.

attached is a patch (on top of a current TM patchset) which forces every SMB2
FLUSH request to take at least 1 ms which will trigger an request-went-async
SMB2 response to the client.

Can you please test this with a macOS client? Please check whether the patch
really produced async resonses for the SMB2 FLUSHes.

Thanks!

-slow
-------------- next part --------------
From 5627492308e009fb3546d6375473e764940a1c28 Mon Sep 17 00:00:00 2001
From: Kevin Anderson <andersonkw2 at gmail.com>
Date: Mon, 14 Nov 2016 19:14:44 +0100
Subject: [PATCH 1/3] vfs_fruit: Add Time Machine support

Add a configuration option to disable/enable Time Machine support via
the FULLSYNC AAPL flag.
---
 source3/modules/vfs_fruit.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index c4277b9..6716e9a 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -137,6 +137,7 @@ struct fruit_config_data {
 	bool veto_appledouble;
 	bool posix_rename;
 	bool aapl_zero_file_id;
+	bool time_machine;
 
 	/*
 	 * Additional options, all enabled by default,
@@ -1606,6 +1607,9 @@ static int init_fruit_config(vfs_handle_struct *handle)
 	config->use_aapl = lp_parm_bool(
 		-1, FRUIT_PARAM_TYPE_NAME, "aapl", true);
 
+	config->time_machine = lp_parm_bool(
+		-1, FRUIT_PARAM_TYPE_NAME, "time machine", false);
+
 	config->unix_info_enabled = lp_parm_bool(
 		-1, FRUIT_PARAM_TYPE_NAME, "nfs_aces", true);
 
@@ -2176,6 +2180,7 @@ static NTSTATUS check_aapl(vfs_handle_struct *handle,
 	DATA_BLOB blob = data_blob_talloc(req, NULL, 0);
 	uint64_t req_bitmap, client_caps;
 	uint64_t server_caps = SMB2_CRTCTX_AAPL_UNIX_BASED;
+	uint64_t volume_caps = 0;
 	smb_ucs2_t *model;
 	size_t modellen;
 
@@ -2260,6 +2265,10 @@ static NTSTATUS check_aapl(vfs_handle_struct *handle,
 			break;
 		}
 
+		if (config->time_machine) {
+			volume_caps |= SMB2_CRTCTX_AAPL_FULL_SYNC;
+		}
+
 		SBVAL(p, 0, caps);
 
 		ok = data_blob_append(req, &blob, p, 8);
@@ -2688,6 +2697,13 @@ static int fruit_connect(vfs_handle_struct *handle,
 			"0x0d:0xf00d");
 	}
 
+	if (config->time_machine) {
+		lp_do_parameter(SNUM(handle->conn), "durable handles", "yes");
+		lp_do_parameter(SNUM(handle->conn), "strict sync", "yes");
+		lp_do_parameter(SNUM(handle->conn), "kernel oplocks", "no");
+		lp_do_parameter(SNUM(handle->conn), "kernel share modes", "no");
+		lp_do_parameter(SNUM(handle->conn), "posix locking", "no");
+	}
 	return rc;
 }
 
-- 
2.9.3


From 8333d9ed5c654f74136acd23addd504d0ede4434 Mon Sep 17 00:00:00 2001
From: Kevin Anderson <andersonkw2 at gmail.com>
Date: Sun, 23 Oct 2016 20:32:27 -0400
Subject: [PATCH 2/3] docs/vfs_fruit: Add Time Machine support

Add the capability to advertise FULLSYNC volume capabilities
to clients that request them. This is mainly used for supporting
Mac OS Time Machine backups from clients. The capability does
not perform any additional action.
---
 docs-xml/manpages/vfs_fruit.8.xml | 33 +++++++++++++++++++++++++++++++++
 libcli/smb/smb2_create_ctx.h      |  1 +
 2 files changed, 34 insertions(+)

diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml
index 0bddd4a..3455644 100644
--- a/docs-xml/manpages/vfs_fruit.8.xml
+++ b/docs-xml/manpages/vfs_fruit.8.xml
@@ -298,6 +298,39 @@
 	  </varlistentry>
 
 	  <varlistentry>
+	    <term>fruit:time machine = [ yes | no ]</term>
+	    <listitem>
+	      <para>Controls if Time Machine support via the FULLSYNC volume
+	      capability is advertised to clients.</para>
+
+	      <itemizedlist>
+		<listitem><para><command>no (default)</command> Disables
+		advertising Time Machine support and the FULLSYNC volume
+		capability to clients.</para></listitem>
+
+		<listitem><para><command>yes</command> - Enables advertising
+		Time Machine support and the FULLSYNC volume capability to
+		clients. This is necessary for supporting Time Machine backups
+		from Mac OSX clients. This value only advertises the capability
+		and does nothing else with the sync requests from clients.
+		</para></listitem>
+
+	      </itemizedlist>
+
+	      <para>This option enforces the following settings per share (or
+	      for all shares if enabled globally):</para>
+	      <itemizedlist>
+		<listitem><para><command>durable handles = yes</command></para></listitem>
+		<listitem><para><command>kernel oplocks = no</command></para></listitem>
+		<listitem><para><command>kernel share modes = no</command></para></listitem>
+		<listitem><para><command>posix locking = no</command></para></listitem>
+		<listitem><para><command>strict sync = yes</command></para></listitem>
+	      </itemizedlist>
+
+	    </listitem>
+	  </varlistentry>
+
+	  <varlistentry>
 	    <term>readdir_attr:aapl_rsize = yes | no</term>
 	    <listitem>
 	      <para>Return resource fork size in SMB2 FIND responses.</para>
diff --git a/libcli/smb/smb2_create_ctx.h b/libcli/smb/smb2_create_ctx.h
index cb194f5..0e0b713 100644
--- a/libcli/smb/smb2_create_ctx.h
+++ b/libcli/smb/smb2_create_ctx.h
@@ -42,5 +42,6 @@
 /* "AAPL" Volume Capabilities bitmap */
 #define SMB2_CRTCTX_AAPL_SUPPORT_RESOLVE_ID 1
 #define SMB2_CRTCTX_AAPL_CASE_SENSITIVE     2
+#define SMB2_CRTCTX_AAPL_FULL_SYNC          4
 
 #endif
-- 
2.9.3


From 5ba37f9c4d320efefc1c888a6c7eaa240499abed Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 23 Jun 2017 13:17:37 +0200
Subject: [PATCH 3/3] HACK: Force an interim async repsonse

---
 source3/modules/vfs_default.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index d339f39..9af4e92 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1015,6 +1015,9 @@ static void vfs_fsync_do(void *private_data)
 
 	PROFILE_TIMESTAMP(&start_time);
 
+	/* Force an interim async repsonse */
+	msleep(1000);
+
 	do {
 		state->ret = fsync(state->fd);
 	} while ((state->ret == -1) && (errno == EINTR));
-- 
2.9.3



More information about the samba-technical mailing list