Re: [systemd-devel] [PATCH] journald: fix syslog facility for messages coming from kmsg

2014-07-28 Thread Michal Sekletar 
On Sun, Jul 27, 2014 at 07:57:47PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> On Sun, Jul 27, 2014 at 01:11:07PM +0200, Michal Sekletar wrote:
> > On Sat, Jul 26, 2014 at 09:11:47PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> > > Hm, what was wrong with the facility before?
> > 
> > I think that we should always add SYSLOG_FACILITY field. Now, if (priority &
> > LOG_FACMASK) == LOG_KERN we don't do that. We set SYSLOG_IDENTIFIER to 
> > "kernel"
> > tough. But missing SYSLOG_FACILITY field confuses some tools, most notably
> > rsyslog, thus rsyslog filters like "kern.* /var/log/kernel.log" doesn't 
> > work and
> > rsyslog don't output kernel log messages to /var/log/kernel.log.
> 
> Agreed, even though for slightly different reason :)
> syslog(2) describes LOG_KERN as the facility for kernel messages, so we should
> set it so. So I'm +1 for pushing this change, but *please* add the explanation
> to the commit message. It makes it much easier for downstreams, especially for
> people who see the commit in isolation on some stable branch a few months down
> the road.

Pushed, with explanation. Thanks!

> 
> Zbyszek
> 
> > > On Fri, Jul 25, 2014 at 03:04:44PM +0200, Michal Sekletar wrote:
> > > > ---
> > > >  src/journal/journald-kmsg.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
> > > > index 12992e7..8d24344 100644
> > > > --- a/src/journal/journald-kmsg.c
> > > > +++ b/src/journal/journald-kmsg.c
> > > > @@ -274,6 +274,9 @@ static void dev_kmsg_record(Server *s, char *p, 
> > > > size_t l) {
> > > >  if (asprintf(&syslog_priority, "PRIORITY=%i", priority & 
> > > > LOG_PRIMASK) >= 0)
> > > >  IOVEC_SET_STRING(iovec[n++], syslog_priority);
> > > >  
> > > > +if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", priority 
> > > > & LOG_FACMASK) >= 0)
> > > > +IOVEC_SET_STRING(iovec[n++], syslog_facility);
> > > > +
> > > >  if ((priority & LOG_FACMASK) == LOG_KERN)
> > > >  IOVEC_SET_STRING(iovec[n++], 
> > > > "SYSLOG_IDENTIFIER=kernel");
> > > >  else {
> > > > @@ -295,9 +298,6 @@ static void dev_kmsg_record(Server *s, char *p, 
> > > > size_t l) {
> > > >  if (syslog_pid)
> > > >  IOVEC_SET_STRING(iovec[n++], 
> > > > syslog_pid);
> > > >  }
> > > > -
> > > > -if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", 
> > > > LOG_FAC(priority)) >= 0)
> > > > -IOVEC_SET_STRING(iovec[n++], syslog_facility);
> > > >  }
> > > >  
> > > >  message = cunescape_length_with_prefix(p, pl, "MESSAGE=");
> > > > -- 
> > > > 2.0.1
> > > > 
> > > > ___
> > > > systemd-devel mailing list
> > > > systemd-devel@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> > > > 
> > 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] journald: fix syslog facility for messages coming from kmsg

2014-07-27 Thread Zbigniew Jędrzejewski-Szmek 
On Sun, Jul 27, 2014 at 01:11:07PM +0200, Michal Sekletar wrote:
> On Sat, Jul 26, 2014 at 09:11:47PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> > Hm, what was wrong with the facility before?
> 
> I think that we should always add SYSLOG_FACILITY field. Now, if (priority &
> LOG_FACMASK) == LOG_KERN we don't do that. We set SYSLOG_IDENTIFIER to 
> "kernel"
> tough. But missing SYSLOG_FACILITY field confuses some tools, most notably
> rsyslog, thus rsyslog filters like "kern.* /var/log/kernel.log" doesn't work 
> and
> rsyslog don't output kernel log messages to /var/log/kernel.log.

