#2961 PR merged: Copy the console= kernel arguments from the original system

Labels: enhancement, cleanup, fixed / solved / done, minor bug

jsmeix opened issue at 2023-03-28 12:19:

Overhauled the whole automated serial console setup
for the recovery system in particular
prep/GNU/Linux/200_include_serial_console.sh
and lib/serial-functions.sh which results that
rescue/GNU/Linux/400_use_serial_console.sh is obsolete, see
https://github.com/rear/rear/pull/2749#issuecomment-1196650631

According to that the basic idea is by default to only

copy all the console= kernel arguments from the running system

which means to no longer auto-enable serial consoles
for all "real serial devices" in the ReaR recovery system.

That new default behaviour is described in default.conf

A "real serial device" is one where its kernel device node
is a /dev/ttyS* or /dev/hvsi* character device and where

# stty -F /dev/tty...

results a 'speed' line,
e.g. on my test VM /dev/ttyS0 is a "real serial device"
(regarless that /dev/ttyS0 is only a virtual serial device)
but /dev/ttyS1 is not a "real serial device":

# stty -F /dev/ttyS0
speed 9600 baud; line = 0;
-brkint -imaxbel

# stty -F /dev/ttyS1
stty: /dev/ttyS1: Input/output error

What did not change is when the user explicitly specified
SERIAL_CONSOLE_DEVICES
SERIAL_CONSOLE_DEVICES_KERNEL
SERIAL_CONSOLE_DEVICE_SYSLINUX
SERIAL_CONSOLE_DEVICE_GRUB
What the user specified there is used or set up as specified.

There are no new configuration variables.

jsmeix commented at 2023-03-28 12:30:

Tomorrow I will do some initial testing.

jsmeix commented at 2023-03-29 10:19:

My first test result of the new default behaviour
(i.e. when no SERIAL_CONSOLE config variables are specified):

I booted the original system (a QEMU/KVM virtual machine)
with artificial (I do not have serial port hardware)
additional kernel 'console' command line options:

... console=ttyS1,9600n8 ... console=ttyS3 ... console=tty0 ...

and got

# dmesg | grep console
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root console=ttyS1,9600n8 resume=/dev/system/swap console=ttyS3 crashkernel=203M,high console=tty0 mitigations=off
[    0.122561] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root console=ttyS1,9600n8 resume=/dev/system/swap console=ttyS3 crashkernel=203M,high console=tty0 mitigations=off
[    0.146431] printk: console [tty0] enabled
[    0.219572] printk: console [ttyS1] enabled
[    5.980855] qxl 0000:00:02.0: vgaarb: deactivate vga console

# ls -l /dev/ttyS[0-9]
crw-rw---- 1 root dialout 4, 64 Mar 29 11:10 /dev/ttyS0
crw-rw---- 1 root dialout 4, 65 Mar 29 11:10 /dev/ttyS1
crw-rw---- 1 root dialout 4, 66 Mar 29 11:10 /dev/ttyS2
crw-rw---- 1 root dialout 4, 67 Mar 29 11:10 /dev/ttyS3
crw-rw---- 1 root dialout 4, 68 Mar 29 11:10 /dev/ttyS4
crw-rw---- 1 root dialout 4, 69 Mar 29 11:10 /dev/ttyS5
crw-rw---- 1 root dialout 4, 70 Mar 29 11:10 /dev/ttyS6
crw-rw---- 1 root dialout 4, 71 Mar 29 11:10 /dev/ttyS7
crw-rw---- 1 root dialout 4, 72 Mar 29 11:10 /dev/ttyS8
crw-rw---- 1 root dialout 4, 73 Mar 29 11:10 /dev/ttyS9

# uname -a
Linux localhost 5.3.18-57-default #1 SMP Wed Apr 28 10:54:41 UTC 2021 (ba3c2e9) x86_64 x86_64 x86_64 GNU/Linux

That there is first console [tty0] enabled
and then console [ttyS1] enabled
but no 'console [ttyS3] enabled' matches what
https://www.kernel.org/doc/html/v5.3/admin-guide/serial-console.html
reads (excerpts)

So, for example:

console=ttyS1,9600 console=tty0

defines that opening /dev/console will get you the
current foreground virtual console, and kernel messages
will appear on both the VGA console and
the 2nd serial port (ttyS1 or COM2) at 9600 baud.

Note that you can only define one console
per device type (serial, video).

If no console device is specified, the first device found
capable of acting as a system console will be used.
At this time, the system first looks for a VGA card and
then for a serial port.
So if you don't have a VGA card in your system
the first serial port will automatically become the console.

ReaR on the original system (which runs SLES15 SP3):

# egrep -v '^#|^$' etc/rear/local.conf
FIRMWARE_FILES=( 'no' )
MODULES=( 'loaded_modules' )
PROGRESS_MODE="plain"
PROGRESS_WAIT_SECONDS="5"
OUTPUT=ISO
BACKUP=NETFS
BACKUP_OPTIONS="nfsvers=3,nolock"
BACKUP_URL=nfs://192.168.122.1/nfs
REQUIRED_PROGS+=( snapper chattr )
PROGS+=( lsattr )
COPY_AS_IS+=( /usr/lib/snapper/installation-helper /etc/snapper/config-templates/default )
BACKUP_PROG_INCLUDE=( /boot/grub2/x86_64-efi /boot/grub2/i386-pc /opt /srv /usr/local /root /tmp /var ) 
POST_RECOVERY_SCRIPT=( 'if snapper --no-dbus -r $TARGET_FS_ROOT get-config | grep -q "^QGROUP.*[0-9]/[0-9]" ; then snapper --no-dbus -r $TARGET_FS_ROOT set-config QGROUP= ; snapper --no-dbus -r $TARGET_FS_ROOT setup-quota && echo snapper setup-quota done || echo snapper setup-quota failed ; else echo snapper setup-quota not used ; fi' )
SSH_ROOT_PASSWORD='rear'
USE_DHCLIENT="yes"

# usr/sbin/rear -D mkrescue
...
Using build area: /var/tmp/rear.9suWIs5VRXA4OhL
...
Running 'rescue' stage ======================
Creating recovery system root filesystem skeleton layout
Adding 'console=ttyS1,9600n8' to KERNEL_CMDLINE
Adding 'console=ttyS3' to KERNEL_CMDLINE
Adding 'console=tty0' to KERNEL_CMDLINE
Handling network interface 'eth0'
eth0 is a physical device
Handled network interface 'eth0'
Skipping 'lo': not bound to any physical interface.
skipping usr/share/rear/rescue/GNU/Linux/400_use_serial_console.sh
...

# find /var/tmp/rear.9suWIs5VRXA4OhL -xdev -type f | xargs grep 'console=ttyS1,9600n8'
...
/var/tmp/rear.9suWIs5VRXA4OhL/tmp/isofs/isolinux/isolinux.cfg:
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS1,9600n8 console=ttyS3 console=tty0
/var/tmp/rear.9suWIs5VRXA4OhL/tmp/isofs/isolinux/isolinux.cfg:
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS1,9600n8 console=ttyS3 console=tty0 auto_recover

Excerpt from the ReaR log file:

+ source /root/rear.github.master/usr/share/rear/prep/GNU/Linux/200_include_serial_console.sh
...
+++ cat /proc/cmdline
++ for kernel_option in $( cat /proc/cmdline )
++ test BOOT_IMAGE = console
++ for kernel_option in $( cat /proc/cmdline )
++ test root = console
++ for kernel_option in $( cat /proc/cmdline )
++ test console = console
++ USE_SERIAL_CONSOLE=yes
++ COPY_KERNEL_PARAMETERS+=(console)
++ break
+ source_return_code=0

ReaR recovery system on replacement hardware/VM:

RESCUE localhost:~ # dmesg | grep console
[    0.000000] Command line: initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS1,9600n8 console=ttyS3 console=tty0 debug BOOT_IMAGE=kernel 
[    0.147923] Kernel command line: initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS1,9600n8 console=ttyS3 console=tty0 debug BOOT_IMAGE=kernel 
[    0.278021] printk: console [tty0] enabled
[    0.398808] printk: console [ttyS1] enabled
[    5.186534] qxl 0000:00:02.0: vgaarb: deactivate vga console

All recovery system startup messages appear for me
so the new default behaviour seems in particular to fix
https://github.com/rear/rear/issues/2843

I will verify this in a second test where I do not use
any kernel 'console' command line options...

jsmeix commented at 2023-03-29 11:26:

Second test where I do not use
any kernel 'console' command line options
on the original system:

# dmesg | egrep 'Kernel command line|console'
[    0.149486] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root resume=/dev/system/swap crashkernel=203M,high mitigations=off
[    0.173618] printk: console [tty0] enabled
[    5.807052] qxl 0000:00:02.0: vgaarb: deactivate vga console

# usr/sbin/rear -D mkrescue
...
Using build area: /var/tmp/rear.Vdrk58sZjJRSsc9
...

# grep 'root=' /var/tmp/rear.Vdrk58sZjJRSsc9/tmp/isofs/isolinux/isolinux.cfg
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 auto_recover

so there is also no kernel 'console' command line option set
for the ReaR recovery system.

Excerpt from the ReaR log file:

+ source /root/rear.github.master/usr/share/rear/prep/GNU/Linux/200_include_serial_console.sh
...
+++ cat /proc/cmdline
++ for kernel_option in $( cat /proc/cmdline )
++ test BOOT_IMAGE = console
++ for kernel_option in $( cat /proc/cmdline )
++ test root = console
++ for kernel_option in $( cat /proc/cmdline )
++ test resume = console
++ for kernel_option in $( cat /proc/cmdline )
++ test crashkernel = console
++ for kernel_option in $( cat /proc/cmdline )
++ test mitigations = console
+ source_return_code=0

ReaR recovery system on replacement hardware/VM:

RESCUE localhost:~ # dmesg | egrep 'Kernel command line|console'
[    0.132793] Kernel command line: initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 BOOT_IMAGE=kernel 
[    0.216293] printk: console [tty0] enabled
[    4.087957] qxl 0000:00:02.0: vgaarb: deactivate vga console

All recovery system startup messages appear for me
so the new default behaviour fixes
https://github.com/rear/rear/issues/2843
in particular when no kernel 'console' command line options
are used on the original system.

jsmeix commented at 2023-04-24 05:57:

This pull request is currently in a not yet finished sate.
I was interrupted to work on this one by other things.
I will continue here as time permits.

schlomo commented at 2023-04-24 07:00:

Ah, sorry. Please kindly set it to draft mode for everybody to see the not-yet-ready state right away.

jsmeix commented at 2023-04-24 07:29:

@schlomo
thank you for the link to the matching GitHub documentation!
(I had noticed that "convert to draft" button since some time
but never found time to find out what that actually means.)

github-actions commented at 2023-06-24 02:48:

Stale pull request message

github-actions commented at 2023-08-26 01:58:

Stale pull request message

jsmeix commented at 2023-09-27 12:31:

With latest changes
when I neither use any kernel 'console' option on the original system
nor any ...SERIAL_CONSOLE... config variables in local.conf
things look OK to me in the booted recovery system:

RESCUE localhost:~ # dmesg | egrep 'Kernel command line|console'
[    0.122326] Kernel command line: initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 BOOT_IMAGE=kernel 
[    0.198826] printk: console [tty0] enabled
[    3.548304] qxl 0000:00:02.0: vgaarb: deactivate vga console

and all recovery system startup messages appear for me.

jsmeix commented at 2023-09-27 13:09:

With latest changes
when I use those kernel 'console' option on the original system

# dmesg | egrep 'Kernel command line|console'
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root console=ttyS1,9600n8 console=ttyS3 resume=/dev/system/swap console=tty0 crashkernel=203M,high mitigations=off
[    0.122261] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root console=ttyS1,9600n8 console=ttyS3 resume=/dev/system/swap console=tty0 crashkernel=203M,high mitigations=off
[    0.146570] printk: console [tty0] enabled
[    0.220130] printk: console [ttyS1] enabled
[    5.803066] qxl 0000:00:02.0: vgaarb: deactivate vga console

bot not any ...SERIAL_CONSOLE... config variables in local.conf
things look OK to me after "rear mkrescue" with OUTPUT=ISO:

/var/tmp/rear.WE9BOXnDTf0jksX/tmp/isolinux/isolinux.cfg
and its identical copy
/var/tmp/rear.WE9BOXnDTf0jksX/tmp/isofs/isolinux/isolinux.cfg
contain

append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS1,9600n8 console=ttyS3 console=tty0
...
initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS1,9600n8 console=ttyS3 console=tty0 auto_recover

What I cannot test is that the SYSLINUX bootloader
also gets some serial 1 9600 setting because
none of my /dev/ttyS* devices on my test VM
is a real serial device and accordingly
in the function make_syslinux_config()
in lib/bootloader-functions.sh
in the code

    for devnode in $( get_serial_console_devices ) ; do
        # Add SYSLINUX serial console config for real serial devices:
        if speed=$( get_serial_device_speed $devnode ) ; then
            ...
            echo "serial $port $speed"

the if speed=$( get_serial_device_speed $devnode )
never becomes true.

jsmeix commented at 2023-09-27 13:15:

Oh - wait!
/dev/ttyS0 on my test VM seems to behave like a "real" serial device
cf. the function get_serial_device_speed() in lib/serial-functions.sh

# ( set -o pipefail ; stty -F /dev/ttyS0 | awk '/^speed / { print $2 }' ) && echo OK || echo FAIL

9600
OK

in contrast to /dev/ttyS1 on my test VM

# ( set -o pipefail ; stty -F /dev/ttyS1 | awk '/^speed / { print $2 }' ) && echo OK || echo FAIL

stty: /dev/ttyS1: Input/output error
FAIL

Tomorrow I will re-test with kernel command line option console=ttyS0,9600n8

jsmeix commented at 2023-09-28 08:58:

