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

Jakub Hrozek jhrozek at redhat.com
Mon Sep 9 16:12:48 UTC 2013


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 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

ACK to both.



More information about the sssd-devel mailing list