On Fri, Oct 25, 2013 at 02:46:31PM +0200, Lukas Slebodnik wrote: > On (25/10/13 11:54), Sumit Bose wrote: > >On Thu, Oct 24, 2013 at 11:54:30AM +0200, Lukas Slebodnik wrote: > >> ehlo, > >> > >> Global catalog port was ignored from SRV record, hard coded value was used > >> every time. > >> 1st patch should fix it. > > > >good catch, but see my comment below. > > > >> > >> 2nd patch add fall back to LDAP if GC is not available. > >> with this patch SSSD will not go offline if GC is not available, > >> but it may happen that SSSD will try to connect to GC often. > >> I am not sure, but we can add new option do disable GC lookup completely. > > > >The patch is working as expected, so ACK. I still can see some debug > >messages saying that SSD will go offline before the fallback to LDAP > >happens, but I think this is aceptable. > > > I forgot to move this debug message inside the if statement. > and debug message has high debug level SSSDBG_CRIT_FAILURE. > It make sense. > > >I don't think that we need an option to disable the GC lookup > >completely. Since we are doing SRV lookups to find the GC server there > >should be always one wokring GC. If not, there is either a firewall > >issue or the AD domain has other problems. > > > Thank you for review. > > >bye, > >Sumit > > > >> > >> LS > > > >> From abf7cfeaa9826fb52e9a18d3bd5812003c38f118 Mon Sep 17 00:00:00 2001 > >> From: Lukas Slebodnik <lsleb...@redhat.com> > >> Date: Thu, 24 Oct 2013 11:03:32 +0200 > >> Subject: [PATCH 1/3] AD: Prefer GC port from SRV record > >> > >> We had a hard coded value of Global Catalog port (3268). > >> Informations from SRV record was ignored. > >> This patch prefer port number from SRV record and hard coded value is used > >> only > >> as a fall back if port number was not initialized. > >> --- > >> src/providers/ad/ad_common.c | 7 +++++-- > >> 1 file changed, 5 insertions(+), 2 deletions(-) > >> > >> diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c > >> index > >> ab62d64a943a726724698d1b7eb9f15bc40b389e..1c4bd088b30eed4f71a787ce4c3bd60366b72def > >> 100644 > >> --- a/src/providers/ad/ad_common.c > >> +++ b/src/providers/ad/ad_common.c > >> @@ -729,12 +729,15 @@ ad_resolve_callback(void *private_data, struct > >> fo_server *server) > >> talloc_zfree(service->gc->uri); > >> talloc_zfree(service->gc->sockaddr); > >> if (sdata && sdata->gc) { > >> + int new_port = fo_get_server_port(server); > > > >Please move the declaration to the top of the function (see > >http://www.freeipa.org/page/Coding_Style) > > > Fixed. > (I thought that declaration at the beginning of block is allowed.) > > >> + new_port = (new_port == 0) ? AD_GC_PORT : new_port; > >> + > >> service->gc->uri = talloc_asprintf(service->gc, "%s:%d", > >> - new_uri, AD_GC_PORT); > >> + new_uri, new_port); > >> > >> service->gc->sockaddr = resolv_get_sockaddr_address(service->gc, > >> srvaddr, > >> - AD_GC_PORT); > >> + new_port); > >> } else { > >> /* Make sure there always is an URI even if we know that this > >> * server doesn't support GC. That way the lookup would go through > >> -- > >> 1.8.3.1 > >> > > Updated patches are attached.
ACK bye, Sumit > > LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel