-
Notifications
You must be signed in to change notification settings - Fork 8
ephemeral: Support extracting UKIs #144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds support for extracting the kernel and initramfs from Unified Kernel Images (UKIs) for ephemeral VMs. The approach of using objcopy is straightforward and effective. My review focuses on improving the robustness of the implementation by addressing potential panics from non-UTF-8 file paths and fixing non-deterministic behavior in kernel selection. The changes are otherwise well-structured and clear.
We want `bcvk ephemeral` to work for systems with UKIs; there's two choices here. We could switch in this case to actually booting via EFI firmware, and rely on the systemd-stub credentials to do things like inject kargs. But here I chose to do something a bit simpler - we extract the kernel+initramfs via `objcopy` from the UKI and use those in exactly the same way we do things today. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <[email protected]>
| debug!("Extracting kernel and initramfs from UKI: {:?}", uki_path); | ||
|
|
||
| // Extract .linux section (kernel) from UKI | ||
| Command::new("objcopy") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested this, but I suspect if binutils isn't installed this is probably going to error with a bare "No such file or directory", and then down below it's going to map into "Failed to extract kernel from UKI: No such file or directory" which sounds like the UKI is missing. The sort of thing future me would waste entirely too much time on before I finally figured out that actually objcopy is missing 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good thing to think about but there's a map_err in there which adds error context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry yes you're right. Will look at adding a check for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want
bcvk ephemeralto work for systems with UKIs; there's two choices here. We could switch in this case to actually booting via EFI firmware, and rely on the systemd-stub credentials to do things like inject kargs.But here I chose to do something a bit simpler - we extract the kernel+initramfs via
objcopyfrom the UKI and use those in exactly the same way we do things today.