Bug 243831 - Review Request: rsyslog - the enhanced syslogd for Linux and Unix
Review Request: rsyslog - the enhanced syslogd for Linux and Unix
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tomas Mraz
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-12 05:15 EDT by Peter Vrabec
Modified: 2008-07-13 15:06 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-22 09:33:50 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tmraz: fedora‑review+


Attachments (Terms of Use)
Specfile patch (1.72 KB, patch)
2007-06-22 09:34 EDT, Jose Pedro Oliveira
no flags Details | Diff

  None (edit)
Description Peter Vrabec 2007-06-12 05:15:33 EDT
Spec URL: http://people.redhat.com/pvrabec/rpms/rsyslog.spec
SRPM URL: http://people.redhat.com/pvrabec/rpms/rsyslog-1.13.2-1.src.rpm
Description: rsyslog is an enhanced multi-threaded syslogd supporting, among others, MySQL,syslog/tcp, RFC 3195, permitted sender lists, filtering on any message part,and fine grain output format control. It is quite compatible to stock sysklogd and can be used as a drop-in replacement. Its advanced features make it suitable for enterprise-class, encryption protected syslog relay chains while at the same time being very easy to setup for the novice user.
Comment 1 Tomas Mraz 2007-06-12 17:28:23 EDT
First few comments:

- the init script enables the rsyslogd by default in runlevels 2 3 4 5. This is
not OK if we are not immediately replacing the old syslogd with rsyslog. Or do
we want to replace it immediately?

- the rsyslogd binary links to libmysqlclient which then pulls openssl and other
libraries. I'm not sure this is desirable as that means that basically every
system installed will have to be installed with the mysql package. Would it be
possible to compile rsyslogd twice (once with and once without mysql support)
and put the mysql-enabled one in a subpackage? It would have to be named
differently of course. The init script could then switch between the two
binaries. If the mysql-enabled one was installed it would be run and if not the
regular one would be run.
Comment 2 Steve Grubb 2007-06-12 17:39:55 EDT
Yes I think we want a drop-in replacement.

On the other point, I agree that requiring mysql libraries and openssl is not
good. What about a dlopen() approach? Freeradius has the same problems and its
solved by using dlopen.
Comment 3 Peter Vrabec 2007-06-13 08:14:43 EDT
There are some options, how we can solve sql problem:
1, rsyslog + rsyslog-mysql
- this is weird, isn't it?
2, rsyslog + dlopen(libmysql.so)
- I was told that this isn't very nice solution, is there any other opinion?
3, rsyslog + plug-in interface + sql plug-in
- does it make sense? Is there a place for more plug-ins in rsyslog? 

I can rebuild rsyslog without sql support for now and ask upstream, what they 
think about the problem.
Comment 4 Steve Grubb 2007-06-13 08:51:27 EDT
Yes, I think its a good idea to make it without mysql short term and work with
upstream on the more permanent solution.

As for dlopen, its a good solution as long as you understand it slows down
startup time. For a long-running daemon that should be fine. But no matter what,
we should work with upstream for the solution. Thanks.
Comment 5 Tomas Mraz 2007-06-13 09:12:27 EDT
The dlopen solution should not be used on libraries which are not plugins -
specifically designed for that. For example you don't have ABI stability
guarantee checked by SONAME then. So I don't think dlopening libmysqlclient
directly is a supportable way how to do it.
Comment 6 Peter Vrabec 2007-06-13 09:30:09 EDT
DB support is off
SRPM URL: http://people.redhat.com/pvrabec/rpms/rsyslog-1.13.2-2.src.rpm
Comment 7 Rainer Gerhards 2007-06-13 09:54:15 EDT
I am the maintainer of the rsyslog project.

