[PATCH] Update few debug messages

[Lukas Slebodnik]

ehlo,

I noticed the same noise in log files as user in ticker 2678.
It was a little bit related to the latest changes with setting
initgroups flag in the right time.

LS

[Pavel Reichl]

On 06/18/2015 06:16 PM, Lukas Slebodnik wrote:
> ehlo,
>
> I noticed the same noise in log files as user in ticker 2678.
> It was a little bit related to the latest changes with setting
> initgroups flag in the right time.
>
> LS

Hello,
the patch won't apply on current master. Does it depend on some patch 
which is being reviewed?
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Lukas Slebodnik]

On (19/06/15 10:37), Pavel Reichl wrote:
> On 06/18/2015 06:16 PM, Lukas Slebodnik wrote:
> > ehlo,
> >
> > I noticed the same noise in log files as user in ticker 2678.
> > It was a little bit related to the latest changes with setting
> > initgroups flag in the right time.
> >
> > LS
>
> Hello,
> the patch won't apply on current master. Does it depend on some patch which
> is being reviewed?

Yes,
It is related to "[PATCH] SDAP: Remove user from cache for missing user in
LDAP". I didn't send them together because this patch does not fix regression.

LS

[Pavel Reichl]

On 06/19/2015 10:46 AM, Lukas Slebodnik wrote:
> On (19/06/15 10:37), Pavel Reichl wrote:
> > On 06/18/2015 06:16 PM, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > I noticed the same noise in log files as user in ticker 2678.
> > > It was a little bit related to the latest changes with setting
> > > initgroups flag in the right time.
> > >
> > > LS
> >
> > Hello,
> > the patch won't apply on current master. Does it depend on some patch which
> > is being reviewed?
>
> Yes,
> It is related to "[PATCH] SDAP: Remove user from cache for missing user in
> LDAP". I didn't send them together because this patch does not fix regression.
>
> LS

OK thanks, the only nitpick I found is usage of strerror, I know that 
Michal has patch to change this globally but I think that we are in 
agreement to use sss_strerror in changed/new code anyway, right?
>            } else if (ret != ENOENT) {
>                DEBUG(SSSDBG_OP_FAILURE,
>                      "sysdb_search_object_by_uuid did not return 
>
> a " \
>                      "single result.\n");
>                      "sysdb_search_object_by_uuid failed or 
>
> returned "
>                      "more than one result [%d][%s].\n",
>                      ret, strerror(ret));

Otherwise code LGTM and CI passed, but I assume that I can't ack before 
"[PATCH] SDAP: Remove user from cache for missing user in LDAP" gets 
pushed to master anyway.
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Lukas Slebodnik]

On (19/06/15 12:53), Pavel Reichl wrote:
> On 06/19/2015 10:46 AM, Lukas Slebodnik wrote:
> > On (19/06/15 10:37), Pavel Reichl wrote:
> > > On 06/18/2015 06:16 PM, Lukas Slebodnik wrote:
> > > > ehlo,
> > > >
> > > > I noticed the same noise in log files as user in ticker 2678.
> > > > It was a little bit related to the latest changes with setting
> > > > initgroups flag in the right time.
> > > >
> > > > LS
> > >
> > > Hello,
> > > the patch won't apply on current master. Does it depend on some patch which
> > > is being reviewed?
> >
> > Yes,
> > It is related to "[PATCH] SDAP: Remove user from cache for missing user in
> > LDAP". I didn't send them together because this patch does not fix regression.
> >
> > LS
>
> OK thanks, the only nitpick I found is usage of strerror, I know that Michal
> has patch to change this globally but I think that we are in agreement to use
> sss_strerror in changed/new code anyway, right?
> >            } else if (ret != ENOENT) {
> >               DEBUG(SSSDBG_OP_FAILURE,
> >                      "sysdb_search_object_by_uuid did not return a "
> >
> > \
> >                      "single result.\n");
> >                      "sysdb_search_object_by_uuid failed or returned
> >
> > "
> >                      "more than one result [%d][%s].\n",
> >                      ret, strerror(ret));
>
> Otherwise code LGTM and CI passed, but I assume that I can't ack before
> "[PATCH] SDAP: Remove user from cache for missing user in LDAP" gets pushed
> to master anyway.

Patch could not be applied on top of master due to conflict.

Updated patch is attached.

LS

[Pavel Reichl]

On 07/14/2015 01:21 PM, Lukas Slebodnik wrote:
> +++ b/src/db/sysdb_search.c
> @@ -1613,10 +1613,11 @@ errno_t sysdb_get_real_name(TALLOC_CTX *mem_ctx,
>                                                     &res);
>                   if (ret == EOK && res->count == 1) {
>                       msg = res->msgs[0];
>            } else {
>            } else if (ret != ENOENT) {
>                 DEBUG(SSSDBG_OP_FAILURE,
>                      "sysdb_search_object_by_uuid did not return a " \
>                      "single result.\n");
>                      "sysdb_search_object_by_uuid failed or returned "
>                      "more than one result [%d][%s].\n",
>                      ret, strerror(ret));

Hello,

I asked you to replace 'strerror' by 'sss_strerror'. But I don't think 
it's too important. Local CI passed, so ACK.

