[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