No loops for grub - #348
Open
mwhudson wants to merge 3 commits into
Open
Conversation
ty (the static type checker run by `make lint-ty`) flagged three
genuine issues in code introduced by the MBR support PR:
- grubutil._part_num used `# type: ignore[union-attr]` (mypy syntax)
to reach `partition_number` on a union containing variants that
don't define it. Capture the result of getattr() in a local
instead, removing the need for any ignore.
- ImageService._get_partition_numbers was annotated with
`GPTVolume | MBRVolume`, but its only caller passes
`GPTVolume | MBRVolume | HybridVolume`. The implementation already
handles HybridVolume correctly (volume_schema is HYBRID, so the
MBR-extended path is skipped); widen the parameter type to match.
- test_part_num_{mbr_plain,mbr_extended,gpt} build their structure
list from MagicMock instances. ty doesn't see through
`spec=MBRStructureItem`, so the call to _part_num looked
ill-typed. Cast to MBRStructureList / GPTStructureList at
construction so the rest of the test reads naturally.
`make lint-ty` and `make lint-ruff` both pass clean afterwards.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Phase B of the unprivileged-LXD refactor. The old setup_grub flow used losetup --find --show --partscan and a real chroot rooted at an image partition mount, with /dev mounted as devtmpfs. Both losetup and mount -t devtmpfs require init_user_ns CAP_SYS_ADMIN and are blocked inside unprivileged user namespaces — so neither works in an unprivileged LXD container. The new flow (amd64 only — UEFI x86_64-efi + BIOS i386-pc) avoids loops and image mounts entirely: - prepare_grub_assets runs BEFORE the partition-format loop. It chroots directly into the rootfs prime dir (which IS the mmdebstrap output — it already contains shim, grub-efi-amd64-signed, /usr/lib/grub, etc.) with /dev mounted as a --bind of the host's /dev instead of devtmpfs. Inside the chroot it runs update-grub (producing /boot/grub/grub.cfg in the rootfs prime dir naturally) and grub-mkimage (producing core.img for the BIOS bootloader). - It then populates the ESP prime dir with shim (EFI/BOOT/BOOTX64.EFI and EFI/ubuntu/shimx64.efi), signed grub (EFI/ubuntu/grubx64.efi) and a small chainload grub.cfg stub. Filenames are surveyed at runtime since they vary across Ubuntu releases. - install_grub_to_image runs AFTER finalize_images. It dd's boot.img into MBR bytes 0..440 (preserving the partition table at 440..512) and dd's core.img into the BIOS-boot (ef02) partition for GPT, or into the post-MBR gap for plain MBR. Integration option chosen: pre-format population. mke2fs -d and mkfs.vfat + mcopy pick up the grub.cfg and ESP files from the prime dirs at format time, so no image-mount step is needed. The only post-assembly write is the dd of boot.img + core.img — done directly against the image file with a known sector offset (gptutil already had get_partition_sector_offset). arm64, armhf, riscv64, and any other arch emit a TODO progress message and skip. No loop-based fallback — the goal is to make the unprivileged-container build work. Scope-respecting: - services/pack.py: only adds calls before the format loop and after finalize_images. The format loop body is untouched (to minimize conflicts with the parallel Phase A effort). - pack/chroot.py: untouched — Mount already supports --bind. - pack/image.py, gptutil.py, mbrutil.py, services/image.py: untouched. Tests: - test_grubutil.py: fully rewritten. Old _image_mounts / _part_num tests are gone (those helpers were tied to the loop flow). New tests assert: /dev is bind-not-devtmpfs; non-amd64 emits TODO and returns None; GPT-with-EFI populates the ESP prime dir; GPT-with- BIOS-boot returns the ef02 partition name; MBR returns no ef02; missing shim raises; install_grub_to_image dd's the right offsets. - test_pack.py: updated assertion from setup_grub to prepare_grub_assets + install_grub_to_image. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
|
This looks interesting. I wonder if, with this approach, we'd be able to remove special handling for grub altogether and just make grub a parts plugin? |
Separate "which bytes go where" (GRUB policy) from "write bytes to a disk offset" (mechanism), so the disk-writing code no longer knows about GRUB and can be reused for any future raw payload. - Add imagecraft/pack/rawcontent.py: a bootloader-agnostic applier with RawContent records, MbrBootCode/SectorOffset/PartitionStart targets, and apply_raw_content() (dd ... conv=notrunc, resolving PartitionStart against the on-disk partition table at apply time). - grubutil: add grub_raw_content() (policy) and reduce install_grub_to_image() to a thin shim delegating to the generic applier; drop the now-unused gptutil import. - pack service: feed grub_raw_content() records to apply_raw_content() directly instead of constructing an Image for its disk_path. - Tests: new test_rawcontent.py for the applier; grub_raw_content policy tests in test_grubutil.py; updated pack-service patch targets. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
I don't really know what a plugin can do but I don't think it can dd the content into the image file directly? I guess an interface can be added to allow it to do so but is it worth it? I guess the way to answer that is to go look for what it takes to be bootable on other machines. I know some ARM boards require dd-ing the second stage bootloader to a fixed offset and things like that but it's hard to speculate without use cases. I spent a while talking to a robot about this and it made the commit i've just pushed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 2 of my "no loop device" effort
make lint && make test.