The Samba-Bugzilla – Attachment 14221 Details for
Bug 13446
dfree cache returning incorrect data for sub directory mounts
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patches for 4.7
patches-4.7 (text/plain), 11.66 KB, created by
Christof Schmitt
on 2018-06-04 19:57:46 UTC
(
hide
)
Description:
patches for 4.7
Filename:
MIME Type:
Creator:
Christof Schmitt
Created:
2018-06-04 19:57:46 UTC
Size:
11.66 KB
patch
obsolete
>From 7f5bd96fa5aef1a93866516a9bff78f515bb5720 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 299d55b..39a8f49 100755 >--- a/selftest/target/Samba3.pm >+++ b/selftest/target/Samba3.pm >@@ -2007,6 +2007,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 1c28d939debc35de76f355817459199267eeeb7d 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 dd23c7d..74b7092 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -345,3 +345,4 @@ > ^samba.tests.ntlmauth.python\(ktest\).ntlmauth.NtlmAuthTests.test_ntlm_connection\(ktest\) > # Disabling NTLM means you can't use samr to change the password > ^samba.tests.ntlmauth.python\(ktest\).ntlmauth.NtlmAuthTests.test_samr_change_password\(ktest\) >+^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 2c33ff1dca94b2e51778e89aea4c6b5e79646454 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 b87746b..c6a5b86 100644 >--- a/lib/util/memcache.h >+++ b/lib/util/memcache.h >@@ -44,7 +44,8 @@ enum memcache_number { > SINGLETON_CACHE_TALLOC, /* talloc */ > SINGLETON_CACHE, > SMB1_SEARCH_OFFSET_MAP, >- SHARE_MODE_LOCK_CACHE /* talloc */ >+ SHARE_MODE_LOCK_CACHE, /* talloc */ >+ DFREE_CACHE, > }; > > /* >-- >1.8.3.1 > > >From 77ad10f52cfa39f36f10441d93de7ba964f4465d 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 74b7092..dd23c7d 100644 >--- a/selftest/knownfail >+++ b/selftest/knownfail >@@ -345,4 +345,3 @@ > ^samba.tests.ntlmauth.python\(ktest\).ntlmauth.NtlmAuthTests.test_ntlm_connection\(ktest\) > # Disabling NTLM means you can't use samr to change the password > ^samba.tests.ntlmauth.python\(ktest\).ntlmauth.NtlmAuthTests.test_samr_change_password\(ktest\) >-^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 cfdb0da7d0b494a1a45453a4a2f1a76d980e0a85 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 a688341..d814a30 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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
jra
:
review+
Actions:
View
Attachments on
bug 13446
:
14220
| 14221