Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

biosdevname: actor only enable biosdevname for current kernel version #970

Open
hjensas opened this issue Sep 27, 2022 · 3 comments
Open
Labels
bug Something isn't working

Comments

@hjensas
Copy link

hjensas commented Sep 27, 2022

Actual behavior
The actor which enables biosdevname only does so for the currently running kernel.
When the kernel is updated and the system boots the new kernel, biosdevname won't be enabled. Configuration files will reference the biosdevname consistent interface names, but the systemd consistent interface nameing scheme is active.

To Reproduce

  1. Run Leapp (RHEL7->RHEL8) on a DELL box which does not explicitly disable biosdevname.
  2. Update the kernel after Leapp completed.

Expected behavior
Leapp should add the kernel argument to enable biosdevname to the defaults, so that kernel update can happen without the side effect of disabling biosdevname post Leapp upgrade.

System information:

  • OS and version: (e.g. Fedora 29 or $ cat /etc/system-release)
Red Hat Enterprise Linux release 8.2 (Ootpa)
leapp-repository-deps-el8-5.0.0-100.202004161145Z.de6626f.master.el8.noarch

Attach (or provide link to) log files if applicable (optional - may contain confidential information):

 leapp/leapp-upgrade.log:2022-01-26 15:38:06.286 INFO     PID: 7786 leapp.workflow.FactsCollection.biosdevname: Biosdevname naming scheme in use, explicitely enabling biosdevname on RHEL-8

Additional context
This actor applies the kernel command line option with:

  cmd = ['grubby', '--update-kernel=/boot/vmlinuz-{}'.format(kernel_version), '--args="{}={}"'.format(arg.key, arg.value)]

When you run grubby with one specific kernel, it will only add the argument to that specific kernel version.
Instead, it shoule use --update-kernel=ALL to set it for all kernels See manual page GRUBBY(8).

       --args=kernel-args
              When  a  new  kernel  is added, this specifies the command line arguments which should be passed to the kernel by default (note they are merged with the arguments of the default entry if --copy-default is used).  When
              --update-kernel is used, this specifies new arguments to add to the argument list. Multiple, space separated arguments may be used. If an argument already exists the new value replaces the old values. The root= kernel
              argument gets special handling if the configuration file has special handling for specifying the root filesystem.

       --update-kernel=kernel-path
              The  entries  for  kernels matching kernel-path are updated. Currently the only items that can be updated is the kernel argument list, which is modified via the --args and --remove-args options. If the ALL argument is
              used the variable  GRUB_CMDLINE_LINUX in /etc/default/grub is updated with the latest kernel argument list, unless the --no-etc-grub-update option is used or the file does not exist (e.g., on s390x).

I believe the correct command to use would be:

    cmd = ['grubby', '--update-kernel=ALL', '--args="{}={}"'.format(arg.key, arg.value)]

Using ALL should ensure the option is added to GRUB_CMDLINE_LINUX.

Update: The code was refactored in commit 1c8a052 so the command examples are not exactly as above in current code.

@hjensas hjensas added the bug Something isn't working label Sep 27, 2022
@pirat89
Copy link
Member

pirat89 commented Sep 28, 2022

@hjensas Hi, thank you for the report and investigation. I agree that biosdevname=1 should be persistent - so with the installation of another kernel, biosdevname is preserved. However it may need to be handled differently instead of using the ALL value (adding explanation below). Most likely, the solution will handle the GRUB_CMDLINE_LINUX value directly. Keeping for the discussion.

We have been thinking in the past about using ALL as an argument, however that will affect also other installed kernels which is unwanted. The mentioned actor is not adding just a biosdevname argument, but also others - and also removes kernel arguments. The problem with that behaviour is that it changes kernel options for the original installed kernel, which usually it's not important, as the administrator is expected to remove the old kernel manually anyway. However, if the administrator backed up the system via LVM snapshots using boom, doing so we could harm the bootloader entry supposed to be used for the emergency rollback. Also administrators could have present additional custom bootloader entries and affecting them could be considered problematic also.

@hjensas
Copy link
Author

hjensas commented Sep 28, 2022

@pirat89 Thanks for taking a first look. I understand the problem with using 'ALL'.

I guess it may make sense to change the modify_kernel_args_in_boot_cfg method, add an update_default flag. If the flag is set True the GRUB_CMDLINE_LINUX can be updated in a separate step, not using grubby. Individual actor can then "decide" if only the specific kernel should have the option, or if it should be the default when adding new kernels as well.

@pirat89
Copy link
Member

pirat89 commented Mar 7, 2023

@oamg/developers fyi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants