Attachment 14220 Details for Bug 13446 – patches for 4.8

[patch] patches for 4.8

patches-4.8 (text/plain), 11.76 KB, created by Christof Schmitt on 2018-06-04 19:28:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Christof Schmitt

Created: 2018-06-04 19:28:43 UTC

Size: 11.76 KB

patch

obsolete

>From e5d0c01947eccafaab6930e69c8c8fabf95b6fd0 Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Wed, 23 May 2018 11:07:54 -0700
>Subject: [PATCH 1/5] selftest: Add dfq_cache share with 'dfree cache time' set
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 7ffcbd5ce1222971cb9879f78765d87cdc4102a8)
>---
> selftest/target/Samba3.pm | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
>index 52c7d3e..7b0303a 100755
>--- a/selftest/target/Samba3.pm
>+++ b/selftest/target/Samba3.pm
>@@ -2140,6 +2140,12 @@ sub provision($$$$$$$$$)
> 	vfs objects = acl_xattr fake_acls xattr_tdb fake_dfq
> 	admin users = $unix_name
> 	include = $dfqconffile
>+[dfq_cache]
>+	path = $shrdir/dfree
>+	vfs objects = acl_xattr fake_acls xattr_tdb fake_dfq
>+	admin users = $unix_name
>+	include = $dfqconffile
>+	dfree cache time = 60
> [dfq_owner]
> 	path = $shrdir/dfree
> 	vfs objects = acl_xattr fake_acls xattr_tdb fake_dfq
>-- 
>1.8.3.1
>
>
>From 1bc25ae8d35acfd7c444bf3184123204b7a193f7 Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Wed, 23 May 2018 11:25:42 -0700
>Subject: [PATCH 2/5] selftest: Add test for 'dfree cache'
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit a55b3d2fcc2f7737a2702bf908dcf1f80969bf21)
>---
> selftest/knownfail                       |  1 +
> source3/script/tests/test_dfree_quota.sh | 35 ++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 710fd33..0709b2d 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -343,3 +343,4 @@
> # Disabling NTLM means you can't use samr to change the password
> ^samba.tests.ntlmdisabled.python$ktest$.ntlmdisabled.NtlmDisabledTests.test_samr_change_password$ktest$
> ^samba.tests.ntlmdisabled.python$ad_dc_no_ntlm$.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection$ad_dc_no_ntlm$
>+^samba3.blackbox.dfree_quota $fileserver$.Test dfree cache$fileserver$
>diff --git a/source3/script/tests/test_dfree_quota.sh b/source3/script/tests/test_dfree_quota.sh
>index 6e227c4..abd82b4 100755
>--- a/source3/script/tests/test_dfree_quota.sh
>+++ b/source3/script/tests/test_dfree_quota.sh
>@@ -130,6 +130,35 @@ test_smbclient_dfree() {
> 	return $status
> }
> 
>+# Issue two queries to different directories in one session to test
>+# caching effects
>+test_smbclient_dfree_2() {
>+	name="$1"
>+	share="$2"
>+	dir1="$3"
>+	dir2="$4"
>+	confs="$5"
>+	expected="$6"
>+	subunit_start_test "$name"
>+	setup_conf $confs
>+	output=$($VALGRIND $smbclient //$SERVER/$share \
>+			   -c "cd $dir1; du; cd ..; cd $dir2 ; du" $@ 2>&1)
>+	status=$?
>+	if [ "$status" = "0" ]; then
>+		received=$(echo "$output" | \
>+				   awk '/blocks of size/ {print $1, $5, $6}' | \
>+				   tr '\n' ' ')
>+		if [ "$expected" = "$received" ]; then
>+			subunit_pass_test "$name"
>+		else
>+			echo "$output" | subunit_fail_test "$name"
>+		fi
>+	else
>+		echo "$output" | subunit_fail_test "$name"
>+	fi
>+	return $status
>+}
>+
> test_smbcquotas() {
> 	name="$1"
>     conf="$2"
>@@ -164,6 +193,12 @@ test_smbclient_dfree "Test large disk" dfq "." "conf3 ." "1125899906842624 1024.
> #basic quota test (SMB1 only)
> test_smbcquotas "Test user quota" confq1 $USERNAME "40960/4096000/3072000" -U$USERNAME%$PASSWORD --option=clientmaxprotocol=NT1 || failed=`expr $failed + 1`
> 
>+# Test dfree cache through queries in two different directories
>+test_smbclient_dfree_2 "Test dfree cache" dfq_cache "." "subdir1" \
>+		       "conf1 . conf2 subdir1" "10 1024. 5 20 1024. 10 " \
>+		       -U$USERNAME%$PASSWORD --option=clientmaxprotocol=SMB3 \
>+	|| failed=`expr $failed + 1`
>+
> #quota limit > disk size, remaining quota > disk free
> test_smbclient_dfree "Test dfree share root df vs quota case 1" dfq "." "confdfq1 ." "80 1024. 40" -U$USERNAME%$PASSWORD --option=clientmaxprotocol=SMB3 || failed=`expr $failed + 1`
> #quota limit > disk size, remaining quota < disk free
>-- 
>1.8.3.1
>
>
>From 6777e3de8c33d1e4a270ac2f7e11be4ed4d2853a Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Wed, 16 May 2018 13:05:36 -0700
>Subject: [PATCH 3/5] memcache: Add new cache type for dfree information
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit f5d05562679f6aa691b98b4a75952f7dda7ed343)
>---
> lib/util/memcache.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/lib/util/memcache.h b/lib/util/memcache.h
>index 670cbd6..4331c2f 100644
>--- a/lib/util/memcache.h
>+++ b/lib/util/memcache.h
>@@ -45,7 +45,8 @@ enum memcache_number {
> 	SINGLETON_CACHE,
> 	SMB1_SEARCH_OFFSET_MAP,
> 	SHARE_MODE_LOCK_CACHE,	/* talloc */
>-	VIRUSFILTER_SCAN_RESULTS_CACHE_TALLOC /* talloc */
>+	VIRUSFILTER_SCAN_RESULTS_CACHE_TALLOC, /* talloc */
>+	DFREE_CACHE,
> };
> 
> /*
>-- 
>1.8.3.1
>
>
>From 544beec2d7c839396459423b3d1aeebc8d5290ef Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Wed, 16 May 2018 13:17:52 -0700
>Subject: [PATCH 4/5] smbd: Cache dfree information based on query path
>
>Sub directories in a SMB share can have different free space information
>(e.g. when a different file system is mounted there). Caching the dfree
>information per SMB share will return invalid data. Address this by
>switching to memcache and store the cached data based on the query path.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 8f121747b06ca78cf51801a3931b2ddd1a424c77)
>---
> selftest/knownfail   |  1 -
> source3/smbd/dfree.c | 99 ++++++++++++++++++++++++++++++++++++++++++----------
> 2 files changed, 80 insertions(+), 20 deletions(-)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 0709b2d..710fd33 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -343,4 +343,3 @@
> # Disabling NTLM means you can't use samr to change the password
> ^samba.tests.ntlmdisabled.python$ktest$.ntlmdisabled.NtlmDisabledTests.test_samr_change_password$ktest$
> ^samba.tests.ntlmdisabled.python$ad_dc_no_ntlm$.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection$ad_dc_no_ntlm$
>-^samba3.blackbox.dfree_quota $fileserver$.Test dfree cache$fileserver$
>diff --git a/source3/smbd/dfree.c b/source3/smbd/dfree.c
>index a702d08..5b20707 100644
>--- a/source3/smbd/dfree.c
>+++ b/source3/smbd/dfree.c
>@@ -21,6 +21,7 @@
> #include "smbd/smbd.h"
> #include "smbd/globals.h"
> #include "lib/util_file.h"
>+#include "lib/util/memcache.h"
> 
> /****************************************************************************
>  Normalise for DOS usage.
>@@ -167,48 +168,108 @@ dfree_done:
> 
> /****************************************************************************
>  Potentially returned cached dfree info.
>+
>+ Depending on the file system layout and file system features, the free space
>+ information can be different for different sub directories underneath a SMB
>+ share. Store the cache information in memcache using the query path as the
>+ key to accomodate this.
> ****************************************************************************/
> 
> uint64_t get_dfree_info(connection_struct *conn, struct smb_filename *fname,
> 			uint64_t *bsize, uint64_t *dfree, uint64_t *dsize)
> {
> 	int dfree_cache_time = lp_dfree_cache_time(SNUM(conn));
>-	struct dfree_cached_info *dfc = conn->dfree_info;
>+	struct dfree_cached_info *dfc = NULL;
>+	struct dfree_cached_info dfc_new = { 0 };
> 	uint64_t dfree_ret;
>+	char tmpbuf[PATH_MAX];
>+	char *full_path = NULL;
>+	char *to_free = NULL;
>+	char *key_path = NULL;
>+	size_t len;
>+	DATA_BLOB key, value;
>+	bool found;
> 
> 	if (!dfree_cache_time) {
> 		return sys_disk_free(conn, fname, bsize, dfree, dsize);
> 	}
> 
>+	len = full_path_tos(conn->connectpath,
>+			    fname->base_name,
>+			    tmpbuf,
>+			    sizeof(tmpbuf),
>+			    &full_path,
>+			    &to_free);
>+	if (len == -1) {
>+		errno = ENOMEM;
>+		return -1;
>+	}
>+
>+	if (VALID_STAT(fname->st) && S_ISREG(fname->st.st_ex_mode)) {
>+		/*
>+		 * In case of a file use the parent directory to reduce number
>+		 * of cache entries.
>+		 */
>+		bool ok;
>+
>+		ok = parent_dirname(talloc_tos(),
>+				    full_path,
>+				    &key_path,
>+				    NULL);
>+		TALLOC_FREE(to_free); /* We're done with full_path */
>+
>+		if (!ok) {
>+			errno = ENOMEM;
>+			return -1;
>+		}
>+
>+		/*
>+		 * key_path is always a talloced object.
>+		 */
>+		to_free = key_path;
>+	} else {
>+		/*
>+		 * key_path might not be a talloced object; rely on
>+		 * to_free set from full_path_tos.
>+		 */
>+		key_path = full_path;
>+	}
>+
>+	key = data_blob_const(key_path, strlen(key_path));
>+	found = memcache_lookup(smbd_memcache(),
>+				DFREE_CACHE,
>+				key,
>+				&value);
>+	dfc = found ? (struct dfree_cached_info *)value.data : NULL;
>+
> 	if (dfc && (conn->lastused - dfc->last_dfree_time < dfree_cache_time)) {
>-		/* Return cached info. */
>+		DBG_DEBUG("Returning dfree cache entry for %s\n", key_path);
> 		*bsize = dfc->bsize;
> 		*dfree = dfc->dfree;
> 		*dsize = dfc->dsize;
>-		return dfc->dfree_ret;
>+		dfree_ret = dfc->dfree_ret;
>+		goto out;
> 	}
> 
> 	dfree_ret = sys_disk_free(conn, fname, bsize, dfree, dsize);
> 
> 	if (dfree_ret == (uint64_t)-1) {
> 		/* Don't cache bad data. */
>-		return dfree_ret;
>-	}
>-
>-	/* No cached info or time to refresh. */
>-	if (!dfc) {
>-		dfc = talloc(conn, struct dfree_cached_info);
>-		if (!dfc) {
>-			return dfree_ret;
>-		}
>-		conn->dfree_info = dfc;
>+		goto out;
> 	}
> 
>-	dfc->bsize = *bsize;
>-	dfc->dfree = *dfree;
>-	dfc->dsize = *dsize;
>-	dfc->dfree_ret = dfree_ret;
>-	dfc->last_dfree_time = conn->lastused;
>-
>+	DBG_DEBUG("Creating dfree cache entry for %s\n", key_path);
>+	dfc_new.bsize = *bsize;
>+	dfc_new.dfree = *dfree;
>+	dfc_new.dsize = *dsize;
>+	dfc_new.dfree_ret = dfree_ret;
>+	dfc_new.last_dfree_time = conn->lastused;
>+	memcache_add(smbd_memcache(),
>+		     DFREE_CACHE,
>+		     key,
>+		     data_blob_const(&dfc_new, sizeof(dfc_new)));
>+
>+out:
>+	TALLOC_FREE(to_free);
> 	return dfree_ret;
> }
>-- 
>1.8.3.1
>
>
>From bc3dd6f476567d213005c8692be6452c89155635 Mon Sep 17 00:00:00 2001
>From: Christof Schmitt <cs@samba.org>
>Date: Fri, 18 May 2018 20:51:58 -0700
>Subject: [PATCH 5/5] smbd: Flush dfree memcache on service reload
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13446
>
>Signed-off-by: Christof Schmitt <cs@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit e30d0c0e0d11f65b2d1886be3c0fe9e32eaf3926)
>---
> source3/smbd/dfree.c         | 5 +++++
> source3/smbd/proto.h         | 1 +
> source3/smbd/server_reload.c | 1 +
> 3 files changed, 7 insertions(+)
>
>diff --git a/source3/smbd/dfree.c b/source3/smbd/dfree.c
>index 5b20707..d280e1e 100644
>--- a/source3/smbd/dfree.c
>+++ b/source3/smbd/dfree.c
>@@ -273,3 +273,8 @@ out:
> 	TALLOC_FREE(to_free);
> 	return dfree_ret;
> }
>+
>+void flush_dfree_cache(void)
>+{
>+	memcache_flush(smbd_memcache(), DFREE_CACHE);
>+}
>diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
>index fe37640..9bb919d 100644
>--- a/source3/smbd/proto.h
>+++ b/source3/smbd/proto.h
>@@ -176,6 +176,7 @@ uint64_t sys_disk_free(connection_struct *conn, struct smb_filename *fname,
> 		       uint64_t *bsize, uint64_t *dfree, uint64_t *dsize);
> uint64_t get_dfree_info(connection_struct *conn, struct smb_filename *fname,
> 			uint64_t *bsize, uint64_t *dfree, uint64_t *dsize);
>+void flush_dfree_cache(void);
> 
> /* The following definitions come from smbd/dir.c  */
> 
>diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c
>index c93c077..9b62096 100644
>--- a/source3/smbd/server_reload.c
>+++ b/source3/smbd/server_reload.c
>@@ -164,6 +164,7 @@ bool reload_services(struct smbd_server_connection *sconn,
> 
> 	mangle_reset_cache();
> 	reset_stat_cache();
>+	flush_dfree_cache();
> 
> 	/* this forces service parameters to be flushed */
> 	set_current_service(NULL,0,True);
>-- 
>1.8.3.1
>

Flags:

jra: review+

Actions: View

Attachments on bug 13446: 14220 | 14221