Attachment 8484 Details for Bug 9461 – Patch to avoid timeout on FSMO operations

[patch] Patch to avoid timeout on FSMO operations

fsmo-timeout.patch (text/plain), 7.95 KB, created by Andrew Bartlett on 2013-01-25 03:13:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2013-01-25 03:13:53 UTC

Size: 7.95 KB

patch

obsolete

>From d0b9241b6af0e394cd80f9958f00e2268f141288 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>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 <metze@samba.org>
>(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 <abartlet@samba.org>
>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 <metze@samba.org>
>(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 <abartlet@samba.org>
>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 <metze@samba.org>
>(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
>

Flags:

metze: review+

Actions: View

Attachments on bug 9461: 8279 | 8483 | 8484 | 8506 | 8874 | 9238 | 9239