From Steve Danneman @ Isilon.
authorJeremy Allison <jra@samba.org>
Thu, 26 Jun 2008 21:02:39 +0000 (14:02 -0700)
committerJeremy Allison <jra@samba.org>
Thu, 26 Jun 2008 21:02:39 +0000 (14:02 -0700)
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)

source3/winbindd/winbindd.h
source3/winbindd/winbindd_async.c
source3/winbindd/winbindd_domain.c
source3/winbindd/winbindd_group.c
source3/winbindd/winbindd_misc.c
source3/winbindd/winbindd_proto.h
source3/winbindd/winbindd_user.c

index 301d877e615123aa406077e6121a6e109f30224c..b2fe8b67e7ca58cb199fd09ee639af024a3505c1 100644 (file)
@@ -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
index bc0f9d836b4603c405aac8604f349e1a866d703c..e47666462e238b0706aa0055d0db7585c2c43aba 100644 (file)
@@ -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);
 
index 4d10f49ca217b8ee4bff3ddad98175feec0e8b0f..2e8c6175ca09087f13e9d40089bc12df02976ec7 100644 (file)
@@ -49,6 +49,10 @@ static const struct winbindd_child_dispatch_table domain_dispatch_table[] = {
                .name           = "LOOKUPRIDS",
                .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,
index d10609a83f1ab42766ec528d16e11654c13a6aa1..f9f946f3f3de4cef8a882788c83189d884c31691 100644 (file)
@@ -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
index 8933cf27945d058dcd58af1807bb72cbaeecb6da..01a4054d4424b781e9512123434694bb44e7601b 100644 (file)
@@ -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 {
index 356d4aaf79aed15ff55ab6d3b0576412e12c4765..84ea67b9ec802999e74f6c6c919abb4020bf9622 100644 (file)
@@ -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);
index 6241d84fe6d0621ecf5e92499e608a30fc079469..45918383b7538906510195dceef4c5b76da80e7c 100644 (file)
@@ -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);
 }