From Steve Danneman @ Isilon.

Attached is the companion patch to
(037b9689d9042a398cb91e4628a82fcdfa913c21), which
made handling of WINBINDD_LIST_GROUPS asynchronous.

Because most all of the list_groups code was reusable, I abstracted it,
and implemented both list_groups and list_users on top of it.

On my large test domain a "wbinfo -u" call went from 70 seconds to 30
seconds with this patch.  Plus, the parent process is no longer blocked
from receiving new requests during that time.

Steven Danneman | Software Development Engineer
Isilon Systems    P +1-206-315-7500     F +1-206-315-7501
www.isilon.com
(This used to be commit 5188f2861137ff06d5399561d55d7d00c3a08644)

7 files changed, 220 insertions, 217 deletions

diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h
index 301d877e61..b2fe8b67e7 100644
--- a/source3/winbindd/winbindd.h
+++ b/source3/winbindd/winbindd.h
@@ -363,7 +363,12 @@ struct winbindd_tdc_domain {
 	uint32 trust_type;
 };
 
-
+/* Switch for listing users or groups */
+enum ent_type {
+	LIST_USERS = 0,
+	LIST_GROUPS,
+};
+ 
 #include "winbindd/winbindd_proto.h"
 
 #define WINBINDD_ESTABLISH_LOOP 30
diff --git a/source3/winbindd/winbindd_async.c b/source3/winbindd/winbindd_async.c
index bc0f9d836b..e47666462e 100644
--- a/source3/winbindd/winbindd_async.c
+++ b/source3/winbindd/winbindd_async.c
@@ -453,8 +453,8 @@ enum winbindd_result winbindd_dual_lookupname(struct winbindd_domain *domain,
 	return WINBINDD_OK;
 }
 
