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

[Ondrej Kos]

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

[Jakub Hrozek]

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.

[Ondrej Kos]

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
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

New patch attached.

[Jakub Hrozek]

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
> > [e-mail]
> > 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.

[Ondrej Kos]

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
> > > [e-mail]
> > > 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
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

New patch attached

Ondra

[Jakub Hrozek]

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
> > > > [e-mail]
> > > > 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
> > [e-mail]
> > 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.

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.

[Ondrej Kos]

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
> > > > > [e-mail]
> > > > > 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
> > > [e-mail]
> > > 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
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

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
> > > > > > [e-mail]
> > > > > > 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
> > > > [e-mail]
> > > > 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
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> -- 
> Ondrej Kos
> Associate Software Engineer
> Identity Management - SSSD
> Red Hat Czech
> From: Ondrej Kos 
> 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 
> 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.

[Ondrej Kos]

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
> > > > > > > [e-mail]
> > > > > > > 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
> > > > > [e-mail]
> > > > > 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
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
> > --
> > Ondrej Kos
> > Associate Software Engineer
> > Identity Management - SSSD
> > Red Hat Czech
> > From: Ondrej Kos 
> > 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 
> > 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
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

new patches attached

[Jakub Hrozek]

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
> > > > > > >>[e-mail]
> > > > > > >>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
> > > > > > [e-mail]
> > > > > > 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
> > > > [e-mail]
> > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
> > > --
> > > Ondrej Kos
> > > Associate Software Engineer
> > > Identity Management - SSSD
> > > Red Hat Czech
> > > From: Ondrej Kos 
> > > 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 
> > > 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
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> new patches attached

ACK to both.

[Jakub Hrozek]

On Mon, Sep 09, 2013 at 06:12:48PM +0200, Jakub Hrozek wrote:
> 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
> > > > > > >>>[e-mail]
> > > > > > >>>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
> > > > > > >[e-mail]
> > > > > > >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
> > > > > [e-mail]
> > > > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > > >
> > > > --
> > > > Ondrej Kos
> > > > Associate Software Engineer
> > > > Identity Management - SSSD
> > > > Red Hat Czech
> > > > From: Ondrej Kos 
> > > > 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 
> > > > 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
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
> > new patches attached
>
> ACK to both.

Pushed to master.