Attachment 9186 Details for Bug 5917 – Patchset that went into master.

[patch] Patchset that went into master.

Bug5917-rodc.patchset (text/plain), 10.10 KB, created by Jeremy Allison on 2013-09-03 23:31:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2013-09-03 23:31:07 UTC

Size: 10.10 KB

patch

obsolete

>From dd12bfbcbf359c1642cc2e968aec62ae904aad5d Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 3 Sep 2013 12:13:45 -0700
>Subject: [PATCH 1/5] dsgetdcname_cache_fetch() doesn't use the site_name
> parameter so don't pass it.
>
>Bug 5917 - Samba does not work on site with Read Only Domain Controller
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Richard Sharpe <rsharpe@samba.org>
>---
> source3/libsmb/dsgetdcname.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
>index 028a31b..326f6f7 100644
>--- a/source3/libsmb/dsgetdcname.c
>+++ b/source3/libsmb/dsgetdcname.c
>@@ -320,7 +320,6 @@ static NTSTATUS dsgetdcname_cache_fetch(TALLOC_CTX *mem_ctx,
> 					const char *domain_name,
> 					const struct GUID *domain_guid,
> 					uint32_t flags,
>-					const char *site_name,
> 					struct netr_DsRGetDCNameInfo **info_p)
> {
> 	char *key;
>@@ -393,7 +392,7 @@ static NTSTATUS dsgetdcname_cached(TALLOC_CTX *mem_ctx,
> 	NTSTATUS status;
> 
> 	status = dsgetdcname_cache_fetch(mem_ctx, domain_name, domain_guid,
>-					 flags, site_name, info);
>+					 flags, info);
> 	if (!NT_STATUS_IS_OK(status)
> 	    && !NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
> 		DEBUG(10,("dsgetdcname_cached: cache fetch failed with: %s\n",
>-- 
>1.8.4
>
>
>From 66006be7ef703b2935334633d27641050cee5f58 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 3 Sep 2013 12:04:37 -0700
>Subject: [PATCH 2/5] Refactor dsgetdcname to be called via a wrapper function.
>
>Bug 5917 - Samba does not work on site with Read Only Domain Controller
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Richard Sharpe <rsharpe@samba.org>
>---
> source3/libsmb/dsgetdcname.c | 31 ++++++++++++++++++++++++++-----
> 1 file changed, 26 insertions(+), 5 deletions(-)
>
>diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
>index 326f6f7..819fa20 100644
>--- a/source3/libsmb/dsgetdcname.c
>+++ b/source3/libsmb/dsgetdcname.c
>@@ -1093,12 +1093,10 @@ static bool is_closest_site(struct netr_DsRGetDCNameInfo *info)
> }
> 
> /********************************************************************
>- dsgetdcname.
>-
>- This will be the only public function here.
>+ Internal dsgetdcname.
> ********************************************************************/
> 
>-NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
>+static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
> 		     struct messaging_context *msg_ctx,
> 		     const char *domain_name,
> 		     const struct GUID *domain_guid,
>@@ -1112,7 +1110,7 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
> 	bool first = true;
> 	struct netr_DsRGetDCNameInfo *first_info = NULL;
> 
>-	DEBUG(10,("dsgetdcname: domain_name: %s, "
>+	DEBUG(10,("dsgetdcname_internal: domain_name: %s, "
> 		  "domain_guid: %s, site_name: %s, flags: 0x%08x\n",
> 		  domain_name,
> 		  domain_guid ? GUID_string(mem_ctx, domain_guid) : "(null)",
>@@ -1174,3 +1172,26 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
> 	*info = myinfo;
> 	return NT_STATUS_OK;
> }
>+
>+/********************************************************************
>+ dsgetdcname.
>+
>+ This will be the only public function here.
>+********************************************************************/
>+
>+NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
>+		     struct messaging_context *msg_ctx,
>+		     const char *domain_name,
>+		     const struct GUID *domain_guid,
>+		     const char *site_name,
>+		     uint32_t flags,
>+		     struct netr_DsRGetDCNameInfo **info)
>+{
>+	return dsgetdcname_internal(mem_ctx,
>+				msg_ctx,
>+				domain_name,
>+				domain_guid,
>+				site_name,
>+				flags,
>+				info);
>+}
>-- 
>1.8.4
>
>
>From 181c11066bd53b07015a199f56eb71182e89ff71 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 3 Sep 2013 12:08:46 -0700
>Subject: [PATCH 3/5] Move the manipulation of site_name into the caller
> function dsgetdcname().
>
>Leave dsgetdcname_internal() only using const char *site_name.
>
>Bug 5917 - Samba does not work on site with Read Only Domain Controller
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Richard Sharpe <rsharpe@samba.org>
>---
> source3/libsmb/dsgetdcname.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
>diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
>index 819fa20..cad2128 100644
>--- a/source3/libsmb/dsgetdcname.c
>+++ b/source3/libsmb/dsgetdcname.c
>@@ -1106,7 +1106,6 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
> {
> 	NTSTATUS status = NT_STATUS_DOMAIN_CONTROLLER_NOT_FOUND;
> 	struct netr_DsRGetDCNameInfo *myinfo = NULL;
>-	char *query_site = NULL;
> 	bool first = true;
> 	struct netr_DsRGetDCNameInfo *first_info = NULL;
> 
>@@ -1114,7 +1113,7 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
> 		  "domain_guid: %s, site_name: %s, flags: 0x%08x\n",
> 		  domain_name,
> 		  domain_guid ? GUID_string(mem_ctx, domain_guid) : "(null)",
>-		  site_name, flags));
>+		  site_name ? site_name : "(null)", flags));
> 
> 	*info = NULL;
> 
>@@ -1123,18 +1122,12 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
> 		return NT_STATUS_INVALID_PARAMETER;
> 	}
> 
>-	if ((site_name == NULL) || (site_name[0] == '\0')) {
>-		query_site = sitename_fetch(domain_name);
>-	} else {
>-		query_site = SMB_STRDUP(site_name);
>-	}
>-
> 	if (flags & DS_FORCE_REDISCOVERY) {
> 		goto rediscover;
> 	}
> 
> 	status = dsgetdcname_cached(mem_ctx, msg_ctx, domain_name, domain_guid,
>-				    flags, query_site, &myinfo);
>+				    flags, site_name, &myinfo);
> 	if (NT_STATUS_IS_OK(status)) {
> 		goto done;
> 	}
>@@ -1145,12 +1138,10 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
> 
>  rediscover:
> 	status = dsgetdcname_rediscover(mem_ctx, msg_ctx, domain_name,
>-					domain_guid, flags, query_site,
>+					domain_guid, flags, site_name,
> 					&myinfo);
> 
>  done:
>-	SAFE_FREE(query_site);
>-
> 	if (!NT_STATUS_IS_OK(status)) {
> 		if (!first) {
> 			*info = first_info;
>@@ -1165,7 +1156,7 @@ static NTSTATUS dsgetdcname_internal(TALLOC_CTX *mem_ctx,
> 		first = false;
> 		first_info = myinfo;
> 		/* TODO: may use the next_closest_site here */
>-		query_site = SMB_STRDUP(myinfo->client_site_name);
>+		site_name = myinfo->client_site_name;
> 		goto rediscover;
> 	}
> 
>@@ -1187,11 +1178,24 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
> 		     uint32_t flags,
> 		     struct netr_DsRGetDCNameInfo **info)
> {
>-	return dsgetdcname_internal(mem_ctx,
>+	NTSTATUS status;
>+	char *query_site = NULL;
>+
>+	if ((site_name == NULL) || (site_name[0] == '\0')) {
>+		query_site = sitename_fetch(domain_name);
>+	} else {
>+		query_site = SMB_STRDUP(site_name);
>+	}
>+
>+	status = dsgetdcname_internal(mem_ctx,
> 				msg_ctx,
> 				domain_name,
> 				domain_guid,
>-				site_name,
>+				query_site,
> 				flags,
> 				info);
>+
>+	SAFE_FREE(query_site);
>+
>+	return status;
> }
>-- 
>1.8.4
>
>
>From 68e7b1c9446c7d1274b0fb85b59b90ac1a7f6041 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 3 Sep 2013 12:20:52 -0700
>Subject: [PATCH 4/5] Move the retry logic when site_name is passed in a NULL
> or "" to the wrapper function.
>
>Bug 5917 - Samba does not work on site with Read Only Domain Controller
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: Richard Sharpe <rsharpe@samba.org>
>---
> source3/libsmb/dsgetdcname.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
>diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
>index cad2128..bdaa250 100644
>--- a/source3/libsmb/dsgetdcname.c
>+++ b/source3/libsmb/dsgetdcname.c
>@@ -1179,12 +1179,14 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
> 		     struct netr_DsRGetDCNameInfo **info)
> {
> 	NTSTATUS status;
>-	char *query_site = NULL;
>+	const char *query_site = NULL;
>+	char *ptr_to_free = NULL;
> 
> 	if ((site_name == NULL) || (site_name[0] == '\0')) {
>-		query_site = sitename_fetch(domain_name);
>+		ptr_to_free = sitename_fetch(domain_name);
>+		query_site = ptr_to_free;
> 	} else {
>-		query_site = SMB_STRDUP(site_name);
>+		query_site = site_name;
> 	}
> 
> 	status = dsgetdcname_internal(mem_ctx,
>@@ -1195,7 +1197,22 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
> 				flags,
> 				info);
> 
>-	SAFE_FREE(query_site);
>+	SAFE_FREE(ptr_to_free);
>+
>+	if (!NT_STATUS_EQUAL(status, NT_STATUS_DOMAIN_CONTROLLER_NOT_FOUND)) {
>+		return status;
>+	}
>+
>+	/* Should we try again with site_name == NULL ? */
>+	if ((site_name == NULL) || (site_name[0] == '\0')) {
>+		status = dsgetdcname_internal(mem_ctx,
>+					msg_ctx,
>+					domain_name,
>+					domain_guid,
>+					NULL,
>+					flags,
>+					info);
>+	}
> 
> 	return status;
> }
>-- 
>1.8.4
>
>
>From bdab6f9431715fbfd28f8cc0dfb4dde2966f22f3 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 3 Sep 2013 14:07:43 -0700
>Subject: [PATCH 5/5] Optimization. Don't do the retry logic if
> sitename_fetch() returned NULL, we already did a NULL query.
>
>Bug 5917 - Samba does not work on site with Read Only Domain Controller
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Wed Sep  4 01:19:05 CEST 2013 on sn-devel-104
>---
> source3/libsmb/dsgetdcname.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/source3/libsmb/dsgetdcname.c b/source3/libsmb/dsgetdcname.c
>index bdaa250..6818b01 100644
>--- a/source3/libsmb/dsgetdcname.c
>+++ b/source3/libsmb/dsgetdcname.c
>@@ -1181,9 +1181,13 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
> 	NTSTATUS status;
> 	const char *query_site = NULL;
> 	char *ptr_to_free = NULL;
>+	bool retry_query_with_null = false;
> 
> 	if ((site_name == NULL) || (site_name[0] == '\0')) {
> 		ptr_to_free = sitename_fetch(domain_name);
>+		if (ptr_to_free != NULL) {
>+			retry_query_with_null = true;
>+		}
> 		query_site = ptr_to_free;
> 	} else {
> 		query_site = site_name;
>@@ -1204,7 +1208,7 @@ NTSTATUS dsgetdcname(TALLOC_CTX *mem_ctx,
> 	}
> 
> 	/* Should we try again with site_name == NULL ? */
>-	if ((site_name == NULL) || (site_name[0] == '\0')) {
>+	if (retry_query_with_null) {
> 		status = dsgetdcname_internal(mem_ctx,
> 					msg_ctx,
> 					domain_name,
>-- 
>1.8.4
>

Actions: View

Attachments on bug 5917: 3756 | 3757 | 3758 | 3759 | 3760 | 3761 | 3762 | 9178 | 9179 | 9182 | 9183 | 9184 | 9186 | 9187