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

Pavel Březina pbrezina at redhat.com
Tue Jul 28 10:13:31 UTC 2015


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.




More information about the sssd-devel mailing list