Message ID | 20201229181125.704233-1-andreas.hilse@googlemail.com |
---|---|
State | Accepted |
Headers | show |
Series | [v2,1/1] package/sysklogd: bump to version 2.1.2 | expand |
Hi Andreas, I noticed this hasn't been merged yet, so I thought I'd provide my feedback. Sorry for the delay! Overall it looks good, and I have a few questions, and a little reveal. On Tue, Dec 29, 2020 at 19:11, Andreas Hilse via buildroot <buildroot@busybox.net> wrote: > - fixes: sysklogd 1.6 klogd with newer glibcs: kernel messages are logged to user facility > - sysklogd removed klogd, functionality has been moved to syslogd > - now supports config fragments in /etc/syslog.d > - disabled sysklogd logger to not interfere with other loggers > - license has changed from GPL-2.0+ to BSD-3-Clause These two could also be relevant: - RFC3164 style logging, fully RFC compliant not just BSD style - RFC5424 style logging to file and remote log servers > + The continuation of the original sysklogd package, based on > + the original Berkeley syslog daemon. Now with kernel logging, > + and log rotation built-in. It ca both receive from and send ^ Minor, spelling. > + to remote syslog servers. The v2.x series include extended > + support for RFC5424 with an alt. syslogp() API for clients. [snip] > +sha256 09fe1edca882a9ae976cdbf516786edc1629347b9507d45dc005d9d969f94702 sysklogd-2.1.2.tar.gz > +sha256 7a71d7603a7c4456df441463e54da35acf151c1be0879246de63544f1f34f477 LICENSE > +sha256 569321607efe6ba0ecb820b3cb23d933e92f6ab50d2f8fcceb23fc8a43756ef0 0001-Define-_GNU_SOURCE_required_for_O_CLOEXEC_on_uClibc.patch I actually released v2.2.0 the other day, and there's a SHA256 on GitHub now. This version comes with proper /dev/kmsg support on Linux, which support multiple consumers, instead of the old /proc/kmsg support. If /dev/kmsg isn't found it falls back to the old. It also has the build fix for O_CLOEXEC and another fix for an ARMv5 build warning. I've tested it with a few Bootlin toolchains in our NetBox repo, so it should be a much smoother ride. > +SYSKLOGD_CONF_OPTS = --exec-prefix=/ --without-logger The logger tool that comes with sysklogd is quite advanced, allowing the user to leverage the RFC5424 `syslogp()` API to its fullest. However, it collides with its namesake in BusyBox, but so does the logger tool in util-linux. I'd very much like to see it in Buildroot, but I can submit a patch later to add menuconfig support :) Best regards /Joachim
On Tue, 29 Dec 2020 19:11:25 +0100 Andreas Hilse via buildroot <buildroot@busybox.net> wrote: > - fixes: sysklogd 1.6 klogd with newer glibcs: kernel messages are logged to user facility > - sysklogd removed klogd, functionality has been moved to syslogd > - now supports config fragments in /etc/syslog.d > - disabled sysklogd logger to not interfere with other loggers > - license has changed from GPL-2.0+ to BSD-3-Clause > > Signed-off-by: Andreas Hilse <andreas.hilse@googlemail.com> > > --- > Changes v1 -> v2: > - added patch for O_CLOEXEC build failure with uClibc > - mention license change > - extended help text in Config.in Applied to master, thanks. Thomas
Hello Joachim, Thanks for your additional feedback! On Sat, 16 Jan 2021 20:33:37 +0100 Joachim Wiberg <troglobit@gmail.com> wrote: > On Tue, Dec 29, 2020 at 19:11, Andreas Hilse via buildroot <buildroot@busybox.net> wrote: > > - fixes: sysklogd 1.6 klogd with newer glibcs: kernel messages are logged to user facility > > - sysklogd removed klogd, functionality has been moved to syslogd > > - now supports config fragments in /etc/syslog.d > > - disabled sysklogd logger to not interfere with other loggers > > - license has changed from GPL-2.0+ to BSD-3-Clause > > These two could also be relevant: > > - RFC3164 style logging, fully RFC compliant not just BSD style > - RFC5424 style logging to file and remote log servers Right, but the commit log mainly documents the changes that have an impact on packaging, not necessarily all the additional (good) features of the the new upstream release. > > + The continuation of the original sysklogd package, based on > > + the original Berkeley syslog daemon. Now with kernel logging, > > + and log rotation built-in. It ca both receive from and send > ^ > Minor, spelling. Fixed. > > + to remote syslog servers. The v2.x series include extended > > + support for RFC5424 with an alt. syslogp() API for clients. > [snip] > > +sha256 09fe1edca882a9ae976cdbf516786edc1629347b9507d45dc005d9d969f94702 sysklogd-2.1.2.tar.gz > > +sha256 7a71d7603a7c4456df441463e54da35acf151c1be0879246de63544f1f34f477 LICENSE > > +sha256 569321607efe6ba0ecb820b3cb23d933e92f6ab50d2f8fcceb23fc8a43756ef0 0001-Define-_GNU_SOURCE_required_for_O_CLOEXEC_on_uClibc.patch > > I actually released v2.2.0 the other day, and there's a SHA256 on GitHub > now. This version comes with proper /dev/kmsg support on Linux, which > support multiple consumers, instead of the old /proc/kmsg support. If > /dev/kmsg isn't found it falls back to the old. It also has the build > fix for O_CLOEXEC and another fix for an ARMv5 build warning. > > I've tested it with a few Bootlin toolchains in our NetBox repo, so it > should be a much smoother ride. Will definitely be good to have a follow-up patch bumping to 2.2.0. > > +SYSKLOGD_CONF_OPTS = --exec-prefix=/ --without-logger > > The logger tool that comes with sysklogd is quite advanced, allowing the > user to leverage the RFC5424 `syslogp()` API to its fullest. However, > it collides with its namesake in BusyBox, but so does the logger tool in > util-linux. I'd very much like to see it in Buildroot, but I can submit > a patch later to add menuconfig support :) Patch welcome indeed! Regarding the collision with Busybox, we do have a mechanism in place to handle that, so it should be fine. Busybox already has the sysklogd package in its dependencies, which means that sysklogd if enabled will always be built before Busybox. And the Busybox installation is careful to not overwrite any file. So if /bin/foobar has been installed by sysklogd, even if Busybox has support for /bin/foobar, it will not overwrite /bin/foobar with its own symlink. Best regards, Thomas
Hello Thomas, Hello Joachim, > > > +SYSKLOGD_CONF_OPTS = --exec-prefix=/ --without-logger > > > > The logger tool that comes with sysklogd is quite advanced, allowing the > > user to leverage the RFC5424 `syslogp()` API to its fullest. However, > > it collides with its namesake in BusyBox, but so does the logger tool in > > util-linux. I'd very much like to see it in Buildroot, but I can submit > > a patch later to add menuconfig support :) > > Patch welcome indeed! > > Regarding the collision with Busybox, we do have a mechanism in place > to handle that, so it should be fine. Busybox already has the sysklogd > package in its dependencies, which means that sysklogd if enabled will > always be built before Busybox. And the Busybox installation is careful > to not overwrite any file. So if /bin/foobar has been installed by > sysklogd, even if Busybox has support for /bin/foobar, it will not > overwrite /bin/foobar with its own symlink. > The issue I had with the logger tool is that the base dir for sysklogd (exec-prefix) is /, thus it is placed in /bin. The busybox logger is placed in /usr/bin so I ended up with both in my rootfs. To make this work sysklogd exec-prefix should be /usr - which implies additional changes in the package files. Would this be ok? Best regards Andreas
On Wed, 20 Jan 2021 11:11:30 +0100 Andreas Hilse <andreas.hilse@googlemail.com> wrote: > The issue I had with the logger tool is that the base dir for sysklogd > (exec-prefix) is /, thus it is placed in /bin. The busybox logger is > placed in /usr/bin so I ended up with both in my rootfs. > > To make this work sysklogd exec-prefix should be /usr - which implies > additional changes in the package files. > Would this be ok? Yes, it's certainly OK to change sysklogd.mk as necessary. Thomas
Hi :) On Tue, Jan 19, 2021 at 22:26, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > On Sat, 16 Jan 2021 20:33:37 +0100 > Joachim Wiberg <troglobit@gmail.com> wrote: >> These two could also be relevant: >> - RFC3164 style logging, fully RFC compliant not just BSD style >> - RFC5424 style logging to file and remote log servers > Right, but the commit log mainly documents the changes that have an > impact on packaging, not necessarily all the additional (good) features > of the the new upstream release. Ah, of course, thank you for taking the time to explain! >> I've tested it with a few Bootlin toolchains in our NetBox repo, so it >> should be a much smoother ride. > Will definitely be good to have a follow-up patch bumping to 2.2.0. I'll get on it then :) >> > +SYSKLOGD_CONF_OPTS = --exec-prefix=/ --without-logger >> The logger tool that comes with sysklogd is quite advanced, allowing the >> user to leverage the RFC5424 `syslogp()` API to its fullest. However, >> it collides with its namesake in BusyBox, but so does the logger tool in >> util-linux. I'd very much like to see it in Buildroot, but I can submit >> a patch later to add menuconfig support :) > Patch welcome indeed! Great! > Regarding the collision with Busybox, we do have a mechanism in place > to handle that, so it should be fine. Busybox already has the sysklogd > package in its dependencies, which means that sysklogd if enabled will > always be built before Busybox. And the Busybox installation is careful > to not overwrite any file. So if /bin/foobar has been installed by > sysklogd, even if Busybox has support for /bin/foobar, it will not > overwrite /bin/foobar with its own symlink. Wow, that's awesome, didn't know about that collision handling, clever! Very cool :) Best regards /Joachim
diff --git a/package/sysklogd/0001-Define-_GNU_SOURCE_required_for_O_CLOEXEC_on_uClibc.patch b/package/sysklogd/0001-Define-_GNU_SOURCE_required_for_O_CLOEXEC_on_uClibc.patch new file mode 100644 index 0000000000..d794b7f2f3 --- /dev/null +++ b/package/sysklogd/0001-Define-_GNU_SOURCE_required_for_O_CLOEXEC_on_uClibc.patch @@ -0,0 +1,43 @@ +From 93117e58016de6f604e67263b5135adbba032367 Mon Sep 17 00:00:00 2001 +From: Joachim Wiberg <troglobit@gmail.com> +Date: Sun, 30 Aug 2020 11:41:45 +0200 +Subject: [PATCH] Define _GNU_SOURCE, required for O_CLOEXEC on uClibc + +When building sysklogd on a uClibc system we must define _GNU_SOURCE to +get O_CLOEXEC. + +Since _GNU_SOURCE is also required for asprintf() with GLIBC, as used by +the pidfile() replacement function, we drop it from there and rely on +AM_CPPFLAGS for all sources. + +Signed-off-by: Joachim Wiberg <troglobit@gmail.com> +--- + lib/pidfile.c | 1 - + src/Makefile.am | 2 +- + 2 files changed, 1 insertion(+), 2 deletions(-) + +diff --git a/lib/pidfile.c b/lib/pidfile.c +index a26de73..886a29c 100644 +--- a/lib/pidfile.c ++++ b/lib/pidfile.c +@@ -31,7 +31,6 @@ + * POSSIBILITY OF SUCH DAMAGE. + */ + +-#define _GNU_SOURCE /* Needed with GLIBC to get asprintf() */ + #include <sys/stat.h> /* utimensat() */ + #include <sys/time.h> /* utimensat() on *BSD */ + #include <sys/types.h> +diff --git a/src/Makefile.am b/src/Makefile.am +index 9b80a02..1909ada 100644 +--- a/src/Makefile.am ++++ b/src/Makefile.am +@@ -36,7 +36,7 @@ endif + AM_CFLAGS = -W -Wall -Wextra + AM_CFLAGS += -Wno-unused-result -Wno-unused-parameter -fno-strict-aliasing + AM_CPPFLAGS = -DSYSCONFDIR=\"@sysconfdir@\" -DRUNSTATEDIR=\"@runstatedir@\" +-AM_CPPFLAGS += -D_BSD_SOURCE -D_DEFAULT_SOURCE ++AM_CPPFLAGS += -D_BSD_SOURCE -D_DEFAULT_SOURCE -D_GNU_SOURCE + + syslogd_SOURCES = syslogd.c syslogd.h socket.c socket.h syslog.h + syslogd_SOURCES += timer.c timer.h queue.h compat.h diff --git a/package/sysklogd/Config.in b/package/sysklogd/Config.in index fda58e020e..18ef0e1e1e 100644 --- a/package/sysklogd/Config.in +++ b/package/sysklogd/Config.in @@ -1,8 +1,12 @@ config BR2_PACKAGE_SYSKLOGD - bool "syslogd & klogd" + bool "sysklogd" depends on BR2_USE_MMU # fork() depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS help - System log daemons syslogd and klogd. + The continuation of the original sysklogd package, based on + the original Berkeley syslog daemon. Now with kernel logging, + and log rotation built-in. It ca both receive from and send + to remote syslog servers. The v2.x series include extended + support for RFC5424 with an alt. syslogp() API for clients. https://github.com/troglobit/sysklogd/ diff --git a/package/sysklogd/S02klogd b/package/sysklogd/S02klogd deleted file mode 100644 index ba728aa99a..0000000000 --- a/package/sysklogd/S02klogd +++ /dev/null @@ -1,65 +0,0 @@ -#!/bin/sh - -DAEMON="klogd" -PIDFILE="/var/run/$DAEMON.pid" - -KLOGD_ARGS="" - -KLOGD_RELOAD="0" - -# shellcheck source=/dev/null -[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON" - -start() { - printf 'Starting %s: ' "$DAEMON" - # shellcheck disable=SC2086 # we need the word splitting - start-stop-daemon -S -q -p "$PIDFILE" -x "/sbin/$DAEMON" \ - -- $KLOGD_ARGS - status=$? - if [ "$status" -eq 0 ]; then - echo "OK" - else - echo "FAIL" - fi - return "$status" -} - -stop() { - printf 'Stopping %s: ' "$DAEMON" - start-stop-daemon -K -q -p "$PIDFILE" - status=$? - if [ "$status" -eq 0 ]; then - echo "OK" - else - echo "FAIL" - fi - return "$status" -} - -restart() { - stop - sleep 1 - start -} - -# SIGUSR1 makes klogd reload kernel module symbols -# SIGUSR2 makes klogd reload static kernel symbols and kernel module symbols -reload() { - printf 'Reloading %s: ' "$DAEMON" - start-stop-daemon -K -s "$KLOGD_RELOAD" -q -p "$PIDFILE" - status=$? - if [ "$status" -eq 0 ]; then - echo "OK" - else - echo "FAIL" - fi - return "$status" -} - -case "$1" in - start|stop|restart|reload) - "$1";; - *) - echo "Usage: $0 {start|stop|restart|reload}" - exit 1 -esac diff --git a/package/sysklogd/klogd.service b/package/sysklogd/klogd.service deleted file mode 100644 index b5dbb93d7d..0000000000 --- a/package/sysklogd/klogd.service +++ /dev/null @@ -1,11 +0,0 @@ -[Unit] -Description=Kernel Log Daemon - -[Service] -ExecStart=/sbin/klogd -n -StandardOutput=null -Restart=on-failure - -[Install] -WantedBy=multi-user.target -WantedBy=syslogd.service diff --git a/package/sysklogd/sysklogd.hash b/package/sysklogd/sysklogd.hash index 6f7ab6ece8..9f9fd192e1 100644 --- a/package/sysklogd/sysklogd.hash +++ b/package/sysklogd/sysklogd.hash @@ -1,3 +1,4 @@ # Locally calculated -sha256 1e9e18564c5bba474954d55ea6e2a0e3dc1bc145d8973c5fd098b088a9be9ceb sysklogd-1.6.tar.gz -sha256 91df39d1816bfb17a4dda2d3d2c83b1f6f2d38d53e53e41e8f97ad5ac46a0cad COPYING +sha256 09fe1edca882a9ae976cdbf516786edc1629347b9507d45dc005d9d969f94702 sysklogd-2.1.2.tar.gz +sha256 7a71d7603a7c4456df441463e54da35acf151c1be0879246de63544f1f34f477 LICENSE +sha256 569321607efe6ba0ecb820b3cb23d933e92f6ab50d2f8fcceb23fc8a43756ef0 0001-Define-_GNU_SOURCE_required_for_O_CLOEXEC_on_uClibc.patch diff --git a/package/sysklogd/sysklogd.mk b/package/sysklogd/sysklogd.mk index 7a25d0eb6f..cd013f658c 100644 --- a/package/sysklogd/sysklogd.mk +++ b/package/sysklogd/sysklogd.mk @@ -4,13 +4,13 @@ # ################################################################################ -SYSKLOGD_VERSION = 1.6 +SYSKLOGD_VERSION = 2.1.2 SYSKLOGD_SITE = $(call github,troglobit,sysklogd,v$(SYSKLOGD_VERSION)) -SYSKLOGD_LICENSE = GPL-2.0+ -SYSKLOGD_LICENSE_FILES = COPYING +SYSKLOGD_LICENSE = BSD-3-Clause +SYSKLOGD_LICENSE_FILES = LICENSE # From git SYSKLOGD_AUTORECONF = YES -SYSKLOGD_CONF_OPTS = --exec-prefix=/ +SYSKLOGD_CONF_OPTS = --exec-prefix=/ --without-logger define SYSKLOGD_INSTALL_SAMPLE_CONFIG $(INSTALL) -D -m 0644 package/sysklogd/syslog.conf \ @@ -22,15 +22,11 @@ SYSKLOGD_POST_INSTALL_TARGET_HOOKS += SYSKLOGD_INSTALL_SAMPLE_CONFIG define SYSKLOGD_INSTALL_INIT_SYSV $(INSTALL) -m 755 -D package/sysklogd/S01syslogd \ $(TARGET_DIR)/etc/init.d/S01syslogd - $(INSTALL) -m 755 -D package/sysklogd/S02klogd \ - $(TARGET_DIR)/etc/init.d/S02klogd endef define SYSKLOGD_INSTALL_INIT_SYSTEMD $(INSTALL) -D -m 644 $(SYSKLOGD_PKGDIR)/syslogd.service \ $(TARGET_DIR)/usr/lib/systemd/system/syslogd.service - $(INSTALL) -D -m 644 $(SYSKLOGD_PKGDIR)/klogd.service \ - $(TARGET_DIR)/usr/lib/systemd/system/klogd.service endef $(eval $(autotools-package)) diff --git a/package/sysklogd/syslog.conf b/package/sysklogd/syslog.conf index 3184139052..411bd645ff 100644 --- a/package/sysklogd/syslog.conf +++ b/package/sysklogd/syslog.conf @@ -5,3 +5,5 @@ auth,authpriv.* /var/log/auth.log user.* /var/log/user.log *.emerg * + +include /etc/syslog.d/*.conf diff --git a/package/sysklogd/syslogd.service b/package/sysklogd/syslogd.service index adaac679f2..1bf1bb3bd9 100644 --- a/package/sysklogd/syslogd.service +++ b/package/sysklogd/syslogd.service @@ -1,10 +1,9 @@ [Unit] Description=System Logging Service Requires=syslog.socket -Wants=klogd.service [Service] -ExecStart=/sbin/syslogd -m 0 -n +ExecStart=/sbin/syslogd -m 0 -F StandardOutput=null Restart=on-failure