Conversation
travier
left a comment
There was a problem hiding this comment.
Looking good. Not tested.
|
I've integrated this with bootc locally and things work. We probably need tests in here as well |
While collecting UKI from /boot/EFI/Linux or /usr/lib/modules we now also collect UKI addons from any directory that ends with `.efi.extra.d` Signed-off-by: Johan-Liebert1 <pragyanpoudyal41999@gmail.com>
fc18730 to
6b56c83
Compare
| pub file_path: PathBuf, | ||
| // The Portable Executable binary | ||
| pub file: RegularFile<ObjectID>, | ||
| pub pe_type: PEType, |
There was a problem hiding this comment.
Doing it this way seems a bit weird. The addons are always associated with a specific kernel image, right? I was imagining that we'd add something like a boxed slice of RegularFile to each Type2Entry so we can see which addons go with with kernel.
Or did I miss something?
There was a problem hiding this comment.
Yeah we could definitely do it by adding a Vec of RegularFile to a single Type2Entry, but apparently there are global addons as well which can be placed in $BOOT/loader/addons/
| } | ||
| } | ||
|
|
||
| pub const EFI_EXT: &str = ".efi"; |
There was a problem hiding this comment.
I usually prefer to just put these sort of things closer to where they're used or just write them directly...
There was a problem hiding this comment.
I use them in bootc as well, so thought making these constants made sense
|
@allisonkarlitskaya I'm thinking about merging |
6ed9210 to
07e4a22
Compare
|
Merged |
| pub fn load_all(root: &Directory<ObjectID>) -> Result<Vec<Self>> { | ||
| let mut entries = vec![]; | ||
|
|
||
| match root.get_directory("/boot/EFI/Linux".as_ref()) { |
There was a problem hiding this comment.
Wait isn't this the /usr/lib/modules case? Why are we looking in /boot?
This seems to be exacerbating a long-running debate about the role of /boot in container images.
https://uapi-group.org/specifications/specs/unified_kernel_image/#locations-and-naming-for-uki-auxiliary-resources seems very clear that this content should be in /usr/lib/ and not in /boot in images.
There was a problem hiding this comment.
This is more to do with how currently we only ignore /boot (and now /sysroot) in our EROFS image generation. I believe this has been a long running on our TODO list
There was a problem hiding this comment.
The reason we mask /boot for the primary UKI is to break the circular dependency with the composefs digest.
But that's not a problem for auxiliary UKI resources right? Although it's kind of interesting because I guess in theory actually one could have the expected composefs digest as an auxiliary UKI resource instead...that's an interesting thought because it helps break up the "build" of sealed image....
There was a problem hiding this comment.
Auxiliary UKI resources can live anywhere, but usually they'll be alongside the UKI they extend, at least that's what I think. Of course we'll need to mask the directory the addon is in if it has composefs cmdline in it.
Merge `Type2Entry` and `UsrLibModulesUki` structs keeping only `Type2Entry` Signed-off-by: Johan-Liebert1 <pragyanpoudyal41999@gmail.com>
07e4a22 to
42544e1
Compare
While collecting UKI from /boot/EFI/Linux or /usr/lib/modules we now also collect UKI addons from any directory that ends with
.efi.extra.dxref #126