Auto-enabling serial console support
for the recovery system bootloader SYSLINUX/EXTLINUX
seems to work for me (I use plain 'OUTPUT=ISO' without
any ...SERIAL_CONSOLE... config variables in local.conf).

With kernel command line option 'console=ttyS0,9600n8'
on the original system
I get in isolinux/isolinux.cfg an additional first line

serial 0 9600

Details:

I use those kernel 'console' options on the original system

# dmesg | grep 'console'

[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root console=ttyS0,9600n8 resume=/dev/system/swap console=tty0 crashkernel=203M,high mitigations=off
[    0.126268] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root console=ttyS0,9600n8 resume=/dev/system/swap console=tty0 crashkernel=203M,high mitigations=off
[    0.155543] printk: console [tty0] enabled
[    0.279798] printk: console [ttyS0] enabled
[    5.922218] qxl 0000:00:02.0: vgaarb: deactivate vga console

# usr/sbin/rear -D mkrescue
...
Running 'rescue' stage ======================
...
Adding 'console=ttyS0,9600n8' to KERNEL_CMDLINE
Adding 'console=tty0' to KERNEL_CMDLINE

/var/tmp/rear.wcQCs4TpMP7m9zv/tmp/isolinux/isolinux.cfg
and its identical copy
/var/tmp/rear.wcQCs4TpMP7m9zv/tmp/isofs/isolinux/isolinux.cfg
contain

serial 0 9600
...
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS0,9600n8 console=tty0
...
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS0,9600n8 console=tty0 auto_recover

var/log/rear/rear-localhost.log
contains

+ source /root/rear.pull2961/usr/share/rear/output/ISO/Linux-i386/300_create_isolinux.sh
...
+++ get_serial_console_devices
...
+++ for kernel_option in $( cat /proc/cmdline )
+++ test console = console
+++ console_option_value=ttyS0,9600n8
+++ console_option_device=ttyS0
+++ [[ ttyS0 == ttyS* ]]
+++ test -c /dev/ttyS0
+++ echo /dev/ttyS0
...
++ for devnode in $( get_serial_console_devices )
+++ get_serial_device_speed /dev/ttyS0
+++ local devnode=/dev/ttyS0
+++ test -c /dev/ttyS0
+++ set -o pipefail
+++ awk '/^speed / { print $2 }'
+++ stty -F /dev/ttyS0
++ speed=9600
+++ grep -o -E '[0-9]+$'
+++ echo /dev/ttyS0
++ port=0
++ test 0 -lt 4
++ echo 'serial 0 9600'
++ break

I cannot test if the serial console actually works
in the recovery system bootloader SYSLINUX/EXTLINUX
because I don't know (yet) how to access a serial console
that runs in a QEMU/KVM VM from its host system
(I use virt-manager for my VMs).

But this recovery system boots well for me,
in particular the SYSLINUX/EXTLINUX boot menue
and all recovery system startup messages appear for me.

In the booted recovery system:

RESCUE localhost:~ # dmesg | grep 'console'

[    0.000000] Command line: initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS0,9600n8 console=tty0 BOOT_IMAGE=kernel 
[    0.148959] Kernel command line: initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=ttyS0,9600n8 console=tty0 BOOT_IMAGE=kernel 
[    0.239802] printk: console [tty0] enabled
[    0.349881] printk: console [ttyS0] enabled
[    4.881906] qxl 0000:00:02.0: vgaarb: deactivate vga console

jsmeix commented at 2023-09-28 09:03:

@rear/contributors
please have a look,
at least only a look at my changed code,
perhaps you spot some obvious mistakes?
Ideally in particular if you have a serial console
please test how it works in reality (as time permits).
Many thanks in advance!

jsmeix commented at 2023-09-28 10:55:

Tested 'OUTPUT=ISO'
with ...SERIAL_CONSOLE... config variables in local.conf
(but without 'USE_SERIAL_CONSOLE=yes' in local.conf):

SERIAL_CONSOLE_DEVICES="/dev/ttyS1 /dev/ttyS2"
SERIAL_CONSOLE_DEVICES_KERNEL="/dev/tty0 console=ttyS3,9600"
SERIAL_CONSOLE_DEVICE_SYSLINUX="/dev/ttyS0"
OUTPUT=ISO

I still have the kernel command line options

console=ttyS0,9600n8 ... console=tty0

on the original system.

# usr/sbin/rear -D mkrescue
...
Running 'prep' stage ======================
Appended ' console=tty0,38400 console=ttyS3,9600' to KERNEL_CMDLINE
...

/var/tmp/rear.aqE5a3dPrA3CxXh/tmp/isolinux/isolinux.cfg
contains

serial 0 9600
...
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=tty0,38400 console=ttyS3,9600
...
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=tty0,38400 console=ttyS3,9600 auto_recover

var/log/rear/rear-localhost.log
contains

+ source /root/rear.pull2961/usr/share/rear/prep/GNU/Linux/200_include_serial_console.sh
...
++ test '/dev/ttyS1 /dev/ttyS2'
++ serial_console_devices='/dev/ttyS1 /dev/ttyS2'
++ test '/dev/tty0 console=ttyS3,9600'
++ serial_console_devices='/dev/tty0 console=ttyS3,9600'
...
++ for serial_console in $serial_console_devices
++ test -c /dev/tty0
+++ get_serial_device_speed /dev/tty0
+++ local devnode=/dev/tty0
+++ test -c /dev/tty0
+++ set -o pipefail
+++ awk '/^speed / { print $2 }'
+++ stty -F /dev/tty0
++ speed=38400
++ cmdline_add_console+=' console=tty0,38400'
++ for serial_console in $serial_console_devices
++ test -c console=ttyS3,9600
++ cmdline_add_console+=' console=ttyS3,9600'
++ test ' console=tty0,38400 console=ttyS3,9600'
++ KERNEL_CMDLINE+=' console=tty0,38400 console=ttyS3,9600'
++ DebugPrint 'Appended '\'' console=tty0,38400 console=ttyS3,9600'\'' to KERNEL_CMDLINE'
...
++ USE_SERIAL_CONSOLE=yes
.
.
.
+ source /root/rear.pull2961/usr/share/rear/output/ISO/Linux-i386/300_create_isolinux.sh
...
++ make_syslinux_config /var/tmp/rear.aqE5a3dPrA3CxXh/tmp/isolinux isolinux
...
++ is_true yes
++ case "$1" in
++ return 0
++ test /dev/ttyS0
++ test -c /dev/ttyS0
+++ grep -o -E '[0-9]+$'
+++ echo /dev/ttyS0
++ port=0
++ test 0 -lt 4
+++ get_serial_device_speed /dev/ttyS0
+++ local devnode=/dev/ttyS0
+++ test -c /dev/ttyS0
+++ set -o pipefail
+++ awk '/^speed / { print $2 }'
+++ stty -F /dev/ttyS0
++ speed=9600
++ echo 'serial 0 9600'

That console=tty0,38400 was appended to KERNEL_CMDLINE
may look unexpected because in local.conf

SERIAL_CONSOLE_DEVICES_KERNEL="/dev/tty0 ..."

was specified (i.e. plain '/dev/tty0' without speed)
but this is how the automatism works
when stty -F /dev/tty0 outputs a 'speed':

# stty -F /dev/tty0 | awk '/^speed / { print $2 }'

38400

When the automated speed setting is not wanted,
the user can specify exactly what he needs like

SERIAL_CONSOLE_DEVICES_KERNEL="console=tty0 console=ttyS3,9600"

which results

# usr/sbin/rear -D mkrescue
...
Appended ' console=tty0 console=ttyS3,9600' to KERNEL_CMDLINE

and /var/tmp/rear.Ju1JhZsYWExbgYC/tmp/isolinux/isolinux.cfg
contains

serial 0 9600
...
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=tty0 console=ttyS3,9600
...
append initrd=initrd.cgz root=/dev/ram0 vga=normal rw  selinux=0 console=tty0 console=ttyS3,9600 auto_recover

jsmeix commented at 2023-09-28 11:23:

Testing disabling serial console support:

USE_SERIAL_CONSOLE='no'
SERIAL_CONSOLE_DEVICES="/dev/ttyS1 /dev/ttyS2"
SERIAL_CONSOLE_DEVICES_KERNEL="console=tty0 console=ttyS3,9600"
SERIAL_CONSOLE_DEVICE_SYSLINUX="/dev/ttyS0"

I still have the kernel command line options

console=ttyS0,9600n8 ... console=tty0

on the original system.

# usr/sbin/rear -D mkrescue
...
Using build area: /var/tmp/rear.OfMaitQfo5N03qX
...
[no output about 'serial' or 'console']

# egrep 'serial|console' /var/tmp/rear.OfMaitQfo5N03qX/tmp/isolinux/isolinux.cfg
[no output]

# find /var/tmp/rear.OfMaitQfo5N03qX/rootfs/
...
/var/tmp/rear.OfMaitQfo5N03qX/rootfs/bin/agetty
...
/var/tmp/rear.OfMaitQfo5N03qX/rootfs/bin/stty

pcahyna commented at 2023-10-03 10:57:

@jsmeix sorry for the delay of review due to holidays last week, I plan to have a look at this later this week.

jsmeix commented at 2023-10-04 14:28:

@pcahyna
no worries - I was also on vacation.
There was a long weekend in Germany
with Monday vacation and Tuesday public holiday.

jsmeix commented at 2023-10-09 11:21:

Testing with OUTPUT=USB and USB_BOOTLOADER="grub":

On my original VM (with BIOS)
I have no 'console' kernel command line option set.
I use a virtual 10 GiB IDE disk /dev/sdb for OUTPUT=USB.

etc/rear/local.conf (excerpts):

SERIAL_CONSOLE_DEVICES="/dev/ttyS1 /dev/ttyS2"
SERIAL_CONSOLE_DEVICES_KERNEL="console=tty0 console=ttyS3,9600"
SERIAL_CONSOLE_DEVICE_SYSLINUX="/dev/ttyS4"
SERIAL_CONSOLE_DEVICE_GRUB="/dev/ttyS0"
OUTPUT=USB
USB_DEVICE_FILESYSTEM_PERCENTAGE=90
USB_DEVICE_FILESYSTEM_LABEL='MY-DATA'
USB_BOOT_PART_SIZE=2048
USB_BOOTLOADER="grub"
USB_DEVICE_BOOT_LABEL=MY-BOOT
OUTPUT_URL=usb:///dev/disk/by-label/MY-BOOT
USB_DEVICE_PARTED_LABEL=gpt
USB_BOOTLOADER="grub"
BACKUP=NETFS
BACKUP_URL=usb:///dev/disk/by-label/MY-DATA

# usr/sbin/rear -D format /dev/sdb
...
Repartitioning /dev/sdb
Creating partition table of type gpt on /dev/sdb
Making a BIOS bootable device /dev/sdb
Creating BIOS boot partition /dev/sdb1
Setting 'bios_grub' flag on BIOS boot partition /dev/sdb1
Making an EFI bootable device /dev/sdb
Creating EFI system partition /dev/sdb2 with size 1024 MiB aligned at 8 MiB
Setting 'esp' flag on EFI partition /dev/sdb2
Creating boot partition /dev/sdb3 with size 2048 MiB aligned at 8 MiB
Setting 'legacy_boot' flag on boot partition /dev/sdb3
Creating ReaR data partition /dev/sdb4 up to 90% of /dev/sdb
Creating vfat filesystem on EFI system partition on /dev/sdb2
Creating ext2 filesystem with label 'MY-BOOT' on boot partition /dev/sdb3
Creating ext3 filesystem with label 'MY-DATA' on ReaR data partition /dev/sdb4

# parted -s /dev/sdb unit GiB print

Model: ATA QEMU HARDDISK (scsi)
Disk /dev/sdb: 10.0GiB
Sector size (logical/physical): 512B/512B
Partition Table: gpt
Disk Flags: 
Number  Start    End      Size     File system  Name     Flags
 1      0.00GiB  0.01GiB  0.01GiB               primary  bios_grub
 2      0.01GiB  1.01GiB  1.00GiB  fat32        primary  boot, esp
 3      1.01GiB  3.01GiB  2.00GiB  ext2         primary  legacy_boot
 4      3.01GiB  9.00GiB  5.99GiB  ext3         primary

By the way I made via
https://github.com/rear/rear/commit/d725000f4613a702fef3eac065912c743a3ee9f8
a better (more generic) description in default.conf
of USB_DEVICE_FILESYSTEM_PERCENTAGE
that also takes a possible 'bios_grub' partition
and an optional boot partition into account.

# usr/sbin/rear -D mkbackup
...
Running 'prep' stage ======================
Appended ' console=tty0 console=ttyS3,9600' to KERNEL_CMDLINE
...
Running 'output' stage ======================
Skip configuring EFI partition '/dev/disk/by-label/REAR-EFI' for EFI boot (USING_UEFI_BOOTLOADER is not 'true')
Using GRUB2 as USB bootloader for legacy BIOS boot on /dev/sdb (USB_BOOTLOADER='grub')
Installing GRUB2 as USB bootloader on /dev/sdb
Creating GRUB2 config for legacy BIOS boot as USB bootloader
Configuring GRUB2 kernel /rear/localhost/20231009.1306/kernel
Configuring GRUB2 initrd /rear/localhost/20231009.1306/initrd.cgz
Configuring GRUB2 root device as 'search --no-floppy --set=root --label MY-BOOT'
...

# mount /dev/sdb3 /tmp/MY-BOOT

# egrep 'serial|console' /tmp/MY-BOOT/boot/grub2/grub.cfg

serial --unit=0 --speed=9600
    linux /rear/localhost/20231009.1306/kernel root=UUID=2fda6d6f-3d6e-4da9-8509-dd829d41434b  selinux=0 console=tty0 console=ttyS3,9600

"rear recover" works for me.

But with the (artificial and only for this test specified)
"console=tty0 console=ttyS3,9600" kernel command line options
I don't get the ReaR recovery system startup messages.
By default (without specified console kernel command line options)
I get the ReaR recovery system startup messages.

pcahyna commented at 2023-10-09 11:52:

@jsmeix sorry for the delay, looking now.
Can you please edit the description to describe what is the new behavior intended by the PR ?
Does the PR introduce any new configuration variables, or change in meaning of existing configuration variables ?

jsmeix commented at 2023-10-09 12:24:

@pcahyna
I enhanced the description of this pull request
to describe what the new behavior is
and what did not change.

Related to that is
https://github.com/rear/rear/commit/d27cdabaeb7d3d23e437a8c3d554de57683e3597

pcahyna commented at 2023-10-09 13:19:

@jsmeix I have not looked at the code yet, only at default.conf - this is intentional, the behavior should be easy to understand from the comments in default.conf only.

jsmeix commented at 2023-10-09 13:25:

@pcahyna
now I do no longer understand anything here.
I became totally confused now.
I give up for now.

pcahyna commented at 2023-10-09 13:27:

@jsmeix oops, sorry for that :-(

jsmeix commented at 2023-10-09 13:36:

@pcahyna
don't worry - I will try again tomorrow.
If I continue now I probably mess up things.

jsmeix commented at 2023-10-10 09:53:

@pcahyna
via
https://github.com/rear/rear/pull/2961/commits/c108f0dd57782dde92ba4e9af5895ddb9bed46d8
I made in default.conf a better description
how SERIAL_CONSOLE_DEVICES works in particular
together with SERIAL_CONSOLE_DEVICES_KERNEL

Example:
Provided USE_SERIAL_CONSOLE is not set to 'no'
and 'getty' or 'agetty' and 'stty' can be found,
then when there are
'console=ttyS1,9600' and 'console=tty0' in /proc/cmdline
then by default
'console=ttyS1,9600' and 'console=tty0'
get copied for the recovery system kernel and
only /dev/ttyS1 is used as serial console
for the recovery system bootloader (SYSLINUX or GRUB).

I added this example (but with /dev/ttyS0) to default.conf via
https://github.com/rear/rear/pull/2961/commits/12174397d9cd6264189699ce9a0c341aa306d098

jsmeix commented at 2023-10-10 10:42:

Tested latest code with

OUTPUT=USB
USB_BOOTLOADER="grub"
USB_DEVICE_PARTED_LABEL=gpt
USB_BOOTLOADER="grub"
BACKUP=NETFS
BACKUP_URL=usb:///dev/disk/by-label/REAR-000

with all ...SERIAL_CONSOLE... config variables empty by default.

Without a 'console' kernel command line option:

# usr/sbin/rear -D mkrescue
...
Running 'prep' stage ======================
No 'console=...' setting for recovery system kernel (none in /proc/cmdline)
...

With 'console' kernel command line options:

# cat /proc/cmdline 
BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root console=ttyS0,9600 resume=/dev/system/swap crashkernel=203M,high console=tty0 mitigations=off

# usr/sbin/rear -D mkrescue
...
Running 'rescue' stage ======================
...
Adding 'console=ttyS0,9600' to KERNEL_CMDLINE
Adding 'console=tty0' to KERNEL_CMDLINE
...

# mount /dev/sdb3 /tmp/REAR-000

# cat /tmp/REAR-000/boot/grub2/grub.cfg
...
serial --unit=0 --speed=9600
...
    linux /rear/localhost/20231010.1234/kernel root=UUID=2fda6d6f-3d6e-4da9-8509-dd829d41434b  selinux=0 console=ttyS0,9600 console=tty0
...

Excerpts from var/log/rear/rear-localhost.log

+ source /root/rear.pull2961/usr/share/rear/output/USB/Linux-i386/300_create_grub.sh
...
+++ create_grub2_serial_entry
...
++++ get_serial_console_devices
...
++++ for kernel_option in $( cat /proc/cmdline )
++++ test console = console
++++ console_option_value=ttyS0,9600
++++ console_option_device=ttyS0
++++ [[ ttyS0 == ttyS* ]]
++++ test -c /dev/ttyS0
++++ echo /dev/ttyS0
...
++++ for kernel_option in $( cat /proc/cmdline )
++++ test console = console
++++ console_option_value=tty0
++++ console_option_device=tty0
++++ [[ tty0 == ttyS* ]]
++++ [[ tty0 == hvsi* ]]
++++ continue
...
+++ for devnode in $( get_serial_console_devices )
++++ get_serial_device_speed /dev/ttyS0
++++ local devnode=/dev/ttyS0
++++ test -c /dev/ttyS0
++++ set -o pipefail
++++ stty -F /dev/ttyS0
++++ awk '/^speed / { print $2 }'
+++ speed=9600
++++ grep -o -E '[0-9]+$'
++++ echo /dev/ttyS0
+++ unit=0
+++ test 0 -lt 4
+++ echo 'serial --unit=0 --speed=9600'
+++ break

The speed '9600' that is used for /dev/ttyS0 for the bootloader
is not the speed value of the kernel 'console=ttyS0,9600' option
but what stty -F /dev/ttyS0 shows

# stty -F /dev/ttyS0
speed 9600 baud; line = 0;
min = 1; time = 0;
-brkint -icrnl -imaxbel iutf8
-isig -icanon -iexten -echo -echoe -echok -echoctl -echoke

Both speed values are the same in my case so all looks well.
But I don't know how things behave on other systems.
Regardless how things behave on other systems:
In general when ReaR's serial console setup automatism
does not result what is needed in a specific case,
what is needed can be explicitly specified
via the ...SERIAL_CONSOLE... config variables.

jsmeix commented at 2023-10-11 10:09:

@pcahyna
via
https://github.com/rear/rear/pull/2961/commits/478355980e80eecab90fe586f2ce7a49738c9839
I described in default.conf what it means when the user specifies
USE_SERIAL_CONSOLE="yes" or USE_SERIAL_CONSOLE="no" and
I fixed a false description that
SERIAL_CONSOLE_DEVICE_SYSLINUX and SERIAL_CONSOLE_DEVICE_GRUB
work when USE_SERIAL_CONSOLE is not specified as 'no'
because actually SERIAL_CONSOLE_DEVICE_SYSLINUX
and SERIAL_CONSOLE_DEVICE_GRUB work only
when USE_SERIAL_CONSOLE is 'yes'.

See the code parts that are mentioned in the comment about
"auto-enable serial console support for the recovery system bootloader"
in prep/GNU/Linux/200_include_serial_console.sh in this pull request.

jsmeix commented at 2023-10-11 12:24:

@pcahyna
via
https://github.com/rear/rear/pull/2961/commits/21ab8065936998230f61412e7ed701cc2fc975de
I describe in default.conf more explicitly that

With USE_SERIAL_CONSOLE="yes" plus appropriate SERIAL_CONSOLE_DEVICE... settings
serial consoles can be specified for the recovery system kernel and bootloader
for example when there is no 'console=...' option in /proc/cmdline
or when serial consoles for the recovery system kernel and bootloader
should differ from what 'console=...' options in /proc/cmdline tell.

jsmeix commented at 2023-10-12 06:07:

What is strange is that it seems GitHub somehow
hides or removes review conversations automatically:

There was a review conversation between
@pcahyna and me about possibly confusing
"USE_SERIAL_CONSOLE is not set to 'no'"
for SERIAL_CONSOLE_DEVICES and SERIAL_CONSOLE_DEVICES_KERNEL
versus "USE_SERIAL_CONSOLE is 'yes'" for
SERIAL_CONSOLE_DEVICE_SYSLINUX and SERIAL_CONSOLE_DEVICE_GRUB.

But now this conversation seems to be somehow lost?
At least I cannot find it anymore in
https://github.com/rear/rear/pull/2961/files
(it is not one of the two that are marked as resolved).

Ah!
Now I noticed that "Conversations [v]" dropdown menu on
https://github.com/rear/rear/pull/2961/files
There a missing conversation is shown as "unresolved"
which I set to "resolved" right now (hopefully my latest
enhanced and fixed description in default.conf is OK now).

But the above mentioned conversation about "possibly confusing ..."
is not shown by this "Conversations [v]" dropdown menu
so currently this conversation still seems to be lost.

At least on first glance it looks as if
https://github.com/isaacs/github/issues/1786
is somehow related (but I did not actively rebase).

jsmeix commented at 2023-10-12 08:37:

@pcahyna
if you kept the GitHub notifications mails
of our above mentioned conversation about
"possibly confusing ..."
then please post our conversation comments directly here
(i.e. here in the pull request "Conversation" tab
but not as code comments via the "Files changed" tab)
so we still have that conversation.

pcahyna commented at 2023-10-13 10:31:

@jsmeix sorry for the late reply, I was offline yesterday.

Is the missing conversation here? https://github.com/rear/rear/commit/478355980e80eecab90fe586f2ce7a49738c9839

If so, it is because I commented on a specific commit and not in the PR (but, the commit is part of the PR). Not very intuitive though. I supposed that comments on one commit in a PR would show up in the general PR review/comment thread as well, but apparently that's not the case.

pcahyna commented at 2023-10-13 10:34:

And I suspect that the comment thread will be lost if you rebase the branch and thus lose the original commit.

pcahyna commented at 2023-10-13 10:39:

@pcahyna if you kept the GitHub notifications mails of our above mentioned conversation about "possibly confusing ..." then please post our conversation comments directly here (i.e. here in the pull request "Conversation" tab but not as code comments via the "Files changed" tab) so we still have that conversation.

I just tried to do this via the "Files chaged" tab anyway. Let's see if that's enough to preserve the thread.

jsmeix commented at 2023-10-13 11:29:

@pcahyna
thank you!
Yes
https://github.com/rear/rear/commit/478355980e80eecab90fe586f2ce7a49738c9839
still contains the conversation that I missed.

jsmeix commented at 2023-10-16 11:56:

@pcahyna
in
https://github.com/rear/rear/pull/2961#discussion_r1358300539
you wrote

I fully understand the motivation of not wanting
to change behavior except when it is needed
to fix the bug that motivated this change.
For the future (ReaR 3.0) though, I would like
to think again about the semantics and possibly change it.

Do I understand you right that you do not object
when I merge this pull request in its current state?

To make things clear to me clould you please
explicitly "Approve" or explicitly "Request Changes"
at "Finish your review" in a pull request review?

jsmeix commented at 2023-10-16 12:09:

@pcahyna
in your
https://github.com/rear/rear/pull/2961#discussion_r1358300539
you wrote

In particular, I don't like that there are conditions stated as
  "provided USE_SERIAL_CONSOLE is not set to 'no':"
while other conditions are stated as:
  "provided USE_SERIAL_CONSOLE is 'yes':"
which makes harder to understand the impact of settings
when reading default.conf (the same empty value has a
different consequence in one place than in another place).
Would it make sense to change it to
  "provided USE_SERIAL_CONSOLE is 'yes':"
everywhere (and also in the code) in a future version,
for consistency and simplicity?

Assume default.conf would read

# Which serial devices should be used for serial consoles
# provided USE_SERIAL_CONSOLE is 'yes':
# E.g. SERIAL_CONSOLE_DEVICES="/dev/ttyS0 /dev/ttyS1"
# By default (when empty) the devices of the 'console=...' options in /proc/cmdline
# that exist as /dev/ttyS* or /dev/hvsi* character device nodes are used
# (which excludes /dev/tty0 when there is 'console=tty0' in /proc/cmdline):
SERIAL_CONSOLE_DEVICES=
#
# Serial consoles for the kernel of the recovery system
# provided USE_SERIAL_CONSOLE is 'yes':
# By default (when SERIAL_CONSOLE_DEVICES_KERNEL and SERIAL_CONSOLE_DEVICES are empty)
# serial consoles get enabled for the recovery system kernel via COPY_KERNEL_PARAMETERS
# for all 'console=...' options that are found in /proc/cmdline.
# SERIAL_CONSOLE_DEVICES_KERNEL can be device nodes like "/dev/ttyS0 /dev/ttyS1"
# or 'console=...' kernel parameters like "console=ttyS1,9600 console=tty0" or both like
# SERIAL_CONSOLE_DEVICES_KERNEL="/dev/ttyS0 console=ttyS1,9600 console=tty0"
# When SERIAL_CONSOLE_DEVICES_KERNEL is empty but SERIAL_CONSOLE_DEVICES is specified
# then the specified SERIAL_CONSOLE_DEVICES are used for the kernel:
SERIAL_CONSOLE_DEVICES_KERNEL=

Now it contradicts because "is 'yes'"
contradicts "By default (when [...] empty)".

Some setting(s) must match the empty default case.

I still think the logic in the code is "perfectly right", cf.
https://github.com/rear/rear/pull/2961#discussion_r1358150770

I think what could be further improved is how default.conf
explains this "pefrectly right" logic sufficiently well
to the user.

pcahyna commented at 2023-10-16 12:59:

@jsmeix

@pcahyna in #2961 (comment) you wrote

I fully understand the motivation of not wanting
to change behavior except when it is needed
to fix the bug that motivated this change.
For the future (ReaR 3.0) though, I would like
to think again about the semantics and possibly change it.

Do I understand you right that you do not object when I merge this pull request in its current state?

I don't object to this part (if the semantics does not change from the previous state). Let's not discuss this part further here, because I need to focus on other parts of the PR (I have not yet reviewed all of it, sorry for the slow progress).

I need to think more about this discussion after the PR is merged, and perhaps open a separate issue for it.

jsmeix commented at 2023-10-17 09:20:

@pcahyna
please tell me when you finished
your review of the other parts of this PR
because then I would like to merge it
provided you do not "Request Changes".

pcahyna commented at 2023-10-18 08:39:

GitHub has hidden this conversation https://github.com/rear/rear/pull/2961#discussion_r1350293254 because the code has changed, but it is still relevant. Let me copy the last relevant comment:

This is a reasonable behavior. It is hard to see from the rules described in default.conf comments why it works this, though.

By default (when empty) the devices of the 'console=...' options in /proc/cmdline
that exist as /dev/ttyS* or /dev/hvsi* character device nodes are used
(which excludes /dev/tty0 when there is 'console=tty0' in /proc/cmdline):

this sounds that the example above (console=ttyS1,9600 console=tty0) SERIAL_CONSOLE_DEVICES gets set to /dev/ttyS1 only and /dev/tty0 is excluded.

When SERIAL_CONSOLE_DEVICES_KERNEL is empty but SERIAL_CONSOLE_DEVICES is specified
then the specified SERIAL_CONSOLE_DEVICES are used for the kernel:

this sounds that only /dev/ttyS1 will then be used as the console= device, not /dev/tty0. But actually, according to your comment above, both will be used, which seems to contradict the description.

Maybe the lines

By default (when empty) the devices of the 'console=...' options in /proc/cmdline
that exist as /dev/ttyS* or /dev/hvsi* character device nodes are used
(which excludes /dev/tty0 when there is 'console=tty0' in /proc/cmdline):

apply only to SERIAL_CONSOLE_DEVICE_SYSLINUX and SERIAL_CONSOLE_DEVICE_GRUB, but not to SERIAL_CONSOLE_DEVICE itself. (i.e. only console= arguments matching /dev/ttyS* or /dev/hvsi* will be used for the bootloader, but all arguments will be used for the kernel)? In that case, the description should be moved to the bootloader part?

pcahyna commented at 2023-10-18 10:14:

Maybe the lines

By default (when empty) the devices of the 'console=...' options in /proc/cmdline
that exist as /dev/ttyS* or /dev/hvsi* character device nodes are used
(which excludes /dev/tty0 when there is 'console=tty0' in /proc/cmdline):

apply only to SERIAL_CONSOLE_DEVICE_SYSLINUX and SERIAL_CONSOLE_DEVICE_GRUB, but not to SERIAL_CONSOLE_DEVICE itself. (i.e. only console= arguments matching /dev/ttyS* or /dev/hvsi* will be used for the bootloader, but all arguments will be used for the kernel)? In that case, the description should be moved to the bootloader part?

I have now checked the code and this seems to be indeed the case. See my https://github.com/rear/rear/pull/2961/files#r1363613310 suggestion.

jsmeix commented at 2023-10-18 10:53:

Argh!
I don't see your
https://github.com/rear/rear/pull/2961/files#r1363613310
suggestion :-(

I was working on my
https://github.com/rear/rear/pull/2961/commits/6d410a2fda92d763bc2f27736ae37bd5b144460a
that I committed right now.

It seems because of my commit GitHub (again)
deleted user data (i.e. your suggestion).
If this is true it would be really bad
because user data has to be sacrosanct for programs
(i.e. in this case what runs on the GitHub server).

pcahyna commented at 2023-10-18 11:03:

Argh! I don't see your https://github.com/rear/rear/pull/2961/files#r1363613310 suggestion :-(

I was working on my 6d410a2 that I committed right now.

It seems because of my commit GitHub (again) deleted user data (i.e. your suggestion). If this is true it would be really bad because user data has to be sacrosanct for programs (i.e. in this case what runs on the GitHub server).

I don't see the comment either myself. That's because your https://github.com/rear/rear/commit/6d410a2fda92d763bc2f27736ae37bd5b144460a deleted the code lines where the comment was anchored.

Apparently the comment is preserved in the discussion thread, here https://github.com/rear/rear/pull/2961#discussion_r1363613310 , can you see it?

jsmeix commented at 2023-10-18 11:10:

Yes, I can see it in
https://github.com/rear/rear/pull/2961#discussion_r1363613310
and I think I resolved the description issues properly now via
https://github.com/rear/rear/pull/2961/commits/6d410a2fda92d763bc2f27736ae37bd5b144460a

jsmeix commented at 2023-10-18 11:20:

For ReaR 3.0 we may (provided we agree on it)
remove the SERIAL_CONSOLE_DEVICES config variable
because it is not needed for the default cases
(i.e. where /proc/cmdline tells correctly about consoles)
and in special cases it makes all more complicated
(our code and for the user to understand how things work)
without actual benefit - for example

SERIAL_CONSOLE_DEVICES="/dev/ttyS0 /dev/tty0"

is less clear what that actually means
compared to the equivalent but explicit

SERIAL_CONSOLE_DEVICES_KERNEL="/dev/ttyS0 /dev/tty0"
SERIAL_CONSOLE_DEVICE_SYSLINUX="/dev/ttyS0"
SERIAL_CONSOLE_DEVICE_GRUB="/dev/ttyS0"

pcahyna commented at 2023-10-19 11:11:

@jsmeix

I believe that at least for the kernel part, we should rename it to something like just CONSOLE_DEVICES_KERNEL, because there is nothing serial port specific (/dev/tty0 is not a serial port).

Regarding the unified SERIAL_CONSOLE_DEVICES, I suppose the motivation was to govern all places with a single variable, which is understandable from the usability point of view. One usually wants to have the system console on the same console device as the bootloader console. It is not very useful to see bootloader messages but then not the system output or vice-versa. Implementing this properly is difficult though and I doubt that it is done properly now. For example, on the kernel command line, the last console= argument determines what will be used as the system console, but for the bootloader, we use the first device in the list, which is inconsistent. (Actually, it is even more complicated, see
https://www.kernel.org/doc/html/latest/admin-guide/serial-console.html )

I would like to at least unify the setting between bootloaders, i.e. have a single SERIAL_CONSOLE_DEVICE_BOOTLOADER instead of SERIAL_CONSOLE_DEVICE_SYSLINUX and SERIAL_CONSOLE_DEVICE_GRUB. One should not worry too much about what bootloader to use. But since one can specify direct bootloader commands there, even this will be difficult.

I also suspect that the current automatism with default values does not do the right thing with respect to the bootloader, e.g. if console settings are console=ttyS1,9600 console=tty0, serial console will be enabled and the bootloader will use ttyS1 as the console, but the kernel uses and will use tty0 (because it is the last). I suppose it has been this way before your changes though, so it it should not be fixed in this PR, even if it is really the case.

pcahyna commented at 2023-10-19 11:33:

@jsmeix

I also suspect that the current automatism with default values does not do the right thing with respect to the bootloader, e.g. if console settings are console=ttyS1,9600 console=tty0, serial console will be enabled and the bootloader will use ttyS1 as the console, but the kernel uses and will use tty0 (because it is the last). I suppose it has been this way before your changes though, so it it should not be fixed in this PR, even if it is really the case.

now I am thinking, is it true what I wrote? The previous code seems to enable USE_SERIAL_CONSOLE only if there is a serial device on the system and the serial device seems to be usable (via get_serial_device_speed (), while the new code enables USE_SERIAL_CONSOLE whenever it sees a console= kernel argument, regardless of whether it points to a serial device or whether the device is usable.

OTOH, it probably does not make any difference for the bootloader, because the bootloader code in bootloader-functions.sh takes care of skipping serial devices that are not usable, and without any usable serial device, the bootloader code will behave the same way regardless of whether USE_SERIAL_CONSOLE is true or false.

jsmeix commented at 2023-10-19 11:55:

@pcahyna
but I don't want to do backward incompatible changes
like changed config variable names or their meaning
in this pull request, cf. "What did not change"
in the initial description of this pull request
https://github.com/rear/rear/pull/2961#issue-1643830703

So if the current state of this pull request
is acceptable for you I would like to merge it,
preferably tomorrow afternoon (unless you object).

pcahyna commented at 2023-10-19 11:58:

@jsmeix I agree - my first comment was a reaction to

For ReaR 3.0 we may (provided we agree on it)
remove the SERIAL_CONSOLE_DEVICES config variable
because it is not needed for the default cases

jsmeix commented at 2023-10-19 12:00:

I made two separated SERIAL_CONSOLE_DEVICE_SYSLINUX
and SERIAL_CONSOLE_DEVICE_GRUB because of my generic
"final power to the user" principle and as a positive side-effect
the user can specify direct bootloader commands there if needed
and as needed for each bootloader so the user could use one same
settings (i.e. one same local.conf) on different systems
regardless what bootloader he uses on a particular system.
Users with many systems (e.h hundreds of servers) may like that.

jsmeix commented at 2023-10-19 12:57:

@rear/contributors
unless there are objections
I would like to merge it
tomorrow (Friday) afternoon.

pcahyna commented at 2023-10-19 17:48:

I am thinking whether this behavior change for bootloader

The previous code seems to enable USE_SERIAL_CONSOLE only if there is a serial device on the system and the serial device seems to be usable (via get_serial_device_speed (), while the new code enables USE_SERIAL_CONSOLE whenever it sees a console= kernel argument, regardless of whether it points to a serial device or whether the device is usable.

has any practical impact. Right now I think the only case where it would make a difference is a headless machine (without any VGA card and thus not having /dev/tty0) that would have no console= kernel argument and would choose /dev/ttyS0 as its console automatically. In that case, the automatism would not set USE_SERIAL_CONSOLE to yes and would not enable serial console in bootloader, while the previous version would. I don't currently have such a setup to test it, though.

jsmeix commented at 2023-10-20 08:41:

Yes, I see in
https://www.kernel.org/doc/html/latest/admin-guide/serial-console.html

If no console device is specified, the first device
found capable of acting as a system console will be used.
At this time, the system first looks for a VGA card
and then for a serial port.
So if you don't have a VGA card in your system
the first serial port will automatically become the console.

So we may further enhance the autodetection (but not in this PR)
what is actually used as console(s) by the kernel on the original system
by also inspecting the kernel messages ('dmesg')
and perhaps other things (e.g. /proc/consoles).

For example on my homeoffice laptop I have

# cat /proc/cmdline | grep -i console
[no output]

# dmesg | grep -i console
[    0.000000] Console: colour VGA+ 80x25
[    0.000000] printk: console [tty0] enabled
[    4.412715] systemd[1]: Starting Setup Virtual Console...
[    5.245630] i915 0000:00:02.0: vgaarb: deactivate vga console
[    5.247161] Console: switching to colour dummy device 80x25
[    7.441732] Console: switching to colour frame buffer device 200x56
[   29.629040] systemd[1]: Condition check resulted in Dispatch Password Requests to Console Directory Watch being skipped.

Unfortunately it seems to be impossible to find out
which more or less "real" device(s) belong to /dev/console
(nowadays /dev/console is no longer a link but a character device)
cf.
https://www.kernel.org/doc/html/latest/admin-guide/devices.html

Virtual consoles and the console device
...
The console device, /dev/console, is the device
to which system messages should be sent, and
on which logins should be permitted in single-user mode.
Starting with Linux 2.1.71, /dev/console is managed by the kernel;
for previous versions it should be a symbolic link
to either /dev/tty0 ...

Perhaps /proc/consoles is nowadays the best source of information
to tell what device(s) are actually used as console(s)?
For example on my homeoffice laptop I have

# cat /proc/consoles 
tty0                 -WU (EC p  )    4:2

https://unix.stackexchange.com/questions/485156/what-is-dev-console-used-for
states

The “list of consoles” is indeed the list of consoles
defined by the console= boot parameters
(or the default console, if there are none).
You can see the consoles defined in this way
by looking at /proc/consoles.
/dev/console does indeed provide access [to the last of these]
(https://www.kernel.org/doc/html/latest/admin-guide/serial-console.html)

where
https://www.kernel.org/doc/html/latest/admin-guide/serial-console.html
reads

You can specify multiple console= options on the kernel command line.
The behavior is well defined when each device type is mentioned only once.
In this case, the output will appear on all requested consoles.
And the last device will be used when you open /dev/console.
So, for example:
  console=ttyS1,9600 console=tty0
defines that opening /dev/console will get you
the current foreground virtual console,
and kernel messages will appear on both the VGA console
and the 2nd serial port (ttyS1 or COM2) at 9600 baud.

Now there is the question if /proc/consoles shows
the 'console' kernel options as specified
or if /proc/consoles shows what the kernel actually
uses as consoles?
Cf.
https://github.com/rear/rear/pull/2961#issuecomment-1488329126

... kernel 'console' command line options:

... console=ttyS1,9600n8 ... console=ttyS3 ... console=tty0 ...

# dmesg | grep console
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root console=ttyS1,9600n8 resume=/dev/system/swap console=ttyS3 crashkernel=203M,high console=tty0 mitigations=off
[    0.122561] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-5.3.18-57-default root=/dev/mapper/system-root console=ttyS1,9600n8 resume=/dev/system/swap console=ttyS3 crashkernel=203M,high console=tty0 mitigations=off
[    0.146431] printk: console [tty0] enabled
[    0.219572] printk: console [ttyS1] enabled
[    5.980855] qxl 0000:00:02.0: vgaarb: deactivate vga console

By the way:
https://www.kernel.org/doc/html/latest/admin-guide/serial-console.html
seems rather old because it talks about LILO
and at the bottom there is "11-Jun-2000"
so perhaps meanwhile some details could be different?

pcahyna commented at 2023-10-20 10:28:

I believe the best place to look at is /sys/class/tty/console/active. See https://github.com/rear/rear/pull/2749#issuecomment-1196760461

jsmeix commented at 2023-10-20 11:25:

Via
https://github.com/rear/rear/pull/2961/commits/75a23e7942a0d5e4c8f1f6012d7c043fc2838db0
I additionally describe now in default.conf
when manual console setup is needed
for the example of a headless machine
(without VGA card but with /dev/ttyS0), cf.
https://github.com/rear/rear/pull/2961#issuecomment-1771450364

jsmeix commented at 2023-10-20 11:25:

I will merge this PR soon...

pcahyna commented at 2023-10-20 16:37:

Thank you @jsmeix a lot for this enhancement, I think that it can greatly improve the user experience in some situations. I now also understand much better what is the intent of the code.

[Export of Github issue for rear/rear.]