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

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


On 07/28/2015 01:42 PM, Jakub Hrozek wrote:
> 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?

Attached. I'm sorry for taking over your contribution :-)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-AD-Use-ad_site-also-when-site-search-fails.patch
Type: text/x-patch
Size: 1484 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150728/398ef880/attachment.bin>


More information about the sssd-devel mailing list