Ooops I didn't save my comments Diff comments:
> diff --git a/HACKING.rst b/HACKING.rst > index f2b618d..f723906 100644 > --- a/HACKING.rst > +++ b/HACKING.rst > @@ -46,6 +46,12 @@ Do these things once > git remote add LP_USER ssh://[email protected]/~LP_USER/curtin > git push LP_USER master > > +* Install `libapt-pkg-dev` as it is needed by tox: Thanks, done > + > + .. code:: sh > + > + sudo apt install libapt-pkg-dev > + > .. _repository: https://git.launchpad.net/curtin > .. _contributor license agreement: https://ubuntu.com/legal/contributors > .. _contributor-agreement-canonical: > https://launchpad.net/%7Econtributor-agreement-canonical/+members > diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py > index 0c4d7fd..34c4a5d 100644 > --- a/curtin/commands/curthooks.py > +++ b/curtin/commands/curthooks.py > @@ -873,6 +873,38 @@ def translate_old_grub_schema(cfg): > cfg['boot'] = grub_cfg > > > +def setup_extlinux( > + cfg: dict, > + target: str): > + """Set up an extlinux.conf file > + > + :param: cfg: A config dict containing config.BootCfg in cfg['boot']. > + # :param: target: A string specifying the path to the chroot mountpoint. done > + """ > + bootcfg = config.fromdict(config.BootCfg, cfg.get('boot', {})) > + > + # If there is a separate boot partition, set fw_boot_dir to empty > + storage_cfg = cfg.get('storage', {}).get('config', {}) > + fw_boot_dir = '/boot' > + root = None > + for item in storage_cfg: OK done, but I had to get the storage dict, i.e. call extract_storage_ordered_dict() > + if item['type'] == 'mount': > + if item['path'] == '/boot': > + fw_boot_dir = '' > + elif item['path'] == '/': > + root = item > + > + if not root: > + raise ValueError("Storage configuration has no root directory") > + > + storage_config_dict = block_meta.extract_storage_ordered_dict(cfg) Hmmm, well it seems that block_meta.mount_data() needs it. I get an error like this if I try to pass just 'cfg: TestInstallExtlinux::test_single_partition_cfg: ValueError: mount entry refers to non-existant device vdb-part1_format: ({'id': 'vdb-part1_mount', 'type': 'mount', 'path': '/', 'device': 'vdb-part1_format', 'spec': '/dev/sda1'}) Since I am now using it above, perhaps this is OK? Here is what def block_meta.mount_data(info, storage_config) is passed for the storage_config in each case: bad (passing just 'cfg'): storage_config = {'config': [{'id': 'vdb', 'name': 'vdb', 'path': '/dev/vdb', 'ptable': 'gpt', ...}, {'device': 'vdb', 'id': 'vdb-part1...part1'}, {'device': 'vdb-part1_format', 'id': 'vdb-part1_mount', 'path': '/', 'spec': '/dev/sda1', ...}], 'version': 1} good (passing storage_cfg): storage_config OrderedDict({'vdb': {'id': 'vdb', 'type': 'disk', 'name': 'vdb', 'path': '/dev/vdb', 'ptable': 'gpt'}, 'vdb-part1': {'id': 'vdb-part1', 'type': 'partition', 'device': 'vdb', 'number': 1}, 'vdb-part1_format': {'id': 'vdb-part1_format', 'type': 'format', 'volume': 'vdb-part1', 'fstype': 'ext4'}, 'vdb-part1_mount': {'id': 'vdb-part1_mount', 'type': 'mount', 'path': '/', 'device': 'vdb-part1_format', 'spec': '/dev/sda1'}}) The line: if info['device'] not in storage_config: in mount_data() seems to require a dict? > + > + fdata = block_meta.mount_data(root, storage_config_dict) > + spec = block_meta.resolve_fdata_spec(fdata) > + > + install_extlinux(bootcfg, target, fw_boot_dir, spec) > + > + > def setup_boot( > cfg: dict, > target: str, > diff --git a/curtin/commands/install_extlinux.py > b/curtin/commands/install_extlinux.py > index b52b591..dc1bd64 100644 > --- a/curtin/commands/install_extlinux.py > +++ b/curtin/commands/install_extlinux.py > @@ -21,18 +23,25 @@ def build_content(bootcfg: config.BootCfg, target: str): > associated with fdt and fdtdir options. > > We assume that the boot directory is available as /boot in the target. > + > + :param: bootcfg: A boot-config dict > + :param: target: A string specifying the path to the chroot mountpoint. > + :param: fw_boot_dir: Firmware's view of the /boot directory; when there > is > + a separate /boot partition, firmware will access that as the root > + directory of the filesystem, so '' should be passed here. When the > boot > + directory is just a subdirectory of '/', then '/boot' should be > passed > + :param: root_spec: Root device to pass to kernel > """ > 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} > +\tlinux {fw_boot_dir}/{kernel_path} > +\tinitrd {fw_boot_dir}/{initrd_path} > \tappend {params}''' > > buf = io.StringIO() > - params = 'ro quiet' > - alternatives = ['default', 'recovery'] > + params = f'{root_spec} ro quiet' Oh thank you! That fills in a missing gap for me. However I added this in the original MP - this is just making it selectable. In other words, the single/quite stuff is already there. Would it be able to sort this out in a future MP, along the lines of get_carryover_params()? > menu_label = 'Linux' > > # For the recovery option, remove 'quiet' and add 'single' -- https://code.launchpad.net/~sjg1/curtin/+git/curtin/+merge/484176 Your team curtin developers is subscribed to branch curtin:master. -- Mailing list: https://launchpad.net/~curtin-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~curtin-dev More help : https://help.launchpad.net/ListHelp