In the long-term, a plugin interface is probably the best solution. However, I
am not sure if that is something that I can get in very quickly. My next plan is
to support TLS-encrypted syslog inside rsyslog. I am not sure if *that* is
really something for a plugin (but maybe it is...). TLS encryption is probably
something that most people are interested in. However, that means that we will
need to have the openssl package on all boxes that use it. This shows there is a
broader scope of the problem discussed here...
Comment 8 manuel wolfshant 2007-06-13 17:09:53 EDT
Just my 2c: I would not mind at all having openssl as a Require, for the simple
reason that openssh is already installed on all my boxes and it requires openssl
anyway. OTOH I doubt that in a world where other databases exist (starting with
sqlite and pgsql) everyone would need/use centralized logging in mysql.
Therefore I think that a plugin mechanism for this part of the project would be
a very nice addon.
Comment 9 Tomas Mraz 2007-06-13 17:19:44 EDT
There is a license problem (note that sysklogd has exactly the same problem as
it comes from the klogd) - there is BSD-advertising licensed source syslog.c and
GPL licensed source klogd.c linked together. I don't think this is right.
Comment 10 Rainer Gerhards 2007-06-14 02:43:23 EDT
On the license issue: I have just checked glibc and the syslog.c included there
has exactly the same problem. Doesn't that mean that no program can link to
syslog.c without any problems? Wasn't glibc supposed to be all GPLed? (Sorry if
that sounds foulish, I am not so much a license guy). I am asking all these
questions because I am looking for alternate code to resolve the issue...

And, yes, a plugin interface would be especially useful for other databases (as
well as mail forwarding and much more)
Comment 11 Tomas Mraz 2007-06-14 03:39:30 EDT
Well the advertising clause is removed in the newer glibc sources. As the
University of California given up the advertising clause it 'should' be possible
to remove it here as well. The question is whether the later changes when the
code was part of the klogd are covered by the advertising clause or not. But
IANAL so this should be consulted with them.

But I tried to simply remove the syslog.c alltogether and use the glibc's
implementation and it seems to work fine. Even more with the included syslog.c
'#000' is appended to every log message but that might be some problem i rsyslog
as well. When glibc syslog is used it doesn't happen.

But the klogd has another problem - it calls syslog() from signal handler and
this shouldn't be done if we call glibc's syslog() as this is not signal handler
safe function. To interrupt the read() so the klogd is killed immediately and
not after kernel message arrives sigaction() should be used instead of signal()
to set the signal handlers and just set a flag in the signal handler to exit the
main loop.
Comment 12 Rainer Gerhards 2007-06-14 03:43:17 EDT
Our messages crossed.

I've done some more investigation on syslog.c. I barely remembered that it was a
slightly patched version for the sake of klogd (I didn't care much about klogd
until recently). Actually it needs a special version because the glibc version
disallows kernel logging. A interesting thread showed up:

http://www.ecos.sourceware.org/ml/libc-alpha/2000-03/msg00097.html

In short: there even is a problem with that version. I'll now replace it with
the glibc version, which also removes the advertising clause. I'll then make the
necessary modifications to allow kernel logging. I'll also look at the other
klogd issue. As I said, klogd so far is a carry-over from sysklogd and not modified.
Comment 13 Tomas Mraz 2007-06-14 04:10:24 EDT
Ah yes, you're right. But you'll have to modify the glibc code as it uses many
internal glibc functions.
Comment 14 Rainer Gerhards 2007-06-14 09:13:12 EDT
So, I have solved the syslog.c issue. I analyzed what klogd actually does and it
turned out that it needed only few parts of the syslog() functionality. I've now
crafted a new interface and rewritten the file from scratch. While doing so, I
have removed some bugs and also enhanced performance. The new version is already
in CVS:

http://rsyslog.cvs.sourceforge.net/rsyslog/rsyslog/syslog.c?revision=1.4&view=markup

