| Message ID | 31MWDEN7Q9XOV.2001N5J6G4U9K@wilsonb.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#47905] gnu: Add rasdaemon. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
On Tue, Apr 20, 2021 at 01:24:37PM +0900, elaexuotee--- via Guix-patches via wrote: > This is my first patch to gnu/services (and by inclusion doc/guix.texi), so > please scrutinize with abandon! > > I did all the standard sanity checks for the package definition (guix lint, > etc/indent-code.el, guix build --check); however, I wasn't sure about the > service, so the sanity checks were limited to testing in a vm. Thanks for letting us know how you tested it. That's really valuable for reviewers.
On Tue, Apr 20, 2021 at 01:24:37PM +0900, elaexuotee--- via Guix-patches via wrote: > +(define-public rasdaemon Overall LGTM. > + (license license:gpl2+))) I checked the license headers of the ras-*.c files. Some of them are GPL2+, and some are GPL2 only. I think that we should mark it GPL2 only, based on that.
On Tue, Apr 20, 2021 at 01:24:37PM +0900, elaexuotee--- via Guix-patches via wrote: > * doc/guix.texi (Linux Services): Document it. It could be added to Monitoring Services instead. Although, RAS is Linux-only, so I leave that decision up to you. > +@cindex rasdaemon > +@cindex Platform Reliability, Availability and Serviceability daemon > +@subsubheading Rasdaemon Service > + > +The Rasdaemon service provides a monitor for Platform Reliability, > +Availability, and Serviceability (RAS) events in the Linux kernel. It would be nice to link to some upstream documentation of RAS, as we do for the documentation of the Zram Device Service. I think it could also be improved with addition of one or two sentences about how to make use of the service. As logging appears to be disabled by default, how are users expected to learn of the events monitored by rasdaemon? After writing that, I looked at rasdaemon-shepherd-service and see that it keeps a log file. Is that the same data as the optional SQLite database, but unstructured? Overall, the docs should clarify this :)
On Tue, Apr 20, 2021 at 01:24:37PM +0900, elaexuotee--- via Guix-patches via wrote: > +;;; > +;;; Reliability, Availability, and Servicability (RAS) daemon Typo -------------------------------> Serviceability Otherwise, this LGTM. I'm no expert on Guix services but this one seems simple and straightforward. You mentioned that you tested in a VM. What should I look for in the VM to verify that the service is working?
Leo Famulari <leo@famulari.name> wrote: > On Tue, Apr 20, 2021 at 01:24:37PM +0900, elaexuotee--- via Guix-patches via wrote: > > +(define-public rasdaemon > > Overall LGTM. > > > + (license license:gpl2+))) > > I checked the license headers of the ras-*.c files. > > Some of them are GPL2+, and some are GPL2 only. I think that we should > mark it GPL2 only, based on that. Oh! Thanks. Nice catch. This is not the first time I have missed license details, so I looked for some method to find all licenses in a project: $ licensecheck --recursive --machine . | awk -F$'\t' '{print $2}' | sort -u FSF All Permissive License GNU Lesser General Public License (v2.1) GPL (v2 or later) GPL (v2 or later) (with incorrect FSF address) GPL (v2) UNKNOWN The `FSF All Permissive License' just comes from the INSTALL file. In general, I'm aware that we can include multiple licenses, so the above output would look like: (license `(,license:fsf-free ,license:lgpl2.1 ,license:gpl2 ,license:gpl2+)) However, legally-speaking, was is the correct approach here?
Leo Famulari <leo@famulari.name> wrote: > On Tue, Apr 20, 2021 at 01:24:37PM +0900, elaexuotee--- via Guix-patches via wrote: > > * doc/guix.texi (Linux Services): Document it. > > It could be added to Monitoring Services instead. Although, RAS is > Linux-only, so I leave that decision up to you. Hrm. Monitoring Services does make sense. Kind of a tough call. Will give it more thought but probably keep in Linux since anyone wanting this service probably knows exactly what they are looking for. > It would be nice to link to some upstream documentation of RAS, as we do > for the documentation of the Zram Device Service. > > I think it could also be improved with addition of one or two sentences > about how to make use of the service. As logging appears to be disabled > by default, how are users expected to learn of the events monitored by > rasdaemon? > > After writing that, I looked at rasdaemon-shepherd-service and see that > it keeps a log file. Is that the same data as the optional SQLite > database, but unstructured? > > Overall, the docs should clarify this :) Agreed. My documentation was lazy. Will improve. Thanks for the specific pointers. About the sqlite db, I honestly don't know much about how it compares to the logs. From a cursory glance at the code it looks basically like what you said, a structured rendering of the logs. Will make some kind of note about this. Speaking of, the log output looks like "rasdaemon: <message>", so it's probably just as good to put this in the syslog rather than a dedicated file. How can one do that?
Leo Famulari <leo@famulari.name> wrote: > On Tue, Apr 20, 2021 at 01:24:37PM +0900, elaexuotee--- via Guix-patches via wrote: > > +;;; > > +;;; Reliability, Availability, and Servicability (RAS) daemon > > Typo -------------------------------> Serviceability > > Otherwise, this LGTM. I'm no expert on Guix services but this one seems > simple and straightforward. > > You mentioned that you tested in a VM. What should I look for in the VM > to verify that the service is working? Ah, good catch. Just check that the daemon is running and log/database files exist. For a non-broken system, the sqlite database will just contain a few empty tables and the logs will just notify you that the various monitors are active: $ herd status rasdaemon $ pgrep -a rasdaemon $ cat /var/log/rasdaemon $ sqlite3 /var/lib/rasdaemon/ras-mc_event.db I honestly don't know a lot about RAS, yet, so that's the extent of what I can easily probe.
On Tue, Apr 20, 2021 at 02:54:49PM +0900, elaexuotee@wilsonb.com wrote: > The `FSF All Permissive License' just comes from the INSTALL file. In general, > I'm aware that we can include multiple licenses, so the above output would look > like: > > (license `(,license:fsf-free ,license:lgpl2.1 ,license:gpl2 ,license:gpl2+)) > > However, legally-speaking, was is the correct approach here? Overall, the program is distributed under the GPL version 2, based on COPYING (whether or not "later versions" are allowed depends on license headers of individual files). Some components may have other licenses, but I'd say the whole thing — the "program" as we use it — is GPL 2. In general, we redistribute the program under a single license, so that is what the license field should say. Maybe if there is some really valuable component that can be used under a different license, we can add a code comment about it. But, I don't think it's helpful to list the licenses of files such as INSTALL, nor is it unusual that they have a different license than the whole. I would refer to this page for more advice about the GPL: https://www.gnu.org/licenses/gpl-faq.html#AllCompatibility I'm not a legal expert, and my understanding is that none of this stuff is really "settled" or "well understood" legally — that would require extensive and repeated litigation, at least in the USA, which has not occurred.
On Tue, Apr 20, 2021 at 03:18:19PM +0900, elaexuotee@wilsonb.com wrote: > Speaking of, the log output looks like "rasdaemon: <message>", so it's probably > just as good to put this in the syslog rather than a dedicated file. How can > one do that? I poked around in the existing services, and it seems that the service daemons themselves have the ability to log to syslog, based upon their own configuration. For example, the OpenSSH service 'requires' syslogd, but only in terms of making sure that syslogd is running before sshd starts. I don't see any extension of the syslog service. (I could be wrong — services are my weak point) So, I guess that rasdaemon needs to learn how to find and use syslog, if it hasn't yet.
On Tue, Apr 20, 2021 at 03:23:14PM +0900, elaexuotee@wilsonb.com wrote: > Just check that the daemon is running and log/database files exist. For a > non-broken system, the sqlite database will just contain a few empty tables and > the logs will just notify you that the various monitors are active: > > $ herd status rasdaemon > $ pgrep -a rasdaemon > $ cat /var/log/rasdaemon > $ sqlite3 /var/lib/rasdaemon/ras-mc_event.db > > I honestly don't know a lot about RAS, yet, so that's the extent of what I can > easily probe. Thanks, I'll try it out.
From 1d4e60a9578d3572bf1caff11d0b17e4bc8f1ea9 Mon Sep 17 00:00:00 2001 From: "B. Wilson" <elaexuotee@wilsonb.com> Date: Tue, 20 Apr 2021 11:49:26 +0900 Subject: [PATCH] gnu: Add rasdaemon. To: guix-patches@gnu.org * gnu/packages/linux.scm (rasdaemon): New variable. * gnu/services/linux.scm (rasdaemon-configuration) (rasdaemon-configuration?, rasdaemon-configuration-record?) (rasdaemon-service-type): New variables. * doc/guix.texi (Linux Services): Document it. --- doc/guix.texi | 29 +++++++++++++++++++++++++ gnu/packages/linux.scm | 45 ++++++++++++++++++++++++++++++++++++++ gnu/services/linux.scm | 49 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) diff --git a/doc/guix.texi b/doc/guix.texi index 58bcfbdbb5..73479b8fcf 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -88,6 +88,7 @@ Copyright @copyright{} 2020 John Soo@* Copyright @copyright{} 2020 Jonathan Brielmaier@* Copyright @copyright{} 2020 Edgar Vincent@* Copyright @copyright{} 2021 Maxime Devos@* +Copyright @copyright{} 2021 B. Wilson@* Permission is granted to copy, distribute and/or modify this document under the terms of the GNU Free Documentation License, Version 1.3 or @@ -31457,6 +31458,34 @@ parameters, can be done as follow: @end lisp @end deffn +@cindex rasdaemon +@cindex Platform Reliability, Availability and Serviceability daemon +@subsubheading Rasdaemon Service + +The Rasdaemon service provides a monitor for Platform Reliability, +Availability, and Serviceability (RAS) events in the Linux kernel. + +@defvr {Scheme Variable} rasdaemon-service-type +Service type for the @command{rasdaemon} service. It accepts a +@code{rasdaemon-configuration} object. Instantiating like + +@lisp +(service rasdaemon-service-type) +@end lisp + +will load with a default configuration. +@end defvr + +@deftp {Data Type} rasdaemon-configuration +The data type representing the configuration of @command{rasdaemon}. + +@table @asis +@item @code{record?} (default: @code{#f}) +A boolean indicating whether to record the events in an SQLite database. The +database location is hard-coded to @file{/var/lib/rasdaemon/ras-mc_event.db}. +@end table +@end deftp + @cindex zram @cindex compressed swap @cindex Compressed RAM-based block devices diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm index 1ea9d80834..fe464bcc8e 100644 --- a/gnu/packages/linux.scm +++ b/gnu/packages/linux.scm @@ -53,6 +53,7 @@ ;;; Copyright © 2020 Zhu Zihao <all_but_last@163.com> ;;; Copyright © 2020 David Dashyan <mail@davie.li> ;;; Copyright © 2020 pukkamustard <pukkamustard@posteo.net> +;;; Copyright © 2021 B. Wilson <elaexuotee@wilsonb.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -130,6 +131,7 @@ #:use-module (gnu packages sdl) #:use-module (gnu packages serialization) #:use-module (gnu packages slang) + #:use-module (gnu packages sqlite) #:use-module (gnu packages texinfo) #:use-module (gnu packages tls) #:use-module (gnu packages valgrind) @@ -8037,3 +8039,46 @@ kernel side implementation.") read-only file system optimized for resource-scarce devices. This package provides user-space tools for creating EROFS file systems.") (license license:gpl2+))) + +(define-public rasdaemon + (package + (name "rasdaemon") + (version "0.6.6") + (source + (origin + (method git-fetch) + (uri (git-reference + (url "https://github.com/mchehab/rasdaemon") + (commit (string-append "v" version)))) + (file-name (git-file-name name version)) + (sha256 + (base32 "13g39x19lfjf9izdcb0nlyfjrgpliivhv4nw3ndgyzi59l3yqc0v")))) + (native-inputs `(("autoconf" ,autoconf) + ("automake" ,automake) + ("libtool" ,libtool))) + (inputs `(("sqlite" ,sqlite))) + (arguments + `(#:configure-flags '("--enable-all" + "--localstatedir=/var") + #:phases + (modify-phases %standard-phases + (add-before 'configure 'munge-autotools + (lambda _ + ;; For some reason upstream forces sysconfdir=/etc. This results + ;; in EPERM during the install phase. Removing the offending + ;; line lets sysconfdir correctly pick up DESTDIR. + (substitute* "configure.ac" + (("^test .* sysconfdir=/etc\n$") "")) + ;; Upstream tries to create /var/lib/rasdaemon at install time. + ;; This results in EPERM on guix. Instead, the service should + ;; create this at activation time. + (substitute* "Makefile.am" + (("^\\s*\\$\\(install_sh\\) -d .*@RASSTATEDIR@.*$") ""))))))) + (build-system gnu-build-system) + (home-page "https://github.com/mchehab/rasdaemon") + (synopsis "Platform Reliability, Availability and Serviceability tools") + (description "The @code{rasdaemon} program is a daemon which monitors the +platform Reliablity, Availability and Serviceability (RAS) reports from the +Linux kernel trace events. These trace events are logged in +/sys/kernel/debug/tracing, reporting them via syslog/journald.") + (license license:gpl2+))) diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm index 340b330030..01723efc07 100644 --- a/gnu/services/linux.scm +++ b/gnu/services/linux.scm @@ -3,6 +3,7 @@ ;;; Copyright © 2020 Brice Waegeneire <brice@waegenei.re> ;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il> ;;; Copyright © 2021 raid5atemyhomework <raid5atemyhomework@protonmail.com> +;;; Copyright © 2021 B. Wilson <elaexuotee@wilsonb.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -47,6 +48,11 @@ kernel-module-loader-service-type + rasdaemon-configuration + rasdaemon-configuration? + rasdaemon-configuration-record? + rasdaemon-service-type + zram-device-configuration zram-device-configuration? zram-device-configuration-size @@ -188,6 +194,49 @@ representation." (extend append) (default-value '()))) + +;;; +;;; Reliability, Availability, and Servicability (RAS) daemon +;;; + +(define-record-type* <rasdaemon-configuration> + rasdaemon-configuration make-rasdaemon-configuration + rasdaemon-configuration? + (record? rasdaemon-configuration-record? (default #f))) + +(define (rasdaemon-configuration->command-line-args config) + "Translate <rasdaemon-configuration> to its command line arguments + representation" + (let ((record? (rasdaemon-configuration-record? config))) + `(,(file-append rasdaemon "/sbin/rasdaemon") + "--foreground" ,@(if record? '("--record") '())))) + +(define (rasdaemon-activation config) + (let ((record? (rasdaemon-configuration-record? config)) + (rasdaemon-dir "/var/lib/rasdaemon")) + (with-imported-modules '((guix build utils)) + #~(if #$record? (mkdir-p #$rasdaemon-dir))))) + +(define (rasdaemon-shepherd-service config) + (shepherd-service + (documentation "Run rasdaemon") + (provision '(rasdaemon)) + (start #~(make-forkexec-constructor + '#$(rasdaemon-configuration->command-line-args config) + #:log-file "/var/log/rasdaemon.log")) + (stop #~(make-kill-destructor)))) + +(define rasdaemon-service-type + (service-type + (name 'rasdaemon) + (default-value (rasdaemon-configuration)) + (extensions + (list (service-extension shepherd-root-service-type + (compose list rasdaemon-shepherd-service)) + (service-extension activation-service-type rasdaemon-activation))) + (compose concatenate) + (description "Run @command{rasdaemon}, the RAS monitor"))) + ;;; ;;; Kernel module loader. -- 2.31.1