Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Lukas Slebodnik]

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 
>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(, "%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(, "%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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Lukas Slebodnik]

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 
>>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(, "%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(, "%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
>

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 mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Petr Cech]

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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Petr Cech]

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'
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Petr Cech]

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  root80 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 sssd6 Oct 21 10:15 gpo_cache
drwx--. 2 sssd sssd6 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 sssd4096 Oct 21 10:15 sssd

Regards

Petr
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Jakub Hrozek]

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) ?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Lukas Slebodnik]

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:
>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) ?
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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Petr Cech]

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


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Petr Cech]

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 
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(, "%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(, "%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;
-- 
2.4.3

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Re: [SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Lukas Slebodnik]

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 
>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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[SSSD] [PATCH] DEBUG: Preventing chown_debug_file if journald on

[Petr Cech]

Hi,
patch for
https://fedorahosted.org/sssd/ticket/2493
is attached.
Petr
>From 1d87d8dd390c229ac603569a604d9cca656c3f1b Mon Sep 17 00:00:00 2001
From: Petr Cech 
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 */
+
 return EOK;
 }
 
-- 
2.4.3

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel