On Mon, Sep 09, 2013 at 06:04:06PM +0200, Ondrej Kos wrote:
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(a)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(a)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(a)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(a)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(a)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(a)lists.fedorahosted.org
>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
new patches attached
ACK to both.