[PATCHES] de-couple vfs_streams_xattr from "ea support"

Uri Simchoni uri at samba.org
Thu Mar 2 19:37:35 UTC 2017


"ea support" conf parameter determines whether smbd will handle SMB
requests related to extended attributes. vfs_streams_xattr currently
requires "ea support = yes", or it won't list streams, but doesn't seem
aligned with the desired meaning of "ea support" which is the
client-facing behavior, not the storage backend behavior. This is mostly
relevant to vfs_fruit and Mac support.

This patch set removes this coupling - it allows EA's to be disabled and
EA-based streams to be enabled.

First two patches are small unrelated cleanups, then the actual change,
then changes to documentation, selftest, and testparm.

Review appreciated,
Uri.

-------------- next part --------------
From 4a3d918f06218f1d4876a07cdbbdff0fdc5b4493 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 2 Mar 2017 08:46:44 +0200
Subject: [PATCH 1/6] smbd: refuse_symlink() - do not fail if the file does not
 exist

If the file does not exist, it is not a symlink. Current callers
use this function to see if extended attributes can be set / fetched.
Allow them to try and leave the error code at the discretion of the
VFS.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/smbd/trans2.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index f58aacf..a221461 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -75,8 +75,11 @@ static NTSTATUS refuse_symlink(connection_struct *conn,
 		int ret = vfs_stat_smb_basename(conn,
 				smb_fname,
 				&sbuf);
-		if (ret == -1) {
+		if (ret == -1 && errno != ENOENT) {
 			return map_nt_error_from_unix(errno);
+		} else if (ret == -1) {
+			/* it's not a symlink.. */
+			return NT_STATUS_OK;
 		}
 		pst = &sbuf;
 	}
-- 
2.9.3


From f9ad911949f5632ed74d9ebf179af98dcb47e009 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 2 Mar 2017 08:49:54 +0200
Subject: [PATCH 2/6] smbd: get_ea_list_from_file_path() - remove a duplicate
 statement

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/smbd/trans2.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index a221461..dc6dc71 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -416,7 +416,6 @@ static NTSTATUS get_ea_list_from_file_path(TALLOC_CTX *mem_ctx,
 	}
 
 	if (num_names == 0) {
-		*ea_list = NULL;
 		return NT_STATUS_OK;
 	}
 
-- 
2.9.3


From b11307a25df0b75b58bf62194efc67bcb923d0c0 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 2 Mar 2017 08:39:56 +0200
Subject: [PATCH 3/6] smbd: remove coupling between get_ea_names_from_file()
 and "ea support"

The "ea support" configuration variable determines whether smbd
should attempt to manipulate extended attributes via SMB protocol.
It does not pertain to the underlying storage and its support for
extended attributes.

get_ea_names_from_file() is being used also by vfs_streams_xattr -
a module which has nothing to do with client-visible extended
attributes. As such, vfs_streams_xattr should be able to operate
irrespective of the value of "ea support".

This patch moves the check for ea support to the callers.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/smbd/nttrans.c | 30 ++++++++++++++++++------------
 source3/smbd/trans2.c  |  8 ++++----
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index 5f122a9..a5fc625 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -688,16 +688,19 @@ void reply_ntcreate_and_X(struct smb_request *req)
 	p += 8;
 	if (flags & EXTENDED_RESPONSE_REQUIRED) {
 		uint16_t file_status = (NO_EAS|NO_SUBSTREAMS|NO_REPARSETAG);
-		size_t num_names = 0;
 		unsigned int num_streams = 0;
 		struct stream_struct *streams = NULL;
 
-		/* Do we have any EA's ? */
-		status = get_ea_names_from_file(ctx, conn, fsp,
-				smb_fname, NULL, &num_names);
-		if (NT_STATUS_IS_OK(status) && num_names) {
-			file_status &= ~NO_EAS;
+		if (lp_ea_support(SNUM(conn))) {
+			size_t num_names = 0;
+			/* Do we have any EA's ? */
+			status = get_ea_names_from_file(
+			    ctx, conn, fsp, smb_fname, NULL, &num_names);
+			if (NT_STATUS_IS_OK(status) && num_names) {
+				file_status &= ~NO_EAS;
+			}
 		}
+
 		status = vfs_streaminfo(conn, NULL, smb_fname, ctx,
 			&num_streams, &streams);
 		/* There is always one stream, ::$DATA. */
@@ -1334,16 +1337,19 @@ static void call_nt_transact_create(connection_struct *conn,
 	p += 8;
 	if (flags & EXTENDED_RESPONSE_REQUIRED) {
 		uint16_t file_status = (NO_EAS|NO_SUBSTREAMS|NO_REPARSETAG);
-		size_t num_names = 0;
 		unsigned int num_streams = 0;
 		struct stream_struct *streams = NULL;
 
-		/* Do we have any EA's ? */
-		status = get_ea_names_from_file(ctx, conn, fsp,
-				smb_fname, NULL, &num_names);
-		if (NT_STATUS_IS_OK(status) && num_names) {
-			file_status &= ~NO_EAS;
+		if (lp_ea_support(SNUM(conn))) {
+			size_t num_names = 0;
+			/* Do we have any EA's ? */
+			status = get_ea_names_from_file(
+			    ctx, conn, fsp, smb_fname, NULL, &num_names);
+			if (NT_STATUS_IS_OK(status) && num_names) {
+				file_status &= ~NO_EAS;
+			}
 		}
+
 		status = vfs_streaminfo(conn, NULL, smb_fname, ctx,
 			&num_streams, &streams);
 		/* There is always one stream, ::$DATA. */
diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c
index dc6dc71..b6bf93f 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -262,10 +262,6 @@ NTSTATUS get_ea_names_from_file(TALLOC_CTX *mem_ctx,
 	}
 	*pnum_names = 0;
 
-	if (!lp_ea_support(SNUM(conn))) {
-		return NT_STATUS_OK;
-	}
-
 	status = refuse_symlink(conn, fsp, smb_fname);
 	if (!NT_STATUS_IS_OK(status)) {
 		/*
@@ -397,6 +393,10 @@ static NTSTATUS get_ea_list_from_file_path(TALLOC_CTX *mem_ctx,
 	*pea_total_len = 0;
 	*ea_list = NULL;
 
+	if (!lp_ea_support(SNUM(conn))) {
+		return NT_STATUS_OK;
+	}
+
 	if (fsp) {
 		posix_pathnames =
 			(fsp->fsp_name->flags & SMB_FILENAME_POSIX_PATH);
-- 
2.9.3


From 3b84e062b73ec7cec97b96fc0591241fbd6bd35f Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 2 Mar 2017 12:56:25 +0200
Subject: [PATCH 4/6] testparm: remove check for "ea support" in fruit shares

Now that ea support is not required for vfs_fruit, drop the
check that it's enabled in shares using vfs_fruit.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/utils/testparm.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/source3/utils/testparm.c b/source3/utils/testparm.c
index 3e80c39..7883bca 100644
--- a/source3/utils/testparm.c
+++ b/source3/utils/testparm.c
@@ -606,12 +606,6 @@ static void do_per_share_checks(int s)
 	vfs_objects = lp_vfs_objects(s);
 	if (vfs_objects && str_list_check(vfs_objects, "fruit")) {
 		uses_fruit = true;
-		if (!lp_ea_support(s) && !lp_ea_support(-1)) {
-			fprintf(stderr,
-				"ERROR: Service \"%s\" uses vfs_fruit, but "
-				"that requires \"ea support = yes\".\n\n",
-				lp_servicename(talloc_tos(), s));
-		}
 	} else {
 		doesnt_use_fruit = true;
 	}
-- 
2.9.3


From 852e8bb58f67c724a807a9c65f57cb8fc333d04f Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 2 Mar 2017 12:59:16 +0200
Subject: [PATCH 5/6] vfs_fruit: drop "ea support" from the manpage

Now that ea support is not required, drop that
comment from the man page.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 docs-xml/manpages/vfs_fruit.8.xml | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml
index d209a22..fbe30d3 100644
--- a/docs-xml/manpages/vfs_fruit.8.xml
+++ b/docs-xml/manpages/vfs_fruit.8.xml
@@ -45,8 +45,6 @@
 	<command>vfs_streams_xattr</command> which must be loaded
 	together with <command>vfs_fruit</command>.</para>
 
-	<para>vfs_fruit requires "ea support = yes".</para>
-
 	<para>Be careful when mixing shares with and without
 	vfs_fruit. OS X clients negotiate SMB2 AAPL protocol
 	extensions on the first tcon, so mixing shares with and
@@ -273,7 +271,6 @@
 
 <programlisting>
         <smbconfsection name="[share]"/>
-	<smbconfoption name="ea support">yes</smbconfoption>
 	<smbconfoption name="vfs objects">catia fruit streams_xattr</smbconfoption>
 	<smbconfoption name="fruit:resource">file</smbconfoption>
 	<smbconfoption name="fruit:metadata">netatalk</smbconfoption>
-- 
2.9.3


From f9897c1a1e5ed829967711372f1b5d65f839ae5b Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 2 Mar 2017 13:02:25 +0200
Subject: [PATCH 6/6] selftest: remove "ea support" from vfs_fruit-related
 setups.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 selftest/target/Samba3.pm | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index aa50f07..775dc16 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1654,7 +1654,6 @@ sub provision($$$$$$$$)
 [vfs_fruit]
 	path = $shrdir
 	vfs objects = catia fruit streams_xattr acl_xattr
-	ea support = yes
 	fruit:resource = file
 	fruit:metadata = netatalk
 	fruit:locking = netatalk
@@ -1663,26 +1662,22 @@ sub provision($$$$$$$$)
 [vfs_fruit_metadata_stream]
 	path = $shrdir
 	vfs objects = fruit streams_xattr acl_xattr
-	ea support = yes
 	fruit:resource = file
 	fruit:metadata = stream
 
 [vfs_fruit_stream_depot]
 	path = $shrdir
 	vfs objects = fruit streams_depot acl_xattr
-	ea support = yes
 	fruit:resource = stream
 	fruit:metadata = stream
 
 [vfs_wo_fruit]
 	path = $shrdir
 	vfs objects = streams_xattr acl_xattr
-	ea support = yes
 
 [vfs_wo_fruit_stream_depot]
 	path = $shrdir
 	vfs objects = streams_depot acl_xattr
-	ea support = yes
 
 [badname-tmp]
 	path = $badnames_shrdir
-- 
2.9.3



More information about the samba-technical mailing list