Subject: Re: [bug#30498] [WIP v2 shepherd] shepherd: If /dev/kmsg is writable, use it for logging.

From: [email-masked] (Ludovic Courtès)
Date: Mon, 26 Feb 2018 19:04:44 +0100

Hi!

Danny Milosavljevic <[email-masked]> skribis:

> * modules/shepherd.scm (main): If /dev/kmsg is used, don't log to console
> again - use only /dev/kmsg.  Also redirect stderr to /dev/kmsg in
> that case.
> * modules/shepherd/comm.scm (%current-logfile-date-format): New variable.
> (make-shepherd-output-port): Use it.  Export.
> * modules/shepherd/support.scm (default-logfile-date-format): New variable.
> (default-logfile): Use /dev/kmsg if writable.
> (default-logfile-date-format): Drop duplicate timestamp.

[...]

> --- a/modules/shepherd.scm
> +++ b/modules/shepherd.scm
> @@ -141,12 +141,20 @@
>      ;; Enable logging as first action.
>      (start-logging logfile)
>  
> +    (if (string=? logfile "/dev/kmsg")
> +        ;; Prevent duplicate messages.
> +        (set-current-output-port (%make-void-port "w")))

Nitpick: I’d use ‘when’ here since there’s no ‘else’ branch.

> +(define %current-logfile-date-format
> +  (make-parameter default-logfile-date-format))

Please add a one-line comment explaining what the format is.

Could you also add a couple of lines in the .texi file explaining that
/dev/kmsg is used when it’s available and we’re running as root?

As for cryptsetup, what if the service that runs crypsetup simply
parameterizes ‘log-output-port’ to /dev/console?  That would give us the
current behavior, right?

Apart from that it LGTM, thank you!

Ludo’.