Skip to content
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

Add UnifiedKernelImageFormat= (attempt 2) #2733

Merged
merged 3 commits into from
Jun 4, 2024

Conversation

NekkoDroid
Copy link
Contributor

@NekkoDroid NekkoDroid commented May 30, 2024

This is currently incomplete as it doesn't handle the Type1 entry case as I'm unsure how the nested directories should be handled as allowing nesting doesn't work with UKI auto discovery, see #2731 (comment). Provisionally - is replaced by / for Type1 entries.

I'd love some input on how to better formulate the documentation because it seems very superficial right now.

Tested UKI cases:

  • BootloaderEntryFormat= -> /boot/EFI/Linux/<entry-token>-<kver>-<usrhash>.efi ✅ (have no tries file set)
  • BootloaderEntryFormat=%i_%v -> /boot/EFI/Linux/<image-id>_<image-version>.efi
  • BootloaderEntryFormat=hello~&c+&h-&k-&e -> /boot/EFI/Linux/hello~+<usrhash>-<kver>-<entry-token>.efi - &c ignored, expected ✅
  • BootloaderEntryFormat=&e/&k -> config parse error, expected ✅
  • BootloaderEntryFormat=&& -> /boot/EFI/Linux/&.efi

@NekkoDroid
Copy link
Contributor Author

I now have an untested commit that keeps Type1 by default organized how it is now, but if BootloaderEntryFormat= is specified all directories are put directly in /boot/<bootloader-entry-format>. If that is acceptable I can commit and push it.

The other option might be to replace all - in BootloaderEntryFormat= with / to organize the directories.

mkosi/__init__.py Outdated Show resolved Hide resolved
mkosi/resources/mkosi.md Outdated Show resolved Hide resolved
mkosi/__init__.py Show resolved Hide resolved
mkosi/resources/mkosi.md Outdated Show resolved Hide resolved
@NekkoDroid NekkoDroid force-pushed the bootloader-entry-format branch 3 times, most recently from a482f49 to b01009e Compare May 30, 2024 19:26
@NekkoDroid
Copy link
Contributor Author

NekkoDroid commented May 30, 2024

I now added support for Type1. For now I made it replace all - in the BootloaderEntryFormat to / to figure out the directory, although I am not too happy about it as it feels a bit weird and is probably erronious to do the way its currently done.

mkosi/__init__.py Outdated Show resolved Hide resolved
@NekkoDroid NekkoDroid force-pushed the bootloader-entry-format branch 3 times, most recently from 73a4329 to f2bcdbd Compare May 30, 2024 21:27
mkosi/__init__.py Outdated Show resolved Hide resolved
mkosi/__init__.py Outdated Show resolved Hide resolved
mkosi/__init__.py Outdated Show resolved Hide resolved
mkosi/resources/mkosi.md Outdated Show resolved Hide resolved
mkosi/__init__.py Outdated Show resolved Hide resolved
mkosi/resources/mkosi.md Outdated Show resolved Hide resolved
mkosi/resources/mkosi.md Outdated Show resolved Hide resolved
@NekkoDroid NekkoDroid marked this pull request as ready for review May 31, 2024 12:31
@NekkoDroid NekkoDroid force-pushed the bootloader-entry-format branch 3 times, most recently from 0b711c5 to 1287ec8 Compare May 31, 2024 18:42
@DaanDeMeyer
Copy link
Contributor

DaanDeMeyer commented Jun 4, 2024

@NekkoDroid Can you merge the 2nd, 3rd and 4th commits? makes it easier to review

Copy link
Contributor

@DaanDeMeyer DaanDeMeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I read the boot loader spec again, and type1/type2 entries are different enough after all that this option shouldn't be used for both (my bad).

So let's rename this back to UnifiedKernelImageName= and not make it apply to type1 after all. If we introduce a separate type1 name setting later, it should likely take a full path relative to $BOOT instead of just a filename, but we can figure that out later.

mkosi/__init__.py Outdated Show resolved Hide resolved
@NekkoDroid NekkoDroid force-pushed the bootloader-entry-format branch 3 times, most recently from 26dd22f to 5c976b1 Compare June 4, 2024 11:37
@NekkoDroid NekkoDroid changed the title Add BootloaderEntryFormat= Add UnifiedKernelImageFormat= (attempt 2) Jun 4, 2024
Copy link
Contributor

@DaanDeMeyer DaanDeMeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you write a few unit tests for expand_kernel_specifiers()? Aside from that LGTM

This can be used to control the name to use for the UKI during image
generation. Special `&` specifiers can be used to include kernel
specific information in the filename.

This is useful for the `systemd-sysupdate` case, as you can set this to
`%i_%v` to use a format that can be parse by its configuration. The
current format used includes both a roothash as well as the kernel
version which both can't be matched by sysupdate.
@DaanDeMeyer
Copy link
Contributor

Thanks for working on this!

@DaanDeMeyer DaanDeMeyer linked an issue Jun 4, 2024 that may be closed by this pull request
@DaanDeMeyer DaanDeMeyer merged commit c575b47 into systemd:main Jun 4, 2024
23 of 32 checks passed
@NekkoDroid NekkoDroid deleted the bootloader-entry-format branch June 4, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Make UKI naming configurable
3 participants