#2749 PR merged: Add console=tty0 to cmdline only if no real serial device was found

Labels: bug, fixed / solved / done

lzaoral opened issue at 2022-01-26 12:14:

Pull Request Details:
  • Type: Bug Fix

  • Impact: Normal

  • Reference to related issue (URL): None

  • How was this pull request tested? RHEL 8.5 VM with a serial console

  • Brief description of the changes in this pull request:

Changes usr/share/rear/lib/serial-functions.sh.

Without this change console=tty0 would be added whenever the last
device returned by get_serial_console_devices was not classified
as a real serial device even though the previous ones were.

Thus, the cmdline on a machine with ttyS[0-3] where only ttyS0
is real was changed to

console=ttyS0,115200 console=tty0

In such case, only kernel messages would appear on ttyS0 [1].

[1] https://www.kernel.org/doc/html/latest/admin-guide/serial-console.html

jsmeix commented at 2022-01-26 13:49:

@rear/contributors
I am not a serial console user (in particular not on my homeoffice laptop ;-)
so I would appreciate a review by one of you as time permits
ideally by someonw who is a serial console user.

jsmeix commented at 2022-01-31 07:09:

@rear/contributors
if there are no objections I would like to merge it tomorrow afternoon

pcahyna commented at 2022-01-31 09:07:

I am curious (I have never dived into the console code) why does ReaR discover console devices and constructs the console= kernel parameters itself. Wouldn't it be better to simply copy all the console= kernel arguments from the running system? One could assume that if they were good enough for the running system, they will be good enough also for the rescue system (I suppose one will typically use the same console setup for interacting with the rescue system as one usually uses for administration of the running system).

jsmeix commented at 2022-01-31 10:36:

Because I am not a serial console user I can only tell
what I see from plain looking at the code pieces where I look at
but I can not tell how all code in ReaR actually works in the end.

As far as I see the kernel commad line the recovery system is initially set via
rescue/GNU/Linux/290_kernel_cmdline.sh
which only uses what there is in COPY_KERNEL_PARAMETERS
so via COPY_KERNEL_PARAMETERS+=( console )
plus likely also USE_SERIAL_CONSOLE="no" to skip the automatisms
it should be possible for the user to copy all console= kernel arguments
from his running system.

I don't know why 'console' is not by default in COPY_KERNEL_PARAMETERS.
I think this is because serial console support is implemented on its own
based on USE_SERIAL_CONSOLE
which is set to "yes" in prep/GNU/Linux/200_include_serial_console.sh
when there is at least one real serial device found
plus the other SERIAL_CONSOLE_... config variables.

See also https://github.com/rear/rear/pull/2699

jsmeix commented at 2022-02-02 12:21:

@lzaoral
thank you for reporting the issue and for your fix!

jsmeix commented at 2022-07-27 08:36:

It seems this change causes a minor regression in some cases:
https://github.com/rear/rear/issues/2843

As far as I see it (as not a serial console user)
the change here is right (it fixes wrong working code)
but that wrong working code had "by accident" resulted
that console=tty0 was appended to the kernel commandline
which had made things work for my use case "by accident", cf.
https://github.com/rear/rear/issues/2843#issuecomment-1196337765

jsmeix commented at 2022-07-27 12:14:

Meanwhile I think the proposal from @pcahyna
https://github.com/rear/rear/pull/2749#issuecomment-1025517957

simply copy all the console= kernel arguments from the running system

is the right way for an automated serial console setup.

In particular because the primary use case for ReaR
is to recreate a system same as it was before
so the default assumption is that the replacement system
is in the same environment as the original system was
(i.e. same networking environment, same attached devices
like disks, (VGA) monitors, keyboards, serial consoles, ...)
so using the 'console' kernel settings from the original system
should be the right default behaviour for the recovery system.

In general:
Whether or not there is a 'console' setting for the
recovery system bootloader and the recovery system kernel
should not depend on whether or not there is a serial device.
Instead it should depend on whether or not a serial console
is actually used on the original system.