Agreed, even though for slightly different reason :)
syslog(2) describes LOG_KERN as the facility for kernel messages, so we should
set it so. So I'm +1 for pushing this change, but *please* add the explanation
to the commit message. It makes it much easier for downstreams, especially for
people who see the commit in isolation on some stable branch a few months down
the road.

Zbyszek

> > On Fri, Jul 25, 2014 at 03:04:44PM +0200, Michal Sekletar wrote:
> > > ---
> > >  src/journal/journald-kmsg.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
> > > index 12992e7..8d24344 100644
> > > --- a/src/journal/journald-kmsg.c
> > > +++ b/src/journal/journald-kmsg.c
> > > @@ -274,6 +274,9 @@ static void dev_kmsg_record(Server *s, char *p, 
> > > size_t l) {
> > >  if (asprintf(&syslog_priority, "PRIORITY=%i", priority & 
> > > LOG_PRIMASK) >= 0)
> > >  IOVEC_SET_STRING(iovec[n++], syslog_priority);
> > >  
> > > +if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", priority & 
> > > LOG_FACMASK) >= 0)
> > > +IOVEC_SET_STRING(iovec[n++], syslog_facility);
> > > +
> > >  if ((priority & LOG_FACMASK) == LOG_KERN)
> > >  IOVEC_SET_STRING(iovec[n++], "SYSLOG_IDENTIFIER=kernel");
> > >  else {
> > > @@ -295,9 +298,6 @@ static void dev_kmsg_record(Server *s, char *p, 
> > > size_t l) {
> > >  if (syslog_pid)
> > >  IOVEC_SET_STRING(iovec[n++], syslog_pid);
> > >  }
> > > -
> > > -if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", 
> > > LOG_FAC(priority)) >= 0)
> > > -IOVEC_SET_STRING(iovec[n++], syslog_facility);
> > >  }
> > >  
> > >  message = cunescape_length_with_prefix(p, pl, "MESSAGE=");
> > > -- 
> > > 2.0.1
> > > 
> > > ___
> > > systemd-devel mailing list
> > > systemd-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> > > 
> 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] journald: fix syslog facility for messages coming from kmsg

2014-07-27 Thread Michal Sekletar 
On Sat, Jul 26, 2014 at 09:11:47PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> Hm, what was wrong with the facility before?

I think that we should always add SYSLOG_FACILITY field. Now, if (priority &
LOG_FACMASK) == LOG_KERN we don't do that. We set SYSLOG_IDENTIFIER to "kernel"
tough. But missing SYSLOG_FACILITY field confuses some tools, most notably
rsyslog, thus rsyslog filters like "kern.* /var/log/kernel.log" doesn't work and
rsyslog don't output kernel log messages to /var/log/kernel.log.

Michal

> Zbyszek
> 
> On Fri, Jul 25, 2014 at 03:04:44PM +0200, Michal Sekletar wrote:
> > ---
> >  src/journal/journald-kmsg.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
> > index 12992e7..8d24344 100644
> > --- a/src/journal/journald-kmsg.c
> > +++ b/src/journal/journald-kmsg.c
> > @@ -274,6 +274,9 @@ static void dev_kmsg_record(Server *s, char *p, size_t 
> > l) {
> >  if (asprintf(&syslog_priority, "PRIORITY=%i", priority & 
> > LOG_PRIMASK) >= 0)
> >  IOVEC_SET_STRING(iovec[n++], syslog_priority);
> >  
> > +if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", priority & 
> > LOG_FACMASK) >= 0)
> > +IOVEC_SET_STRING(iovec[n++], syslog_facility);
> > +
> >  if ((priority & LOG_FACMASK) == LOG_KERN)
> >  IOVEC_SET_STRING(iovec[n++], "SYSLOG_IDENTIFIER=kernel");
> >  else {
> > @@ -295,9 +298,6 @@ static void dev_kmsg_record(Server *s, char *p, size_t 
> > l) {
> >  if (syslog_pid)
> >  IOVEC_SET_STRING(iovec[n++], syslog_pid);
> >  }
> > -
> > -if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", 
> > LOG_FAC(priority)) >= 0)
> > -IOVEC_SET_STRING(iovec[n++], syslog_facility);
> >  }
> >  
> >  message = cunescape_length_with_prefix(p, pl, "MESSAGE=");
> > -- 
> > 2.0.1
> > 
> > ___
> > systemd-devel mailing list
> > systemd-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> > 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] journald: fix syslog facility for messages coming from kmsg

