[SSSD] [PATCH] DB: Add user/group lookup by SID

Ondrej Kos okos at redhat.com
Mon Sep 9 16:04:06 UTC 2013


On 09/09/2013 05:51 PM, Jakub Hrozek wrote:
> On Mon, Sep 09, 2013 at 02:55:57PM +0200, Ondrej Kos wrote:
>> On 09/06/2013 02:12 PM, Jakub Hrozek wrote:
>>> On Wed, Sep 04, 2013 at 09:30:37AM +0200, Ondrej Kos wrote:
>>>> On 09/03/2013 12:08 AM, Jakub Hrozek wrote:
>>>>> On Tue, Aug 27, 2013 at 12:19:39PM +0200, Ondrej Kos wrote:
>>>>>> On 08/26/2013 03:53 PM, Jakub Hrozek wrote:
>>>>>>> On Mon, Aug 26, 2013 at 02:58:18PM +0200, Ondrej Kos wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Attached patch adds sysdb routine to search users/groups by their
>>>>>>>> SID, which will be needed for ticket 1568.
>>>>>>>>
>>>>>>>> I'm sending it now, because one of the patches I have in this
>>>>>>>> working branch (store group SID) was already written and posted on
>>>>>>>> the list by Sumit, so not to waste time again :)
>>>>>>>>
>>>>>>>
>>>>>>> There is quite some code duplication between the two functions. Can we
>>>>>>> have a single one that would also take a search base and either
>>>>>>> objectlass or filter as arguments? The objectclass or filter would then
>>>>>>> be and-end with SYSDB_SID_STR=%s. User and group functions could then be
>>>>>>> just thin wrappers.
>>>>>>>
>>>>>>> Also I would prefer a unit test for any new sysdb API.
>>>>>>> _______________________________________________
>>>>>>> sssd-devel mailing list
>>>>>>> sssd-devel at lists.fedorahosted.org
>>>>>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>>>>>
>>>>>>
>>>>>> New patch attached.
>>>>>
>>>>> This is better, but do you need the generic function and the enum
>>>>> exposed in the header? Can you make the generic function static and move
>>>>> the enum inside the module?
>>>>>
>>>>> Also, instead of the enum, maybe the function can accept the format
>>>>> strings directly and then we wouldn't need the adhoc enum at all.
>>>>> _______________________________________________
>>>>> sssd-devel mailing list
>>>>> sssd-devel at lists.fedorahosted.org
>>>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>>>
>>>> New patch attached
>>>>
>>>> Ondra
>>>
>>> Only two more changes:
>>>
>>> enum sysdb_sid_search_type can be removed, it's not used anywhere.
>> oh, i forgot it there
>>>
>>> sysdb_search_entry_by_sid_str should report failure on a higher DEBUG
>>> level. Feel free to also file a ticket or send a patch to fix the other
>>> calls. I think in general OP_FAILURE should be much better.
>> done, and patch improving debug level of other search functions is
>> also attached.
>>> _______________________________________________
>>> sssd-devel mailing list
>>> sssd-devel at lists.fedorahosted.org
>>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>>>
>>
>>
>> --
>> Ondrej Kos
>> Associate Software Engineer
>> Identity Management - SSSD
>> Red Hat Czech
>
>> From: Ondrej Kos <okos at redhat.com>
>> Date: Wed, 21 Aug 2013 15:17:00 +0200
>> Subject: [PATCH 1/4] DB: Add user/group lookup by SID
>
> ACK
>
>>  From 6fa07dcc178270fbbb2868b58a94c2d814a36408 Mon Sep 17 00:00:00 2001
>> From: Ondrej Kos <okos at redhat.com>
>> Date: Mon, 9 Sep 2013 14:54:19 +0200
>> Subject: [PATCH 2/4] DB: Rise search functions debug levels
>>
>> ---
>>   src/db/sysdb_ops.c | 36 ++++++++++++++++++------------------
>>   1 file changed, 18 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
>> index 859581aa5b516238db266732a7b9843e23a2edef..bb39ca711b3926f59e14ba73d04f1436c992fae5 100644
>> --- a/src/db/sysdb_ops.c
>> +++ b/src/db/sysdb_ops.c
>> @@ -342,10 +342,10 @@ int sysdb_search_user_by_name(TALLOC_CTX *mem_ctx,
>>
>>   done:
>>       if (ret == ENOENT) {
>> -        DEBUG(SSSDBG_TRACE_FUNC, ("No such entry\n"));
>> +        DEBUG(SSSDBG_OP_FAILURE, ("No such entry\n"));
>
> Sorry if I confused you, but when I said "report failure" earlier, I
> meant other error codes than ENOENT. ENOENT is very often OK, because
> the code would check if an entry exists first before downloading it etc.
> In these cases, we shouldn't log at high log level and just let caller
> decide if the absence of the entry is a failure or not.
>
>>       }
>>       else if (ret) {
>> -        DEBUG(SSSDBG_TRACE_FUNC, ("Error: %d (%s)\n", ret, strerror(ret)));
>> +        DEBUG(SSSDBG_OP_FAILURE, ("Error: %d (%s)\n", ret, strerror(ret)));
>
> This change is fine.
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>

new patches attached

-- 
Ondrej Kos
Associate Software Engineer
Identity Management - SSSD
Red Hat Czech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-DB-Add-user-group-lookup-by-SID.patch
Type: text/x-patch
Size: 10539 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130909/1764d246/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-DB-Rise-search-functions-debug-levels.patch
Type: text/x-patch
Size: 3354 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130909/1764d246/attachment-0001.bin>


More information about the sssd-devel mailing list