[PATCH] services: agetty: Make tty optional and add agetty instance to base services.

  • Done
  • quality assurance status badge
Details
2 participants
  • Danny Milosavljevic
  • Ludovic Courtès
Owner
unassigned
Submitted by
Danny Milosavljevic
Severity
normal
D
D
Danny Milosavljevic wrote on 5 Feb 2018 09:15
(address . guix-patches@gnu.org)(name . Danny Milosavljevic)(address . dannym@scratchpost.org)
20180205081553.5930-1-dannym@scratchpost.org
* gnu/system/install.scm (agetty-default-service): Delete variable.
(beaglebone-black-installation-os): Do not specify tty.
(a20-olinuxino-lime-installation-os): Do not specify tty.
(a20-olinuxino-lime2-emmc-installation-os): Do not specify tty.
(a20-olinuxino-micro-installation-os): Do not specify tty.
(banana-pi-m2-ultra-installation-os): Do not specify tty.
(nintendo-nes-classic-edition-installation-os): Do not specify tty.
(embedded-installation-os): Move agetty-service instantiation to...
* gnu/services/base.scm (%base-services): ...here.
(default-serial-port): New variable.
(agetty-shepherd-service): Make tty optional, default to the above.
* doc/guix.texi (agetty-configuration): Update "tty" documentation.
---
doc/guix.texi | 15 +++++++++-
gnu/services/base.scm | 77 ++++++++++++++++++++++++++++++++++++++++----------
gnu/system/install.scm | 37 ++++++++----------------
3 files changed, 88 insertions(+), 41 deletions(-)

Toggle diff (236 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 2b27a675c..4e5d9e33f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -9621,7 +9621,20 @@ man page for more information.
@item @code{tty}
The name of the console this agetty runs on, as a string---e.g.,
-@code{"ttyS0"}. This argument is mandatory.
+@code{"ttyS0"}. This argument is optional, it will default to
+a reasonable default serial port used by Linux.
+
+For this, if there is a value for an option "agetty.tty" in the Linux
+command line, agetty will extract the device name of the serial port
+from it and use that.
+
+If not and if there is a value for an option "console" with a tty in
+the Linux command line, agetty will extract the device name of the
+serial port from it and use that.
+
+In both cases, agetty will leave the other serial device settings
+(baud rate etc) alone - in the hope that Linux pinned them to the
+correct values.
@item @code{baud-rate} (default: @code{#f})
A string containing a comma-separated list of one or more baud rates, in
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 8e30bcd34..8e740bf6c 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -817,7 +817,7 @@ the message of the day, among other things."
agetty-configuration?
(agetty agetty-configuration-agetty ;<package>
(default util-linux))
- (tty agetty-configuration-tty) ;string
+ (tty agetty-configuration-tty) ;string | #f
(term agetty-term ;string | #f
(default #f))
(baud-rate agetty-baud-rate ;string | #f
@@ -890,6 +890,42 @@ the message of the day, among other things."
;;; (default #f))
)
+(define (default-serial-port)
+ "Return a gexp that determines a reasonable default serial port
+to use as the tty. This is primarily useful for headless systems."
+ #~(begin
+ ;; console=device,options
+ ;; device: can be tty0, ttyS0, lp0, ttyUSB0 (serial).
+ ;; options: BBBBPNF. P n|o|e, N number of bits,
+ ;; F flow control (r RTS)
+ (use-modules (gnu build linux-boot))
+ (let* ((not-comma (char-set-complement (char-set #\,)))
+ (command (linux-command-line))
+ (agetty-specs (find-long-options "agetty.tty" command))
+ (console-specs (filter (lambda (spec)
+ (not (or
+ (string-prefix? "tty0" spec)
+ (string-prefix? "tty1" spec)
+ (string-prefix? "tty2" spec)
+ (string-prefix? "tty3" spec)
+ (string-prefix? "tty4" spec)
+ (string-prefix? "tty5" spec)
+ (string-prefix? "tty6" spec)
+ (string-prefix? "tty7" spec)
+ (string-prefix? "tty8" spec)
+ (string-prefix? "tty9" spec))))
+ (filter
+ (lambda (spec)
+ (string-prefix? "tty" spec))
+ (find-long-options "console" command))))
+ (specs (append agetty-specs console-specs))
+ (spec (if (null? specs)
+ #f
+ (car specs))))
+ (and spec
+ ;; Extract device name.
+ (car (string-tokenize spec not-comma))))))
+
(define agetty-shepherd-service
(match-lambda
(($ <agetty-configuration> agetty tty term baud-rate auto-login
@@ -901,7 +937,7 @@ the message of the day, among other things."
(list
(shepherd-service
(documentation "Run agetty on a tty.")
- (provision (list (symbol-append 'term- (string->symbol tty))))
+ (provision (list (symbol-append 'term- (string->symbol (or tty "auto")))))
;; Since the login prompt shows the host name, wait for the 'host-name'
;; service to be done. Also wait for udev essentially so that the tty
@@ -946,6 +982,9 @@ the message of the day, among other things."
('always "--local-line=always")
('never "-local-line=never")))
#~())
+ #$@(if tty
+ #~()
+ #~("--keep-baud"))
#$@(if extract-baud?
#~("--extract-baud")
#~())
@@ -1009,7 +1048,7 @@ the message of the day, among other things."
#$@(if login-pause?
#~("--login-pause")
#~())
- #$tty
+ #$(or tty (default-serial-port))
#$@(if baud-rate
#~(#$baud-rate)
#~())
@@ -1058,18 +1097,21 @@ the tty to run, among other things."
;; text is not lost in the middle of kernel messages (XXX).
(requirement '(user-processes host-name udev))
- (start #~(make-forkexec-constructor
- (list #$(file-append mingetty "/sbin/mingetty")
- "--noclear" #$tty
- #$@(if auto-login
- #~("--autologin" #$auto-login)
- #~())
- #$@(if login-program
- #~("--loginprog" #$login-program)
- #~())
- #$@(if login-pause?
- #~("--loginpause")
- #~()))))
+ (start #~(let ((tty #$(default-serial-port)))
+ (if tty
+ (make-forkexec-constructor
+ (list #$(file-append mingetty "/sbin/mingetty")
+ "--noclear" #$tty
+ #$@(if auto-login
+ #~("--autologin" #$auto-login)
+ #~())
+ #$@(if login-program
+ #~("--loginprog" #$login-program)
+ #~())
+ #$@(if login-pause?
+ #~("--loginpause")
+ #~())))
+ (const #f)))) ; never start.
(stop #~(make-kill-destructor)))))))
(define mingetty-service-type
@@ -2012,6 +2054,11 @@ This service is not part of @var{%base-services}."
(cons tty %default-console-font))
'("tty1" "tty2" "tty3" "tty4" "tty5" "tty6")))
+ (agetty-service (agetty-configuration
+ (extra-options '("-L")) ; no carrier detect
+ (term "vt100")
+ (tty #f))) ; automatic
+
(mingetty-service (mingetty-configuration
(tty "tty1")))
(mingetty-service (mingetty-configuration
diff --git a/gnu/system/install.scm b/gnu/system/install.scm
index e4b2e8237..db06b7a52 100644
--- a/gnu/system/install.scm
+++ b/gnu/system/install.scm
@@ -381,19 +381,10 @@ You have been warned. Thanks for being so brave.\x1b[0m
nvi ;:wq!
%base-packages))))
-(define* (agetty-default-service #:optional (tty "ttyS0"))
- "Return an agetty-service on the given TTY"
- (agetty-service (agetty-configuration
- (extra-options '("-L"))
- (baud-rate "115200")
- (term "vt100")
- (tty tty))))
-
-(define* (embedded-installation-os bootloader bootloader-target tty
+(define* (embedded-installation-os bootloader bootloader-target
#:key (extra-modules '()))
"Return an installation os for embedded systems.
The initrd gets the extra modules EXTRA-MODULES.
-A getty is provided on TTY.
The bootloader BOOTLOADER is installed to BOOTLOADER-TARGET."
(operating-system
(inherit installation-os)
@@ -404,43 +395,39 @@ The bootloader BOOTLOADER is installed to BOOTLOADER-TARGET."
(initrd (lambda (fs . rest)
(apply base-initrd fs
#:extra-modules extra-modules
- rest)))
- (services (cons* (agetty-default-service tty)
- (operating-system-user-services installation-os)))))
+ rest)))))
(define beaglebone-black-installation-os
(embedded-installation-os u-boot-beaglebone-black-bootloader
"/dev/sda"
- "ttyO0"
#:extra-modules
;; This module is required to mount the sd card.
'("omap_hsmmc")))
-
(define a20-olinuxino-lime-installation-os
(embedded-installation-os u-boot-a20-olinuxino-lime-bootloader
- "/dev/mmcblk0" ; SD card storage
- "ttyS0"))
+ ;; SD card storage
+ "/dev/mmcblk0"))
(define a20-olinuxino-lime2-emmc-installation-os
(embedded-installation-os u-boot-a20-olinuxino-lime2-bootloader
- "/dev/mmcblk1" ; eMMC storage
- "ttyS0"))
+ ;; eMMC storage
+ "/dev/mmcblk1"))
(define a20-olinuxino-micro-installation-os
(embedded-installation-os u-boot-a20-olinuxino-micro-bootloader
- "/dev/mmcblk0" ; SD card storage
- "ttyS0"))
+ ;; SD card storage
+ "/dev/mmcblk0"))
(define banana-pi-m2-ultra-installation-os
(embedded-installation-os u-boot-banana-pi-m2-ultra-bootloader
- "/dev/mmcblk1" ; eMMC storage
- "ttyS0"))
+ ;; eMMC storage
+ "/dev/mmcblk1"))
(define nintendo-nes-classic-edition-installation-os
(embedded-installation-os u-boot-nintendo-nes-classic-edition-bootloader
- "/dev/mmcblk0" ; SD card (solder it yourself)
- "ttyS0"))
+ ;; SD card storage (solder it yourself)
+ "/dev/mmcblk0"))
;; Return the default os here so 'guix system' can consume it directly.
installation-os
L
L
Ludovic Courtès wrote on 9 Feb 2018 14:51
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 30355@debbugs.gnu.org)
87d11e1dbj.fsf@gnu.org
Hi,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

Toggle quote (13 lines)
> * gnu/system/install.scm (agetty-default-service): Delete variable.
> (beaglebone-black-installation-os): Do not specify tty.
> (a20-olinuxino-lime-installation-os): Do not specify tty.
> (a20-olinuxino-lime2-emmc-installation-os): Do not specify tty.
> (a20-olinuxino-micro-installation-os): Do not specify tty.
> (banana-pi-m2-ultra-installation-os): Do not specify tty.
> (nintendo-nes-classic-edition-installation-os): Do not specify tty.
> (embedded-installation-os): Move agetty-service instantiation to...
> * gnu/services/base.scm (%base-services): ...here.
> (default-serial-port): New variable.
> (agetty-shepherd-service): Make tty optional, default to the above.
> * doc/guix.texi (agetty-configuration): Update "tty" documentation.

I would prefer the install.scm part to be a separate patch, if that’s
not too much of a burden for you.

I have a few minor comments:

Toggle quote (6 lines)
> @item @code{tty}
> The name of the console this agetty runs on, as a string---e.g.,
> -@code{"ttyS0"}. This argument is mandatory.
> +@code{"ttyS0"}. This argument is optional, it will default to
> +a reasonable default serial port used by Linux.

s/Linux/the kernel Linux/ (to avoid ambiguities…)

Toggle quote (8 lines)
> +For this, if there is a value for an option "agetty.tty" in the Linux
> +command line, agetty will extract the device name of the serial port
> +from it and use that.
> +
> +If not and if there is a value for an option "console" with a tty in
> +the Linux command line, agetty will extract the device name of the
> +serial port from it and use that.

@code{agetty.tty}, @code{console}, etc.

Write “kernel command line” instead of “Linux command line”.

Toggle quote (4 lines)
> +In both cases, agetty will leave the other serial device settings
> +(baud rate etc) alone - in the hope that Linux pinned them to the
> +correct values.

s/etc/etc./
s/alone - in the hope/alone---in the hope/

(The three hyphens map to an em dash in the output.)

Toggle quote (29 lines)
> +(define (default-serial-port)
> + "Return a gexp that determines a reasonable default serial port
> +to use as the tty. This is primarily useful for headless systems."
> + #~(begin
> + ;; console=device,options
> + ;; device: can be tty0, ttyS0, lp0, ttyUSB0 (serial).
> + ;; options: BBBBPNF. P n|o|e, N number of bits,
> + ;; F flow control (r RTS)
> + (use-modules (gnu build linux-boot))
> + (let* ((not-comma (char-set-complement (char-set #\,)))
> + (command (linux-command-line))
> + (agetty-specs (find-long-options "agetty.tty" command))
> + (console-specs (filter (lambda (spec)
> + (not (or
> + (string-prefix? "tty0" spec)
> + (string-prefix? "tty1" spec)
> + (string-prefix? "tty2" spec)
> + (string-prefix? "tty3" spec)
> + (string-prefix? "tty4" spec)
> + (string-prefix? "tty5" spec)
> + (string-prefix? "tty6" spec)
> + (string-prefix? "tty7" spec)
> + (string-prefix? "tty8" spec)
> + (string-prefix? "tty9" spec))))
> + (filter
> + (lambda (spec)
> + (string-prefix? "tty" spec))
> + (find-long-options "console" command))))

Please make that a single ‘filter’ or ‘remove’ call, for clarity.

Toggle quote (8 lines)
> + (specs (append agetty-specs console-specs))
> + (spec (if (null? specs)
> + #f
> + (car specs))))
> + (and spec
> + ;; Extract device name.
> + (car (string-tokenize spec not-comma))))))

Rather:

(match (append agetty-specs console-specs)
(() #f)
((spec _ ...)
(string-tokenize spec not-comma)))

The rest LGTM. OK to push with these changes!

Thanks,
Ludo’.
D
D
Danny Milosavljevic wrote on 9 Feb 2018 20:49
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 30355@debbugs.gnu.org)
20180209204912.379ddae7@scratchpost.org
Toggle quote (2 lines)
> Please make that a single ‘filter’ or ‘remove’ call, for clarity.

OK!

Toggle quote (7 lines)
> Rather:
>
> (match (append agetty-specs console-specs)
> (() #f)
> ((spec _ ...)
> (string-tokenize spec not-comma)))

Crashes boot process with form error ().

I can't get "match" to work at all in this location.

It works fine when I start guile on a normally running system, but
in this initrd guile thing, (use-modules (ice-9 match)) doesn't help
either. What does help is (@ (ice-9 match) match). Why?

(define (default-serial-port)
"Return a gexp that determines a reasonable default serial port
to use as the tty. This is primarily useful for headless systems."
#~(begin
;; console=device,options
;; device: can be tty0, ttyS0, lp0, ttyUSB0 (serial).
;; options: BBBBPNF. P n|o|e, N number of bits,
;; F flow control (r RTS)
(use-modules (gnu build linux-boot))
(let* ((not-comma (char-set-complement (char-set #\,)))
(command (linux-command-line))
(agetty-specs (find-long-options "agetty.tty" command))
(console-specs (filter (lambda (spec)
(and (string-prefix? "tty" spec)
(not (or
(string-prefix? "tty0" spec)
(string-prefix? "tty1" spec)
(string-prefix? "tty2" spec)
(string-prefix? "tty3" spec)
(string-prefix? "tty4" spec)
(string-prefix? "tty5" spec)
(string-prefix? "tty6" spec)
(string-prefix? "tty7" spec)
(string-prefix? "tty8" spec)
(string-prefix? "tty9" spec)))))
(find-long-options "console" command)))
(specs (append agetty-specs console-specs)))
(use-modules (ice-9 match))
(match specs
(() #f) ; form error here
((spec _ ...) ; underscore is undefined
;; Extract device name from first spec.
(match (string-tokenize spec not-comma)
((device-name _ ...) ; underscore is undefined
device-name)))))))

Works just fine if I execute it in a running system, mind you...
L
L
Ludovic Courtès wrote on 9 Feb 2018 23:00
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 30355@debbugs.gnu.org)
877erlygas.fsf@gnu.org
Danny Milosavljevic <dannym@scratchpost.org> skribis:

Toggle quote (55 lines)
>> Please make that a single ‘filter’ or ‘remove’ call, for clarity.
>
> OK!
>
>> Rather:
>>
>> (match (append agetty-specs console-specs)
>> (() #f)
>> ((spec _ ...)
>> (string-tokenize spec not-comma)))
>
> Crashes boot process with form error ().
>
> I can't get "match" to work at all in this location.
>
> It works fine when I start guile on a normally running system, but
> in this initrd guile thing, (use-modules (ice-9 match)) doesn't help
> either. What does help is (@ (ice-9 match) match). Why?
>
> (define (default-serial-port)
> "Return a gexp that determines a reasonable default serial port
> to use as the tty. This is primarily useful for headless systems."
> #~(begin
> ;; console=device,options
> ;; device: can be tty0, ttyS0, lp0, ttyUSB0 (serial).
> ;; options: BBBBPNF. P n|o|e, N number of bits,
> ;; F flow control (r RTS)
> (use-modules (gnu build linux-boot))
> (let* ((not-comma (char-set-complement (char-set #\,)))
> (command (linux-command-line))
> (agetty-specs (find-long-options "agetty.tty" command))
> (console-specs (filter (lambda (spec)
> (and (string-prefix? "tty" spec)
> (not (or
> (string-prefix? "tty0" spec)
> (string-prefix? "tty1" spec)
> (string-prefix? "tty2" spec)
> (string-prefix? "tty3" spec)
> (string-prefix? "tty4" spec)
> (string-prefix? "tty5" spec)
> (string-prefix? "tty6" spec)
> (string-prefix? "tty7" spec)
> (string-prefix? "tty8" spec)
> (string-prefix? "tty9" spec)))))
> (find-long-options "console" command)))
> (specs (append agetty-specs console-specs)))
> (use-modules (ice-9 match))
> (match specs
> (() #f) ; form error here
> ((spec _ ...) ; underscore is undefined
> ;; Extract device name from first spec.
> (match (string-tokenize spec not-comma)
> ((device-name _ ...) ; underscore is undefined
> device-name)))))))

Try moving the ‘use-modules’ form at the top so you have:

(use-modules (gnu build linux-boot) (ice-9 match))

The inner ‘use-modules’ you had cannot work because the expander doesn’t
“see” it when compiling, and thus the ‘match’ macro is missing at
expansion time (thus the compiler assumes that code is calling a ‘match’
procedure that’ll be define at run time.) However it can work when you
interpret it instead of compiling because the compiler will have
expansion and execution intermingled.

HTH,
Ludo’.
D
D
Danny Milosavljevic wrote on 10 Feb 2018 23:07
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 30355@debbugs.gnu.org)
20180210230711.693d1452@scratchpost.org
Hi Ludo,

I tried this now:

(define (default-serial-port)
"Return a gexp that determines a reasonable default serial port
to use as the tty. This is primarily useful for headless systems."
#~(begin
;; console=device,options
;; device: can be tty0, ttyS0, lp0, ttyUSB0 (serial).
;; options: BBBBPNF. P n|o|e, N number of bits,
;; F flow control (r RTS)
(use-modules (gnu build linux-boot) (ice-9 match))
(let* ((not-comma (char-set-complement (char-set #\,)))
(command (linux-command-line))
(agetty-specs (find-long-options "agetty.tty" command))
(console-specs (filter (lambda (spec)
(and (string-prefix? "tty" spec)
(not (or
(string-prefix? "tty0" spec)
(string-prefix? "tty1" spec)
(string-prefix? "tty2" spec)
(string-prefix? "tty3" spec)
(string-prefix? "tty4" spec)
(string-prefix? "tty5" spec)
(string-prefix? "tty6" spec)
(string-prefix? "tty7" spec)
(string-prefix? "tty8" spec)
(string-prefix? "tty9" spec)))))
(find-long-options "console" command)))
(specs (append agetty-specs console-specs)))
(match specs
(() #f)
((spec _ ...)
;; Extract device name from first spec.
(match (string-tokenize spec not-comma)
((device-name _ ...)
device-name)))))))

I get

$ ./pre-inst-env guix system reconfigure /etc/config.scm --fallback
...
guix system: error: exception caught while executing 'eval' on service 'root':
ERROR: Syntax error:
unknown location: unexpected syntax in form ()

(works totally fine with (@ (ice-9 match) match) instead)

Maybe (ice-9 match) was imported already and the use-module is ignored.

I get the same effect when I do the following in a new guile interpreter:

,use (ice-9 match)
(define match 42)
,use (ice-9 match)
(match #t (() #f))

So maybe there's a "match" variable captured from somewhere. Icky...
L
L
Ludovic Courtès wrote on 13 Feb 2018 23:04
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 30355@debbugs.gnu.org)
87bmgsv95x.fsf@gnu.org
Heya,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

Toggle quote (12 lines)
> I tried this now:
>
> (define (default-serial-port)
> "Return a gexp that determines a reasonable default serial port
> to use as the tty. This is primarily useful for headless systems."
> #~(begin
> ;; console=device,options
> ;; device: can be tty0, ttyS0, lp0, ttyUSB0 (serial).
> ;; options: BBBBPNF. P n|o|e, N number of bits,
> ;; F flow control (r RTS)
> (use-modules (gnu build linux-boot) (ice-9 match))

[...]

Toggle quote (16 lines)
> (match specs
> (() #f)
> ((spec _ ...)
> ;; Extract device name from first spec.
> (match (string-tokenize spec not-comma)
> ((device-name _ ...)
> device-name)))))))
>
> I get
>
> $ ./pre-inst-env guix system reconfigure /etc/config.scm --fallback
> ...
> guix system: error: exception caught while executing 'eval' on service 'root':
> ERROR: Syntax error:
> unknown location: unexpected syntax in form ()

That’s a sign that the outermost ‘match’ wasn’t macro-expanded, hence
the error about ().

I got it: that’s because this gexp is spliced in a non-top-level
context. The end result is something like:

(let ((foo bar)
(baz (begin (use-modules …) …)))
…)

and the ‘use-modules’ there doesn’t have the desired effect.

The fix is to remove this ‘use-modules’ form and instead to pass the
modules as the ‘modules’ field of ‘shepherd-service’:

(shepherd-service
(modules '((ice-9 match) …))
;; …
)

Does that work for you?

Sorry for overlooking this before!

Ludo’.
D
D
Danny Milosavljevic wrote on 10 Feb 2019 22:12
(no subject)
(address . control@debbugs.gnu.org)
20190210221243.39823f05@scratchpost.org
close 30355
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAlxgk8sACgkQ5xo1VCww
uqUCmgf8DQXH3RpXMNtDuzGnTlygSmB8nIDIR0mn2DN3WuHj0loUGsu2rjqK1ZAZ
glRVlWhHJq/Hw3W6tVDNVM5qjTHMNjQBt2j+KF49SrB2Xy23GddDWxyTAnK6iRnq
0JV7MzAyppAw8dglJfcJ30HCvjaCCC2Iu896BK/k7pDwiZkPb4QFjpvtDJm/NUId
+NQXSKRz/e5/G2wptR9GaIFRK4+yWSPBQ5V9vxPi1uUtah/ncmGV37tSj/PHktde
VivWc+N356XaJph2TLueuZ9HyYQNLXsbGIdnmGN2sAjwr1JIZtqFQxzSrT9QEK0D
kIN3yUSEl+489al1+QKcEIT88VPSUg==
=j5ay
-----END PGP SIGNATURE-----


?