-
Notifications
You must be signed in to change notification settings - Fork 150
build_library: support uefi boot from iso #551
base: master
Are you sure you want to change the base?
Conversation
@@ -140,6 +140,10 @@ info "Generating ${GRUB_DIR}/load.cfg" | |||
ESP_FSID=$(sudo grub-probe -t fs_uuid -d "${LOOP_DEV}p1") | |||
sudo_clobber "${ESP_DIR}/${GRUB_DIR}/load.cfg" <<EOF | |||
search.fs_uuid ${ESP_FSID} root \$root |
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.
So if you boot an ISO on a system that has a disk installed from the same build this fs_uuid is search is going to find it. When booting from ISO fs_uuid should not get called.
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.
is there another option to determine if we've booted from iso?
i suggested doing a search.file /coreos/cpio.gz, but @mjg59 mentioned that wouldn't work well if someone's put cpio.gz on disk somewhere....
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 don't know, perhaps by querying UEFI for the boot disk? I don't know if there is an existing mechanism to translate what UEFI is using as the ESP to grub disk name bit if there is that'd probably be better than searching for a fs uuid like 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.
Also the search for ESP can be moved from load.cfg to grub.cfg, being in here is left over from pre-secure-boot when grub.cfg was read from the ESP.
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.
does this imply dropping load.cfg?
@@ -57,23 +72,25 @@ if [ "$net_default_server" != "" ]; then | |||
fi | |||
|
|||
# Search for the OEM partition, load additional configuration if found. | |||
if [ "$secure_boot" = "0" ]; then | |||
if [ "$secure_boot" = "0" -a -z "$isoboot" ]; then |
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.
Does this form of and actually work in grub? I know in the past we've switched to a nested if because and didn't work but that was probably using a different syntax.
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.
did you confirm that this syntax is properly supported by grub?
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.
-a
is already used other places in this config, like line 77 below
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.
Ok, that must work then. Must have been && that isn't valid.
current mformat doesn't have that -i arg, do you have another PR to bump mtools? |
@marineam ptal, seems to work for amd64, arm64 is not tested yet... |
Looking good, please sanity check with qemu arm64 though :) |
LGTM assuming arm64 doesn't explode :) |
mcopy -i isolinux/efi.img -o "${base_dir}"/boot/EFI/boot/* "::EFI/BOOT/" | ||
|
||
# syslinux won't work on arm64, and there is no point in isohybrid | ||
# for arm64/uefi. |
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.
Why is there no point to isohybrid? Isn't that needed to be able to use the ISO as a usb stick/drive/thingy instead of a cdrom? Or does isohybrid just not work for arm64?
PTAL cc @mjg59. confirmed this works in qemu-system-amd64/OVMF and qemu-system-aarch64/ArmVirtPkg. |
This isn't how the Fedora images work, so I think we're still doing something wrong here |
Bump |
No description provided.