zipl: add BootLoaderSpec support

[martinezjavier]

The BootLoaderSpec (BLS) defines a file format for boot configurations, so bootloaders can parse these files and create their boot menu entries by using the information provided by them.

From Fedora 28 there will be an option to use BootLoaderSpec snippets to update the bootloader menu entries. This still won't be the default, but the idea is to use it as default once all the architectures are supported. So this pull-request adds BLS support to the zipl user-space tool.

This will allow to configure the boot items as drop-in files in a directory instead of having to parse and modify the zipl.conf file.

If the /boot/loader/entries exists and there are BLS files there, then these are parsed and configuration sections are added without the need to have these in a zipl.conf file. A different BLS directory can be specified from the command line using the --blsdir option.

It can be tested using a zipl.conf file with no boot configuration sections:

[defaultboot]
defaultauto
prompt=1
timeout=5
default=4.15.9-300.fc27.s390x
target=/boot

and BLS entries, for example /boot/loader/entries/8ee920ff9a434127928b5c04a594d3d5-4.15.9-300.fc27.s390x.conf:

title Fedora (4.15.9-300.fc27.s390x) 27 (Server Edition)
version 4.15.9-300.fc27.s390x
linux /boot/vmlinuz-4.15.9-300.fc27.s390x
initrd /boot/initramfs-4.15.9-300.fc27.s390x.img
options root=/dev/mapper/fedora-root rd.lvm.lv=fedora/root rd.lvm.lv=fedora/swap
id fedora-20170308173431-4.15.9-300.fc27.s390x

[hoeppnerj]

@stefan-haberland @oberpar Can you have a look into it?

[martinezjavier]

@stefan-haberland I've pushed a new version of the patches that addressed the issues you pointed out.

Changes since v1:

• Drop the util_proc_is_mountpoint() function and don't check if /boot is a mount point.
• Add the absolute path to the kernel and initramfs images in the BLS fragments instead of a relative path to the root of the boot partition.
• Change zipl-switch-to-blscfg script to generate BLS fragments from the IPL sections in the zipl.conf file instead of searching for installed kernels in /boot.
• Print the used BLS config files in the standard output to make it easier to debug if a error occurs.

[martinezjavier]

@stefan-haberland thanks a lot for the detailed review and testing!

[root@s83lp74 ~]# ./zipl -b /etc Using config file '/etc/zipl.conf' Using BLS config file '/etc/zipl.conf'

Right, the problem is that since both zipl.conf and the BLS use a .conf suffix, it believes that's a BLS file.

I've improved scan_bls() and scan_bls_field() functions so they are more robust and only parse BLS files that are in the correct format.

So executing your example command should lead to:

# ./zipl -b /etc
Using config file '/etc/zipl.conf'
Using BLS config file '/etc/zipl.conf'
Error: BLS parsing '/etc': Incorrect BLS field in config file /etc/zipl.conf

[martinezjavier]

If there is an error during processing and it happens that the script is called again the old backup file is overwritten with the new (mostly empty) zipl.conf... leading to a loss of the first working zipl.conf. Maybe with an error in the script you can roll back the changes to the original zipl.conf.

Indeed, I've fixed the issues that lead to this but now also check if there's an old backup file and exit if that's the case to avoid overriding an old backup file:

$ ./scripts/zipl-switch-to-blscfg --bls-directory ./blsdir --config-file ./RHEL-7.5.conf 
Backup file ./RHEL-7.5.conf.bak already exists

[martinezjavier]

If /etc/machine-id is missing the script does work but the conf files start with a -. Maybe we can use a default value here or remove the -.

Right, I made this optional. So if there isn't an machine-id file it would lead to the following:

$ tree ./blsdir
./blsdir
├── 0-rescue.conf
├── 3.10.0+.conf
└── vmlinuz.conf

[martinezjavier]

With the following config from a RHEL7.5 system I get some errors and the last RHEL-7.5.conf file is mostly empty...

The script (wrongly) assumed that the kernel images file names would always be vmlinuz-X so I now take into account for images without that prefix.