I think in
https://github.com/rear/rear/pull/2749#issuecomment-1025517957
"ReaR discover console devices" is not exactly right.
I think - as far as I understand the code in
prep/GNU/Linux/200_include_serial_console.sh

# If possible auto-enable serial console when not specified:
if [[ -z "$USE_SERIAL_CONSOLE" ]] ; then
    local devnode speed=""
    for devnode in $( get_serial_console_devices ) ; do
        # Enable serial console when there is at least one real serial device:
        if speed=$( get_serial_device_speed $devnode ) ; then
            USE_SERIAL_CONSOLE="yes"
            break
        fi
    done
fi

currently ReaR does not discover console devices
(i.e. serial device nodes that are used as serial consoles)
but it discovers serial device nodes that are real serial devices
(regardless for what those real serial devices are used).
So (as far as I understand it) the current code could
auto-enable serial console for real serial devices
that are not used as serial consoles.
This could auto-enable output of boot messages and so on
via serial devices that are not used as serial consoles
which is plain wrong (perhaps even insecure or harmful).

lzaoral commented at 2022-07-27 13:25:

There are two ways that I know of to get the device that /dev/console actually points to:

  1. Parse /sys/class/tty/console/active and get the last entry. [1]
  2. Parse /proc/consoles and select a device with the C flag. [2]

Some useful info about the console kernel cmdline option can also be found here [3].

[1] https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-tty
[2] https://www.kernel.org/doc/html/latest/filesystems/proc.html?highlight=console#proc-consoles
[3] https://www.kernel.org/doc/html/latest/admin-guide/serial-console.html?highlight=console#linux-serial-console

lzaoral commented at 2022-07-27 13:28:

Also, both methods can be used to list all devices that are used as (serial) console devices at that moment.

hpannenb commented at 2022-07-27 14:26:

My 2 cents: To me it looks like it depends on the kernel and its command line only. E.g. the last console parameters in the kernel command line is used for /dev/console and if no console parameters are available /dev/tty0 is automatically used by the kernel.

So if one requires a specific setup this should either be configured in the kernel command line or with means of different CONSOLE variables in the ReaR configuration. From a first look an auto detection does not seem to add a benefit here.

jsmeix commented at 2022-07-28 08:41:

Regarding the proposal from @pcahyna
https://github.com/rear/rear/pull/2749#issuecomment-1025517957

Wouldn't it be better to simply copy
all the console= kernel arguments
from the running system?

We have COPY_KERNEL_PARAMETERS
but currently the code in
rescue/GNU/Linux/290_kernel_cmdline.sh
works only for one (the first one) kernel parameter
for a particular kernel option name (like 'console').

So with COPY_KERNEL_PARAMETERS+=( console )
when on the original system /proc/cmdline contains

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

only console=ttyS0,9600 gets added
to KERNEL_CMDLINE for the recovery system.

"rear -D mkrescue" log messages for this case (excepts):

++ LogPrint 'Adding console=ttyS0,9600 to KERNEL_CMDLINE'
...
++ Log 'Current kernel option [console=tty0] supperseeded by [console=ttyS0,9600] in your rear configuration: (KERNEL_CMDLINE)'

The COPY_KERNEL_PARAMETERS description in default.conf
does not match what the implementation does, in particular

If the key-value kernel parameter is already set
in KERNEL_CMDLINE variable it will always superseed
the one detected on the current system

is wrong because actually if such a kernel parameter key
is already specified in the KERNEL_CMDLINE variable
the already specified setting will supersede the one
detected on the current system (i.e. only the key matters,
not the key-value) which makes sense because when some

KERNEL_CMDLINE="option=value"

is specified a option=different_value in /proc/cmdline
should be superseded by what is specified in KERNEL_CMDLINE.

jsmeix commented at 2022-07-28 13:26:

As far as I could simulate it with
https://github.com/rear/rear/pull/2844

COPY_KERNEL_PARAMETERS+=( console )

seems to work - i.e. when /proc/cmdline contains e.g.

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

then KERNEL_CMDLINE will become

... console=ttyS0,9600 console=tty0

[Export of Github issue for rear/rear.]