Hi, patch for https://fedorahosted.org/sssd/ticket/2493 is attached. Petr
On (10/09/15 17:08), Petr Cech wrote:
Hi, patch for https://fedorahosted.org/sssd/ticket/2493 is attached. Petr
From 1d87d8dd390c229ac603569a604d9cca656c3f1b Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Thu, 10 Sep 2015 10:05:59 -0400 Subject: [PATCH] DEBUG: Preventing chown_debug_file if journald on
There is function chown_debug_file() which didn't check if the SSSD is compiled with journald support.
This patch add simple checking of this state.
Resolves: https://fedorahosted.org/sssd/ticket/2493
src/util/debug.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/util/debug.c b/src/util/debug.c index 69df54386101973548108c3194a1bfd111f046f0..70d136dbfc996a4bcbd246861c55c6eba7a5b65b 100644 --- a/src/util/debug.c +++ b/src/util/debug.c @@ -316,6 +316,8 @@ int chown_debug_file(const char *filename, const char *log_file; errno_t ret;
+#ifndef WITH_JOURNALD
- if (filename == NULL) { log_file = debug_log_file; } else {
@@ -336,6 +338,8 @@ int chown_debug_file(const char *filename, return ret; }
+#endif /* WITH_JOURNALD */
I do not understand how is the function chown_debug_file related to journald. sssd can be compiled with journald support and in the same time can log to the files. This is a default for fedora and rehl7.
If someone want to enable logging all messages to journald then it is required manula change to the file /etc/systemd/system/sssd.service.d/journal.conf
LS
On 09/11/2015 11:02 AM, Lukas Slebodnik wrote:
I do not understand how is the function chown_debug_file related to journald. sssd can be compiled with journald support and in the same time can log to the files. This is a default for fedora and rehl7.
If someone want to enable logging all messages to journald then it is required manula change to the file /etc/systemd/system/sssd.service.d/journal.conf
LS
Right, thanks, it wasn't good way how to fix it. There is another fixing patch attached.
I used variable debug_file which inform us if we use logfiles. And I hope that I can ignore variable debug_to_stderr.
Petr
ping
On 09/14/2015 04:36 PM, Petr Cech wrote:
On 09/11/2015 11:02 AM, Lukas Slebodnik wrote:
I do not understand how is the function chown_debug_file related to journald. sssd can be compiled with journald support and in the same time can log to the files. This is a default for fedora and rehl7.
If someone want to enable logging all messages to journald then it is required manula change to the file /etc/systemd/system/sssd.service.d/journal.conf
LS
Right, thanks, it wasn't good way how to fix it. There is another fixing patch attached.
I used variable debug_file which inform us if we use logfiles. And I hope that I can ignore variable debug_to_stderr.
Petr
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Fri, Sep 25, 2015 at 02:05:14PM +0200, Petr Cech wrote:
ping
I can't start sssd as a service with this patch applied after switching from root to non-root: 1) add user=sssd to the [sssd] section 2) chown root.root /var/log/sssd/*.log 3) systemctl start sssd
I must admit I no longer remember what the irritating messages the ticket talks about were. Was is that the file is not there if only journald support is used? Would it make more sense to ignore ENOENT errors in that case (or even always) ?
On (05/10/15 10:21), Jakub Hrozek wrote:
On Fri, Sep 25, 2015 at 02:05:14PM +0200, Petr Cech wrote:
ping
I can't start sssd as a service with this patch applied after switching from root to non-root:
- add user=sssd to the [sssd] section
- chown root.root /var/log/sssd/*.log
- systemctl start sssd
I must admit I no longer remember what the irritating messages the ticket talks about were. Was is that the file is not there if only journald support is used? Would it make more sense to ignore ENOENT errors in that case (or even always) ?
Could it be related to problems with changing own if logging to stderr? IIRC krb5_child initially log to stderr before initializing logs to file and therefore some messages were sent to journald.
LS
On 10/05/2015 10:21 AM, Jakub Hrozek wrote:
On Fri, Sep 25, 2015 at 02:05:14PM +0200, Petr Cech wrote:
ping
I can't start sssd as a service with this patch applied after switching from root to non-root: 1) add user=sssd to the [sssd] section 2) chown root.root /var/log/sssd/*.log 3) systemctl start sssd
I must admit I no longer remember what the irritating messages the ticket talks about were. Was is that the file is not there if only journald support is used? Would it make more sense to ignore ENOENT errors in that case (or even always) ?
Hi Jakub,
ticket says "chown_debug_file() is called unconditionally and it does not make any check if debug to files is active or not. This might cause irritating error messages e.g. when journald is used."
I must admit I didn't check my patch currently. But I tried to reproduce your problem with starting SSSD after switching root to non-root user. I think I catch the problem.
I tried it (switch the user) without my patch applied---and the result was the same, it couldn't start. Problem is in step number 2, you wrote root instead of sssd.
I tried full installation now (make, create rpm, install from rpm) and it is possible to run SSSD without and with my patch applied.
Maybe this could help to find short way like in step 2:
[root@albireo sssd]# ll /var/lib/ drwxr-xr-x. 8 root root 80 Oct 21 10:15 sss
[root@albireo sssd]# ll /var/lib/sss drwx------. 2 sssd sssd 4096 Oct 21 10:25 db drwxr-xr-x. 2 sssd sssd 6 Oct 21 10:15 gpo_cache drwx------. 2 sssd sssd 6 Oct 21 10:15 keytabs drwxr-xr-x. 2 sssd sssd 48 Oct 21 10:25 mc drwxr-xr-x. 3 sssd sssd 40 Oct 21 10:25 pipes drwxr-xr-x. 3 sssd sssd 27 Oct 21 10:25 pubconf
[root@albireo sssd]# ll /var/log drwxr-x---. 2 sssd sssd 4096 Oct 21 10:15 sssd
Regards
Petr
How to reproduce:
Sumit wrote explanation to ticket comment. Better is if .log files missing. And you need run SSSD logging only to journal. Lukas wrote in soe mail above in thread, how to enable it.
Thanks to all.
Petr
On 10/23/2015 02:18 PM, Petr Cech wrote:
How to reproduce:
Sumit wrote explanation to ticket comment. Better is if .log files missing. And you need run SSSD logging only to journal. Lukas wrote in soe mail above in thread, how to enable it.
Thanks to all.
Petr
# sudo bash # systemctl stop sssd # vim /etc/systemd/system/sssd.service.d/journal.conf # rm /var/log/sssd/*.log # systemctl daemon-reload # systemctl start sssd # journalctl -r | grep 'chown failed'
On (14/09/15 16:36), Petr Cech wrote:
On 09/11/2015 11:02 AM, Lukas Slebodnik wrote:
I do not understand how is the function chown_debug_file related to journald. sssd can be compiled with journald support and in the same time can log to the files. This is a default for fedora and rehl7.
If someone want to enable logging all messages to journald then it is required manula change to the file /etc/systemd/system/sssd.service.d/journal.conf
LS
Right, thanks, it wasn't good way how to fix it. There is another fixing patch attached.
I used variable debug_file which inform us if we use logfiles. And I hope that I can ignore variable debug_to_stderr.
Petr
From 8cb0a4a6b59259e9096ae6f5926595b7b10d6b27 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Thu, 10 Sep 2015 10:05:59 -0400 Subject: [PATCH] DEBUG: Preventing chown_debug_file if journald on
There is function chown_debug_file() which didn't check if the SSSD is compiled with journald support.
This patch add simple checking of this state.
Resolves: https://fedorahosted.org/sssd/ticket/2493
src/util/debug.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/src/util/debug.c b/src/util/debug.c index 69df54386101973548108c3194a1bfd111f046f0..b6ab368db824bbd297dcb410c3e669d911ff0d33 100644 --- a/src/util/debug.c +++ b/src/util/debug.c @@ -316,24 +316,27 @@ int chown_debug_file(const char *filename, const char *log_file; errno_t ret;
- if (filename == NULL) {
log_file = debug_log_file;- } else {
log_file = filename;- }
- if (debug_file) {
- ret = asprintf(&logpath, "%s/%s.log", LOG_PATH, log_file);
- if (ret == -1) {
return ENOMEM;- }
if (filename == NULL) {log_file = debug_log_file;} else {log_file = filename;}
- ret = chown(logpath, uid, gid);
- free(logpath);
- if (ret != 0) {
ret = errno;DEBUG(SSSDBG_FATAL_FAILURE, "chown failed for [%s]: [%d]\n",log_file, ret);return ret;
ret = asprintf(&logpath, "%s/%s.log", LOG_PATH, log_file);if (ret == -1) {return ENOMEM;}ret = chown(logpath, uid, gid);free(logpath);if (ret != 0) {ret = errno;DEBUG(SSSDBG_FATAL_FAILURE, "chown failed for [%s]: [%d]\n",log_file, ret);return ret;}}
return EOK;
ACK
The error message is not displayed when logging to journald or to stderr. Becasue if debug_file is not set then we will not log to files and in this case it does not make sense to call chown().
http://sssd-ci.duckdns.org/logs/job/31/41/summary.html
LS
On (27/10/15 10:39), Lukas Slebodnik wrote:
On (14/09/15 16:36), Petr Cech wrote:
On 09/11/2015 11:02 AM, Lukas Slebodnik wrote:
I do not understand how is the function chown_debug_file related to journald. sssd can be compiled with journald support and in the same time can log to the files. This is a default for fedora and rehl7.
If someone want to enable logging all messages to journald then it is required manula change to the file /etc/systemd/system/sssd.service.d/journal.conf
LS
Right, thanks, it wasn't good way how to fix it. There is another fixing patch attached.
I used variable debug_file which inform us if we use logfiles. And I hope that I can ignore variable debug_to_stderr.
Petr
From 8cb0a4a6b59259e9096ae6f5926595b7b10d6b27 Mon Sep 17 00:00:00 2001 From: Petr Cech pcech@redhat.com Date: Thu, 10 Sep 2015 10:05:59 -0400 Subject: [PATCH] DEBUG: Preventing chown_debug_file if journald on
There is function chown_debug_file() which didn't check if the SSSD is compiled with journald support.
This patch add simple checking of this state.
Resolves: https://fedorahosted.org/sssd/ticket/2493
src/util/debug.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/src/util/debug.c b/src/util/debug.c index 69df54386101973548108c3194a1bfd111f046f0..b6ab368db824bbd297dcb410c3e669d911ff0d33 100644 --- a/src/util/debug.c +++ b/src/util/debug.c @@ -316,24 +316,27 @@ int chown_debug_file(const char *filename, const char *log_file; errno_t ret;
- if (filename == NULL) {
log_file = debug_log_file;- } else {
log_file = filename;- }
- if (debug_file) {
- ret = asprintf(&logpath, "%s/%s.log", LOG_PATH, log_file);
- if (ret == -1) {
return ENOMEM;- }
if (filename == NULL) {log_file = debug_log_file;} else {log_file = filename;}
- ret = chown(logpath, uid, gid);
- free(logpath);
- if (ret != 0) {
ret = errno;DEBUG(SSSDBG_FATAL_FAILURE, "chown failed for [%s]: [%d]\n",log_file, ret);return ret;
ret = asprintf(&logpath, "%s/%s.log", LOG_PATH, log_file);if (ret == -1) {return ENOMEM;}ret = chown(logpath, uid, gid);free(logpath);if (ret != 0) {ret = errno;DEBUG(SSSDBG_FATAL_FAILURE, "chown failed for [%s]: [%d]\n",log_file, ret);return ret;}}
return EOK;
ACK
The error message is not displayed when logging to journald or to stderr. Becasue if debug_file is not set then we will not log to files and in this case it does not make sense to call chown().
BTW we could see the error message in cwrap test. src/tests/cwrap/server-tests.log
master: * 152fed23797c8950ca18cf6dc2bddb61a3f615c8
sssd-1-13: * 6e2822b151c21ce6e3287a0cf25d40e9f10a6127
LS
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Petr Cech