[SSSD] [PATCH] AD: Use ad_site also when site search fails

Jakub Hrozek jhrozek at redhat.com
Tue Jul 28 11:42:35 UTC 2015


On Tue, Jul 28, 2015 at 12:13:31PM +0200, Pavel Březina wrote:
> On 07/27/2015 09:35 PM, Jakub Hrozek wrote:
> >On Mon, Jul 27, 2015 at 07:38:12PM +0200, Pavel Březina wrote:
> >>On 07/27/2015 05:38 PM, Jakub Hrozek wrote:
> >>>On Mon, Jul 27, 2015 at 12:32:59PM +0200, Pavel Březina wrote:
> >>>>On 07/21/2015 09:45 PM, Jakub Hrozek wrote:
> >>>>>Hi,
> >>>>>
> >>>>>the attached patch helps AD clients that have trouble detecting the
> >>>>>right site. Please see the patch and the commit message for more
> >>>>>details.
> >>>>
> >>>>Hmm... nack. I think this change makes better job:
> >>>>
> >>>>     if (state->ctx->ad_site_override != NULL) {
> >>>>         DEBUG(SSSDBG_TRACE_INTERNAL,
> >>>>               "Ignoring AD site found by DNS discovery: '%s', "
> >>>>               "using configured value: '%s' instead.\n",
> >>>>               state->site, state->ctx->ad_site_override);
> >>>>         state->site = state->ctx->ad_site_override;
> >>>>+       ret = EOK;
> >>>>     }
> >>>
> >>>That was my first version, too, but it appears that the EOK branch tries
> >>>to access state->forest that is not known -- keep in mind that this is
> >>>for cases where ad_get_client_site_recv() returned ENOENT so its output
> >>>parameters might not be available.
> >>>
> >>>Can we infer the forest somehow?
> >>
> >>Is forest not known or is it just not set because _recv returned error?
> >
> >I don't think we can make any assumptions unless we change the _recv to
> >return a special error code for the case where the request was only able
> >to read the forest. But looking at the request, both are read at the
> >same time using ad_get_client_site_parse_ndr().
> >
> >>The
> >>latter one can be solved... but it may be even more conditions :-)
> >>
> >>OK then.
> >
> >Can you review the patch, please?
> 
> How about this:
> 
> diff --git a/src/providers/ad/ad_srv.c b/src/providers/ad/ad_srv.c
> index f3a4e64..dabcf9f 100644
> --- a/src/providers/ad/ad_srv.c
> +++ b/src/providers/ad/ad_srv.c
> @@ -774,6 +774,12 @@ static void ad_srv_plugin_site_done(struct tevent_req
> *subreq)
>                "using configured value: '%s' instead.\n",
>                state->site, state->ctx->ad_site_override);
>          state->site = state->ctx->ad_site_override;
> +
> +        if (state->forest == NULL) {
> +            state->forest = state->discovery_domain;
> +        }
> +
> +        ret = EOK;
>      }
> 
> With some debug message probably.

Yes, that's better, do you want to send this as a git-formatted patch?



More information about the sssd-devel mailing list