I'll have a look at klogd and its signal handlers soon. In general, it looks
like there is some room for improvement ;)
Comment 15 Tomas Mraz 2007-06-14 15:33:18 EDT
The signal handling is not a problem which blocks review so if you want to make
a release now which could Peter use for new .src.rpm it can be reviewed and
accepted for Fedora.
Comment 16 Jose Pedro Oliveira 2007-06-14 15:47:49 EDT
Regarding comments #1 and # 2

We can't ship rsyslog with the init script enabled by default. It will break
things (sysklogd and syslog-ng).

jpo
Comment 17 Steve Grubb 2007-06-14 15:58:08 EDT
Regarding comment #16. The plan is to replace syslogd since its no longer
maintained upstream. This should be a drop-in replacement since its forked from
the same codebase. As such it needs to have the same status that syslogd had or
the switchover will fail. We could put a conflicts with syslogd statement in it
to ease the transition.
Comment 18 Tomas Mraz 2007-06-14 16:03:15 EDT
Perhaps Obsoletes/Provides as well?
Comment 19 Jose Pedro Oliveira 2007-06-14 16:07:26 EDT
(In reply to comment #17)
> Regarding comment #16. The plan is to replace syslogd since its no longer
> maintained upstream. This should be a drop-in replacement since its forked from
> the same codebase. As such it needs to have the same status that syslogd had or
> the switchover will fail. We could put a conflicts with syslogd statement in it
> to ease the transition.


Then you will have to use the same pidfile and logrotate file as used in
sysklogd.  At least syslog-ng depends on their existence.  

jpo
Comment 20 Jose Pedro Oliveira 2007-06-14 16:14:22 EDT
A longer explanation:

Due to a logrotate limitation (unable to function if it detects the same file
being rotate in diferent scripts) I had to ship the same logrotate script as
shipped in sysklogd (same MD5 digest to avoid file conflicts).  In order to be
able to ship it I also had to modified the name of the default pidfile name used
by syslog-ng.


$ cat /etc/sysconfig/syslog-ng 
#---
# Syslog-ng command line options
# See syslog-ng(8) for more details
#---
SYSLOGNG_OPTIONS="-p /var/run/syslogd.pid"


jpo
Comment 21 Jose Pedro Oliveira 2007-06-14 16:41:09 EDT
The directory /usr/sbin is being created twice and that directory will never be
used.


$ diff  rsyslog.spec.old rsyslog.spec
46,47c46
< mkdir -p $RPM_BUILD_ROOT{/etc,%{_sbindir},%{_mandir}/man{5,8},/usr/sbin}
< mkdir -p $RPM_BUILD_ROOT/sbin
---
> install -d -m 755 $RPM_BUILD_ROOT{/etc,/sbin,%{_mandir}/man{5,8}}
Comment 22 Jose Pedro Oliveira 2007-06-14 16:45:38 EDT
Also the references to the directories "/etc" and "/etc/rc.d/init.d" should be
replaced by their respective macros: _sysconfdir  and _initrddir.
Comment 23 Rainer Gerhards 2007-06-15 05:53:28 EDT
I have created a new release:

http://www.rsyslog.com/Downloads-req-getit-lid-32.phtml

The signal problem is still in the code, but that should be no show stopper (and
soon to be removed).
Comment 24 Jose Pedro Oliveira 2007-06-17 14:08:35 EDT
(In reply to comment #18)
> Perhaps Obsoletes/Provides as well?

The provides statement shouldn't be necessary. The "Provides: syslog" statement
should be enough.  The only packages that require "syslog" are "initscripts" and
"vixie-cron".


From the syslog-ng specfile:
...
#
# Keep initscripts and vixie-cron happy
#
Provides:         syslog
...


Additional information about the "Provides: syslog" statement:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=172885

jpo
Comment 25 Rainer Gerhards 2007-06-18 06:31:34 EDT
I have just release 1.13.4 which contains, among others, the fix for klogd
signal handling.

Download: http://www.rsyslog.com/Downloads-req-getit-lid-33.phtml
Changelog: http://www.rsyslog.com/Article71.phtml
Comment 26 Peter Vrabec 2007-06-18 08:04:03 EDT
SRPM URL: http://people.redhat.com/pvrabec/rpms/rsyslog-1.13.4-1.src.rpm

(comment #20)
Jose, I think logrotate is able to handle this situation since F7.
Comment 27 Tomas Mraz 2007-06-21 16:13:09 EDT
Rainer, would it be possible for you in future to provide a download link in
such form so it would contain the tarball name and otherwise stayed the same
over version changes? If not, it is nothing critical but it is nice to be able
to use directly the source download URL in the package spec.
Comment 28 Rainer Gerhards 2007-06-21 16:28:03 EDT
Tomas,

no problem: the URL is just for me to track download numbers, not really
essential. The target url is always

http://download.adiscon.com/rsyslog/rsyslog-<version>.tar.gz

in the current case:

http://download.adiscon.com/rsyslog/rsyslog-1.13.4.tar.gz

Very early releases missed the /rsyslog path, which I got bashed for by my
webmaster ;)

I might be possible, though, that the url will change in the future. I think
I'll see if I can change the hostname to download.rsyslog.com, which would bring
us on the save side. I keep you posted when this happens (and for the forseable
future, both names will be valid).
Comment 29 Tomas Mraz 2007-06-21 17:00:57 EDT
rpmlint output:

rpmlint -v ~/rsyslog-1.13.4-1.src.rpm
I: rsyslog checking
W: rsyslog unversioned-explicit-provides syslog
- that's OK

rpmlint -v ../RPMS/x86_64/rsyslog-1.13.4-1.x86_64.rpm 
I: rsyslog checking
W: rsyslog service-default-enabled /etc/rc.d/init.d/rsyslog
- OK (comment #2)

rpmlint -v ../RPMS/x86_64/rsyslog-debuginfo-1.13.4-1.x86_64.rpm 
I: rsyslog-debuginfo checking
W: rsyslog-debuginfo spurious-executable-perm
/usr/src/debug/rsyslog-1.13.4/srUtils.c
E: rsyslog-debuginfo wrong-script-end-of-line-encoding
/usr/src/debug/rsyslog-1.13.4/srUtils.c
W: rsyslog-debuginfo spurious-executable-perm
/usr/src/debug/rsyslog-1.13.4/stringbuf.h
E: rsyslog-debuginfo wrong-script-end-of-line-encoding
/usr/src/debug/rsyslog-1.13.4/stringbuf.h
W: rsyslog-debuginfo spurious-executable-perm
/usr/src/debug/rsyslog-1.13.4/stringbuf.c
- OK (just sources)

Please fix:
- use 'Source0: http://download.adiscon.com/rsyslog/%{name}-%{version}.tar.gz'
- be consistent use %{_initrddir} instead of %{_sysconfdir}/rc.d/init.d in %files
- change Summary to: Enhanced system logging and kernel message trapping daemons
- use %defattr(-,root,root,-) in %files
Comment 30 Peter Vrabec 2007-06-22 05:38:34 EDT
SRPM URL: http://people.redhat.com/pvrabec/rpms/rsyslog-1.13.4-2.src.rpm
Comment 31 Rainer Gerhards 2007-06-22 06:51:59 EDT
To keep you busy, I just released a new version:

http://download.adiscon.com/rsyslog/rsyslog-1.13.5.tar.gz
Comment 32 Peter Vrabec 2007-06-22 07:36:58 EDT
thnx. Rainer ;-),

SRPM URL: http://people.redhat.com/pvrabec/rpms/rsyslog-1.13.5-1.src.rpm
Comment 33 Tomas Mraz 2007-06-22 07:59:11 EDT
OK, the package seems fine for Fedora and confirms to guidelines.

rpmlint output is basically the same as in comment #29 which is OK too.

APPROVED
Comment 34 Jose Pedro Oliveira 2007-06-22 09:34:13 EDT
Created attachment 157618 [details]
Specfile patch

Peter,

The patch includes the following modifications to the specfile:

 * adds the dist tag
 * corrects the last changelog entry
 * simplifies the %setup line
 * preserves time stamps (install -p)
 * removes the chmod lines

A few more comments:
 * does the bash (>= 2.0) requirement still makes sense?
 * should we have an obsoletes statement?

jpo
Comment 35 Peter Vrabec 2007-06-25 08:36:25 EDT
New Package CVS Request
=======================
Package Name: rsyslog
Short Description: Enhanced system logging and kernel message trapping daemons
Owners: pvrabec@redhat.com
Branches: F-7
InitialCC:
Comment 36 Peter Vrabec 2007-06-25 09:45:28 EDT
thnx. Jose, patch will be applied.

bash (>= 2.0) ... It's not clear to me why there is this requirement.
obsoletes ... I'm not sure this is necessary, let me find it out.
Comment 37 Kevin Fenzi 2007-06-25 15:17:50 EDT
cvs done. 

Please note that FESCo is planning on discussing if rsyslog should replace
sysklogd in it's thursday meeting. You might hold off on building until after
that. Also, be very carefull when pushing to F-7. I don't think we want to
replace sysklogd in a released version. 
Comment 38 Rainer Gerhards 2007-06-28 03:56:55 EDT
Not sure if it is still OK to post that info here, but 1.14.0 has been released
with IPv6 support for UDP (thanks, Peter!). Support for TCP will follow. 1.14.0
is experimental and needs some real-world review in IPv6 environments (I have
currently only very limited ability to do so).

changelog:
http://www.rsyslog.com/Article76.phtml

download:
http://download.adiscon.com/rsyslog/rsyslog-1.14.0.tar.gz
Comment 39 Peter Vrabec 2007-07-02 06:12:09 EDT
Kevin, are there any news after last FESCo discussion?

(In reply to comment #37)
> cvs done. 
> 
> Please note that FESCo is planning on discussing if rsyslog should replace
> sysklogd in it's thursday meeting. You might hold off on building until 
after
> that. Also, be very carefull when pushing to F-7. I don't think we want to
> replace sysklogd in a released version. 

Comment 40 Peter Vrabec 2007-07-02 10:35:12 EDT
SRPM URL: http://people.redhat.com/pvrabec/rpms/rsyslog-1.14.1-1.src.rpm
IPv6 support completed(tcp,udp) 
Comment 41 Kevin Fenzi 2007-07-02 11:52:25 EDT
(In reply to comment #39)

There was high level discussion about features in general, but nothing specific
to rsyslog. ;( I will ask for it to be added to the schedule for this week. 
Comment 42 Rainer Gerhards 2007-07-04 06:41:40 EDT
I have changed the generic download server name. It now is download.rsyslog.com.
The previous one (download.adiscon.com) is still valid and will remain so for
the foreseeable future.
Comment 43 Peter Lemenkov 2008-01-22 08:19:35 EST
Is it OK to close this Review Request? Don't forget to close tickets after
successful inclusion of package into Fedora.
Comment 44 Lubomir Rintel 2008-07-13 05:49:02 EDT
Package Change Request
======================
Package Name: rsyslog
New Branches: EL-5
Owner of new branches: lkundrak
Cvsextras Commits for new branches: yes

According to wishlist: "Owners were mailed 071210" [1], therefore I assume Peter
has no interest in maintaining this for EPEL.

[1] http://fedoraproject.org/wiki/EPEL/WishList
Comment 45 Rahul Sundaram 2008-07-13 06:23:33 EDT
EL 5.2 already has rsyslog and hence a EPEL branch would conflict. Not allowed. 
Comment 46 Lubomir Rintel 2008-07-13 07:30:43 EDT
Eww, right; I guess it was still 5.1 I last checked it against. Sorry for the noise.

Note You need to log in before you can comment on or make changes to this bug.