[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Tue May 23 03:19:03 UTC 2017


The branch, master has been updated
       via  82bb44d dsdb: Do not search the sam.ldb file when trying to search all partitions
       via  5f0e53f dsdb: Do not write the @INDEXLIST or @ATTRIBUTES records during schema refresh
       via  b8ba010 dsdb: Take out the transaction and prepare_commit locks in the same order
       via  c23b191 ldb_tdb: Call talloc_free(options_dn) as soon as we are done with options_dn
       via  496fecc ldb_tdb: Provide better debugging on end_trans failures
       via  a2a1962 ldb_tdb: Provide better debugging on prepare_commit failures
       via  1fd8ec2 tdb: Improve debugging when the allrecord lock fails to upgrade
      from  4c234a9 s3: smbd: Correctly identify a snapshot path using UCF_GMT_PATHNAME.

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


- Log -----------------------------------------------------------------
commit 82bb44dd3b7f42b90494294b32f8413a39cb2030
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu May 4 10:12:46 2017 +0200

    dsdb: Do not search the sam.ldb file when trying to search all partitions
    
    The sam.ldb file does not contain the same kind of data as the partitions, we do not wish to
    mix these results.  This also avoids taking out locks on the sam.ldb file.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Tue May 23 05:18:04 CEST 2017 on sn-devel-144

commit 5f0e53f1b90369c649688122c0a8742352f13877
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 3 22:53:14 2017 +0200

    dsdb: Do not write the @INDEXLIST or @ATTRIBUTES records during schema refresh
    
    Instead, write it once in the module init, if required, and after a
    modify to the schema partition is detected
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit b8ba0103bf45670c31384c56d6cd63bbef760a0c
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 3 22:51:30 2017 +0200

    dsdb: Take out the transaction and prepare_commit locks in the same order
    
    We must, when starting the transaction and preparing to commit the transaction, take
    out the locks in the same order across all the databases, otherwise we may deadlock.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit c23b191ac4d5d8486f1ad807fb55b7da25e772f4
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu Mar 30 13:11:15 2017 +1300

    ldb_tdb: Call talloc_free(options_dn) as soon as we are done with options_dn
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 496feccad4a8b1e07d6cf441e2835462945b3c61
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu Mar 30 14:27:55 2017 +1300

    ldb_tdb: Provide better debugging on end_trans failures
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit a2a1962dc083346fb53b93eab54ca5c3636b15d3
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu Mar 30 14:26:23 2017 +1300

    ldb_tdb: Provide better debugging on prepare_commit failures
    
    ltdb_index_transaction_commit() does LDB operations, sets the ldb
    error string and returns LDB errors so we should not check the tdb
    error code.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

commit 1fd8ec23c769d15a2de8ad6c96698e92a91b3f64
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu Mar 30 19:11:06 2017 +1300

    tdb: Improve debugging when the allrecord lock fails to upgrade
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Garming Sam <garming at catalyst.net.nz>

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

Summary of changes:
 lib/ldb/ldb_tdb/ldb_cache.c                  |  1 +
 lib/ldb/ldb_tdb/ldb_tdb.c                    | 23 ++++++++++++----
 lib/tdb/common/transaction.c                 | 21 ++++++++++++---
 source4/dsdb/samdb/ldb_modules/partition.c   | 40 +++++++++++++++++-----------
 source4/dsdb/samdb/ldb_modules/schema_load.c | 33 ++++++++++++++++++++---
 source4/dsdb/schema/schema_set.c             | 13 ++++++---
 source4/dsdb/schema/tests/schema_syntax.c    |  2 +-
 source4/libnet/libnet_vampire.c              |  2 +-
 source4/torture/drs/drs_util.c               |  2 +-
 9 files changed, 101 insertions(+), 36 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/ldb/ldb_tdb/ldb_cache.c b/lib/ldb/ldb_tdb/ldb_cache.c
index 5ea09d6..388b461 100644
--- a/lib/ldb/ldb_tdb/ldb_cache.c
+++ b/lib/ldb/ldb_tdb/ldb_cache.c
@@ -385,6 +385,7 @@ int ltdb_cache_load(struct ldb_module *module)
 	if (options_dn == NULL) goto failed;
 
 	r= ltdb_search_dn1(module, options_dn, options, 0);
+	talloc_free(options_dn);
 	if (r != LDB_SUCCESS && r != LDB_ERR_NO_SUCH_OBJECT) {
 		goto failed;
 	}
diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index c0d7a1a..3dfd1cc 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -1121,6 +1121,7 @@ static int ltdb_start_trans(struct ldb_module *module)
 
 static int ltdb_prepare_commit(struct ldb_module *module)
 {
+	int ret;
 	void *data = ldb_module_get_private(module);
 	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
 
@@ -1128,15 +1129,21 @@ static int ltdb_prepare_commit(struct ldb_module *module)
 		return LDB_SUCCESS;
 	}
 
-	if (ltdb_index_transaction_commit(module) != 0) {
+	ret = ltdb_index_transaction_commit(module);
+	if (ret != LDB_SUCCESS) {
 		tdb_transaction_cancel(ltdb->tdb);
 		ltdb->in_transaction--;
-		return ltdb_err_map(tdb_error(ltdb->tdb));
+		return ret;
 	}
 
 	if (tdb_transaction_prepare_commit(ltdb->tdb) != 0) {
+		ret = ltdb_err_map(tdb_error(ltdb->tdb));
 		ltdb->in_transaction--;
-		return ltdb_err_map(tdb_error(ltdb->tdb));
+		ldb_asprintf_errstring(ldb_module_get_ctx(module),
+				       "Failure during tdb_transaction_prepare_commit(): %s -> %s",
+				       tdb_errorstr(ltdb->tdb),
+				       ldb_strerror(ret));
+		return ret;
 	}
 
 	ltdb->prepared_commit = true;
@@ -1146,11 +1153,12 @@ static int ltdb_prepare_commit(struct ldb_module *module)
 
 static int ltdb_end_trans(struct ldb_module *module)
 {
+	int ret;
 	void *data = ldb_module_get_private(module);
 	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
 
 	if (!ltdb->prepared_commit) {
-		int ret = ltdb_prepare_commit(module);
+		ret = ltdb_prepare_commit(module);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
@@ -1160,7 +1168,12 @@ static int ltdb_end_trans(struct ldb_module *module)
 	ltdb->prepared_commit = false;
 
 	if (tdb_transaction_commit(ltdb->tdb) != 0) {
-		return ltdb_err_map(tdb_error(ltdb->tdb));
+		ret = ltdb_err_map(tdb_error(ltdb->tdb));
+		ldb_asprintf_errstring(ldb_module_get_ctx(module),
+				       "Failure during tdb_transaction_commit(): %s -> %s",
+				       tdb_errorstr(ltdb->tdb),
+				       ldb_strerror(ret));
+		return ret;
 	}
 
 	return LDB_SUCCESS;
diff --git a/lib/tdb/common/transaction.c b/lib/tdb/common/transaction.c
index f1050a2..420e754 100644
--- a/lib/tdb/common/transaction.c
+++ b/lib/tdb/common/transaction.c
@@ -986,10 +986,23 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
 
 	/* upgrade the main transaction lock region to a write lock */
 	if (tdb_allrecord_upgrade(tdb) == -1) {
-		TDB_LOG((tdb, TDB_DEBUG_ERROR,
-			"tdb_transaction_prepare_commit: "
-			"failed to upgrade hash locks: %s\n",
-			 tdb_errorstr(tdb)));
+		if (tdb->ecode == TDB_ERR_RDONLY && tdb->read_only) {
+			TDB_LOG((tdb, TDB_DEBUG_ERROR,
+				 "tdb_transaction_prepare_commit: "
+				 "failed to upgrade hash locks: "
+				 "database is read only\n"));
+		} else if (tdb->ecode == TDB_ERR_RDONLY
+			   && tdb->traverse_read) {
+			TDB_LOG((tdb, TDB_DEBUG_ERROR,
+				 "tdb_transaction_prepare_commit: "
+				 "failed to upgrade hash locks: "
+				 "a database traverse is in progress\n"));
+		} else {
+			TDB_LOG((tdb, TDB_DEBUG_ERROR,
+				 "tdb_transaction_prepare_commit: "
+				 "failed to upgrade hash locks: %s\n",
+				 tdb_errorstr(tdb)));
+		}
 		_tdb_transaction_cancel(tdb);
 		return -1;
 	}
diff --git a/source4/dsdb/samdb/ldb_modules/partition.c b/source4/dsdb/samdb/ldb_modules/partition.c
index b501ff1..08a959c 100644
--- a/source4/dsdb/samdb/ldb_modules/partition.c
+++ b/source4/dsdb/samdb/ldb_modules/partition.c
@@ -372,7 +372,7 @@ static int partition_call_first(struct partition_context *ac)
 }
 
 /**
- * Send a request down to all the partitions
+ * Send a request down to all the partitions (but not the sam.ldb file)
  */
 static int partition_send_all(struct ldb_module *module, 
 			      struct partition_context *ac, 
@@ -381,10 +381,8 @@ static int partition_send_all(struct ldb_module *module,
 	unsigned int i;
 	struct partition_private_data *data = talloc_get_type(ldb_module_get_private(module),
 							      struct partition_private_data);
-	int ret = partition_prep_request(ac, NULL);
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
+	int ret;
+
 	for (i=0; data && data->partitions && data->partitions[i]; i++) {
 		ret = partition_prep_request(ac, data->partitions[i]);
 		if (ret != LDB_SUCCESS) {
@@ -815,6 +813,8 @@ static int partition_start_trans(struct ldb_module *module)
 	if (ldb_module_flags(ldb_module_get_ctx(module)) & LDB_FLG_ENABLE_TRACING) {
 		ldb_debug(ldb_module_get_ctx(module), LDB_DEBUG_TRACE, "partition_start_trans() -> (metadata partition)");
 	}
+
+	/* This order must match that in prepare_commit() */
 	ret = ldb_next_start_trans(module);
 	if (ret != LDB_SUCCESS) {
 		return ret;
@@ -826,12 +826,6 @@ static int partition_start_trans(struct ldb_module *module)
 		return ret;
 	}
 
-	ret = partition_metadata_start_trans(module);
-	if (ret != LDB_SUCCESS) {
-		ldb_next_del_trans(module);
-		return ret;
-	}
-
 	for (i=0; data && data->partitions && data->partitions[i]; i++) {
 		if ((module && ldb_module_flags(ldb_module_get_ctx(module)) & LDB_FLG_ENABLE_TRACING)) {
 			ldb_debug(ldb_module_get_ctx(module), LDB_DEBUG_TRACE, "partition_start_trans() -> %s",
@@ -849,6 +843,20 @@ static int partition_start_trans(struct ldb_module *module)
 		}
 	}
 
+	/*
+	 * Because in prepare_commit this must come last, to ensure
+	 * lock ordering we have to do this last here also 
+	 */
+	ret = partition_metadata_start_trans(module);
+	if (ret != LDB_SUCCESS) {
+		/* Back it out, if it fails on one */
+		for (i--; i >= 0; i--) {
+			ldb_next_del_trans(data->partitions[i]->module);
+		}
+		ldb_next_del_trans(module);
+		return ret;
+	}
+
 	data->in_transaction++;
 
 	return LDB_SUCCESS;
@@ -862,6 +870,11 @@ static int partition_prepare_commit(struct ldb_module *module)
 							      struct partition_private_data);
 	int ret;
 
+	ret = ldb_next_prepare_commit(module);
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
 	for (i=0; data && data->partitions && data->partitions[i]; i++) {
 		if ((module && ldb_module_flags(ldb_module_get_ctx(module)) & LDB_FLG_ENABLE_TRACING)) {
 			ldb_debug(ldb_module_get_ctx(module), LDB_DEBUG_TRACE, "partition_prepare_commit() -> %s",
@@ -880,11 +893,6 @@ static int partition_prepare_commit(struct ldb_module *module)
 		ldb_debug(ldb_module_get_ctx(module), LDB_DEBUG_TRACE, "partition_prepare_commit() -> (metadata partition)");
 	}
 
-	ret = ldb_next_prepare_commit(module);
-	if (ret != LDB_SUCCESS) {
-		return ret;
-	}
-
 	/* metadata prepare commit must come last, as other partitions could modify
 	 * the database inside the prepare commit method of a module */
 	return partition_metadata_prepare_commit(module);
diff --git a/source4/dsdb/samdb/ldb_modules/schema_load.c b/source4/dsdb/samdb/ldb_modules/schema_load.c
index d9814e2..6ffa465 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_load.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_load.c
@@ -246,7 +246,7 @@ static struct dsdb_schema *dsdb_schema_refresh(struct ldb_module *module, struct
 		return schema;
 	}
 
-	ret = dsdb_set_schema(ldb, new_schema);
+	ret = dsdb_set_schema(ldb, new_schema, false);
 	if (ret != LDB_SUCCESS) {
 		ldb_debug_set(ldb, LDB_DEBUG_FATAL,
 			      "dsdb_set_schema() failed: %d:%s: %s",
@@ -417,7 +417,7 @@ static int schema_load_init(struct ldb_module *module)
 		}
 
 		/* "dsdb_set_schema()" steals schema into the ldb_context */
-		ret = dsdb_set_schema(ldb, new_schema);
+		ret = dsdb_set_schema(ldb, new_schema, false);
 		if (ret != LDB_SUCCESS) {
 			ldb_debug_set(ldb, LDB_DEBUG_FATAL,
 				      "schema_load_init: dsdb_set_schema() failed: %d:%s: %s",
@@ -449,6 +449,17 @@ static int schema_load_init(struct ldb_module *module)
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
+	/* Now check the @INDEXLIST is correct, or fix it up */
+	ret = dsdb_schema_set_indices_and_attributes(ldb, schema,
+						     true);
+	if (ret != LDB_SUCCESS) {
+		ldb_asprintf_errstring(ldb, "Failed to update "
+				       "@INDEXLIST and @ATTRIBUTES "
+				       "records to match database schema: %s",
+				       ldb_errstring(ldb));
+		return ret;
+	}
+
 	return ret;
 }
 
@@ -504,13 +515,27 @@ static int schema_load_del_transaction(struct ldb_module *module)
 static int schema_load_extended(struct ldb_module *module, struct ldb_request *req)
 {
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
-
+	struct dsdb_schema *schema;
+	int ret;
+	
 	if (strcmp(req->op.extended.oid, DSDB_EXTENDED_SCHEMA_UPDATE_NOW_OID) != 0) {
 		return ldb_next_request(module, req);
 	}
 	/* Force a refresh */
-	dsdb_get_schema(ldb, NULL);
+	schema = dsdb_get_schema(ldb, NULL);
 
+	ret = dsdb_schema_set_indices_and_attributes(ldb,
+						     schema,
+						     true);
+
+	if (ret != LDB_SUCCESS) {
+		ldb_asprintf_errstring(ldb, "Failed to write new "
+				       "@INDEXLIST and @ATTRIBUTES "
+				       "records for updated schema: %s",
+				       ldb_errstring(ldb));
+		return ret;
+	}
+	
 	/* Pass to next module, the partition one should finish the chain */
 	return ldb_next_request(module, req);
 }
diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
index 2e688d0..d3e3cc8 100644
--- a/source4/dsdb/schema/schema_set.c
+++ b/source4/dsdb/schema/schema_set.c
@@ -56,7 +56,9 @@ const struct ldb_schema_attribute *dsdb_attribute_handler_override(struct ldb_co
  * are required so we can operate on a schema-less database (say the
  * backend during emergency fixes) and during the schema load.
  */
-static int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb, struct dsdb_schema *schema, bool write_indices_and_attributes)
+int dsdb_schema_set_indices_and_attributes(struct ldb_context *ldb,
+					   struct dsdb_schema *schema,
+					   bool write_indices_and_attributes)
 {
 	int ret = LDB_SUCCESS;
 	struct ldb_result *res;
@@ -468,7 +470,9 @@ int dsdb_set_schema_refresh_function(struct ldb_context *ldb,
  * Attach the schema to an opaque pointer on the ldb,
  * so ldb modules can find it
  */
-int dsdb_set_schema(struct ldb_context *ldb, struct dsdb_schema *schema)
+int dsdb_set_schema(struct ldb_context *ldb,
+		    struct dsdb_schema *schema,
+		    bool write_indices_and_attributes)
 {
 	struct dsdb_schema *old_schema;
 	int ret;
@@ -493,7 +497,8 @@ int dsdb_set_schema(struct ldb_context *ldb, struct dsdb_schema *schema)
 	talloc_steal(ldb, schema);
 
 	/* Set the new attributes based on the new schema */
-	ret = dsdb_schema_set_indices_and_attributes(ldb, schema, true);
+	ret = dsdb_schema_set_indices_and_attributes(ldb, schema,
+						     write_indices_and_attributes);
 	if (ret != LDB_SUCCESS) {
 		return ret;
 	}
@@ -884,7 +889,7 @@ WERROR dsdb_set_schema_from_ldif(struct ldb_context *ldb,
 		}
 	}
 
-	ret = dsdb_set_schema(ldb, schema);
+	ret = dsdb_set_schema(ldb, schema, true);
 	if (ret != LDB_SUCCESS) {
 		status = WERR_FOOBAR;
 		goto failed;
diff --git a/source4/dsdb/schema/tests/schema_syntax.c b/source4/dsdb/schema/tests/schema_syntax.c
index 419dc3d..cb4d802 100644
--- a/source4/dsdb/schema/tests/schema_syntax.c
+++ b/source4/dsdb/schema/tests/schema_syntax.c
@@ -78,7 +78,7 @@ static bool torture_syntax_add_OR_Name(struct torture_context *tctx,
 	ldb_ldif_read_free(ldb, ldif);
 	torture_assert_werr_ok(tctx, werr, "dsdb_set_attribute_from_ldb() failed!");
 
-	ldb_res = dsdb_set_schema(ldb, schema);
+	ldb_res = dsdb_set_schema(ldb, schema, true);
 	torture_assert_int_equal(tctx, ldb_res, LDB_SUCCESS, "dsdb_set_schema() failed");
 
 	return true;
diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c
index f6bcf29..7f25a3a 100644
--- a/source4/libnet/libnet_vampire.c
+++ b/source4/libnet/libnet_vampire.c
@@ -365,7 +365,7 @@ static WERROR libnet_vampire_cb_apply_schema(struct libnet_vampire_cb_state *s,
 	 * attach the schema we just brought over DRS to the ldb,
 	 * so we can use it in dsdb_convert_object_ex below
 	 */
-	ret = dsdb_set_schema(s->ldb, s->self_made_schema);
+	ret = dsdb_set_schema(s->ldb, s->self_made_schema, true);
 	if (ret != LDB_SUCCESS) {
 		DEBUG(0,("Failed to attach working schema from DRS.\n"));
 		return WERR_INTERNAL_ERROR;
diff --git a/source4/torture/drs/drs_util.c b/source4/torture/drs/drs_util.c
index 7809e67..7c073c9 100644
--- a/source4/torture/drs/drs_util.c
+++ b/source4/torture/drs/drs_util.c
@@ -158,7 +158,7 @@ bool drs_util_dsdb_schema_load_ldb(struct torture_context *tctx,
 
 	talloc_free(res);
 
-	ret = dsdb_set_schema(ldb, ldap_schema);
+	ret = dsdb_set_schema(ldb, ldap_schema, true);
 	if (ret != LDB_SUCCESS) {
 		torture_fail(tctx,
 			     talloc_asprintf(tctx, "dsdb_set_schema() failed: %s", ldb_strerror(ret)));


-- 
Samba Shared Repository



More information about the samba-cvs mailing list