Thanks.

[Michal Židek]

On 07/16/2015 02:35 PM, Pavel Reichl wrote:
> On 07/14/2015 01:21 PM, Lukas Slebodnik wrote:
> > +++ b/src/db/sysdb_search.c
> > @@ -1613,10 +1613,11 @@ errno_t sysdb_get_real_name(TALLOC_CTX *mem_ctx,
> >                                                     &res);
> >                   if (ret == EOK && res->count == 1) {
> >                       msg = res->msgs[0];
> >            } else {
> >            } else if (ret != ENOENT) {
> >                 DEBUG(SSSDBG_OP_FAILURE,
> >                      "sysdb_search_object_by_uuid did not return
> >
> > a " \
> >                      "single result.\n");
> >                      "sysdb_search_object_by_uuid failed or
> >
> > returned "
> >                      "more than one result [%d][%s].\n",
> >                      ret, strerror(ret));
>
> Hello,
>
> I asked you to replace 'strerror' by 'sss_strerror'. But I don't think
> it's too important. Local CI passed, so ACK.
>
> Thanks.

Please replace the strerror with sss_strerror.

Michal

[Lukas Slebodnik]

On (16/07/15 14:35), Pavel Reichl wrote:
> On 07/14/2015 01:21 PM, Lukas Slebodnik wrote:
> > +++ b/src/db/sysdb_search.c
> > @@ -1613,10 +1613,11 @@ errno_t sysdb_get_real_name(TALLOC_CTX *mem_ctx,
> >                                                    &res);
> >                  if (ret == EOK && res->count == 1) {
> >                      msg = res->msgs[0];
> >            } else {
> >            } else if (ret != ENOENT) {
> >                DEBUG(SSSDBG_OP_FAILURE,
> >                      "sysdb_search_object_by_uuid did not return a " \
> >                      "single result.\n");
> >                      "sysdb_search_object_by_uuid failed or returned "
> >                      "more than one result [%d][%s].\n",
> >                      ret, strerror(ret));
>
> Hello,
>
> I asked you to replace 'strerror' by 'sss_strerror'. But I don't think it's
> too important. Local CI passed, so ACK.

Just one message contained sss_strerror. It might have been caused by
improper merging after rebase.

Thank you for a careful review and updated version is attached.

LS

[Pavel Reichl]

On 07/16/2015 02:54 PM, Lukas Slebodnik wrote:
> On (16/07/15 14:35), Pavel Reichl wrote:
> > On 07/14/2015 01:21 PM, Lukas Slebodnik wrote:
> > > +++ b/src/db/sysdb_search.c
> > > @@ -1613,10 +1613,11 @@ errno_t sysdb_get_real_name(TALLOC_CTX *mem_ctx,
> > >                                                     &res);
> > >                   if (ret == EOK && res->count == 1) {
> > >                       msg = res->msgs[0];
> > >            } else {
> > >            } else if (ret != ENOENT) {
> > >                 DEBUG(SSSDBG_OP_FAILURE,
> > >                      "sysdb_search_object_by_uuid did not return a " \
> > >                      "single result.\n");
> > >                      "sysdb_search_object_by_uuid failed or returned "
> > >                      "more than one result [%d][%s].\n",
> > >                      ret, strerror(ret));
> >
> > Hello,
> >
> > I asked you to replace 'strerror' by 'sss_strerror'. But I don't think it's
> > too important. Local CI passed, so ACK.
>
> Just one message contained sss_strerror. It might have been caused by
> improper merging after rebase.
>
> Thank you for a careful review and updated version is attached.
>
> LS

ACK
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

On Thu, Jul 16, 2015 at 04:38:03PM +0200, Pavel Reichl wrote:
> On 07/16/2015 02:54 PM, Lukas Slebodnik wrote:
> > On (16/07/15 14:35), Pavel Reichl wrote:
> > > On 07/14/2015 01:21 PM, Lukas Slebodnik wrote:
> > > > +++ b/src/db/sysdb_search.c
> > > > @@ -1613,10 +1613,11 @@ errno_t sysdb_get_real_name(TALLOC_CTX *mem_ctx,
> > > >                                                    &res);
> > > >                  if (ret == EOK && res->count == 1) {
> > > >                      msg = res->msgs[0];
> > > >            } else {
> > > >            } else if (ret != ENOENT) {
> > > >                DEBUG(SSSDBG_OP_FAILURE,
> > > >                      "sysdb_search_object_by_uuid did not return a " \
> > > >                      "single result.\n");
> > > >                      "sysdb_search_object_by_uuid failed or returned "
> > > >                      "more than one result [%d][%s].\n",
> > > >                      ret, strerror(ret));
> > >
> > > Hello,
> > >
> > > I asked you to replace 'strerror' by 'sss_strerror'. But I don't think it's
> > > too important. Local CI passed, so ACK.
> >
> > Just one message contained sss_strerror. It might have been caused by
> > improper merging after rebase.
> >
> > Thank you for a careful review and updated version is attached.
> >
> > LS
>
> ACK

* master: 890ae77c52e36f222655d710439d2fe25f15101b