-/* This is the first callback after enumerating groups from a domain */
-static void listgroups_recv(TALLOC_CTX *mem_ctx, bool success,
+/* This is the first callback after enumerating users/groups from a domain */
+static void listent_recv(TALLOC_CTX *mem_ctx, bool success,
 	                    struct winbindd_response *response,
 			    void *c, void *private_data)
 {
@@ -462,7 +462,7 @@ static void listgroups_recv(TALLOC_CTX *mem_ctx, bool success,
 		(void (*)(void *, bool, fstring, char*))c;
 
 	if (!success || response->result != WINBINDD_OK) {
-		DEBUG(5, ("list_groups() failed!\n"));
+		DEBUG(5, ("list_ent() failed!\n"));
 		cont(private_data, False, response->data.name.dom_name, NULL);
 		return;
 	}
@@ -473,30 +473,95 @@ static void listgroups_recv(TALLOC_CTX *mem_ctx, bool success,
 	SAFE_FREE(response->extra_data.data);
 }
 
-/* Request the name of all groups in a single domain */
-void winbindd_listgroups_async(TALLOC_CTX *mem_ctx,
+/* Request the name of all users/groups in a single domain */
+void winbindd_listent_async(TALLOC_CTX *mem_ctx,
 	                       struct winbindd_domain *domain,
 	                       void (*cont)(void *private_data, bool success,
 				     fstring dom_name, char* extra_data),
-			       void *private_data)
+			       void *private_data, enum ent_type type)
 {
 	struct winbindd_request request;
 
 	ZERO_STRUCT(request);
-	request.cmd = WINBINDD_LIST_GROUPS;
+	if (type == LIST_USERS)
+		request.cmd = WINBINDD_LIST_USERS;
+	else if (type == LIST_GROUPS)
+		request.cmd = WINBINDD_LIST_GROUPS;
 
-	do_async_domain(mem_ctx, domain, &request, listgroups_recv,
+	do_async_domain(mem_ctx, domain, &request, listent_recv,
 		        (void *)cont, private_data);
 }
+ 
+enum winbindd_result winbindd_dual_list_users(struct winbindd_domain *domain,
+                                              struct winbindd_cli_state *state)
+{
+	WINBIND_USERINFO *info;
+	NTSTATUS status;
+	struct winbindd_methods *methods;
+	uint32 num_entries = 0;
+	char *extra_data = NULL;
+	uint32_t extra_data_len = 0, i;
+
+	/* Must copy domain into response first for debugging in parent */
+	fstrcpy(state->response.data.name.dom_name, domain->name);
+
+	/* Query user info */
+	methods = domain->methods;
+	status = methods->query_user_list(domain, state->mem_ctx, 
+					  &num_entries, &info);
+	
+	if (!NT_STATUS_IS_OK(status))
+		return WINBINDD_ERROR;
+
+	if (num_entries == 0)
+		return WINBINDD_OK;
+
+	/* Allocate some memory for extra data.  Note that we limit
+	   account names to sizeof(fstring) = 256 characters.		
+	   +1 for the ',' between group names */
+	extra_data = (char *)SMB_REALLOC(extra_data, 
+		(sizeof(fstring) + 1) * num_entries);
+ 
+	if (!extra_data) {
+		DEBUG(0,("failed to enlarge buffer!\n"));
+		return WINBINDD_ERROR;
+	}
+
+	/* Pack user list into extra data fields */
+	for (i = 0; i < num_entries; i++) {
+		fstring acct_name, name;
+		
+		if (info[i].acct_name == NULL)
+			fstrcpy(acct_name, "");
+		else
+			fstrcpy(acct_name, info[i].acct_name);
+		
+		fill_domain_username(name, domain->name, acct_name, True);
+		/* Append to extra data */
+		memcpy(&extra_data[extra_data_len], name, strlen(name));
+		extra_data_len += strlen(name);
+		extra_data[extra_data_len++] = ',';
+	}   
+
+	/* Assign extra_data fields in response structure */
+	if (extra_data) {
+		/* remove trailing ',' */
+		extra_data[extra_data_len - 1] = '\0';
+		state->response.extra_data.data = extra_data;
+		state->response.length += extra_data_len;
+	}
+
+	return WINBINDD_OK;
+}
 
 enum winbindd_result winbindd_dual_list_groups(struct winbindd_domain *domain,
                                                struct winbindd_cli_state *state)
 {
 	struct getent_state groups = {};
 	char *extra_data = NULL;
-	unsigned int extra_data_len = 0, i;
+	uint32_t extra_data_len = 0, i;
 
-	/* Must copy domain into response first for bookeeping in parent */
+	/* Must copy domain into response first for debugging in parent */
 	fstrcpy(state->response.data.name.dom_name, domain->name);
 	fstrcpy(groups.domain_name, domain->name);
 
diff --git a/source3/winbindd/winbindd_domain.c b/source3/winbindd/winbindd_domain.c
index 4d10f49ca2..2e8c6175ca 100644
--- a/source3/winbindd/winbindd_domain.c
+++ b/source3/winbindd/winbindd_domain.c
@@ -50,6 +50,10 @@ static const struct winbindd_child_dispatch_table domain_dispatch_table[] = {
 		.struct_cmd	= WINBINDD_LOOKUPRIDS,
 		.struct_fn	= winbindd_dual_lookuprids,
 	},{
+		.name		= "LIST_USERS",
+		.struct_cmd	= WINBINDD_LIST_USERS,
+		.struct_fn	= winbindd_dual_list_users,
+	},{
 		.name		= "LIST_GROUPS",
 		.struct_cmd	= WINBINDD_LIST_GROUPS,
 		.struct_fn	= winbindd_dual_list_groups,
diff --git a/source3/winbindd/winbindd_group.c b/source3/winbindd/winbindd_group.c
index d10609a83f..f9f946f3f3 100644
--- a/source3/winbindd/winbindd_group.c
+++ b/source3/winbindd/winbindd_group.c
@@ -1339,112 +1339,10 @@ void winbindd_getgrent(struct winbindd_cli_state *state)
 		request_error(state);
 }
 
-struct listgroups_state {
-	TALLOC_CTX *mem_ctx;
-	struct winbindd_cli_state *cli_state;
-	unsigned int domain_count;
-	char *extra_data;
-	unsigned int extra_data_len;
-};
-
-static void listgroups_recv(void *private_data, bool success, fstring dom_name,
-	                    char *extra_data);
-
 /* List domain groups without mapping to unix ids */
 void winbindd_list_groups(struct winbindd_cli_state *state)
 {
-	struct winbindd_domain *domain;
-	const char *which_domain;
-	struct listgroups_state *groups_state;
-
-	DEBUG(3, ("[%5lu]: list groups\n", (unsigned long)state->pid));
-
-	/* Ensure null termination */
-	state->request.domain_name[sizeof(state->request.domain_name)-1]='\0';	
-	which_domain = state->request.domain_name;
-	
-	/* Initialize listgroups_state */
-	groups_state = TALLOC_P(state->mem_ctx, struct listgroups_state);
-	if (groups_state == NULL) {
-		DEBUG(0, ("talloc failed\n"));
-		request_error(state);
-		return;
-	}
-
-	groups_state->mem_ctx = state->mem_ctx;
-	groups_state->cli_state = state;
-	groups_state->domain_count = 0;
-	groups_state->extra_data = NULL;
-	groups_state->extra_data_len = 0;
-
-	/* Must count the full list of expected domains before we request data
-	 * from any of them.  Otherwise it's possible for a connection to the
-	 * first domain to fail, call listgroups_recv(), and return to the
-	 * client without checking any other domains. */
-	for (domain = domain_list(); domain; domain = domain->next) {
-		/* if we have a domain name restricting the request and this
-		   one in the list doesn't match, then just bypass the remainder
-		   of the loop */
-		if ( *which_domain && !strequal(which_domain, domain->name) )
-			continue;
-
-		groups_state->domain_count++;
-	}
-
-	/* Make sure we're enumerating at least one domain */
-	if (!groups_state->domain_count) {
-		request_ok(state);
-		return;
-	}
-
-	/* Enumerate list of trusted domains and request group list from each */
-	for (domain = domain_list(); domain; domain = domain->next) {
-		if ( *which_domain && !strequal(which_domain, domain->name) )
-			continue;
-
-		winbindd_listgroups_async(state->mem_ctx, domain,
-			                  listgroups_recv, groups_state);
-	}
-}
-
-static void listgroups_recv(void *private_data, bool success, fstring dom_name,
-	                    char *extra_data)
-{
-	/* extra_data comes to us as a '\0' terminated string of comma
-	   separated groups */
-	struct listgroups_state *state = talloc_get_type_abort(
-		private_data, struct listgroups_state);
-
-	/* Append groups from one domain onto the whole list */
-	if (extra_data) {
-		DEBUG(5, ("listgroups_recv: %s returned groups.\n", dom_name));
-		if (!state->extra_data)
-			state->extra_data = talloc_asprintf(state->mem_ctx,
-							    "%s", extra_data);
-		else
-			state->extra_data = talloc_asprintf_append_buffer(
-							    state->extra_data,
-							    ",%s", extra_data);
-		/* Add one for the '\0' and each additional ',' */
-		state->extra_data_len += strlen(extra_data) + 1;
-	}
-	else {
-		DEBUG(5, ("listgroups_recv: %s returned no groups.\n",
-			dom_name));
-	}
-
-	if (--state->domain_count)
-		/* Still waiting for some child domains to return */
-		return;
-
-	/* Return list of all groups to the client */
-	if (state->extra_data) {
-		state->cli_state->response.extra_data.data =
-			SMB_STRDUP(state->extra_data);
-		state->cli_state->response.length += state->extra_data_len;
-	}
-
-	request_ok(state->cli_state);
+	winbindd_list_ent(state, LIST_GROUPS);
 }
 
 /* Get user supplementary groups.  This is much quicker than trying to
diff --git a/source3/winbindd/winbindd_misc.c b/source3/winbindd/winbindd_misc.c
index 8933cf2794..01a4054d44 100644
--- a/source3/winbindd/winbindd_misc.c
+++ b/source3/winbindd/winbindd_misc.c
@@ -97,6 +97,129 @@ enum winbindd_result winbindd_dual_check_machine_acct(struct winbindd_domain *do
 	return NT_STATUS_IS_OK(result) ? WINBINDD_OK : WINBINDD_ERROR;
 }
 
+/* Helpers for listing user and group names */
+
+const char *ent_type_strings[] = {"users", 
+				  "groups"};
+
+static const char *get_ent_type_string(enum ent_type type)
+{
+	return ent_type_strings[type];
+}
+
+struct listent_state {
+	TALLOC_CTX *mem_ctx;
+	struct winbindd_cli_state *cli_state;
+	enum ent_type type;
+	int domain_count;
+	char *extra_data;
+	uint32_t extra_data_len;
+};
+
+static void listent_recv(void *private_data, bool success, fstring dom_name,
+	                 char *extra_data);
+
+/* List domain users/groups without mapping to unix ids */
+void winbindd_list_ent(struct winbindd_cli_state *state, enum ent_type type)
+{
+	struct winbindd_domain *domain;
+	const char *which_domain;
+	struct listent_state *ent_state;
+
+	DEBUG(3, ("[%5lu]: list %s\n", (unsigned long)state->pid, 
+	      get_ent_type_string(type)));
+
+	/* Ensure null termination */
+	state->request.domain_name[sizeof(state->request.domain_name)-1]='\0';	
+	which_domain = state->request.domain_name;
+	
+	/* Initialize listent_state */
+	ent_state = TALLOC_P(state->mem_ctx, struct listent_state);
+	if (ent_state == NULL) {
+		DEBUG(0, ("talloc failed\n"));
+		request_error(state);
+		return;
+	}
+
+	ent_state->mem_ctx = state->mem_ctx;
+	ent_state->cli_state = state;
+	ent_state->type = type;
+	ent_state->domain_count = 0;
+	ent_state->extra_data = NULL;
+	ent_state->extra_data_len = 0;
+
+	/* Must count the full list of expected domains before we request data
+	 * from any of them.  Otherwise it's possible for a connection to the
+	 * first domain to fail, call listent_recv(), and return to the
+	 * client without checking any other domains. */
+	for (domain = domain_list(); domain; domain = domain->next) {
+		/* if we have a domain name restricting the request and this
+		   one in the list doesn't match, then just bypass the remainder
+		   of the loop */
+		if ( *which_domain && !strequal(which_domain, domain->name) )
+			continue;
+
+		ent_state->domain_count++;
+	}
+
+	/* Make sure we're enumerating at least one domain */
+	if (!ent_state->domain_count) {
+		request_ok(state);
+		return;
+	}
+
+	/* Enumerate list of trusted domains and request user/group list from
+	 * each */
+	for (domain = domain_list(); domain; domain = domain->next) {
+		if ( *which_domain && !strequal(which_domain, domain->name) )
+			continue;
+
+		winbindd_listent_async(state->mem_ctx, domain, 
+			                  listent_recv, ent_state, type);
+	}
+}
+
+static void listent_recv(void *private_data, bool success, fstring dom_name,
+	                 char *extra_data)
+{
+	/* extra_data comes to us as a '\0' terminated string of comma
+	   separated users or groups */
+	struct listent_state *state = talloc_get_type_abort(
+		private_data, struct listent_state);
+
+	/* Append users/groups from one domain onto the whole list */
+	if (extra_data) {
+		DEBUG(5, ("listent_recv: %s returned %s.\n", 
+		      dom_name, get_ent_type_string(state->type)));
+                if (!state->extra_data)
+                        state->extra_data = talloc_asprintf(state->mem_ctx,
+                                                            "%s", extra_data);
+                else
+                        state->extra_data = talloc_asprintf_append(
+                                                            state->extra_data,
+                                                            ",%s", extra_data);
+                /* Add one for the '\0' and each additional ',' */
+                state->extra_data_len += strlen(extra_data) + 1;
+	}
+	else {
+		DEBUG(5, ("listent_recv: %s returned no %s.\n", 
+		      dom_name, get_ent_type_string(state->type)));
+	}
+
+	if (--state->domain_count)
+		/* Still waiting for some child domains to return */
+		return;
+
+	/* Return list of all users/groups to the client */
+	if (state->extra_data) {
+		state->cli_state->response.extra_data.data = 
+			SMB_STRDUP(state->extra_data);
+		state->cli_state->response.length += state->extra_data_len;
+	}
+
+	request_ok(state->cli_state);
+}	
+
 /* Constants and helper functions for determining domain trust types */
 
 enum trust_type {
diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index 356d4aaf79..84ea67b9ec 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -104,6 +104,15 @@ void winbindd_lookupname_async(TALLOC_CTX *mem_ctx,
 			       void *private_data);
 enum winbindd_result winbindd_dual_lookupname(struct winbindd_domain *domain,
 					      struct winbindd_cli_state *state);
+void winbindd_listent_async(TALLOC_CTX *mem_ctx,
+	                       struct winbindd_domain *domain,
+	                       void (*cont)(void *private_data, bool success,
+				     fstring dom_name, char* extra_data),
+			       void *private_data, enum ent_type type);
+enum winbindd_result winbindd_dual_list_users(struct winbindd_domain *domain,
+                                              struct winbindd_cli_state *state);
+enum winbindd_result winbindd_dual_list_groups(struct winbindd_domain *domain,
+                                               struct winbindd_cli_state *state);
 bool print_sidlist(TALLOC_CTX *mem_ctx, const DOM_SID *sids,
 		   size_t num_sids, char **result, ssize_t *len);
 enum winbindd_result winbindd_dual_lookuprids(struct winbindd_domain *domain,
@@ -133,16 +142,6 @@ void query_user_async(TALLOC_CTX *mem_ctx, struct winbindd_domain *domain,
 				   uint32 group_rid),
 		      void *private_data);
 
-void winbindd_listgroups_async(TALLOC_CTX *mem_ctx,
-	                       struct winbindd_domain *domain,
-	                       void (*cont)(void *private_data, bool success,
-				     fstring dom_name, char* extra_data),
-			       void *private_data);
-
-enum winbindd_result winbindd_dual_list_groups(struct winbindd_domain *domain,
-                                               struct winbindd_cli_state *state);
-
-
 /* The following definitions come from winbindd/winbindd_cache.c  */
 
 void winbindd_check_cache_size(time_t t);
@@ -394,6 +393,7 @@ void winbindd_dsgetdcname(struct winbindd_cli_state *state);
 void winbindd_check_machine_acct(struct winbindd_cli_state *state);
 enum winbindd_result winbindd_dual_check_machine_acct(struct winbindd_domain *domain,
 						      struct winbindd_cli_state *state);
+void winbindd_list_ent(struct winbindd_cli_state *state, enum ent_type type);
 void winbindd_list_trusted_domains(struct winbindd_cli_state *state);
 enum winbindd_result winbindd_dual_list_trusted_domains(struct winbindd_domain *domain,
 							struct winbindd_cli_state *state);
diff --git a/source3/winbindd/winbindd_user.c b/source3/winbindd/winbindd_user.c
index 6241d84fe6..45918383b7 100644
--- a/source3/winbindd/winbindd_user.c
+++ b/source3/winbindd/winbindd_user.c
@@ -778,99 +778,7 @@ void winbindd_getpwent(struct winbindd_cli_state *state)
 }
 
 /* List domain users without mapping to unix ids */
-
 void winbindd_list_users(struct winbindd_cli_state *state)
 {
-	struct winbindd_domain *domain;
-	WINBIND_USERINFO *info;
-	const char *which_domain;
-	uint32 num_entries = 0, total_entries = 0;
-	char *extra_data = NULL;
-	int extra_data_len = 0;
-	enum winbindd_result rv = WINBINDD_ERROR;
-
-	DEBUG(3, ("[%5lu]: list users\n", (unsigned long)state->pid));
-
-	/* Ensure null termination */
-	state->request.domain_name[sizeof(state->request.domain_name)-1]='\0';	
-	which_domain = state->request.domain_name;
-	
-	/* Enumerate over trusted domains */
-
-	for (domain = domain_list(); domain; domain = domain->next) {
-		NTSTATUS status;
-		struct winbindd_methods *methods;
-		unsigned int i;
-		
-		/* if we have a domain name restricting the request and this
-		   one in the list doesn't match, then just bypass the remainder
-		   of the loop */
-		   
-		if ( *which_domain && !strequal(which_domain, domain->name) )
-			continue;
-			
-		methods = domain->methods;
-
-		/* Query display info */
-		status = methods->query_user_list(domain, state->mem_ctx, 
-						  &num_entries, &info);
-
-		if (!NT_STATUS_IS_OK(status)) {
-			continue;
-		}
-
-		if (num_entries == 0)
-			continue;
-
-		/* Allocate some memory for extra data */
-		total_entries += num_entries;
-			
-		extra_data = (char *)SMB_REALLOC(
-			extra_data, sizeof(fstring) * total_entries);
-			
-		if (!extra_data) {
-			DEBUG(0,("failed to enlarge buffer!\n"));
-			goto done;
-		}
-
-		/* Pack user list into extra data fields */
-			
-		for (i = 0; i < num_entries; i++) {
-			fstring acct_name, name;
-			
-			if (!info[i].acct_name) {
-				fstrcpy(acct_name, "");
-			} else {
-				fstrcpy(acct_name, info[i].acct_name);
-			}
-			
-			fill_domain_username(name, domain->name, acct_name, True);
-			
-				/* Append to extra data */
-			memcpy(&extra_data[extra_data_len], name, 
-			       strlen(name));
-			extra_data_len += strlen(name);
-			extra_data[extra_data_len++] = ',';
-		}   
-        }
-
-	/* Assign extra_data fields in response structure */
-
-	if (extra_data) {
-		extra_data[extra_data_len - 1] = '\0';
-		state->response.extra_data.data = extra_data;
-		state->response.length += extra_data_len;
-	}
-
-	/* No domains responded but that's still OK so don't return an
-	   error. */
-
-	rv = WINBINDD_OK;
-
- done:
-
-	if (rv == WINBINDD_OK)
-		request_ok(state);
-	else
-		request_error(state);
+	winbindd_list_ent(state, LIST_USERS);
 }