GNU bug report logs - #47905
gnu: Add rasdaemon.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 47905 in the body.
You can then email your comments to 47905 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: elaexuotee <at> wilsonb.com
To: guix-patches <at> gnu.org
Subject: gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 13:24:37 +0900

[Message part 1 (text/plain, inline)]

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.

Cheers!

[0001-gnu-Add-rasdaemon.patch (text/x-patch, attachment)]

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: elaexuotee--- via Guix-patches via <guix-patches <at> gnu.org>
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 01:04:48 -0400

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.

Message #14 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: elaexuotee <at> wilsonb.com
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 01:07:31 -0400

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.

Message #17 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: elaexuotee <at> wilsonb.com
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 01:30:51 -0400

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 :)

Message #20 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: elaexuotee <at> wilsonb.com
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 01:33:21 -0400

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?

Message #23 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: elaexuotee <at> wilsonb.com
To: Leo Famulari <leo <at> famulari.name>
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 14:54:49 +0900

Leo Famulari <leo <at> 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?

Message #26 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: elaexuotee <at> wilsonb.com
To: Leo Famulari <leo <at> famulari.name>
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 15:18:19 +0900

Leo Famulari <leo <at> 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?

Message #29 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: elaexuotee <at> wilsonb.com
To: Leo Famulari <leo <at> famulari.name>
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 15:23:14 +0900

Leo Famulari <leo <at> 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.

Message #32 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: elaexuotee <at> wilsonb.com
To: Leo Famulari <leo <at> famulari.name>
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 16:10:54 +0900

[Message part 1 (text/plain, inline)]

This patch updates the license field to contain lgpl2.1, gpl2, and gpl2+.
I also added a lot to the docs.  Upstream docs are pretty sparse, so I mostly
just pilfered from the Linux kernel admin-guide explanation of RAS.  At the end
of the explanation, I include a URL to that guide directly.

[0001-gnu-Add-rasdaemon.patch (text/x-patch, attachment)]

Message #35 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: elaexuotee <at> wilsonb.com
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 12:08:48 -0400

On Tue, Apr 20, 2021 at 02:54:49PM +0900, elaexuotee <at> 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.

Message #38 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: elaexuotee <at> wilsonb.com
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 12:19:43 -0400

On Tue, Apr 20, 2021 at 03:18:19PM +0900, elaexuotee <at> 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.

Message #41 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: elaexuotee <at> wilsonb.com
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 12:20:04 -0400

On Tue, Apr 20, 2021 at 03:23:14PM +0900, elaexuotee <at> 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.

Message #44 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: elaexuotee <at> wilsonb.com
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Tue, 20 Apr 2021 12:23:16 -0400

On Tue, Apr 20, 2021 at 04:10:54PM +0900, elaexuotee <at> wilsonb.com wrote:
> This patch updates the license field to contain lgpl2.1, gpl2, and gpl2+.

As mentioned in a previous reply, it should contain only gpl2 [0], unless
upstream has explicitly given us a license to distribute rasdaemon under
the other licenses.

[0] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47905#35

Message #47 received at 47905 <at> debbugs.gnu.org (full text, mbox):

From: elaexuotee <at> wilsonb.com
To: Leo Famulari <leo <at> famulari.name>
Cc: 47905 <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Thu, 22 Apr 2021 19:01:20 +0900

[Message part 1 (text/plain, inline)]

Leo Famulari <leo <at> famulari.name> wrote:
> On Tue, Apr 20, 2021 at 04:10:54PM +0900, elaexuotee <at> wilsonb.com wrote:
> > This patch updates the license field to contain lgpl2.1, gpl2, and gpl2+.
> 
> As mentioned in a previous reply, it should contain only gpl2 [0], unless
> upstream has explicitly given us a license to distribute rasdaemon under
> the other licenses.
> 
> [0] https://debbugs.gnu.org/cgi/bugreport.cgi?bug=47905#35


Fixed!  Thanks for holding my hand on this.  That above link cleared up a lot
of lingering questions I had.

Anyway, in addition to the license fix, I also noticed that the daemon
automatically logs to syslogd, so I stripped out /var/log/rasdaemon.log,
including the references in the documentation.

Let me know what you think.

Cheers!

[0001-gnu-Add-rasdaemon.patch (text/x-patch, attachment)]

Message #52 received at 47905-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: elaexuotee <at> wilsonb.com
Cc: 47905-done <at> debbugs.gnu.org
Subject: Re: [bug#47905] gnu: Add rasdaemon.
Date: Sat, 24 Apr 2021 12:33:56 -0400

[Message part 1 (text/plain, inline)]

On Thu, Apr 22, 2021 at 07:01:20PM +0900, elaexuotee <at> wilsonb.com wrote:
> Fixed!  Thanks for holding my hand on this.  That above link cleared up a lot
> of lingering questions I had.

You're welcome! My goal is help contributors keep learning until they
are sending patches that pass code review on the first try, and they
feel confident to start reviewing patches, too. Based on this submission,
I think you are more than knowledgeable enough to review some patches :)

> Let me know what you think.

> From d31cb88e5c34cc0b650ff181ebb6a0d85d045f9d Mon Sep 17 00:00:00 2001
> From: "B. Wilson" <elaexuotee <at> wilsonb.com>
> Date: Tue, 20 Apr 2021 11:49:26 +0900
> Subject: [PATCH] gnu: Add rasdaemon.
> To: guix-patches <at> 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.

It's good! I pushed as 2c93df3d11bf8ceeb5c203416a2533cf32275e1a after
splitting it into two commits: one for the package, and one for the
service and its documentation.

By the way, you might ask upstream about fixing the issues in their
build scripts — the ones you fixed in the 'munge-autotools' phase. I
think most distros will need to do similar fixes.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.