$ cat ./blsdir/e5c131dfee3249cbb9891c2641d8e350-vmlinuz.conf 
version vmlinuz
linux /boot/vmlinuz
options cio_ignore=all,!condev vconsole.keymap=us vconsole.font=latarcyrheb-sun16 LANG=en_US.UTF-8 crashkernel=1G-:128M scsi_mod.scsi_logging_level=4605 printk.time=1 zfcp.dbfsize=100 root=/dev/disk/by-path/ccw-0.0.9580-part1 rd_DASD=0.0.9580
initrd /boot/initramfs

[martinezjavier]

Changes since v2:

• Fixed issues pointed out by @stefan-haberland.

[stefan-haberland]

Thanks for the update. I noticed that you change the name of the section to the image name. This is unfavorable because we might have several sections with the same image but different parameter lines for example. So I would keep the section name as the name of the entry. It might also be a good idea to keep the default= line in the [defaultboot] section in zipl to have the old default section still the default entry after changing to BLS.

[martinezjavier]

@stefan-haberland yes, that's because the idea of BLS is to be able to update the sections just by adding and removing BLS fragments in the BLS directory, without the need to update the zipl.conf file.

But maintaining the default= line will defeat the purpose since it means that the zipl.conf file would need to be updated anyways to set a new default every time that a kernel is added/removed.

Instead for BLS the policy is to always default to the latest kernel and these are sorted using strverscmp(3), which is the reason why the section names were changed to the kernel version so the BLS entries are sorted correctly.

Maybe we could do ${MACHINE_ID}-${version}-${section_name} to allow the same kernel version with different parameters?

[martinezjavier]

@stefan-haberland Ok, I've given some thought to your suggestions. What do you think about the following:

• Add two more options to the script to control if the default is left or not and if the section names will be used as BLS file names.
• Make the default what you suggest (maintain default= and use section names as filenames) but allow to ignore the default= and use ${MACHINE_ID}-${version} as BLS file name to be consistent with what other bootloaders do.

Does that work for you?

[stefan-haberland]

This sounds good to me. Thank you.

One thing that I saw in the man page... I think we should remove the

.SH AUTHOR
Linux on System z development <linux390@de.ibm.com>

lines but I need to talk to a legal guy here regarding this. I am on vacation for one week for now. So I will clarify this next week. Hope this is OK.

[martinezjavier]

That's Ok, enjoy your vacation.

And yes, that's something I carried by copy & pasting from another man page.

Thanks again for all your help!

[martinezjavier]

@stefan-haberland I'll just remove the Author section from the man page. I see that only one man page from the scripts has it anyways.

[martinezjavier]

Changes since v3:

• Add --ignore-default and --use-version-name options.
• By default copy the default option from the zipl.conf file and use the section name as BLS filename.
• Remove the author section from the man page.

[martinezjavier]

@stefan-haberland I did the changes that you suggested in the last version, please let me know if there's anything else to fix.

[martinezjavier]

@sharkcz sorry about that, I just pushed a fix.

[stefan-haberland]

Hi,

thanks for the update. Unfortunately the script still does not use the old section name as "version" name.

So with a config that uses the same kernel image in two different sections we get an error that the section is already specified.

Example:

[defaultboot] default=RHEL7.5 target=/boot

[RHEL7.5] target=/boot image=/boot/vmlinuz parameters="some kernel parm" ramdisk=/boot/initramfs

[RHEL7.5-v2] target=/boot image=/boot/vmlinuz parameters="some important different kernel parm" ramdisk=/boot/initramfs

[martinezjavier]

@stefan-haberland you are right. I changed the BLS filename but forgot about the version field, sorry for missing that.

Could you please test again the latest version I pushed?

[stefan-haberland]

Looks much better. Thanks. I will do some more testing.

[martinezjavier]

@stefan-haberland perfect, thanks a lot for your help!

[stefan-haberland]

Again, thanks for the work. I did some testing and code review and did not find anything to add. If no one else has any objections I will include the pull request.

[martinezjavier]

@stefan-haberland great, and thanks for all your feedback and suggestions. I really appreciate it.