libvirt: Fix custom secure boot logic #170
Merged
Conversation
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
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes several issues with the custom secure boot logic for libvirt domains. The changes correctly avoid using the mutually exclusive firmware="efi" attribute when a custom OVMF path is specified, and add the necessary raw format attributes for both the loader and nvram elements. My review includes one suggestion to improve the robustness of path resolution for the nvram template, ensuring it works correctly regardless of the current working directory. Overall, the changes are well-targeted and address the described issues effectively.
cgwalters
added a commit
to gerblesh/bootc
that referenced
this pull request
Dec 3, 2025
Main goal is to reduce signing logic duplication between the systemd-boot and UKI generation. However, this quickly snowballed into wanting to actually verify by providing a custom secure boot keys to bcvk that things worked. This depends on bootc-dev/bcvk#170 Now as part of that, I ran into what I think are bugs in pesign; this cuts things back over to using sbsign. I'll file a tracker for that separately. Finally as part of this, just remove the TMT example that builds a sealed image but doesn't actually verify it works - it's already drifted from what we do outside here. Ultimately what we need is to shift some of this into the Fedora examples and we just fetch it here anyways. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <[email protected]>
cgwalters
force-pushed
the
nvram-secureboot
branch
from
50fa02c to
dc2c26b
Compare
cgwalters
added a commit
to gerblesh/bootc
that referenced
this pull request
Dec 3, 2025
Main goal is to reduce signing logic duplication between the systemd-boot and UKI generation. However, this quickly snowballed into wanting to actually verify by providing a custom secure boot keys to bcvk that things worked. This depends on bootc-dev/bcvk#170 Now as part of that, I ran into what I think are bugs in pesign; this cuts things back over to using sbsign. I'll file a tracker for that separately. Finally as part of this, just remove the TMT example that builds a sealed image but doesn't actually verify it works - it's already drifted from what we do outside here. Ultimately what we need is to shift some of this into the Fedora examples and we just fetch it here anyways. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <[email protected]>
cgwalters
added a commit
to gerblesh/bootc
that referenced
this pull request
Dec 4, 2025
Main goal is to reduce signing logic duplication between the systemd-boot and UKI generation. However, this quickly snowballed into wanting to actually verify by providing a custom secure boot keys to bcvk that things worked. This depends on bootc-dev/bcvk#170 Now as part of that, I ran into what I think are bugs in pesign; this cuts things back over to using sbsign. I'll file a tracker for that separately. Finally as part of this, just remove the TMT example that builds a sealed image but doesn't actually verify it works - it's already drifted from what we do outside here. Ultimately what we need is to shift some of this into the Fedora examples and we just fetch it here anyways. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <[email protected]>
gursewak1997
reviewed
Dec 5, 2025
- The nvram path needs to be absolute (would be nice if we could pass this as a file descriptor instead, but hard to do AFAIK with libvirt today) - When a custom nvram is specified, we need to avoid using firmware="efi" as it's mutually exclusive with explicit <loader> paths - Also need to explicitly specify `raw` format for nvram No tests yet, but I did test this locally as part of updating bootc's composefs+UKI integration test suite. Signed-off-by: Colin Walters <[email protected]>
cgwalters
force-pushed
the
nvram-secureboot
branch
from
dc2c26b to
ae10852
Compare
jeckersb
approved these changes
Dec 5, 2025
cgwalters
added a commit
to gerblesh/bootc
that referenced
this pull request
Dec 5, 2025
Main goal is to reduce signing logic duplication between the systemd-boot and UKI generation. However, this quickly snowballed into wanting to actually verify by providing a custom secure boot keys to bcvk that things worked. This depends on bootc-dev/bcvk#170 Now as part of that, I ran into what I think are bugs in pesign; this cuts things back over to using sbsign. I'll file a tracker for that separately. Finally as part of this, just remove the TMT example that builds a sealed image but doesn't actually verify it works - it's already drifted from what we do outside here. Ultimately what we need is to shift some of this into the Fedora examples and we just fetch it here anyways. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: Colin Walters <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
rawformat for nvramNo tests yet, but I did test this locally as part of updating bootc's composefs+UKI integration test suite.