2014-07-26 Thread Zbigniew Jędrzejewski-Szmek 
Hm, what was wrong with the facility before?

Zbyszek

On Fri, Jul 25, 2014 at 03:04:44PM +0200, Michal Sekletar wrote:
> ---
>  src/journal/journald-kmsg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
> index 12992e7..8d24344 100644
> --- a/src/journal/journald-kmsg.c
> +++ b/src/journal/journald-kmsg.c
> @@ -274,6 +274,9 @@ static void dev_kmsg_record(Server *s, char *p, size_t l) 
> {
>  if (asprintf(&syslog_priority, "PRIORITY=%i", priority & 
> LOG_PRIMASK) >= 0)
>  IOVEC_SET_STRING(iovec[n++], syslog_priority);
>  
> +if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", priority & 
> LOG_FACMASK) >= 0)
> +IOVEC_SET_STRING(iovec[n++], syslog_facility);
> +
>  if ((priority & LOG_FACMASK) == LOG_KERN)
>  IOVEC_SET_STRING(iovec[n++], "SYSLOG_IDENTIFIER=kernel");
>  else {
> @@ -295,9 +298,6 @@ static void dev_kmsg_record(Server *s, char *p, size_t l) 
> {
>  if (syslog_pid)
>  IOVEC_SET_STRING(iovec[n++], syslog_pid);
>  }
> -
> -if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", 
> LOG_FAC(priority)) >= 0)
> -IOVEC_SET_STRING(iovec[n++], syslog_facility);
>  }
>  
>  message = cunescape_length_with_prefix(p, pl, "MESSAGE=");
> -- 
> 2.0.1
> 
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] journald: fix syslog facility for messages coming from kmsg

2014-07-25 Thread Michal Sekletar 
---
 src/journal/journald-kmsg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c
index 12992e7..8d24344 100644
--- a/src/journal/journald-kmsg.c
+++ b/src/journal/journald-kmsg.c
@@ -274,6 +274,9 @@ static void dev_kmsg_record(Server *s, char *p, size_t l) {
 if (asprintf(&syslog_priority, "PRIORITY=%i", priority & LOG_PRIMASK) 
>= 0)
 IOVEC_SET_STRING(iovec[n++], syslog_priority);
 
+if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", priority & 
LOG_FACMASK) >= 0)
+IOVEC_SET_STRING(iovec[n++], syslog_facility);
+
 if ((priority & LOG_FACMASK) == LOG_KERN)
 IOVEC_SET_STRING(iovec[n++], "SYSLOG_IDENTIFIER=kernel");
 else {
@@ -295,9 +298,6 @@ static void dev_kmsg_record(Server *s, char *p, size_t l) {
 if (syslog_pid)
 IOVEC_SET_STRING(iovec[n++], syslog_pid);
 }
-
-if (asprintf(&syslog_facility, "SYSLOG_FACILITY=%i", 
LOG_FAC(priority)) >= 0)
-IOVEC_SET_STRING(iovec[n++], syslog_facility);
 }
 
 message = cunescape_length_with_prefix(p, pl, "MESSAGE=");
-- 
2.0.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel