← Back to team overview

curtin-dev team mailing list archive

Re: [Merge] ~sjg1/curtin:boot2 into curtin:master

 

I saw you used the Launchpad superscede feature, it's not necessary to do that (and I think it generates some unnecessary noise).  I understand this merge proposal is about targetting master so this is a different merge proposal, but feel free to just `git push -f` to do any updates.

The questions on https://code.launchpad.net/~sjg1/curtin/+git/curtin/+merge/479606 are still open, just marking that here to make it easier for anyone else to follow along.

Diff comments:

> diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
> index 82ebcab..9706494 100644
> --- a/curtin/commands/curthooks.py
> +++ b/curtin/commands/curthooks.py
> @@ -843,16 +838,70 @@ def setup_grub(
>      else:
>          instdevs = ["none"]
>  
> -    update_nvram = grubcfg.update_nvram
> +    update_nvram = bootcfg.update_nvram
>      if uefi_bootable and update_nvram:
>          efi_orig_state = util.get_efibootmgr(target)
> -        uefi_remove_old_loaders(grubcfg, target)
> +        uefi_remove_old_loaders(bootcfg, target)
>  
> -    install_grub(instdevs, target, uefi=uefi_bootable, grubcfg=grubcfg)
> +    install_grub(instdevs, target, uefi=uefi_bootable, bootcfg=bootcfg)
>  
>      if uefi_bootable and update_nvram:
> -        uefi_reorder_loaders(grubcfg, target, efi_orig_state, variant)
> -        uefi_remove_duplicate_entries(grubcfg, target)
> +        uefi_reorder_loaders(bootcfg, target, efi_orig_state, variant)
> +        uefi_remove_duplicate_entries(bootcfg, target)
> +
> +
> +def translate_old_grub_schema(cfg):
> +    """Translate the old top-level 'grub' configure to the new 'boot' one"""
> +    grub_cfg = cfg.get('grub', {})
> +
> +    # Use the 'boot' key, if present
> +    if 'boot' in cfg:
> +        if grub_cfg:
> +            raise ValueError("Configuration has both 'grub' and 'boot' keys")
> +        return
> +
> +    # copy legacy top level name
> +    if 'grub_install_devices' in cfg and 'install_devices' not in cfg:
> +        grub_cfg['install_devices'] = cfg['grub_install_devices']
> +
> +    if 'grub' in cfg:
> +        del cfg['grub']
> +
> +    # Create a bootloaders list with 'grub', which is implied in the old config
> +    grub_cfg['bootloaders'] = ['grub']
> +
> +    cfg['boot'] = grub_cfg
> +
> +
> +def setup_boot(
> +        cfg: dict,
> +        target: str,
> +        machine: str,
> +        stack_prefix: str,
> +        osfamily: str,
> +        variant: str,
> +        ) -> None:
> +    translate_old_grub_schema(cfg)
> +
> +    boot_cfg = cfg['boot']
> +    bootloaders = boot_cfg['bootloaders']
> +
> +    # For now we have a hard-coded mechanism to determine whether grub should
> +    # be installed or not. Even if the grub info is present in the config, we
> +    # check the machine to decide whether or not to install it.
> +    if 'grub' in bootloaders and uses_grub(machine):
> +        with events.ReportEventStack(
> +                name=stack_prefix + '/install-grub',
> +                reporting_enabled=True, level="INFO",
> +                description="installing grub to target devices"):
> +            setup_grub(cfg, target, osfamily=osfamily, variant=variant)
> +
> +    if 'extlinux' in bootloaders:
> +        with events.ReportEventStack(
> +                name=stack_prefix + '/install-extlinux',
> +                reporting_enabled=True, level="INFO",
> +                description="installing extlinux to target devices"):
> +            install_extlinux(cfg, target, osfamily=osfamily, variant=variant)

install_extlinux() doesn't take those arguments yet, so an extlinux install attempt fails here.

>  
>  
>  def update_initramfs(target=None, all_kernels=False):
> diff --git a/curtin/commands/install_extlinux.py b/curtin/commands/install_extlinux.py
> new file mode 100644
> index 0000000..8e378c2
> --- /dev/null
> +++ b/curtin/commands/install_extlinux.py
> @@ -0,0 +1,85 @@
> +# This file is part of curtin. See LICENSE file for copyright and license info.
> +
> +"""This loosely follows the u-boot-update script in the u-boot-menu package"""
> +
> +import io
> +import os
> +
> +from curtin import config
> +from curtin import paths
> +from curtin.log import LOG
> +
> +EXTLINUX_DIR = '/boot/extlinux'
> +
> +
> +def build_content(target: str, bootcfg: config.BootCfg):
> +    """Build the content of the extlinux.conf file
> +
> +    For now this only supports x86, since it does not handle the 'fdt' option.

do you want to assert that so that if someone is running an extlinux test elsewhere they get an earlier error?

> +    Rather than add that, the plan is to use a FIT (Flat Image Tree) which can
> +    handle FDT selection automatically. This should avoid the complexity
> +    associated with fdt and fdtdir options.
> +
> +    We assume that the boot/ directory is in the root partition, rather than
> +    being in a separate partition. TBD.

Normally things are setup so the target system is mounted as target, including /boot if it is on a specific partition.

> +    """
> +    def get_entry(label, params, menu_label_append=''):
> +        return f'''\
> +label {label}
> +\tmenu label {menu_label} {version}{menu_label_append}
> +\tlinux /{kernel_path}
> +\tinitrd /{initrd_path}
> +\tappend root={root} {params}'''
> +
> +    buf = io.StringIO()
> +    params = 'ro quiet'
> +    alternatives = ['default', 'recovery']
> +    menu_label = 'Ubuntu 22.04.5 LTS'

Placeholder content.  I'd rather you be less specific here than fill it in with wrong values.  In the subiquity case you may assume the install ISO has the install cd mounted at /cdrom and /cdrom/.disk has some useful info.

> +    root = '/dev/mapper/vgubuntu-root'  # TODO
> +
> +    # For the recovery option, remove 'quiet' and add 'single'
> +    without_quiet = filter(lambda word: word != 'quiet', params.split())
> +    rec_params = ' '.join(list(without_quiet) + ['single'])
> +
> +    print(f'''\
> +## {EXTLINUX_DIR}/extlinux.conf
> +##
> +## IMPORTANT WARNING
> +##
> +## The configuration of this file is generated automatically.
> +## Do not edit this file manually, use: u-boot-update
> +
> +default l0
> +menu title U-Boot menu
> +prompt 0
> +timeout 50''', file=buf)
> +    for seq, (kernel_path, full_initrd_path, version) in enumerate(
> +            paths.get_kernel_list(target)):
> +        LOG.debug('P: Writing config for %s...', kernel_path)
> +        initrd_path = os.path.basename(full_initrd_path)
> +        print(file=buf)
> +        print(file=buf)
> +        print(get_entry(f'l{seq}', params), file=buf)
> +
> +        if 'recovery' in alternatives:
> +            print(file=buf)
> +            print(get_entry(f'l{seq}r', rec_params, ' (rescue target)'),
> +                  file=buf)
> +
> +    return buf.getvalue()
> +
> +
> +def install_extlinux(
> +        target: str,
> +        bootcfg: config.BootCfg,
> +        ):
> +    """Install extlinux to the target chroot.
> +
> +    :param: target: A string specifying the path to the chroot mountpoint.
> +    :param: bootcfg: An config dict with grub config options.
> +    """
> +    content = build_content(target, bootcfg)
> +    extlinux_path = paths.target_path(target, '/boot/extlinux')
> +    os.makedirs(extlinux_path, exist_ok=True)
> +    with open(extlinux_path + '/extlinux.conf', 'w', encoding='utf-8') as outf:
> +        outf.write(content)


-- 
https://code.launchpad.net/~sjg1/curtin/+git/curtin/+merge/480712
Your team curtin developers is requested to review the proposed merge of ~sjg1/curtin:boot2 into curtin:master.



References