6:58 a.m.
On Wed, Jun 05, 2013 at 10:06:28PM +0200, Jakub Hrozek wrote:
On Wed, Jun 05, 2013 at 11:32:39AM +0200, Jakub Hrozek wrote:
> I pushed the code into my gc branch and I'm attaching an updated tarball
> again. Sorry for the confusion, I only developed and tested on F19.
Sumit found out that the code crashed after performing a SRV lookup if
subdomain user was requested. The attached patches implement SRV lookups
for GC servers properly and protect against cases where no GC servers
are found at all during service discovery, but later subdomain user is
requested.
I've pushed the code into my fedorapeople branch and I'm attaching a
tarball with the latest patches as well.
Thank you, I see no coredump anymore, but still the GC cannot be found
with SRV lookups. But I hve to check my DNS setup as well, so this
might not be a real issue and if, it can be fixed after the beta.
Here are my review comments:
[PATCH 01/15] Do not obfuscate calls with booleans
ACK. I only have one comment. I'm not sure what would be more gdb-friendly,
using defined like in you patch or using real functions for
*_primary_servers_init and _backup_servers_init?
[PATCH 02/15] LDAP: sdap_id_ctx might contain several connections
ACK
[PATCH 03/15] LDAP: Refactor account info handler into a tevent request
ACK
[PATCH 04/15] LDAP: Pass in a connection to ID functions
ACK
[PATCH 05/15] LDAP: new SDAP domain structure
+int
+sdap_domain_destructor(TALLOC_CTX *ctx)
+{
+ struct sdap_domain *dom =
+ talloc_get_type(ctx, struct sdap_domain);
+ DLIST_REMOVE(*(dom->head), dom);
+ return 0;
+}
destructors use void * as argument.
+void
+sdap_domain_remove(struct sss_domain_info *subdom)
+{
+ struct sss_domain_info *diter;
+
+ if (subdom->parent == NULL) return;
+
+ for (diter = subdom->parent->subdomains;
+ diter;
+ diter = get_next_domain(diter, false)) {
+ if (diter == subdom) break;
+ }
+
+ talloc_free(diter);
+}
I think we should be careful to delete sss_domain_info objects, because they
might be still used by some requests.
[PATCH 06/15] LDAP: return sdap search return code to ID
ACK
[PATCH 07/15] Move domain_to_basedn outside IPA subtree
ACK
[PATCH 08/15] New utility function sss_get_domain_name
ACK
PATCH 09/15] LDAP: split a function to create search bases
ACK
[PATCH 10/15] LDAP: store FQDNs for trusted users and groups
ACK
[PATCH 11/15] Split generating primary GID for ID mapped users into a separate function
ACK
[PATCH 12/15] LDAP: Do not store separate GID for subdomain users
ACK
PATCH 13/15] AD: Add additional service to support Global Catalog lookups
ACK
[PATCH 14/15] AD ID lookups - choose GC or LDAP as appropriate
+ switch (ar->entry_type & BE_REQ_TYPE_MASK) {
+ case BE_REQ_USER: /* user */
+ case BE_REQ_GROUP: /* group */
+ case BE_REQ_INITGROUPS: /* init groups for user */
+ if (ad_ctx->gc_ctx && IS_SUBDOMAIN(dom)) {
+ clist[i] = ad_ctx->gc_ctx;
+ i++;
+ } else {
+ clist[i] = ad_ctx->ldap_ctx;
+ }
+ break;
+
+ default:
+ clist[0] = ad_ctx->ldap_ctx;
+ break;
+ }
I guess BE_REQ_BY_SECID and BE_REQ_USER_AND_GROUP should be handled like BE_REQ_USER?
[PATCH 15/15] AD: Store trusted AD domains as subdomains
typo in the commit message 'newe' -> 'newer'
bye,
Sumit
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel