[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

Debbugs page

Danny Milosavljevic wrote 7 years ago
(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
Ludovic Courtès wrote 7 years ago
(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’.
Danny Milosavljevic wrote 7 years ago
(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...
Ludovic Courtès wrote 7 years ago
(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’.
Danny Milosavljevic wrote 7 years ago
(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...
Ludovic Courtès wrote 7 years ago
(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’.
Danny Milosavljevic wrote 6 years ago
(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-----


?
Your comment

This issue is archived.

To comment on this conversation send an email to 30355@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 30355
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help