[PATCH] discover/grub: Add blscfg command support to parse BootLoaderSpec files
Javier Martinez Canillas
javierm at redhat.com
Thu Mar 8 20:51:05 AEDT 2018
On 03/08/2018 06:07 AM, Samuel Mendoza-Jonas wrote:
> On Wed, 2018-03-07 at 20:43 +0100, Javier Martinez Canillas wrote:
>> 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 [0].
>>
>> This allow to configure the boot items as drop-in files in a directory
>> instead of having to parse and modify a bootloader configuration file.
>>
>> The GRUB 2 bootloader provides a blscfg command that parses these files
>> and creates menu entries using this information. Add support for it.
>>
>> [0]: https://www.freedesktop.org/wiki/Specifications/BootLoaderSpec/
>>
>> Signed-off-by: Javier Martinez Canillas <javierm at redhat.com>
>>
>> ---
>>
>> Hello,
>>
>> From Fedora 28 there will be an option to use BootLoaderSpec snippets to
>> update GRUB's boot menu entries. So I'm posting this patch to allow this
>> to also work on ppc64 machines using petitboot, instead of grub-ieee1275.
>
> Hi, thanks for thinking ahead! Is there a straightforward way for me to
Thanks for the quick review! I don't know if I made clear above, but this won't
be the default for Fedora 28. We will provide it as an optional feature and let
users enable it if they want to test, by setting a GRUB_ENABLE_BLSCFG="true" in
the /etc/default/grub config file.
We plan it to be the default though on later releases once we have the support
for all the architectures in place. For now only platforms with GRUB will work
out-of-the-box.
> test this out on Fedora 27 (looks like 28 Beta is later in March)?.
>
I think the easiest way for you to test is to create a /boot/loader/entries dir
and copy there a (or a set of) BLS config. For example, something like following
/boot/loader/entries/6c063c8e48904f2684abde8eea303f41-4.15.6-300.fc27.x86_64.conf:
title Fedora (4.15.6-300.fc27.x86_64) 27 (Twenty Seven)
linux /vmlinuz-4.15.6-300.fc27.x86_64
initrd /initramfs-4.15.6-300.fc27.x86_64.img
options root=/dev/mapper/fedora-root ro rd.lvm.lv=fedora/root rd.luks.uuid=luks-0b078909-4a1c-4a57-91b8-b9f724e86a1a rd.lvm.lv=fedora/swap rhgb quiet LANG=en_US.UTF-8
id fedora-20180214051518-4.15.6-300.fc27.x86_64.x86_64
grub_users $grub_users
grub_arg --unrestricted
grub_class kernel
And then you need a grub.cfg. It could be one that contains just the following:
blscfg
I've also attached the grub.cfg that's generated in F28 when GRUB_ENABLE_BLSCFG
is enabled, in case you want to test a more complete grub.cfg (just remember to
update the root UUID to match your block device or petitboot won't be able to
resolve the grub resource).
With those two you should be able to test this. The BLS snippets are included in
the kernel package and copied to /boot/loader/entries on kernel installation but
that's all platform independent and not interesting for you to test.
I've also included tests, and made sure that they pass with make check. So you
may also want to take a look at those for other use cases. For example, there is
also support to have the kernel command line as a GRUB variable and have defined
in grubenv. That's how we will ship, so users could change it in a single place.
> Also is BLS support in upstream GRUB? I had a quick look but it didn't
> appear so.
>
Not yet, but we plan to upstream it. The problem is that GRUB upstream isn't
really great at reviewing and taking patches in a timely manner, so a change
like this may take months to land. Will accepting it be blocked by upstream
first taking it? I'm asking to know how much I should prioritize engaging
with upstream GRUB if that's the case.
> Some quick comments below:
>
[snip]
>> +
>> +static void bls_finish(struct conf_context *conf)
>> +{
>> + struct bls_state *state = conf->parser_info;
>> + struct discover_context *dc = conf->dc;
>> + struct discover_boot_option *opt = state->opt;
>> + struct boot_option *option = opt->option;
>> + const char *root;
>> +
>> + if (!option->id || !option->name || !option->boot_args ||
>> + !state->image || !state->initrd) {
>
> Do we want to be this strict, or allow boot options to have for example
> just a kernel image, or image & initrd but not boot_args?
>
That's a very good question. I believe we want to be very strict on this and not
allow users to boot an entry that's incorrect. It's mostly defensive programming
really since as mentioned the BLS are generated at build time and shipped in the
kernel package, so they should be correct.
But users can still modify it and that's why I preferred to ignore the ones that
don't have the expected fields. I've added a dev_info below so users can know if
a BLS snippet was ignored due being wrong, by looking at the System status log.
>> + device_handler_status_dev_info(dc->handler, dc->device,
>> + _("BLS file %s is incorrect"),
>> + state->filename);
>> + return;
>> + }
>> +
>> + root = script_env_get(state->script, "root");
>> +
>> + opt->boot_image = create_grub2_resource(opt, conf->dc->device,
>> + root, state->image);
>> + opt->initrd = create_grub2_resource(opt, conf->dc->device,
>> + root, state->initrd);
>> + discover_context_add_boot_option(dc, opt);
>> +
>> + device_handler_status_dev_info(dc->handler, dc->device,
>> + _("Created menu entry from BLS file %s"),
>> + state->filename);
>> +}
>> +
>> +static int bls_filter(const struct dirent *ent __attribute__((unused)))
>
> This says ((unused)) but
>
>> +{
>> + int offset = strlen(ent->d_name) - strlen(".conf");
>
> We use it?
>
Right, sorry about that. During development I just wrote an empty function and
then I forgot to remove the unused attribute when I added the filter logic.
[snip]
>> +
>> + conf_parse_buf(conf, buf, len);
>> +
>> + talloc_free(buf);
>> + talloc_free(state);
>> + talloc_free(filename);
>> + free(bls_entries[n]);
>> +
>> + if (rc)
>> + break;
>
> This check is redundant, we will have break'd after parser_request_file()
>
Indeed. I reorganized the code a little bit and forgot to remove it. Thanks a
lot for pointing it out.
Best regards,
--
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat
-------------- next part --------------
#
# DO NOT EDIT THIS FILE
#
# It is automatically generated by grub2-mkconfig using templates
# from /etc/grub.d and settings from /etc/default/grub
#
### BEGIN /etc/grub.d/00_header ###
set pager=1
if [ -f ${config_directory}/grubenv ]; then
load_env -f ${config_directory}/grubenv
elif [ -s $prefix/grubenv ]; then
load_env
fi
if [ "${next_entry}" ] ; then
set default="${next_entry}"
set next_entry=
save_env next_entry
set boot_once=true
else
set default="${saved_entry}"
fi
if [ x"${feature_menuentry_id}" = xy ]; then
menuentry_id_option="--id"
else
menuentry_id_option=""
fi
export menuentry_id_option
if [ "${prev_saved_entry}" ]; then
set saved_entry="${prev_saved_entry}"
save_env saved_entry
set prev_saved_entry=
save_env prev_saved_entry
set boot_once=true
fi
function savedefault {
if [ -z "${boot_once}" ]; then
saved_entry="${chosen}"
save_env saved_entry
fi
}
function load_video {
if [ x$feature_all_video_module = xy ]; then
insmod all_video
else
insmod efi_gop
insmod efi_uga
insmod ieee1275_fb
insmod vbe
insmod vga
insmod video_bochs
insmod video_cirrus
fi
}
terminal_output console
if [ x$feature_timeout_style = xy ] ; then
set timeout_style=menu
set timeout=5
# Fallback normal timeout code in case the timeout_style feature is
# unavailable.
else
set timeout=5
fi
### END /etc/grub.d/00_header ###
### BEGIN /etc/grub.d/01_users ###
if [ -f ${prefix}/user.cfg ]; then
source ${prefix}/user.cfg
if [ -n "${GRUB2_PASSWORD}" ]; then
set superusers="root"
export superusers
password_pbkdf2 root ${GRUB2_PASSWORD}
fi
fi
### END /etc/grub.d/01_users ###
### BEGIN /etc/grub.d/10_linux ###
insmod part_msdos
insmod ext2
set root='hd0,gpt2'
if [ x$feature_platform_search_hint = xy ]; then
search --no-floppy --fs-uuid --set=root --hint-bios=hd0,gpt2 --hint-efi=hd0,msdos1 --hint-baremetal=ahci0,msdos1 --hint='hd0,msdos1' 0a9325bb-d94a-4833-9ba5-da902689e823
else
search --no-floppy --fs-uuid --set=root 0a9325bb-d94a-4833-9ba5-da902689e823
fi
insmod part_msdos
insmod ext2
set boot='hd0,gpt1'
if [ x$feature_platform_search_hint = xy ]; then
search --no-floppy --fs-uuid --set=boot --hint-bios=hd0,gpt1 --hint-efi=hd0,msdos1 --hint-baremetal=ahci0,msdos1 --hint='hd0,msdos1' AB8D-9B87
else
search --no-floppy --fs-uuid --set=boot AB8D-9B87
fi
blscfg
if [ -s $prefix/grubenv ]; then
load_env
fi
### END /etc/grub.d/10_linux ###
### BEGIN /etc/grub.d/20_linux_xen ###
### END /etc/grub.d/20_linux_xen ###
### BEGIN /etc/grub.d/20_ppc_terminfo ###
### END /etc/grub.d/20_ppc_terminfo ###
### BEGIN /etc/grub.d/30_os-prober ###
### END /etc/grub.d/30_os-prober ###
### BEGIN /etc/grub.d/40_custom ###
# This file provides an easy way to add custom menu entries. Simply type the
# menu entries you want to add after this comment. Be careful not to change
# the 'exec tail' line above.
### END /etc/grub.d/40_custom ###
### BEGIN /etc/grub.d/41_custom ###
if [ -f ${config_directory}/custom.cfg ]; then
source ${config_directory}/custom.cfg
elif [ -z "${config_directory}" -a -f $prefix/custom.cfg ]; then
source $prefix/custom.cfg;
fi
### END /etc/grub.d/41_custom ###
More information about the Petitboot
mailing list