Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
On Wed, Apr 09, 2014 at 02:29:05PM +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
Hi,
this patch is OK, but I think we should also add more debugging to sdap_save_grpmem().
Would it be too much work to also apply the patch to 1.11 ? I think it might come handy with the planned RHEL-6.6 rebase..
On Fri, 2014-04-11 at 12:16 +0200, Jakub Hrozek wrote:
On Wed, Apr 09, 2014 at 02:29:05PM +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
Hi,
this patch is OK, but I think we should also add more debugging to sdap_save_grpmem().
Would it be too much work to also apply the patch to 1.11 ? I think it might come handy with the planned RHEL-6.6 rebase..
Hi,
I added 3 log messages to sdap_save_grpmem() and 1 to sysdb_store_group. I actually can't see any other good spot to add logging in sdap_save_grpmem(), but I could add more logging messages into functions which are called from here...?
I'll be happy to prepare patch for 1.11 but I would prefer to have acked patch for master before doing so.
Thanks.
On Fri, Apr 11, 2014 at 03:28:30PM +0200, Pavel Reichl wrote:
On Fri, 2014-04-11 at 12:16 +0200, Jakub Hrozek wrote:
On Wed, Apr 09, 2014 at 02:29:05PM +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
Hi,
this patch is OK, but I think we should also add more debugging to sdap_save_grpmem().
Would it be too much work to also apply the patch to 1.11 ? I think it might come handy with the planned RHEL-6.6 rebase..
Hi,
I added 3 log messages to sdap_save_grpmem() and 1 to sysdb_store_group. I actually can't see any other good spot to add logging in sdap_save_grpmem(), but I could add more logging messages into functions which are called from here...?
I'll be happy to prepare patch for 1.11 but I would prefer to have acked patch for master before doing so.
Thanks.
I think the current version is good and sdap_save_grpmem() is now documented well.
ACK
Can you also send a version for 1.11 ?
On Mon, Apr 14, 2014 at 12:50:33PM +0200, Jakub Hrozek wrote:
On Fri, Apr 11, 2014 at 03:28:30PM +0200, Pavel Reichl wrote:
On Fri, 2014-04-11 at 12:16 +0200, Jakub Hrozek wrote:
On Wed, Apr 09, 2014 at 02:29:05PM +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
Hi,
this patch is OK, but I think we should also add more debugging to sdap_save_grpmem().
Would it be too much work to also apply the patch to 1.11 ? I think it might come handy with the planned RHEL-6.6 rebase..
Hi,
I added 3 log messages to sdap_save_grpmem() and 1 to sysdb_store_group. I actually can't see any other good spot to add logging in sdap_save_grpmem(), but I could add more logging messages into functions which are called from here...?
I'll be happy to prepare patch for 1.11 but I would prefer to have acked patch for master before doing so.
Thanks.
I think the current version is good and sdap_save_grpmem() is now documented well.
ACK
Can you also send a version for 1.11 ?
Pushed to master: 9123c2abff5780f485764261eb4b180e9ceadf20
On Mon, Apr 14, 2014 at 03:23:10PM +0200, Jakub Hrozek wrote:
On Mon, Apr 14, 2014 at 01:41:48PM +0200, Pavel Reichl wrote:
On Mon, 2014-04-14 at 12:50 +0200, Jakub Hrozek wrote: [snip]
ACK
Can you also send a version for 1.11 ?
Sure.
ACK for sssd-1-11
Pushed to sssd-1-11: 2ba0b0bbac193b7db5d86e542b506978e63f128f
Hello,
some more generally useful logging messages are needed for that bugzilla.
Patches for both master and 1.11 are attached.
Thanks for review!
On Wed, 2014-04-09 at 14:29 +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
On Tue, Apr 15, 2014 at 04:25:50PM +0200, Pavel Reichl wrote:
Hello,
some more generally useful logging messages are needed for that bugzilla.
Patches for both master and 1.11 are attached.
Thanks for review!
It would be nice to explain in the commit message why you used TRACE_LIBS for the messages and not i.e OP_FAILURE.
On Wed, 2014-04-09 at 14:29 +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
From e7e4a06098bce4df600370274babe470b0d71c47 Mon Sep 17 00:00:00 2001 From: Pavel Reichl reichl.pavel@gmail.com Date: Mon, 14 Apr 2014 17:27:23 +0100 Subject: [PATCH] SDAP: augmented logging when adding new group
Resolves: https://fedorahosted.org/sssd/ticket/2239
src/db/sysdb_ops.c | 92 +++++++++++++++++++++++++++++----- src/providers/ldap/sdap_async_groups.c | 2 +- 2 files changed, 81 insertions(+), 13 deletions(-)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 95354d98806a13cf142705dc304b7810b8ee82f4..559f8b51813fa499ec21d9e78189910cacc7b32a 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -1428,7 +1428,15 @@ int sysdb_add_group(struct sysdb_ctx *sysdb, ret = sysdb_search_user_by_name(tmp_ctx, sysdb, domain, name, NULL, &msg); if (ret != ENOENT) {
if (ret == EOK) ret = EEXIST;
if (ret == EOK) {DEBUG(SSSDBG_TRACE_LIBS, ("MPG domain is having a user "
"...domain contains..."
"with the same name - %s.\n", name));ret = EEXIST;} else {DEBUG(SSSDBG_TRACE_LIBS,("sysdb_search_user_by_name failed for user %s.\n",name));} goto done; }
[snip]
goto done; } DEBUG(SSSDBG_MINOR_FAILURE, ("A group with the same GID [%llu] was removed from the " "cache\n", (unsigned long long) gid));
If you're touching the code anyway, can you change the DEBUG calls to use SPRIgid here as well?
ret = sysdb_add_group(sysdb, domain, name, gid, attrs, cache_timeout, now);if (ret) {DEBUG(SSSDBG_MINOR_FAILURE,("sysdb_add_group failed (while renaming group) for: ""%s [%"SPRIgid"].\n", name, gid));} } goto done;
On Thu, 2014-04-17 at 17:30 +0200, Jakub Hrozek wrote:
On Tue, Apr 15, 2014 at 04:25:50PM +0200, Pavel Reichl wrote:
Hello,
some more generally useful logging messages are needed for that bugzilla.
Patches for both master and 1.11 are attached.
Thanks for review!
It would be nice to explain in the commit message why you used TRACE_LIBS for the messages and not i.e OP_FAILURE.
On Wed, 2014-04-09 at 14:29 +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
From e7e4a06098bce4df600370274babe470b0d71c47 Mon Sep 17 00:00:00 2001 From: Pavel Reichl reichl.pavel@gmail.com Date: Mon, 14 Apr 2014 17:27:23 +0100 Subject: [PATCH] SDAP: augmented logging when adding new group
Resolves: https://fedorahosted.org/sssd/ticket/2239
src/db/sysdb_ops.c | 92 +++++++++++++++++++++++++++++----- src/providers/ldap/sdap_async_groups.c | 2 +- 2 files changed, 81 insertions(+), 13 deletions(-)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 95354d98806a13cf142705dc304b7810b8ee82f4..559f8b51813fa499ec21d9e78189910cacc7b32a 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -1428,7 +1428,15 @@ int sysdb_add_group(struct sysdb_ctx *sysdb, ret = sysdb_search_user_by_name(tmp_ctx, sysdb, domain, name, NULL, &msg); if (ret != ENOENT) {
if (ret == EOK) ret = EEXIST;
if (ret == EOK) {DEBUG(SSSDBG_TRACE_LIBS, ("MPG domain is having a user ""...domain contains..."
"with the same name - %s.\n", name));ret = EEXIST;} else {DEBUG(SSSDBG_TRACE_LIBS,("sysdb_search_user_by_name failed for user %s.\n",name));} goto done; }[snip]
goto done; } DEBUG(SSSDBG_MINOR_FAILURE, ("A group with the same GID [%llu] was removed from the " "cache\n", (unsigned long long) gid));If you're touching the code anyway, can you change the DEBUG calls to use SPRIgid here as well?
ret = sysdb_add_group(sysdb, domain, name, gid, attrs, cache_timeout, now);if (ret) {DEBUG(SSSDBG_MINOR_FAILURE,("sysdb_add_group failed (while renaming group) for: ""%s [%"SPRIgid"].\n", name, gid));} } goto done;
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hello Jakub,
sorry for late reply. I attached patch addressing your concerns with previous version. If it desired I'll prepare patch for SSSD 1.11 when version for master is ACKed (to save me from the fun of rebasing this patch multiple times).
Thanks,
Pavel Reichl
On Mon, Apr 28, 2014 at 01:55:00PM +0200, Pavel Reichl wrote:
On Thu, 2014-04-17 at 17:30 +0200, Jakub Hrozek wrote:
On Tue, Apr 15, 2014 at 04:25:50PM +0200, Pavel Reichl wrote:
Hello,
some more generally useful logging messages are needed for that bugzilla.
Patches for both master and 1.11 are attached.
Thanks for review!
It would be nice to explain in the commit message why you used TRACE_LIBS for the messages and not i.e OP_FAILURE.
On Wed, 2014-04-09 at 14:29 +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
From e7e4a06098bce4df600370274babe470b0d71c47 Mon Sep 17 00:00:00 2001 From: Pavel Reichl reichl.pavel@gmail.com Date: Mon, 14 Apr 2014 17:27:23 +0100 Subject: [PATCH] SDAP: augmented logging when adding new group
ACK for master.
I would like a 1.11 version personally.
On Mon, 2014-04-28 at 21:35 +0200, Jakub Hrozek wrote:
On Mon, Apr 28, 2014 at 01:55:00PM +0200, Pavel Reichl wrote:
On Thu, 2014-04-17 at 17:30 +0200, Jakub Hrozek wrote:
On Tue, Apr 15, 2014 at 04:25:50PM +0200, Pavel Reichl wrote:
Hello,
some more generally useful logging messages are needed for that bugzilla.
Patches for both master and 1.11 are attached.
Thanks for review!
It would be nice to explain in the commit message why you used TRACE_LIBS for the messages and not i.e OP_FAILURE.
On Wed, 2014-04-09 at 14:29 +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
From e7e4a06098bce4df600370274babe470b0d71c47 Mon Sep 17 00:00:00 2001 From: Pavel Reichl reichl.pavel@gmail.com Date: Mon, 14 Apr 2014 17:27:23 +0100 Subject: [PATCH] SDAP: augmented logging when adding new group
ACK for master.
I would like a 1.11 version personally. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
OK, patches attached.
On (29/04/14 18:25), Pavel Reichl wrote:
On Mon, 2014-04-28 at 21:35 +0200, Jakub Hrozek wrote:
On Mon, Apr 28, 2014 at 01:55:00PM +0200, Pavel Reichl wrote:
On Thu, 2014-04-17 at 17:30 +0200, Jakub Hrozek wrote:
On Tue, Apr 15, 2014 at 04:25:50PM +0200, Pavel Reichl wrote:
Hello,
some more generally useful logging messages are needed for that bugzilla.
Patches for both master and 1.11 are attached.
Thanks for review!
It would be nice to explain in the commit message why you used TRACE_LIBS for the messages and not i.e OP_FAILURE.
On Wed, 2014-04-09 at 14:29 +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
From e7e4a06098bce4df600370274babe470b0d71c47 Mon Sep 17 00:00:00 2001 From: Pavel Reichl reichl.pavel@gmail.com Date: Mon, 14 Apr 2014 17:27:23 +0100 Subject: [PATCH] SDAP: augmented logging when adding new group
ACK for master.
I would like a 1.11 version personally. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
OK, patches attached.
From 47c158c3dbd63d71a7372823b9d74da252478498 Mon Sep 17 00:00:00 2001 From: Pavel Reichl preichl@redhat.com Date: Mon, 14 Apr 2014 17:27:23 +0100 Subject: [PATCH] SYSDB: augmented logging when adding new group
This patch adds some more log messages to functionality of storing groups into sysdb. As these functions are low level and failures are often handled on higher levels the commonly chosen level is SSSDBG_TRACE_LIBS.
Resolves: https://fedorahosted.org/sssd/ticket/2239
done: if (ret) { @@ -3146,7 +3214,7 @@ int sysdb_cache_auth(struct sysdb_ctx *sysdb, SYSDB_LAST_FAILED_LOGIN, time(NULL)); if (ret != EOK) {
DEBUG(3, ("sysdb_attrs_add_time_t failed\n."));
DEBUG(3, ("sysdb_attrs_add_time_t failed.\n")); goto done;
As you can see this patch was prepared for 1-11 branch with "old-style" DEBUG macro. Unfortunatelly, the second patch 0001-SYSDB-augmented-logging-when-adding-new-group.patch cannot be applied on 1-11 branch
Applying: SYSDB: augmented logging when adding new group error: patch failed: src/db/sysdb_ops.c:1417 error: src/db/sysdb_ops.c: patch does not apply error: patch failed: src/providers/ldap/sdap_async_groups.c:246 error: src/providers/ldap/sdap_async_groups.c: patch does not apply Patch failed at 0001 SYSDB: augmented logging when adding new group The copy of the patch that failed is found in: /dev/shm/sssd/.git/rebase-apply/patch
Could you send new version for 1-11 branch?
LS
On Sat, 2014-05-17 at 23:37 +0200, Pavel Reichl wrote:
On Fri, 2014-05-16 at 16:58 +0200, Lukas Slebodnik wrote: [snip]
Could you send new version for 1-11 branch?
LS
Sure, this is always fun:-).
Sorry we made you redo the DEBUG messages, I didn't realize this patch was still around.
Builds fine and the messages look OK.
ACK for 1.11.
On Sun, 2014-05-18 at 04:51 +0200, Jakub Hrozek wrote:
On Sat, 2014-05-17 at 23:37 +0200, Pavel Reichl wrote:
On Fri, 2014-05-16 at 16:58 +0200, Lukas Slebodnik wrote: [snip]
Could you send new version for 1-11 branch?
LS
Sure, this is always fun:-).
Sorry we made you redo the DEBUG messages, I didn't realize this patch was still around.
Builds fine and the messages look OK.
ACK for 1.11.
Pushed to sssd-1-11 in 098e9067b85b090b60488103a904857924c36dce I just changed the 'resolves' to 'related' in the commit message.
On Mon, 2014-04-28 at 13:55 +0200, Pavel Reichl wrote:
On Thu, 2014-04-17 at 17:30 +0200, Jakub Hrozek wrote:
On Tue, Apr 15, 2014 at 04:25:50PM +0200, Pavel Reichl wrote:
Hello,
some more generally useful logging messages are needed for that bugzilla.
Patches for both master and 1.11 are attached.
Thanks for review!
It would be nice to explain in the commit message why you used TRACE_LIBS for the messages and not i.e OP_FAILURE.
On Wed, 2014-04-09 at 14:29 +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
From e7e4a06098bce4df600370274babe470b0d71c47 Mon Sep 17 00:00:00 2001 From: Pavel Reichl reichl.pavel@gmail.com Date: Mon, 14 Apr 2014 17:27:23 +0100 Subject: [PATCH] SDAP: augmented logging when adding new group
Resolves: https://fedorahosted.org/sssd/ticket/2239
src/db/sysdb_ops.c | 92 +++++++++++++++++++++++++++++----- src/providers/ldap/sdap_async_groups.c | 2 +- 2 files changed, 81 insertions(+), 13 deletions(-)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 95354d98806a13cf142705dc304b7810b8ee82f4..559f8b51813fa499ec21d9e78189910cacc7b32a 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -1428,7 +1428,15 @@ int sysdb_add_group(struct sysdb_ctx *sysdb, ret = sysdb_search_user_by_name(tmp_ctx, sysdb, domain, name, NULL, &msg); if (ret != ENOENT) {
if (ret == EOK) ret = EEXIST;
if (ret == EOK) {DEBUG(SSSDBG_TRACE_LIBS, ("MPG domain is having a user ""...domain contains..."
"with the same name - %s.\n", name));ret = EEXIST;} else {DEBUG(SSSDBG_TRACE_LIBS,("sysdb_search_user_by_name failed for user %s.\n",name));} goto done; }[snip]
goto done; } DEBUG(SSSDBG_MINOR_FAILURE, ("A group with the same GID [%llu] was removed from the " "cache\n", (unsigned long long) gid));If you're touching the code anyway, can you change the DEBUG calls to use SPRIgid here as well?
ret = sysdb_add_group(sysdb, domain, name, gid, attrs, cache_timeout, now);if (ret) {DEBUG(SSSDBG_MINOR_FAILURE,("sysdb_add_group failed (while renaming group) for: ""%s [%"SPRIgid"].\n", name, gid));} } goto done;
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hello Jakub,
sorry for late reply. I attached patch addressing your concerns with previous version. If it desired I'll prepare patch for SSSD 1.11 when version for master is ACKed (to save me from the fun of rebasing this patch multiple times).
Thanks,
Pavel Reichl
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hi,
I think we forgot to push this patch to master. It still applies.
Thanks, PR
On Mon, Jul 14, 2014 at 03:41:15PM +0200, Pavel Reichl wrote:
On Mon, 2014-04-28 at 13:55 +0200, Pavel Reichl wrote:
On Thu, 2014-04-17 at 17:30 +0200, Jakub Hrozek wrote:
On Tue, Apr 15, 2014 at 04:25:50PM +0200, Pavel Reichl wrote:
Hello,
some more generally useful logging messages are needed for that bugzilla.
Patches for both master and 1.11 are attached.
Thanks for review!
It would be nice to explain in the commit message why you used TRACE_LIBS for the messages and not i.e OP_FAILURE.
On Wed, 2014-04-09 at 14:29 +0200, Pavel Reichl wrote:
Hello,
please see attached patch.
This patch was previously written for BZ 1059423. But it now seems that more detailed logging information is generally useful for issues that are emerging from this area lately.
Pavel Reichl
From e7e4a06098bce4df600370274babe470b0d71c47 Mon Sep 17 00:00:00 2001 From: Pavel Reichl reichl.pavel@gmail.com Date: Mon, 14 Apr 2014 17:27:23 +0100 Subject: [PATCH] SDAP: augmented logging when adding new group
Resolves: https://fedorahosted.org/sssd/ticket/2239
src/db/sysdb_ops.c | 92 +++++++++++++++++++++++++++++----- src/providers/ldap/sdap_async_groups.c | 2 +- 2 files changed, 81 insertions(+), 13 deletions(-)
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 95354d98806a13cf142705dc304b7810b8ee82f4..559f8b51813fa499ec21d9e78189910cacc7b32a 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -1428,7 +1428,15 @@ int sysdb_add_group(struct sysdb_ctx *sysdb, ret = sysdb_search_user_by_name(tmp_ctx, sysdb, domain, name, NULL, &msg); if (ret != ENOENT) {
if (ret == EOK) ret = EEXIST;
if (ret == EOK) {DEBUG(SSSDBG_TRACE_LIBS, ("MPG domain is having a user ""...domain contains..."
"with the same name - %s.\n", name));ret = EEXIST;} else {DEBUG(SSSDBG_TRACE_LIBS,("sysdb_search_user_by_name failed for user %s.\n",name));} goto done; }[snip]
goto done; } DEBUG(SSSDBG_MINOR_FAILURE, ("A group with the same GID [%llu] was removed from the " "cache\n", (unsigned long long) gid));If you're touching the code anyway, can you change the DEBUG calls to use SPRIgid here as well?
ret = sysdb_add_group(sysdb, domain, name, gid, attrs, cache_timeout, now);if (ret) {DEBUG(SSSDBG_MINOR_FAILURE,("sysdb_add_group failed (while renaming group) for: ""%s [%"SPRIgid"].\n", name, gid));} } goto done;
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hello Jakub,
sorry for late reply. I attached patch addressing your concerns with previous version. If it desired I'll prepare patch for SSSD 1.11 when version for master is ACKed (to save me from the fun of rebasing this patch multiple times).
Thanks,
Pavel Reichl
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hi,
I think we forgot to push this patch to master. It still applies.
Thanks, PR
Yes, I forgot, sorry about that. I pushed it to master now: 4d69eef4713fac8ade73739dd72f949da51382f4
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Jakub Hrozek -
Lukas Slebodnik -
Pavel Reichl