On 09/17/2013 10:18 PM, Jakub Hrozek wrote:
>On Fri, Sep 13, 2013 at 01:04:28PM +0200, Pavel Březina wrote:
>>On 09/04/2013 08:16 AM, Jakub Hrozek wrote:
>>>This patch depends on my other AD enumeration patches.
>>>
>>>https://fedorahosted.org/sssd/ticket/2067
>>>
>>>Some AD or AD-like servers do not contain the netlogon attribute in the
>>>master domain name. Instead of failing completely, we should just abort
>>>the master domain request and carry on. The only functionality we miss
>>>would get getting users by domain flat name.
>>
>>Nack (minor)
>>
>
>Hi,
>
>thank you for the review.
>
>>> AD: Failure to get flat name is not fatal
>>>
>>>
https://fedorahosted.org/sssd/ticket/2067
>>>
>>> Some AD or AD-like servers do not contain the netlogon attribute in the
>>> master domain name. Instead of failing completely, we should just abort
>>> the master domain request and carry on. The only functionality we miss
>>> would get getting users by domain flat name.
>> ^ be?
>>
>
>Fixed.
>
>>
>>>static void
>>>ad_master_domain_netlogon_done(struct tevent_req *subreq)
>>>{
>>> int ret;
>>> size_t reply_count;
>>> struct sysdb_attrs **reply = NULL;
>>>
>>> struct tevent_req *req = tevent_req_callback_data(subreq,
>>> struct tevent_req);
>>> struct ad_master_domain_state *state =
>>> tevent_req_data(req, struct ad_master_domain_state);
>>>
>>> ret = sdap_get_generic_recv(subreq, state, &reply_count,
&reply);
>>> talloc_zfree(subreq);
>>> if (ret != EOK) {
>>> DEBUG(SSSDBG_OP_FAILURE, ("sdap_get_generic_send request
failed.\n"));
>>> tevent_req_error(req, ret);
>>> return;
>>> }
>>>
>>> if (reply_count != 1) {
>>> if (reply_count == 0) {
>>> DEBUG(SSSDBG_MINOR_FAILURE, ("No netlogon data available.
Flat name " \
>>> "might not be
usable\n"));
>>> } else if (reply_count > 1) {
>>> DEBUG(SSSDBG_MINOR_FAILURE,
>>> ("More than one netlogon info returned.\n"));
>>> }
>>> /* Not fatal. Just quit. */
>>> goto done;
>>> }
>>
>>Removing != 1 condition will make the code nicer.
>
>OK, I removed the condition.
>
>>
>>>
>>> ret = netlogon_get_flat_name(state, reply[0], &state->flat);
>>> if (ret != EOK) {
>>> DEBUG(SSSDBG_MINOR_FAILURE, ("Could not get the flat
name\n"));
>>> /* Not fatal. Just quit. */
>>> goto done;
>>> }
>>>
>>> DEBUG(SSSDBG_TRACE_FUNC, ("Found flat name [%s].\n",
state->flat));
>>>done:
>>> tevent_req_done(req);
>>> return;
>>>}
>
>New patches are attached.
Hi,
sorry, I should have nitpick this earlier. When you fail to obtain a
flat name, logs contains:
[ad_enumeration_master_done] (0x0400): Found flat name [(null)].
We should print that we failed to get flat name but we will continue anyway.
I don't think it will, we won't get to this line at all but rather jump
to the done label using one of the previous if-s. But when reading the
function again, I found a bad indentation, so attached is a revised
patch.