[PATCH] Address some -O3 issues on Ubuntu 10.04
Stefan Metzmacher
metze at samba.org
Fri Aug 5 20:16:42 UTC 2016
Am 05.08.2016 um 17:36 schrieb Jeremy Allison:
> On Thu, Aug 04, 2016 at 11:26:46PM +0200, Stefan Metzmacher wrote:
>>
>> I'm currently running some autobuilds with the following
>> patchset...
>>
>> metze
>
> Metze - autobuilds are still failing and it's been
> a week without real progress.
>
> Did your tests help at all ?
>
> I'm planning to push the patch to skip the broken
> tests today unless I hear any different.
I got distracted a bit today.
I'm currently running autobuilds with the attached patches.
metze
-------------- next part --------------
From 08d03f79de49826dc5dff3bc09193f1404e5f549 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 4 Aug 2016 12:51:24 -0700
Subject: [PATCH 1/7] s4: tests: Skip drs tests.
Please revert once the tests are fixed.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
selftest/skip | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/selftest/skip b/selftest/skip
index ba6718a..879608d 100644
--- a/selftest/skip
+++ b/selftest/skip
@@ -138,3 +138,7 @@ bench # don't run benchmarks in our selftest
^samba4.rpc.unixinfo # This contains a server-side getpwuid call which hangs the server when nss_winbindd is in use
^samba.tests.dcerpc.unix # This contains a server-side getpwuid call which hangs the server when nss_winbindd is in use
GETADDRINFO # socket wrapper doesn't support threads
+#
+# drs tests are currently broken.
+# Revert this when they are working again.
+^samba.*drs.*
--
1.9.1
From 686d39dee23543c36fa7aff76c1745fca75df6ee Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 4 Aug 2016 23:04:32 +0200
Subject: [PATCH 2/7] s4:dsdb/schema: don't change schema->schema_info on
originating schema changes.
The next reload will take care of it.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12114
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/dsdb/samdb/ldb_modules/schema_util.c | 38 +++++++++++++++-------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/source4/dsdb/samdb/ldb_modules/schema_util.c b/source4/dsdb/samdb/ldb_modules/schema_util.c
index 7402c04..47d2411 100644
--- a/source4/dsdb/samdb/ldb_modules/schema_util.c
+++ b/source4/dsdb/samdb/ldb_modules/schema_util.c
@@ -276,9 +276,7 @@ int dsdb_module_schema_info_update(struct ldb_module *ldb_module,
{
int ret;
const struct GUID *invocation_id;
- DATA_BLOB ndr_blob;
struct dsdb_schema_info *schema_info;
- const char *schema_info_str;
WERROR werr;
TALLOC_CTX *temp_ctx = talloc_new(schema);
if (temp_ctx == NULL) {
@@ -321,22 +319,26 @@ int dsdb_module_schema_info_update(struct ldb_module *ldb_module,
return ret;
}
- /* finally, update schema_info in the cache */
- werr = dsdb_blob_from_schema_info(schema_info, temp_ctx, &ndr_blob);
- if (!W_ERROR_IS_OK(werr)) {
- ldb_asprintf_errstring(ldb_module_get_ctx(ldb_module), "Failed to get schema info");
- talloc_free(temp_ctx);
- return ldb_operr(ldb_module_get_ctx(ldb_module));
- }
-
- schema_info_str = hex_encode_talloc(schema, ndr_blob.data, ndr_blob.length);
- if (!schema_info_str) {
- talloc_free(temp_ctx);
- return ldb_module_oom(ldb_module);
- }
-
- talloc_unlink(schema, discard_const(schema->schema_info));
- schema->schema_info = schema_info_str;
+ /*
+ * We don't update the schema->schema_info!
+ * as that would not represent the other information
+ * in schema->*
+ *
+ * We're not sure if the current transaction will go through!
+ * E.g. schema changes are only allowed on the schema master,
+ * otherwise they result in a UNWILLING_TO_PERFORM and a
+ *
+ * Note that schema might a global variable shared between
+ * multiple ldb_contexts. With process model "single" it
+ * means the drsuapi server also uses it.
+ *
+ * We keep it simple and just try to update the
+ * stored value.
+ *
+ * The next schema reload will pick it up, which
+ * then works for originating and replicated changes
+ * in the same way.
+ */
talloc_free(temp_ctx);
return LDB_SUCCESS;
--
1.9.1
From 3279cdca8c760a50d33d59a30c75848196b92d3b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 4 Aug 2016 11:09:21 -0700
Subject: [PATCH 3/7] s4:dsdb/repl: Ensure all error paths in
dreplsrv_op_pull_source_get_changes_trigger() are protected with tevent
returns.
Otherwise dreplsrv_op_pull_source_get_changes_trigger() could infinitely recurse.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source4/dsdb/repl/drepl_out_helpers.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c
index bf8a372..2a74cb7 100644
--- a/source4/dsdb/repl/drepl_out_helpers.c
+++ b/source4/dsdb/repl/drepl_out_helpers.c
@@ -446,6 +446,8 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
if (!W_ERROR_IS_OK(werr)) {
DEBUG(0,(__location__ ": Failed to convert UDV for %s : %s\n",
ldb_dn_get_linearized(partition->dn), win_errstr(werr)));
+ tevent_req_nterror(req, werror_to_ntstatus(werr));
+ return;
}
}
@@ -470,6 +472,7 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
status = dreplsrv_get_gc_partial_attribute_set(service, r, &pas);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0,(__location__ ": Failed to construct GC partial attribute set : %s\n", nt_errstr(status)));
+ tevent_req_nterror(req, status);
return;
}
replica_flags &= ~DRSUAPI_DRS_WRIT_REP;
@@ -482,6 +485,7 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
status = dreplsrv_get_rodc_partial_attribute_set(service, r, &pas, for_schema);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0,(__location__ ": Failed to construct RODC partial attribute set : %s\n", nt_errstr(status)));
+ tevent_req_nterror(req, status);
return;
}
replica_flags &= ~DRSUAPI_DRS_WRIT_REP;
--
1.9.1
From 7ec1c53ef69256ea0456bb1c8141843606ad7a54 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 5 Aug 2016 11:05:37 +0200
Subject: [PATCH 4/7] s4:dsdb/repl: avoid recursion after fetching schema
changes.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/dsdb/repl/drepl_out_helpers.c | 35 +++++++++++++++++++++++++----------
source4/dsdb/repl/drepl_service.h | 7 -------
2 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c
index 2a74cb7..9fe8c3b 100644
--- a/source4/dsdb/repl/drepl_out_helpers.c
+++ b/source4/dsdb/repl/drepl_out_helpers.c
@@ -241,6 +241,13 @@ struct dreplsrv_op_pull_source_state {
struct tevent_context *ev;
struct dreplsrv_out_operation *op;
void *ndr_struct_ptr;
+ /*
+ * Used when we have to re-try with a different NC, eg for
+ * EXOP retry or to get a current schema first
+ */
+ struct dreplsrv_partition_source_dsa *source_dsa_retry;
+ enum drsuapi_DsExtendedOperation extended_op_retry;
+ bool retry_started;
};
static void dreplsrv_op_pull_source_connect_done(struct tevent_req *subreq);
@@ -785,10 +792,17 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
dsdb_repl_flags,
state, &objects);
- if (W_ERROR_EQUAL(status, WERR_DS_DRA_SCHEMA_MISMATCH)
- && state->op->source_dsa_retry == NULL) {
+ if (W_ERROR_EQUAL(status, WERR_DS_DRA_SCHEMA_MISMATCH)) {
struct dreplsrv_partition *p;
+ if (state->retry_started) {
+ nt_status = werror_to_ntstatus(WERR_BAD_NET_RESP);
+ DEBUG(0,("Failed to convert objects after retry: %s/%s\n",
+ win_errstr(status), nt_errstr(nt_status)));
+ tevent_req_nterror(req, nt_status);
+ return;
+ }
+
/*
* Change info sync or extended operation into a fetch
* of the schema partition, so we get all the schema
@@ -802,14 +816,14 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
if (state->op->extended_op == DRSUAPI_EXOP_REPL_SECRET) {
- state->op->extended_op_retry = state->op->extended_op;
+ state->extended_op_retry = state->op->extended_op;
} else {
- state->op->extended_op_retry = DRSUAPI_EXOP_NONE;
+ state->extended_op_retry = DRSUAPI_EXOP_NONE;
}
state->op->extended_op = DRSUAPI_EXOP_NONE;
if (ldb_dn_compare(nc_root, partition->dn) == 0) {
- state->op->source_dsa_retry = state->op->source_dsa;
+ state->source_dsa_retry = state->op->source_dsa;
} else {
status = dreplsrv_partition_find_for_nc(service,
NULL, NULL,
@@ -825,7 +839,7 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
}
status = dreplsrv_partition_source_dsa_by_guid(p,
&state->op->source_dsa->repsFrom1->source_dsa_obj_guid,
- &state->op->source_dsa_retry);
+ &state->source_dsa_retry);
if (!W_ERROR_IS_OK(status)) {
struct GUID_txt_buf str;
@@ -867,6 +881,7 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
}
DEBUG(4,("Wrong schema when applying reply GetNCChanges, retrying\n"));
+ state->retry_started = true;
dreplsrv_op_pull_source_get_changes_trigger(req);
return;
@@ -930,10 +945,10 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
* pulling the schema, then go back and do the original
* operation once we are done.
*/
- if (state->op->source_dsa_retry != NULL) {
- state->op->source_dsa = state->op->source_dsa_retry;
- state->op->extended_op = state->op->extended_op_retry;
- state->op->source_dsa_retry = NULL;
+ if (state->source_dsa_retry != NULL) {
+ state->op->source_dsa = state->source_dsa_retry;
+ state->op->extended_op = state->extended_op_retry;
+ state->source_dsa_retry = NULL;
dreplsrv_op_pull_source_get_changes_trigger(req);
return;
}
diff --git a/source4/dsdb/repl/drepl_service.h b/source4/dsdb/repl/drepl_service.h
index 317fa87..edba4c4 100644
--- a/source4/dsdb/repl/drepl_service.h
+++ b/source4/dsdb/repl/drepl_service.h
@@ -130,13 +130,6 @@ struct dreplsrv_out_operation {
enum drsuapi_DsExtendedError extended_ret;
dreplsrv_extended_callback_t callback;
void *cb_data;
-
- /*
- * Used when we have to re-try with a different NC, eg for
- * EXOP retry or to get a current schema first
- */
- struct dreplsrv_partition_source_dsa *source_dsa_retry;
- enum drsuapi_DsExtendedOperation extended_op_retry;
};
struct dreplsrv_notify_operation {
--
1.9.1
From 6887a8e0122193cb2eb4f1a835fd13c6be1a4948 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 4 Aug 2016 10:03:14 +0200
Subject: [PATCH 5/7] s4:dsdb/schema: store struct dsdb_schema_info instead of
a hexstring
This will simplify the schema checking in future.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/dsdb/schema/schema.h | 2 +-
source4/dsdb/schema/schema_info_attr.c | 27 ++++++++++------
source4/dsdb/schema/schema_init.c | 31 +++++++++---------
source4/dsdb/schema/schema_prefixmap.c | 22 +++++++------
source4/torture/drs/unit/prefixmap_tests.c | 49 ++++++++++++++---------------
source4/torture/drs/unit/schemainfo_tests.c | 12 ++++---
6 files changed, 79 insertions(+), 64 deletions(-)
diff --git a/source4/dsdb/schema/schema.h b/source4/dsdb/schema/schema.h
index 6c3581b..6543eda 100644
--- a/source4/dsdb/schema/schema.h
+++ b/source4/dsdb/schema/schema.h
@@ -212,7 +212,7 @@ struct dsdb_schema {
* this is the content of the schemaInfo attribute of the
* Schema-Partition head object.
*/
- const char *schema_info;
+ struct dsdb_schema_info *schema_info;
struct dsdb_attribute *attributes;
struct dsdb_class *classes;
diff --git a/source4/dsdb/schema/schema_info_attr.c b/source4/dsdb/schema/schema_info_attr.c
index e113033..76cd90d 100644
--- a/source4/dsdb/schema/schema_info_attr.c
+++ b/source4/dsdb/schema/schema_info_attr.c
@@ -175,10 +175,11 @@ WERROR dsdb_blob_from_schema_info(const struct dsdb_schema_info *schema_info,
WERROR dsdb_schema_info_cmp(const struct dsdb_schema *schema,
const struct drsuapi_DsReplicaOIDMapping_Ctr *ctr)
{
- bool bres;
- DATA_BLOB blob;
- char *schema_info_str;
- struct drsuapi_DsReplicaOIDMapping *mapping;
+ TALLOC_CTX *frame = NULL;
+ DATA_BLOB blob = data_blob_null;
+ struct dsdb_schema_info *schema_info = NULL;
+ const struct drsuapi_DsReplicaOIDMapping *mapping = NULL;
+ WERROR werr;
/* we should have at least schemaInfo element */
if (ctr->num_mappings < 1) {
@@ -196,13 +197,21 @@ WERROR dsdb_schema_info_cmp(const struct dsdb_schema *schema,
return WERR_INVALID_PARAMETER;
}
- schema_info_str = hex_encode_talloc(NULL, blob.data, blob.length);
- W_ERROR_HAVE_NO_MEMORY(schema_info_str);
+ frame = talloc_stackframe();
+ werr = dsdb_schema_info_from_blob(&blob, frame, &schema_info);
+ if (!W_ERROR_IS_OK(werr)) {
+ TALLOC_FREE(frame);
+ return werr;
+ }
- bres = strequal(schema->schema_info, schema_info_str);
- talloc_free(schema_info_str);
+ if (schema->schema_info->revision != schema_info->revision) {
+ werr = WERR_DS_DRA_SCHEMA_MISMATCH;
+ } else {
+ werr = WERR_OK;
+ }
- return bres ? WERR_OK : WERR_DS_DRA_SCHEMA_MISMATCH;
+ TALLOC_FREE(frame);
+ return werr;
}
diff --git a/source4/dsdb/schema/schema_init.c b/source4/dsdb/schema/schema_init.c
index 16f213d..c9c55ca 100644
--- a/source4/dsdb/schema/schema_init.c
+++ b/source4/dsdb/schema/schema_init.c
@@ -64,7 +64,11 @@ struct dsdb_schema *dsdb_schema_copy_shallow(TALLOC_CTX *mem_ctx,
goto failed;
}
- schema_copy->schema_info = talloc_strdup(schema_copy, schema->schema_info);
+ schema_copy->schema_info = talloc(schema_copy, struct dsdb_schema_info);
+ if (!schema_copy->schema_info) {
+ goto failed;
+ }
+ *schema_copy->schema_info = *schema->schema_info;
/* copy classes and attributes*/
for (cls = schema->classes; cls; cls = cls->next) {
@@ -107,8 +111,8 @@ WERROR dsdb_load_prefixmap_from_drsuapi(struct dsdb_schema *schema,
const struct drsuapi_DsReplicaOIDMapping_Ctr *ctr)
{
WERROR werr;
- const char *schema_info;
- struct dsdb_schema_prefixmap *pfm;
+ struct dsdb_schema_info *schema_info = NULL;
+ struct dsdb_schema_prefixmap *pfm = NULL;
werr = dsdb_schema_pfm_from_drsuapi_pfm(ctr, true, schema, &pfm, &schema_info);
W_ERROR_NOT_OK_RETURN(werr);
@@ -117,7 +121,7 @@ WERROR dsdb_load_prefixmap_from_drsuapi(struct dsdb_schema *schema,
talloc_free(schema->prefixmap);
schema->prefixmap = pfm;
- talloc_free(discard_const(schema->schema_info));
+ talloc_free(schema->schema_info);
schema->schema_info = schema_info;
return WERR_OK;
@@ -169,8 +173,8 @@ WERROR dsdb_load_oid_mappings_ldb(struct dsdb_schema *schema,
const struct ldb_val *schemaInfo)
{
WERROR werr;
- const char *schema_info;
- struct dsdb_schema_prefixmap *pfm;
+ struct dsdb_schema_info *schema_info = NULL;
+ struct dsdb_schema_prefixmap *pfm = NULL;
TALLOC_CTX *mem_ctx;
/* verify schemaInfo blob is valid one */
@@ -192,19 +196,18 @@ WERROR dsdb_load_oid_mappings_ldb(struct dsdb_schema *schema,
}
/* decode schema_info */
- schema_info = hex_encode_talloc(mem_ctx,
- schemaInfo->data,
- schemaInfo->length);
- if (!schema_info) {
+ werr = dsdb_schema_info_from_blob(schemaInfo, mem_ctx, &schema_info);
+ if (!W_ERROR_IS_OK(werr)) {
+ DEBUG(0, (__location__ " dsdb_schema_info_from_blob failed: %s\n", win_errstr(werr)));
talloc_free(mem_ctx);
- return WERR_NOMEM;
+ return werr;
}
/* store prefixMap and schema_info into cached Schema */
talloc_free(schema->prefixmap);
schema->prefixmap = talloc_steal(schema, pfm);
- talloc_free(discard_const(schema->schema_info));
+ talloc_free(schema->schema_info);
schema->schema_info = talloc_steal(schema, schema_info);
/* clean up locally allocated mem */
@@ -257,8 +260,8 @@ WERROR dsdb_get_oid_mappings_ldb(const struct dsdb_schema *schema,
talloc_free(ctr);
W_ERROR_NOT_OK_RETURN(status);
- *schemaInfo = strhex_to_data_blob(mem_ctx, schema->schema_info);
- W_ERROR_HAVE_NO_MEMORY(schemaInfo->data);
+ status = dsdb_blob_from_schema_info(schema->schema_info, mem_ctx, schemaInfo);
+ W_ERROR_NOT_OK_RETURN(status);
return WERR_OK;
}
diff --git a/source4/dsdb/schema/schema_prefixmap.c b/source4/dsdb/schema/schema_prefixmap.c
index 270e6be..c5acf9c 100644
--- a/source4/dsdb/schema/schema_prefixmap.c
+++ b/source4/dsdb/schema/schema_prefixmap.c
@@ -510,7 +510,7 @@ WERROR dsdb_schema_pfm_from_drsuapi_pfm(const struct drsuapi_DsReplicaOIDMapping
bool have_schema_info,
TALLOC_CTX *mem_ctx,
struct dsdb_schema_prefixmap **_pfm,
- const char **_schema_info)
+ struct dsdb_schema_info **_schema_info)
{
WERROR werr;
uint32_t i;
@@ -561,12 +561,12 @@ WERROR dsdb_schema_pfm_from_drsuapi_pfm(const struct drsuapi_DsReplicaOIDMapping
* but set it here for clarity */
i = ctr->num_mappings - 1;
- *_schema_info = hex_encode_talloc(mem_ctx,
- ctr->mappings[i].oid.binary_oid,
- ctr->mappings[i].oid.length);
- if (!*_schema_info) {
+ blob = data_blob_const(ctr->mappings[i].oid.binary_oid,
+ ctr->mappings[i].oid.length);
+ werr = dsdb_schema_info_from_blob(&blob, mem_ctx, _schema_info);
+ if (!W_ERROR_IS_OK(werr)) {
talloc_free(pfm);
- return WERR_NOMEM;
+ return werr;
}
}
@@ -585,7 +585,7 @@ WERROR dsdb_schema_pfm_from_drsuapi_pfm(const struct drsuapi_DsReplicaOIDMapping
* \param _ctr Out pointer to drsuapi_DsReplicaOIDMapping_Ctr prefix map structure
*/
WERROR dsdb_drsuapi_pfm_from_schema_pfm(const struct dsdb_schema_prefixmap *pfm,
- const char *schema_info,
+ const struct dsdb_schema_info *schema_info,
TALLOC_CTX *mem_ctx,
struct drsuapi_DsReplicaOIDMapping_Ctr **_ctr)
{
@@ -628,14 +628,16 @@ WERROR dsdb_drsuapi_pfm_from_schema_pfm(const struct dsdb_schema_prefixmap *pfm,
/* make schema_info entry if needed */
if (schema_info) {
+ WERROR werr;
+
/* by this time, i should have this value,
* but set it here for clarity */
i = ctr->num_mappings - 1;
- blob = strhex_to_data_blob(ctr, schema_info);
- if (!blob.data) {
+ werr = dsdb_blob_from_schema_info(schema_info, ctr, &blob);
+ if (!W_ERROR_IS_OK(werr)) {
talloc_free(ctr);
- return WERR_NOMEM;
+ return werr;
}
ctr->mappings[i].id_prefix = 0;
diff --git a/source4/torture/drs/unit/prefixmap_tests.c b/source4/torture/drs/unit/prefixmap_tests.c
index 29941eb..969d641 100644
--- a/source4/torture/drs/unit/prefixmap_tests.c
+++ b/source4/torture/drs/unit/prefixmap_tests.c
@@ -26,7 +26,7 @@
#include "torture/rpc/drsuapi.h"
#include "torture/drs/proto.h"
#include "param/param.h"
-
+#include "librpc/ndr/libndr.h"
/**
* Private data to be shared among all test in Test case
@@ -36,7 +36,6 @@ struct drsut_prefixmap_data {
struct dsdb_schema_prefixmap *pfm_full;
/* default schemaInfo value to test with */
- const char *schi_default_str;
struct dsdb_schema_info *schi_default;
struct ldb_context *ldb_ctx;
@@ -539,7 +538,8 @@ static bool torture_drs_unit_pfm_oid_from_attid_check_attid(struct torture_conte
static bool torture_drs_unit_pfm_to_from_drsuapi(struct torture_context *tctx, struct drsut_prefixmap_data *priv)
{
WERROR werr;
- const char *schema_info;
+ struct dsdb_schema_info *schema_info;
+ DATA_BLOB schema_info_blob;
struct dsdb_schema_prefixmap *pfm;
struct drsuapi_DsReplicaOIDMapping_Ctr *ctr;
TALLOC_CTX *mem_ctx;
@@ -548,19 +548,20 @@ static bool torture_drs_unit_pfm_to_from_drsuapi(struct torture_context *tctx, s
torture_assert(tctx, mem_ctx, "Unexpected: Have no memory!");
/* convert Schema_prefixMap to drsuapi_prefixMap */
- werr = dsdb_drsuapi_pfm_from_schema_pfm(priv->pfm_full, priv->schi_default_str, mem_ctx, &ctr);
+ werr = dsdb_drsuapi_pfm_from_schema_pfm(priv->pfm_full, priv->schi_default, mem_ctx, &ctr);
torture_assert_werr_ok(tctx, werr, "dsdb_drsuapi_pfm_from_schema_pfm() failed");
torture_assert(tctx, ctr && ctr->mappings, "drsuapi_prefixMap not constructed correctly");
torture_assert_int_equal(tctx, ctr->num_mappings, priv->pfm_full->length + 1,
"drs_mappings count does not match");
/* look for schema_info entry - it should be the last one */
- schema_info = hex_encode_talloc(mem_ctx,
- ctr->mappings[ctr->num_mappings - 1].oid.binary_oid,
- ctr->mappings[ctr->num_mappings - 1].oid.length);
- torture_assert_str_equal(tctx,
- schema_info,
- priv->schi_default_str,
- "schema_info not stored correctly or not last entry");
+ schema_info_blob = data_blob_const(ctr->mappings[ctr->num_mappings - 1].oid.binary_oid,
+ ctr->mappings[ctr->num_mappings - 1].oid.length);
+ werr = dsdb_schema_info_from_blob(&schema_info_blob, tctx, &schema_info);
+ torture_assert_werr_ok(tctx, werr, "dsdb_schema_info_from_blob failed");
+ torture_assert_int_equal(tctx, schema_info->revision, priv->schi_default->revision,
+ "schema_info (revision) not stored correctly or not last entry");
+ torture_assert(tctx, GUID_equal(&schema_info->invocation_id, &priv->schi_default->invocation_id),
+ "schema_info (invocation_id) not stored correctly or not last entry");
/* compare schema_prefixMap and drsuapi_prefixMap */
werr = dsdb_schema_pfm_contains_drsuapi_pfm(priv->pfm_full, ctr);
@@ -569,7 +570,10 @@ static bool torture_drs_unit_pfm_to_from_drsuapi(struct torture_context *tctx, s
/* convert back drsuapi_prefixMap to schema_prefixMap */
werr = dsdb_schema_pfm_from_drsuapi_pfm(ctr, true, mem_ctx, &pfm, &schema_info);
torture_assert_werr_ok(tctx, werr, "dsdb_schema_pfm_from_drsuapi_pfm() failed");
- torture_assert_str_equal(tctx, schema_info, priv->schi_default_str, "Fetched schema_info is different");
+ torture_assert_int_equal(tctx, schema_info->revision, priv->schi_default->revision,
+ "Fetched schema_info is different (revision)");
+ torture_assert(tctx, GUID_equal(&schema_info->invocation_id, &priv->schi_default->invocation_id),
+ "Fetched schema_info is different (invocation_id)");
/* compare against the original */
if (!_torture_drs_pfm_compare_same(tctx, priv->pfm_full, pfm, true)) {
@@ -599,7 +603,6 @@ static bool torture_drs_unit_pfm_to_from_drsuapi(struct torture_context *tctx, s
static bool torture_drs_unit_pfm_to_from_ldb_val(struct torture_context *tctx, struct drsut_prefixmap_data *priv)
{
WERROR werr;
- const char *schema_info;
struct dsdb_schema *schema;
struct ldb_val pfm_ldb_val;
struct ldb_val schema_info_ldb_val;
@@ -613,7 +616,7 @@ static bool torture_drs_unit_pfm_to_from_ldb_val(struct torture_context *tctx, s
/* set priv->pfm_full as prefixMap for new schema object */
schema->prefixmap = priv->pfm_full;
- schema->schema_info = priv->schi_default_str;
+ schema->schema_info = priv->schi_default;
/* convert schema_prefixMap to ldb_val blob */
werr = dsdb_get_oid_mappings_ldb(schema, mem_ctx, &pfm_ldb_val, &schema_info_ldb_val);
@@ -622,14 +625,6 @@ static bool torture_drs_unit_pfm_to_from_ldb_val(struct torture_context *tctx, s
"pfm_ldb_val not constructed correctly");
torture_assert(tctx, schema_info_ldb_val.data && schema_info_ldb_val.length,
"schema_info_ldb_val not constructed correctly");
- /* look for schema_info entry - it should be the last one */
- schema_info = hex_encode_talloc(mem_ctx,
- schema_info_ldb_val.data,
- schema_info_ldb_val.length);
- torture_assert_str_equal(tctx,
- schema_info,
- priv->schi_default_str,
- "schema_info not stored correctly or not last entry");
/* convert pfm_ldb_val back to schema_prefixMap */
schema->prefixmap = NULL;
@@ -641,6 +636,10 @@ static bool torture_drs_unit_pfm_to_from_ldb_val(struct torture_context *tctx, s
talloc_free(mem_ctx);
return false;
}
+ torture_assert_int_equal(tctx, schema->schema_info->revision, priv->schi_default->revision,
+ "Fetched schema_info is different (revision)");
+ torture_assert(tctx, GUID_equal(&schema->schema_info->invocation_id, &priv->schi_default->invocation_id),
+ "Fetched schema_info is different (invocation_id)");
talloc_free(mem_ctx);
return true;
@@ -664,7 +663,7 @@ static bool torture_drs_unit_pfm_read_write_ldb(struct torture_context *tctx, st
torture_assert(tctx, schema, "Unexpected: failed to allocate schema object");
/* set priv->pfm_full as prefixMap for new schema object */
schema->prefixmap = priv->pfm_full;
- schema->schema_info = priv->schi_default_str;
+ schema->schema_info = priv->schi_default;
/* write prfixMap to ldb */
werr = dsdb_write_prefixes_from_schema_to_ldb(mem_ctx, priv->ldb_ctx, schema);
@@ -701,7 +700,7 @@ static bool torture_drs_unit_dsdb_create_prefix_mapping(struct torture_context *
schema = dsdb_new_schema(mem_ctx);
torture_assert(tctx, schema, "Unexpected: failed to allocate schema object");
/* set priv->pfm_full as prefixMap for new schema object */
- schema->schema_info = priv->schi_default_str;
+ schema->schema_info = priv->schi_default;
werr = _drsut_prefixmap_new(_prefixmap_test_new_data, ARRAY_SIZE(_prefixmap_test_new_data),
schema, &schema->prefixmap);
torture_assert_werr_ok(tctx, werr, "_drsut_prefixmap_new() failed");
@@ -827,8 +826,6 @@ static bool torture_drs_unit_prefixmap_setup(struct torture_context *tctx, struc
werr = dsdb_blob_from_schema_info(priv->schi_default, priv, &blob);
torture_assert_werr_ok(tctx, werr, "dsdb_blob_from_schema_info() failed");
- priv->schi_default_str = data_blob_hex_string_upper(priv, &blob);
-
/* create temporary LDB and populate with data */
if (!torture_drs_unit_ldb_setup(tctx, priv)) {
return false;
diff --git a/source4/torture/drs/unit/schemainfo_tests.c b/source4/torture/drs/unit/schemainfo_tests.c
index 2fb5308..3b492b8 100644
--- a/source4/torture/drs/unit/schemainfo_tests.c
+++ b/source4/torture/drs/unit/schemainfo_tests.c
@@ -304,6 +304,7 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx,
{
DATA_BLOB blob;
struct drsuapi_DsReplicaOIDMapping_Ctr *ctr;
+ struct dsdb_schema_info schema_info;
ctr = talloc_zero(priv, struct drsuapi_DsReplicaOIDMapping_Ctr);
torture_assert(tctx, ctr, "Not enough memory!");
@@ -354,8 +355,10 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx,
"dsdb_schema_info_cmp(): unexpected result");
/* test with correct schemaInfo, but invalid ATTID */
- blob = strhex_to_data_blob(ctr, priv->schema->schema_info);
- torture_assert(tctx, blob.data, "Not enough memory!");
+ schema_info = *priv->schema->schema_info;
+ torture_assert_werr_ok(tctx,
+ dsdb_blob_from_schema_info(&schema_info, tctx, &blob),
+ "dsdb_blob_from_schema_info() failed");
ctr->mappings[0].id_prefix = 1;
ctr->mappings[0].oid.length = blob.length;
ctr->mappings[0].oid.binary_oid = blob.data;
@@ -365,7 +368,6 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx,
"dsdb_schema_info_cmp(): unexpected result");
/* test with valid schemaInfo */
- blob = strhex_to_data_blob(ctr, priv->schema->schema_info);
ctr->mappings[0].id_prefix = 0;
torture_assert_werr_ok(tctx,
dsdb_schema_info_cmp(priv->schema, ctr),
@@ -595,7 +597,9 @@ static bool torture_drs_unit_schemainfo_setup(struct torture_context *tctx,
priv->schema = dsdb_new_schema(priv);
/* set schema_info in dsdb_schema for testing */
- priv->schema->schema_info = talloc_strdup(priv->schema, SCHEMA_INFO_DEFAULT_STR);
+ torture_assert(tctx,
+ _drsut_schemainfo_new(tctx, SCHEMA_INFO_DEFAULT_STR, &priv->schema->schema_info),
+ "Failed to create schema_info test object");
/* pre-cache invocationId for samdb_ntds_invocation_id()
* to work with our mock ldb */
--
1.9.1
From 0e68d758904ed19102ecbd527cd277bf58bb7a31 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Thu, 4 Aug 2016 11:00:34 +0200
Subject: [PATCH 6/7] s4:dsdb/schema: don't treat on older remote schema as
SCHEMA_MISMATCH
It's perfectly valid to replicate from a partner with an older schema
version, otherwise schema changes would block any other replication
until every dc in the forest has the schema changes.
The avoids an endless loop trying to get schema in sync with the partner.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12115
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source4/dsdb/schema/schema_info_attr.c | 11 ++++-
source4/torture/drs/unit/schemainfo_tests.c | 71 ++++++++++++++++++++++++++++-
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/source4/dsdb/schema/schema_info_attr.c b/source4/dsdb/schema/schema_info_attr.c
index 76cd90d..0d54012 100644
--- a/source4/dsdb/schema/schema_info_attr.c
+++ b/source4/dsdb/schema/schema_info_attr.c
@@ -204,8 +204,17 @@ WERROR dsdb_schema_info_cmp(const struct dsdb_schema *schema,
return werr;
}
- if (schema->schema_info->revision != schema_info->revision) {
+ if (schema->schema_info->revision > schema_info->revision) {
+ /*
+ * It's ok if our schema is newer than the remote one
+ */
+ werr = WERR_OK;
+ } else if (schema->schema_info->revision < schema_info->revision) {
werr = WERR_DS_DRA_SCHEMA_MISMATCH;
+ } else if (!GUID_equal(&schema->schema_info->invocation_id,
+ &schema_info->invocation_id))
+ {
+ werr = WERR_DS_DRA_SCHEMA_CONFLICT;
} else {
werr = WERR_OK;
}
diff --git a/source4/torture/drs/unit/schemainfo_tests.c b/source4/torture/drs/unit/schemainfo_tests.c
index 3b492b8..e5a1c87 100644
--- a/source4/torture/drs/unit/schemainfo_tests.c
+++ b/source4/torture/drs/unit/schemainfo_tests.c
@@ -344,14 +344,14 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx,
WERR_INVALID_PARAMETER,
"dsdb_schema_info_cmp(): unexpected result");
- /* test with valid schemaInfo, but not correct one */
+ /* test with valid schemaInfo, but older one should be ok */
blob = strhex_to_data_blob(ctr, "FF0000000000000000000000000000000000000000");
torture_assert(tctx, blob.data, "Not enough memory!");
ctr->mappings[0].oid.length = blob.length;
ctr->mappings[0].oid.binary_oid = blob.data;
torture_assert_werr_equal(tctx,
dsdb_schema_info_cmp(priv->schema, ctr),
- WERR_DS_DRA_SCHEMA_MISMATCH,
+ WERR_OK,
"dsdb_schema_info_cmp(): unexpected result");
/* test with correct schemaInfo, but invalid ATTID */
@@ -373,6 +373,73 @@ static bool test_dsdb_schema_info_cmp(struct torture_context *tctx,
dsdb_schema_info_cmp(priv->schema, ctr),
"dsdb_schema_info_cmp(): unexpected result");
+ /* test with valid schemaInfo, but older revision */
+ schema_info = *priv->schema->schema_info;
+ schema_info.revision -= 1;
+ torture_assert_werr_ok(tctx,
+ dsdb_blob_from_schema_info(&schema_info, tctx, &blob),
+ "dsdb_blob_from_schema_info() failed");
+ ctr->mappings[0].oid.length = blob.length;
+ ctr->mappings[0].oid.binary_oid = blob.data;
+ torture_assert_werr_equal(tctx,
+ dsdb_schema_info_cmp(priv->schema, ctr),
+ WERR_OK,
+ "dsdb_schema_info_cmp(): unexpected result");
+
+ /* test with valid schemaInfo, but newer revision */
+ schema_info = *priv->schema->schema_info;
+ schema_info.revision += 1;
+ torture_assert_werr_ok(tctx,
+ dsdb_blob_from_schema_info(&schema_info, tctx, &blob),
+ "dsdb_blob_from_schema_info() failed");
+ ctr->mappings[0].oid.length = blob.length;
+ ctr->mappings[0].oid.binary_oid = blob.data;
+ torture_assert_werr_equal(tctx,
+ dsdb_schema_info_cmp(priv->schema, ctr),
+ WERR_DS_DRA_SCHEMA_MISMATCH,
+ "dsdb_schema_info_cmp(): unexpected result");
+
+ /* test with valid schemaInfo, but newer revision and other invocationId */
+ schema_info = *priv->schema->schema_info;
+ schema_info.revision += 1;
+ schema_info.invocation_id.time_mid += 1;
+ torture_assert_werr_ok(tctx,
+ dsdb_blob_from_schema_info(&schema_info, tctx, &blob),
+ "dsdb_blob_from_schema_info() failed");
+ ctr->mappings[0].oid.length = blob.length;
+ ctr->mappings[0].oid.binary_oid = blob.data;
+ torture_assert_werr_equal(tctx,
+ dsdb_schema_info_cmp(priv->schema, ctr),
+ WERR_DS_DRA_SCHEMA_MISMATCH,
+ "dsdb_schema_info_cmp(): unexpected result");
+
+ /* test with valid schemaInfo, but older revision and other invocationId */
+ schema_info = *priv->schema->schema_info;
+ schema_info.revision -= 1;
+ schema_info.invocation_id.time_mid += 1;
+ torture_assert_werr_ok(tctx,
+ dsdb_blob_from_schema_info(&schema_info, tctx, &blob),
+ "dsdb_blob_from_schema_info() failed");
+ ctr->mappings[0].oid.length = blob.length;
+ ctr->mappings[0].oid.binary_oid = blob.data;
+ torture_assert_werr_equal(tctx,
+ dsdb_schema_info_cmp(priv->schema, ctr),
+ WERR_OK,
+ "dsdb_schema_info_cmp(): unexpected result");
+
+ /* test with valid schemaInfo, but same revision and other invocationId */
+ schema_info = *priv->schema->schema_info;
+ schema_info.invocation_id.time_mid += 1;
+ torture_assert_werr_ok(tctx,
+ dsdb_blob_from_schema_info(&schema_info, tctx, &blob),
+ "dsdb_blob_from_schema_info() failed");
+ ctr->mappings[0].oid.length = blob.length;
+ ctr->mappings[0].oid.binary_oid = blob.data;
+ torture_assert_werr_equal(tctx,
+ dsdb_schema_info_cmp(priv->schema, ctr),
+ WERR_DS_DRA_SCHEMA_CONFLICT,
+ "dsdb_schema_info_cmp(): unexpected result");
+
talloc_free(ctr);
return true;
}
--
1.9.1
From e49be928b313bdb3ea7625b8bdc2d7ca366a4646 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 5 Aug 2016 22:06:41 +0200
Subject: [PATCH 7/7] Revert "s4: tests: Skip drs tests."
This reverts commit 08d03f79de49826dc5dff3bc09193f1404e5f549.
---
selftest/skip | 4 ----
1 file changed, 4 deletions(-)
diff --git a/selftest/skip b/selftest/skip
index 879608d..ba6718a 100644
--- a/selftest/skip
+++ b/selftest/skip
@@ -138,7 +138,3 @@ bench # don't run benchmarks in our selftest
^samba4.rpc.unixinfo # This contains a server-side getpwuid call which hangs the server when nss_winbindd is in use
^samba.tests.dcerpc.unix # This contains a server-side getpwuid call which hangs the server when nss_winbindd is in use
GETADDRINFO # socket wrapper doesn't support threads
-#
-# drs tests are currently broken.
-# Revert this when they are working again.
-^samba.*drs.*
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160805/d18598dd/signature.sig>
More information about the samba-technical
mailing list