On Thu, Oct 11, 2012 at 05:32:20PM +0200, Pavel Březina wrote:
> On 10/11/2012 04:16 PM, Sumit Bose wrote:
>> Hi,
>>
>> those two patch fixes
https://fedorahosted.org/sssd/ticket/1561 from the
>> sssd side. While testing I found that there is also a server side issue
>> which is tracked by
https://fedorahosted.org/freeipa/ticket/3166 . As
>> long as the issue on the server is not solved this patch can be checked
>> by watching the logs and doing the following steps:
>>
>> - stop sssd
>> - delete all ldb and memory mapped cache files
>> - start sssd
>> - call 'getent passwd UID_of_a_user_from_a_trusted_domain
>>
>> Without the patch you should see in the log that the request is send via
>> ldapsearch to the IPA server, which the patch applied the request will
>> go to the extended operation to map the uid to a name.
>>
>> bye,
>> Sumit
>
> Unfortunately, I will have to leave in a few minutes, so I don't
> have enough time to test those patches. So the following is only
> based upon code observation.
>
> Patch 1:
> Nack until the following is answered.
> Where do we set rctx->get_domains_last_call? I can only find six references:
> 1x diff = now-rctx->get_domains_last_call.tv_sec;
> 5x if (cctx->rctx->get_domains_last_call.tv_sec == 0) {
good catch, looks like setting it felt between the cracks. I added a new
first patch which does this. I didn't change the other two.
>
> If possible, I would like to see a universal callback instead of
> creating 5 callbacks that differs only in one line (2 in this patch,
> another 3 were written earlier). But that may be done later.
yes, I was thinking the same while writing the patch, but I took this
approach, because
- I didn't want to change too much existing code in a minor release
- I think all nss_cmd_get[ug]id* requests in nsssrv_cmd.c are looking
for some heavy refactoring. I think with a reasonable amount of of
abstraction all user and group by-name and by-uid requests can be
handled in a single cond path. I haven't looked much at the other, but
I expect that there is room for improvement, too. I others think this
is a valuable task, too, maybe someone can start this soon, so that we
have it ready early in the 1.10 development cycle so that other
patches touching nsssrv_cmd.c can be written on top of the refactored
version.
>
> Patch 2:
> Code-wise ack.
Thank you for the review.
bye,
Sumit
Ack to all patches. Good job.