[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Wed May 30 05:04:02 UTC 2018


The branch, master has been updated
       via  0196318 dsdb: Add log when ignoring a replicated object outside of partition
       via  5714995 selftest: Use samba.tests.create_test_ou() in replica_sync tests
       via  1eed8c0 selftest: Use samba.tests.create_test_ou() in repl_move tests
       via  6cce06f selftest: Make create_test_ou() return a ldb.Dn
       via  ede668e dsdb partition.c: Make partition_copy_all aysnc.
       via  4e2eb56 ldb: Release ldb 1.4.0
       via  439072d selftest: Add test to show that sam.ldb does not do a full scan in startup
       via  e99c199 ldb: Add tests for when we should expect a full scan
       via  88ae60e ldb: One-level search was incorrectly falling back to full DB scan
       via  9e143ee ldb: Explain why an entry can vanish from the index
       via  3632775 ldb: Indicate that the ltdb_dn_list_sort() in list_union is a bit subtle.
       via  d02cd23 ldb: Save a copy of the index result before calling the callbacks.
       via  41d8c56 subtree_rename: Correct comments
       via  d346e2e dsdb: Remove sort from subtree_delete and add comments.
       via  16a0582 selftest: Lock down the expected parents in BasicTreeDeleteTests
       via  273c55e selftest: Rework BasicDeleteTests.test_all() into setUp() and a test
       via  2dedd49 samldb: Explain why the odd error code is expected.
       via  95a9dbd samldb: Add useful error string to explain why a group may not be deleted.
       via  642dd37 tests: Fix intermittent error in PSO test
       via  5ea1114 repl_meta_data: Cope with the strange but unusual case of isDeleted: FALSE in replmd_process_linked_attribute()
       via  9564adb repl_meta_data: Remove el_count from replmd_delete_internals()
       via  400abe8 s4-repl: Try to give more information in the error codes for prepare_commit failure.
       via  04e3c4b ldb: Reset error string before running prepare_commit() hook
      from  46d1278 vfs_fruit: delete 0 byte size streams if AAPL is enabled

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 0196318a1d027f04ed27d7a05dbefa9ded863b24
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Thu May 24 15:27:45 2018 +1200

    dsdb: Add log when ignoring a replicated object outside of partition
    
    This is probably a note-worthy event for debugging purposes.
    
    (Found while developing the domain rename functionality)
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>
    Pair-Programmed-With: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Wed May 30 07:03:51 CEST 2018 on sn-devel-144

commit 57149959277b92d7a97a3932a2809946a1e1cc8a
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu May 24 14:35:15 2018 +1200

    selftest: Use samba.tests.create_test_ou() in replica_sync tests
    
    This may avoid some flapping tests by ensuring that each part of this
    test runs in a unique namespace, no matter what may be left behind
    or revived via replication.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 1eed8c079daef1d2c1ad2c952edecf3efe4fa140
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 29 09:37:27 2018 +1200

    selftest: Use samba.tests.create_test_ou() in repl_move tests
    
    This may avoid some flapping tests by ensuring that each part of this
    test runs in a unique namespace, no matter what may be left behind
    or revived via replication.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 6cce06f3be7bf8c76298ba643691baec32d8c87a
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu May 24 20:28:13 2018 +1200

    selftest: Make create_test_ou() return a ldb.Dn
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit ede668e8e24c86f0836dfa5740e76d8aca1e0824
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Mon May 21 14:31:57 2018 +1200

    dsdb partition.c: Make partition_copy_all aysnc.
    
    partition_copy_all uses ldb_wait to wait for the update to the primary
    partition to complete, when updating a special dn.  If a module higher
    up the chain inserts a callback, the code blocks in ldb_wait and does
    not complete.  This change replaces the ldb_wait logic with a callback.
    
    Currently there is no code that triggers this bug, however the up coming
    audit logging changes do trigger this bug.
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 4e2eb5660a11cea215d39495844aa76ffb5a1a2e
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue Mar 20 12:58:02 2018 +1300

    ldb: Release ldb 1.4.0
    
    * New LMDB backend (experimental)
    * Comprehensive tests for index behaviour
    * Enforce transactions for writes
    * Enforce read lock use for all reads
    * Fix memory leak in paged_results module.
      We hold at most 10 outstanding paged result cookies
      (bug #13362)
    * Fix compiler warnings
    * Python3 improvements
    * Restore --disable-python build
    * Fix for performance regression on one-level searches
      (bug #13448)
    * Samba's subtree_rename could fail to rename some entries
      (bug #13452)
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 439072d1997cc94b63d269e2760d70c7b1855f2f
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 23 17:31:03 2018 +1200

    selftest: Add test to show that sam.ldb does not do a full scan in startup
    
    We should add some other more complex operations here.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13448
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit e99c199d811e607e7867e7b40d82a1642226c647
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 23 17:15:38 2018 +1200

    ldb: Add tests for when we should expect a full scan
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13448
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 88ae60ed186c9c479722ad62d65a07d0c2e71469
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 29 10:04:29 2018 +1200

    ldb: One-level search was incorrectly falling back to full DB scan
    
    When no search filter is specified, the code falls back to using
    '(|(objectClass=*)(distinguishedName=*)'. ltdb_index_dn() then failed
    because matching against '*' is not indexed. The error return then
    caused the code to fallback to a full-scan of the DB, which could have a
    considerable performance hit.
    
    Instead, we want to continue on and do the ltdb_index_filter() over the
    indexed results that were returned.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13448
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 9e143ee9b9f7be53c193cee3153f64c4dedc07e9
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon May 28 14:12:52 2018 +1200

    ldb: Explain why an entry can vanish from the index
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 3632775d7ad31e06437ed76b8731d9895930caa1
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon May 28 13:02:16 2018 +1200

    ldb: Indicate that the ltdb_dn_list_sort() in list_union is a bit subtle.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit d02cd236dcbd8a44ecc85d1f7e95a48c95c0a479
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon May 28 13:01:18 2018 +1200

    ldb: Save a copy of the index result before calling the callbacks.
    
    Otherwise Samba modules like subtree_rename can fail as they modify the
    index during the callback.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13452
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 41d8c563089ac67863c7e48df6a8c92a5724a1be
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon May 28 11:17:34 2018 +1200

    subtree_rename: Correct comments
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit d346e2ee6b154a0dcf4072a3fc1fd6007369a69d
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon May 28 10:56:21 2018 +1200

    dsdb: Remove sort from subtree_delete and add comments.
    
    The sort was written back when the module did not operate recursivly
    over the tree. Now it is just confusing, so replace with useful
    comments.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 16a0582644800fecb52adad05f43014df5607314
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon May 28 09:28:36 2018 +1200

    selftest: Lock down the expected parents in BasicTreeDeleteTests
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13448
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 273c55e94912013160ffeb6394e4666d59a683fb
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon May 28 08:44:51 2018 +1200

    selftest: Rework BasicDeleteTests.test_all() into setUp() and a test
    
    This will allow running multiple tests against the same tree.  This tree
    is very similar to the tree produced by the KCC test that simply does a
    tree_delete, and I want to lock down the tree_delete behaviour.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13448
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 2dedd49ca3041970ffe02f9adf69fbbb3cee7a4c
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 29 10:40:56 2018 +1200

    samldb: Explain why the odd error code is expected.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 95a9dbd1febe9d2db9b900945fa1d7dc08f33058
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 29 10:39:39 2018 +1200

    samldb: Add useful error string to explain why a group may not be deleted.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 642dd37d51d8fb69b04ec444a590550b1807a0d8
Author: Tim Beale <timbeale at catalyst.net.nz>
Date:   Tue May 29 10:46:50 2018 +1200

    tests: Fix intermittent error in PSO test
    
    Deleting a group fails if the primaryGroupID of a user is set to that of
    the group. This can happen in the PSO tests, as we don't clear the
    primaryGroupID before cleaning up. Normally it seems to work OK, but
    this is relying purely on the subtree delete order.
    
    Update the test to clear the primaryGroupID before the tearDown is
    called, to make things more robust.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13448
    
    Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 5ea111471aff23c66ab9a62b4e3a4db25b7f5738
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 30 10:06:54 2018 +1200

    repl_meta_data: Cope with the strange but unusual case of isDeleted: FALSE in replmd_process_linked_attribute()
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=13448
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 9564adb66fb91ab32ee9249409498989ce725286
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 29 16:52:14 2018 +1200

    repl_meta_data: Remove el_count from replmd_delete_internals()
    
    Instead, use the actual found attribute (less error prone).
    
    This is an attempt to fix:
    
    ./source4/dsdb/repl/replicated_objects.c:945 Failed to prepare commit of transaction:
    attribute isDeleted: invalid modify flags on CN=g1_1527558311141,CN=Users,DC=samba,DC=example,DC=com: 0x0
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 400abe837c43953bc6e94e98941c4d9a23d365f2
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 29 16:50:16 2018 +1200

    s4-repl: Try to give more information in the error codes for prepare_commit failure.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 04e3c4bea2d15aecd88e318573fe563c722dbc28
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 29 16:14:45 2018 +1200

    ldb: Reset error string before running prepare_commit() hook
    
    This ensures that the error string returned to the caller reflects a failure in this call.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

-----------------------------------------------------------------------

Summary of changes:
 lib/ldb/ABI/{ldb-1.3.2.sigs => ldb-1.4.0.sigs}     |   0
 ...b-util.py3-1.3.2.sigs => pyldb-util-1.4.0.sigs} |   0
 ...il.py3-1.3.2.sigs => pyldb-util.py3-1.4.0.sigs} |   0
 lib/ldb/common/ldb.c                               |   2 +
 lib/ldb/ldb_tdb/ldb_index.c                        | 112 ++++++--
 lib/ldb/ldb_tdb/ldb_search.c                       |  11 +-
 lib/ldb/ldb_tdb/ldb_tdb.c                          |  16 ++
 lib/ldb/ldb_tdb/ldb_tdb.h                          |   6 +
 lib/ldb/tests/ldb_mod_op_test.c                    | 275 ++++++++++++++++++
 lib/ldb/tests/python/api.py                        | 100 ++++++-
 lib/ldb/wscript                                    |   2 +-
 python/samba/tests/__init__.py                     |   2 +-
 python/samba/tests/dsdb.py                         |  19 ++
 python/samba/tests/dsdb_schema_attributes.py       |  13 +
 source4/dsdb/repl/replicated_objects.c             |  11 +-
 source4/dsdb/samdb/ldb_modules/partition.c         | 203 ++++++++++++--
 source4/dsdb/samdb/ldb_modules/repl_meta_data.c    |  71 ++++-
 source4/dsdb/samdb/ldb_modules/samldb.c            |  18 +-
 source4/dsdb/samdb/ldb_modules/subtree_delete.c    |  25 +-
 source4/dsdb/samdb/ldb_modules/subtree_rename.c    |  15 +-
 source4/dsdb/tests/python/deletetest.py            | 307 +++++++++++++++------
 source4/dsdb/tests/python/password_settings.py     |   7 +-
 source4/torture/drs/python/getncchanges.py         |   3 +-
 source4/torture/drs/python/repl_move.py            |  92 ++++--
 source4/torture/drs/python/replica_sync.py         |  75 ++---
 25 files changed, 1147 insertions(+), 238 deletions(-)
 copy lib/ldb/ABI/{ldb-1.3.2.sigs => ldb-1.4.0.sigs} (100%)
 copy lib/ldb/ABI/{pyldb-util.py3-1.3.2.sigs => pyldb-util-1.4.0.sigs} (100%)
 copy lib/ldb/ABI/{pyldb-util.py3-1.3.2.sigs => pyldb-util.py3-1.4.0.sigs} (100%)


Changeset truncated at 500 lines:

diff --git a/lib/ldb/ABI/ldb-1.3.2.sigs b/lib/ldb/ABI/ldb-1.4.0.sigs
similarity index 100%
copy from lib/ldb/ABI/ldb-1.3.2.sigs
copy to lib/ldb/ABI/ldb-1.4.0.sigs
diff --git a/lib/ldb/ABI/pyldb-util.py3-1.3.2.sigs b/lib/ldb/ABI/pyldb-util-1.4.0.sigs
similarity index 100%
copy from lib/ldb/ABI/pyldb-util.py3-1.3.2.sigs
copy to lib/ldb/ABI/pyldb-util-1.4.0.sigs
diff --git a/lib/ldb/ABI/pyldb-util.py3-1.3.2.sigs b/lib/ldb/ABI/pyldb-util.py3-1.4.0.sigs
similarity index 100%
copy from lib/ldb/ABI/pyldb-util.py3-1.3.2.sigs
copy to lib/ldb/ABI/pyldb-util.py3-1.4.0.sigs
diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index 2249089..5525e70 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -425,6 +425,8 @@ int ldb_transaction_prepare_commit(struct ldb_context *ldb)
 		return LDB_SUCCESS;
 	}
 
+	ldb_reset_err_string(ldb);
+
 	status = next_module->ops->prepare_commit(next_module);
 	if (status != LDB_SUCCESS) {
 		ldb->transaction_active--;
diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index cd64e69..7bd843e 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -1310,6 +1310,9 @@ static bool list_union(struct ldb_context *ldb,
 	/*
 	 * Sort the lists (if not in GUID DN mode) so we can do
 	 * the de-duplication during the merge
+	 *
+	 * NOTE: This can sort the in-memory index values, as list or
+	 * list2 might not be a copy!
 	 */
 	ltdb_dn_list_sort(ltdb, list);
 	ltdb_dn_list_sort(ltdb, list2);
@@ -1717,26 +1720,61 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 			     uint32_t *match_count,
 			     enum key_truncation scope_one_truncation)
 {
-	struct ldb_context *ldb;
+	struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
 	struct ldb_message *msg;
 	struct ldb_message *filtered_msg;
 	unsigned int i;
+	unsigned int num_keys = 0;
 	uint8_t previous_guid_key[LTDB_GUID_KEY_SIZE] = {};
+	TDB_DATA *keys = NULL;
+
+	/*
+	 * We have to allocate the key list (rather than just walk the
+	 * caller supplied list) as the callback could change the list
+	 * (by modifying an indexed attribute hosted in the in-memory
+	 * index cache!)
+	 */
+	keys = talloc_array(ac, TDB_DATA, dn_list->count);
+	if (keys == NULL) {
+		return ldb_module_oom(ac->module);
+	}
+
+	if (ltdb->cache->GUID_index_attribute != NULL) {
+		/*
+		 * We speculate that the keys will be GUID based and so
+		 * pre-fill in enough space for a GUID (avoiding a pile of
+		 * small allocations)
+		 */
+		struct guid_tdb_key {
+			uint8_t guid_key[LTDB_GUID_KEY_SIZE];
+		} *key_values = NULL;
 
-	ldb = ldb_module_get_ctx(ac->module);
+		key_values = talloc_array(keys,
+					  struct guid_tdb_key,
+					  dn_list->count);
+
+		for (i = 0; i < dn_list->count; i++) {
+			keys[i].dptr = key_values[i].guid_key;
+			keys[i].dsize = sizeof(key_values[i].guid_key);
+		}
+		if (key_values == NULL) {
+			return ldb_module_oom(ac->module);
+		}
+	} else {
+		for (i = 0; i < dn_list->count; i++) {
+			keys[i].dptr = NULL;
+			keys[i].dsize = 0;
+		}
+	}
 
 	for (i = 0; i < dn_list->count; i++) {
-		uint8_t guid_key[LTDB_GUID_KEY_SIZE];
-		TDB_DATA tdb_key = {
-			.dptr = guid_key,
-			.dsize = sizeof(guid_key)
-		};
 		int ret;
-		bool matched;
 
-		ret = ltdb_idx_to_key(ac->module, ltdb,
-				      ac, &dn_list->dn[i],
-				      &tdb_key);
+		ret = ltdb_idx_to_key(ac->module,
+				      ltdb,
+				      keys,
+				      &dn_list->dn[i],
+				      &keys[num_keys]);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
@@ -1753,30 +1791,42 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 			 * LDB_FLAG_INTERNAL_DISABLE_SINGLE_VALUE_CHECK
 			 */
 
-			if (memcmp(previous_guid_key, tdb_key.dptr,
+			if (memcmp(previous_guid_key,
+				   keys[num_keys].dptr,
 				   sizeof(previous_guid_key)) == 0) {
 				continue;
 			}
 
-			memcpy(previous_guid_key, tdb_key.dptr,
+			memcpy(previous_guid_key,
+			       keys[num_keys].dptr,
 			       sizeof(previous_guid_key));
 		}
+		num_keys++;
+	}
 
+
+	/*
+	 * Now that the list is a safe copy, send the callbacks
+	 */
+	for (i = 0; i < num_keys; i++) {
+		int ret;
+		bool matched;
 		msg = ldb_msg_new(ac);
 		if (!msg) {
 			return LDB_ERR_OPERATIONS_ERROR;
 		}
 
-
 		ret = ltdb_search_key(ac->module, ltdb,
-				      tdb_key, msg,
+				      keys[i], msg,
 				      LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC|
 				      LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC);
-		if (tdb_key.dptr != guid_key) {
-			TALLOC_FREE(tdb_key.dptr);
-		}
 		if (ret == LDB_ERR_NO_SUCH_OBJECT) {
-			/* the record has disappeared? yes, this can happen */
+			/*
+			 * the record has disappeared? yes, this can
+			 * happen if the entry is deleted by something
+			 * operating in the callback (not another
+			 * process, as we have a read lock)
+			 */
 			talloc_free(msg);
 			continue;
 		}
@@ -1834,6 +1884,7 @@ static int ltdb_index_filter(struct ltdb_private *ltdb,
 		(*match_count)++;
 	}
 
+	TALLOC_FREE(keys);
 	return LDB_SUCCESS;
 }
 
@@ -1943,20 +1994,21 @@ int ltdb_search_indexed(struct ltdb_context *ac, uint32_t *match_count)
 			}
 			/*
 			 * Here we load the index for the tree.
+			 *
+			 * We only care if this is successful, if the
+			 * index can't trim the result list down then
+			 * the ONELEVEL index is still good enough.
 			 */
 			ret = ltdb_index_dn(ac->module, ltdb, ac->tree,
 					    idx_one_tree_list);
-			if (ret != LDB_SUCCESS) {
-				talloc_free(idx_one_tree_list);
-				talloc_free(dn_list);
-				return ret;
-			}
-
-			if (!list_intersect(ldb, ltdb,
-					    dn_list, idx_one_tree_list)) {
-				talloc_free(idx_one_tree_list);
-				talloc_free(dn_list);
-				return LDB_ERR_OPERATIONS_ERROR;
+			if (ret == LDB_SUCCESS) {
+				if (!list_intersect(ldb, ltdb,
+						    dn_list,
+						    idx_one_tree_list)) {
+					talloc_free(idx_one_tree_list);
+					talloc_free(dn_list);
+					return LDB_ERR_OPERATIONS_ERROR;
+				}
 			}
 		}
 		break;
diff --git a/lib/ldb/ldb_tdb/ldb_search.c b/lib/ldb/ldb_tdb/ldb_search.c
index 35583a1..cfc3714 100644
--- a/lib/ldb/ldb_tdb/ldb_search.c
+++ b/lib/ldb/ldb_tdb/ldb_search.c
@@ -806,7 +806,7 @@ int ltdb_search(struct ltdb_context *ctx)
 		 * callback error */
 		if ( ! ctx->request_terminated && ret != LDB_SUCCESS) {
 			/* Not indexed, so we need to do a full scan */
-			if (ltdb->warn_unindexed) {
+			if (ltdb->warn_unindexed || ltdb->disable_full_db_scan) {
 				/* useful for debugging when slow performance
 				 * is caused by unindexed searches */
 				char *expression = ldb_filter_from_tree(ctx, ctx->tree);
@@ -819,6 +819,7 @@ int ltdb_search(struct ltdb_context *ctx)
 
 				talloc_free(expression);
 			}
+
 			if (match_count != 0) {
 				/* the indexing code gave an error
 				 * after having returned at least one
@@ -831,6 +832,14 @@ int ltdb_search(struct ltdb_context *ctx)
 				ltdb->kv_ops->unlock_read(module);
 				return LDB_ERR_OPERATIONS_ERROR;
 			}
+
+			if (ltdb->disable_full_db_scan) {
+				ldb_set_errstring(ldb,
+						  "ldb FULL SEARCH disabled");
+				ltdb->kv_ops->unlock_read(module);
+				return LDB_ERR_INAPPROPRIATE_MATCHING;
+			}
+
 			ret = ltdb_search_full(ctx);
 			if (ret != LDB_SUCCESS) {
 				ldb_set_errstring(ldb, "Indexed and full searches both failed!\n");
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index f9bc35c..8581604 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -2279,6 +2279,22 @@ int init_store(struct ltdb_private *ltdb,
 		}
 	}
 
+	/*
+	 * Override full DB scans
+	 *
+	 * A full DB scan is expensive on a large database.  This
+	 * option is for testing to show that the full DB scan is not
+	 * triggered.
+	 */
+	{
+		const char *len_str =
+			ldb_options_find(ldb, options,
+					 "disable_full_db_scan_for_self_test");
+		if (len_str != NULL) {
+			ltdb->disable_full_db_scan = true;
+		}
+	}
+
 	return LDB_SUCCESS;
 }
 
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.h b/lib/ldb/ldb_tdb/ldb_tdb.h
index 46af8a1..2896c63 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.h
+++ b/lib/ldb/ldb_tdb/ldb_tdb.h
@@ -79,6 +79,12 @@ struct ltdb_private {
 	unsigned max_key_length;
 
 	/*
+	 * To allow testing that ensures the DB does not fall back
+	 * to a full scan
+	 */
+	bool disable_full_db_scan;
+
+	/*
 	 * The PID that opened this database so we don't work in a
 	 * fork()ed child.
 	 */
diff --git a/lib/ldb/tests/ldb_mod_op_test.c b/lib/ldb/tests/ldb_mod_op_test.c
index 7b6f19c..01667af 100644
--- a/lib/ldb/tests/ldb_mod_op_test.c
+++ b/lib/ldb/tests/ldb_mod_op_test.c
@@ -2487,6 +2487,269 @@ static void test_ldb_modify_before_ldb_wait(void **state)
 	assert_int_equal(res2->count, 1);
 }
 
+/*
+ * This test is also complex.
+ * The purpose is to test if a modify can occur during an ldb_search()
+ * This would be a failure if if in process
+ * (1) and (2):
+ *  - (1) ltdb_search() starts and calls back for one entry
+ *  - (2) one of the entries to be matched is modified
+ *  - (1) the indexed search tries to return the modified entry, but
+ *        it is no longer found, either:
+ *          - despite it still matching (dn changed)
+ *          - it no longer matching (attrs changed)
+ *
+ * We also try un-indexed to show that the behaviour differs on this
+ * point, which it should not (an index should only impact search
+ * speed).
+ */
+
+/*
+ * This purpose of this callback is to trigger a write in the callback
+ * so as to change in in-memory index code while looping over the
+ * index result.
+ */
+
+static int test_ldb_callback_modify_during_search_callback1(struct ldb_request *req,
+						   struct ldb_reply *ares)
+{
+	int ret;
+	struct modify_during_search_test_ctx *ctx = req->context;
+	struct ldb_dn *dn = NULL, *new_dn = NULL;
+	TALLOC_CTX *tmp_ctx = talloc_new(ctx->test_ctx);
+	struct ldb_message *msg = NULL;
+
+	assert_non_null(tmp_ctx);
+
+	switch (ares->type) {
+	case LDB_REPLY_ENTRY:
+	{
+		const struct ldb_val *cn_val
+			= ldb_dn_get_component_val(ares->message->dn, 0);
+		const char *cn = (char *)cn_val->data;
+		ctx->res_count++;
+		if (strcmp(cn, "test_search_cn") == 0) {
+			ctx->got_cn = true;
+		} else if (strcmp(cn, "test_search_2_cn") == 0) {
+			ctx->got_2_cn = true;
+		}
+		if (ctx->res_count == 2) {
+			return LDB_SUCCESS;
+		}
+		break;
+	}
+	case LDB_REPLY_REFERRAL:
+		return LDB_SUCCESS;
+
+	case LDB_REPLY_DONE:
+		return ldb_request_done(req, LDB_SUCCESS);
+	}
+
+	if (ctx->rename) {
+		if (ctx->got_2_cn) {
+			/* Modify this one */
+			dn = ldb_dn_new_fmt(tmp_ctx,
+					    ctx->test_ctx->ldb,
+					    "cn=test_search_2_cn,%s",
+					    ldb_dn_get_linearized(ctx->basedn));
+		} else {
+			dn = ldb_dn_new_fmt(tmp_ctx,
+					    ctx->test_ctx->ldb,
+					    "cn=test_search_cn,%s",
+					    ldb_dn_get_linearized(ctx->basedn));
+		}
+		assert_non_null(dn);
+
+		new_dn = ldb_dn_new_fmt(tmp_ctx,
+					ctx->test_ctx->ldb,
+					"cn=test_search_cn_renamed,"
+					"dc=not_search_test_entry");
+		assert_non_null(new_dn);
+
+		ret = ldb_rename(ctx->test_ctx->ldb, dn, new_dn);
+		assert_int_equal(ret, 0);
+
+	} else {
+		if (ctx->got_2_cn) {
+			/* Delete this one */
+			dn = ldb_dn_new_fmt(tmp_ctx,
+					    ctx->test_ctx->ldb,
+					    "cn=test_search_2_cn,%s",
+					    ldb_dn_get_linearized(ctx->basedn));
+		} else {
+			dn = ldb_dn_new_fmt(tmp_ctx,
+					    ctx->test_ctx->ldb,
+					    "cn=test_search_cn,%s",
+					    ldb_dn_get_linearized(ctx->basedn));
+		}
+		assert_non_null(dn);
+
+		ret = ldb_delete(ctx->test_ctx->ldb, dn);
+		assert_int_equal(ret, 0);
+	}
+
+	/*
+	 * Now fill in the position we just removed from the
+	 * index to ensure we fail the test (otherwise we just read
+	 * past the end of the array and find the value we wanted to
+	 * skip)
+	 */
+	msg = ldb_msg_new(tmp_ctx);
+	assert_non_null(msg);
+
+	/* We deliberatly use ou= not cn= here */
+	msg->dn = ldb_dn_new_fmt(msg,
+				 ctx->test_ctx->ldb,
+				 "ou=test_search_cn_extra,%s",
+				 ldb_dn_get_linearized(ctx->basedn));
+
+	ret = ldb_msg_add_string(msg,
+				 "objectUUID",
+				 "0123456789abcde3");
+
+	ret = ldb_add(ctx->test_ctx->ldb,
+		      msg);
+	assert_int_equal(ret, LDB_SUCCESS);
+
+	TALLOC_FREE(tmp_ctx);
+	return LDB_SUCCESS;
+}
+
+static void test_ldb_callback_modify_during_search(void **state, bool add_index,
+					  bool rename)
+{
+	struct search_test_ctx *search_test_ctx = talloc_get_type_abort(*state,
+			struct search_test_ctx);
+	struct modify_during_search_test_ctx
+		ctx =
+		{ .res_count = 0,
+		  .test_ctx = search_test_ctx->ldb_test_ctx,
+		  .rename = rename
+		};
+
+	int ret;
+	struct ldb_request *req;
+
+	ret = ldb_transaction_start(search_test_ctx->ldb_test_ctx->ldb);
+	assert_int_equal(ret, 0);
+
+	if (add_index) {
+		struct ldb_message *msg;
+		struct ldb_dn *indexlist = ldb_dn_new(search_test_ctx,
+						      search_test_ctx->ldb_test_ctx->ldb,
+						      "@INDEXLIST");
+		assert_non_null(indexlist);
+
+		msg = ldb_msg_new(search_test_ctx);
+		assert_non_null(msg);
+
+		msg->dn = indexlist;
+
+		ret = ldb_msg_add_string(msg, "@IDXONE", "1");
+		assert_int_equal(ret, LDB_SUCCESS);
+		ret = ldb_msg_add_string(msg, "@IDXATTR", "cn");
+		assert_int_equal(ret, LDB_SUCCESS);
+		ret = ldb_add(search_test_ctx->ldb_test_ctx->ldb,
+			      msg);
+		if (ret == LDB_ERR_ENTRY_ALREADY_EXISTS) {
+			msg->elements[0].flags = LDB_FLAG_MOD_ADD;
+			msg->elements[1].flags = LDB_FLAG_MOD_ADD;
+			ret = ldb_modify(search_test_ctx->ldb_test_ctx->ldb,
+					 msg);
+		}
+		assert_int_equal(ret, LDB_SUCCESS);
+
+		/*
+		 * Now bring the IDXONE index into memory by modifying
+		 * it.  This exposes an issue in ldb_tdb
+		 */
+		msg = ldb_msg_new(search_test_ctx);
+		assert_non_null(msg);
+
+		msg->dn = ldb_dn_new_fmt(search_test_ctx,
+					 search_test_ctx->ldb_test_ctx->ldb,
+					 "cn=test_search_cn_extra,%s",
+					 search_test_ctx->base_dn);
+
+		ret = ldb_msg_add_string(msg,
+					 "objectUUID",
+					 "0123456789abcde2");
+
+		ret = ldb_add(search_test_ctx->ldb_test_ctx->ldb,
+			      msg);
+		assert_int_equal(ret, LDB_SUCCESS);
+
+		ret = ldb_delete(search_test_ctx->ldb_test_ctx->ldb,
+				 msg->dn);
+		assert_int_equal(ret, LDB_SUCCESS);
+	}
+
+	tevent_loop_allow_nesting(search_test_ctx->ldb_test_ctx->ev);
+
+	ctx.basedn
+		= ldb_dn_new_fmt(search_test_ctx,
+				 search_test_ctx->ldb_test_ctx->ldb,
+				 "%s",
+				 search_test_ctx->base_dn);
+	assert_non_null(ctx.basedn);
+
+
+	/*


-- 
Samba Shared Repository



More information about the samba-cvs mailing list