From d0b9241b6af0e394cd80f9958f00e2268f141288 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 20 Nov 2012 14:59:17 +1100 Subject: [PATCH 1/3] drs-fsmo: Improve handling of FSMO role takeover. This needs to be more async, and give less scary errors. Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 316fd085ad2b587b82d817358240f84ae054a543) --- source4/dsdb/repl/drepl_fsmo.c | 5 ++--- source4/dsdb/samdb/ldb_modules/rootdse.c | 11 ++++++++++- source4/rpc_server/drsuapi/getncchanges.c | 3 ++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/repl/drepl_fsmo.c b/source4/dsdb/repl/drepl_fsmo.c index 0e83982..37fb684 100644 --- a/source4/dsdb/repl/drepl_fsmo.c +++ b/source4/dsdb/repl/drepl_fsmo.c @@ -108,9 +108,8 @@ NTSTATUS drepl_take_FSMO_role(struct irpc_message *msg, return NT_STATUS_OK; } - if (is_us || - (extended_op == DRSUAPI_EXOP_NONE)) { - DEBUG(0,("FSMO role check failed for DN %s and owner %s \n", + if (is_us) { + DEBUG(5,("FSMO role check failed, we already own DN %s with %s\n", ldb_dn_get_linearized(fsmo_role_dn), ldb_dn_get_linearized(role_owner_dn))); r->out.result = WERR_OK; diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c index ba71b5f..add83d2 100644 --- a/source4/dsdb/samdb/ldb_modules/rootdse.c +++ b/source4/dsdb/samdb/ldb_modules/rootdse.c @@ -1377,9 +1377,18 @@ static int rootdse_become_master(struct ldb_module *module, fsmo->ldb = ldb; fsmo->req = req; - /* we send the call asynchronously, as the ldap client is + /* + * we send the call asynchronously, as the ldap client is * expecting to get an error back if the role transfer fails + * + * We need more than the default 10 seconds IRPC allows, so + * set a longer timeout (default ldb timeout is 300 seconds). + * We send an async reply when we are done. + * + * We are the first module, so don't bother working out how + * long we have spent so far. */ + dcerpc_binding_handle_set_timeout(irpc_handle, req->timeout); treq = dcerpc_drepl_takeFSMORole_send(req, ldb_get_event_context(ldb), irpc_handle, role); if (treq == NULL) { diff --git a/source4/rpc_server/drsuapi/getncchanges.c b/source4/rpc_server/drsuapi/getncchanges.c index c3fd000..575d037 100644 --- a/source4/rpc_server/drsuapi/getncchanges.c +++ b/source4/rpc_server/drsuapi/getncchanges.c @@ -2022,7 +2022,8 @@ allowed: werr = drsuapi_UpdateRefs(b_state, mem_ctx, &ureq); if (!W_ERROR_IS_OK(werr)) { - DEBUG(0,(__location__ ": Failed UpdateRefs in DsGetNCChanges - %s\n", + DEBUG(0,(__location__ ": Failed UpdateRefs on %s for %s in DsGetNCChanges - %s\n", + drs_ObjectIdentifier_to_string(mem_ctx, ncRoot), ureq.dest_dsa_dns_name, win_errstr(werr))); } } -- 1.7.11.7 From 9b3b9ae3e1624ed65fb8689d7684b0d9a9d7e6fc Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 15 Jan 2013 09:56:46 +1100 Subject: [PATCH 2/3] dsdb: Do not hold the transaction over the IRPC call to perform a role transfer This avoids one samba process locking out another from the DB. Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 18d7e5df0eb8fb593e66daf25d142584f44b5b87) --- selftest/knownfail | 1 - source4/dsdb/samdb/ldb_modules/rootdse.c | 27 ++++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index 85634ab..84eb769 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -135,7 +135,6 @@ ^samba4.smb2.acls.*.owner ^samba4.ldap.dirsync.python.dc..__main__.ExtendedDirsyncTests.test_dirsync_deleted_items #^samba4.ldap.dirsync.python.dc..__main__.ExtendedDirsyncTests.* -^samba4.drs.fsmo.python ^samba4.libsmbclient.opendir.opendir # This requires netbios browsing ^samba4.rpc.drsuapi.*.drsuapi.DsGetDomainControllerInfo\(.*\)$ ^samba4.rpc.drsuapi.*.drsuapi.DsCrackNames\(.*\)$ diff --git a/source4/dsdb/samdb/ldb_modules/rootdse.c b/source4/dsdb/samdb/ldb_modules/rootdse.c index add83d2..eaf6451 100644 --- a/source4/dsdb/samdb/ldb_modules/rootdse.c +++ b/source4/dsdb/samdb/ldb_modules/rootdse.c @@ -1297,6 +1297,7 @@ static int rootdse_add(struct ldb_module *module, struct ldb_request *req) struct fsmo_transfer_state { struct ldb_context *ldb; struct ldb_request *req; + struct ldb_module *module; }; /* @@ -1307,6 +1308,7 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq) struct fsmo_transfer_state *fsmo = tevent_req_callback_data(treq, struct fsmo_transfer_state); NTSTATUS status; WERROR werr; + int ret; struct ldb_request *req = fsmo->req; struct ldb_context *ldb = fsmo->ldb; @@ -1314,16 +1316,31 @@ static void rootdse_fsmo_transfer_callback(struct tevent_req *treq) talloc_free(fsmo); if (!NT_STATUS_IS_OK(status)) { ldb_asprintf_errstring(ldb, "Failed FSMO transfer: %s", nt_errstr(status)); + /* + * Now that it is failed, start the transaction up + * again so the wrappers can close it without additional error + */ + ldb_next_start_trans(fsmo->module); ldb_module_done(req, NULL, NULL, LDB_ERR_UNAVAILABLE); return; } if (!W_ERROR_IS_OK(werr)) { ldb_asprintf_errstring(ldb, "Failed FSMO transfer: %s", win_errstr(werr)); + /* + * Now that it is failed, start the transaction up + * again so the wrappers can close it without additional error + */ + ldb_next_start_trans(fsmo->module); ldb_module_done(req, NULL, NULL, LDB_ERR_UNAVAILABLE); return; } - ldb_module_done(req, NULL, NULL, LDB_SUCCESS); + /* + * Now that it is done, start the transaction up again so the + * wrappers can close it without error + */ + ret = ldb_next_start_trans(fsmo->module); + ldb_module_done(req, NULL, NULL, ret); } static int rootdse_become_master(struct ldb_module *module, @@ -1358,6 +1375,13 @@ static int rootdse_become_master(struct ldb_module *module, "RODC cannot become a role master."); } + /* + * We always delete the transaction, not commit it, because + * this gives the least supprise to this supprising action (as + * we will never record anything done to this point + */ + ldb_next_del_trans(module); + msg = imessaging_client_init(tmp_ctx, lp_ctx, ldb_get_event_context(ldb)); if (!msg) { @@ -1376,6 +1400,7 @@ static int rootdse_become_master(struct ldb_module *module, } fsmo->ldb = ldb; fsmo->req = req; + fsmo->module = module; /* * we send the call asynchronously, as the ldap client is -- 1.7.11.7 From a8cf7be22e503f7f92d618671cafeecd4f0031e0 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Sat, 17 Nov 2012 13:49:00 +1100 Subject: [PATCH 3/3] torture: Fix fsmo test to use correct -H samba-tool syntax However, the test still does not pass. Reviewed-by: Stefan Metzmacher (cherry picked from commit a0faf16ae9aefc4963b2583970509b1b23e27ce1) --- source4/torture/drs/python/fsmo.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/torture/drs/python/fsmo.py b/source4/torture/drs/python/fsmo.py index f1fa6ef..8a1e9ff 100644 --- a/source4/torture/drs/python/fsmo.py +++ b/source4/torture/drs/python/fsmo.py @@ -61,8 +61,8 @@ class DrsFsmoTestCase(drs_base.DrsBaseTestCase): creds = self.get_credentials() cmd_line_auth = "-U%s/%s%%%s" % (creds.get_domain(), creds.get_username(), creds.get_password()) - # bin/samba-tool fsmo transfer --role=role --url=ldap://DC:389 - cmd_line = "%s fsmo transfer --role=%s --url=ldap://%s:389 %s" % (net_cmd, role, DC, + # bin/samba-tool fsmo transfer --role=role -H ldap://DC:389 + cmd_line = "%s fsmo transfer --role=%s -H ldap://%s:389 %s" % (net_cmd, role, DC, cmd_line_auth) ret = os.system(cmd_line) self.assertEquals(ret, 0, "Transferring role %s to %s has failed!" % (role, DC)) -- 1.7.11.7