-
-
Notifications
You must be signed in to change notification settings - Fork 186
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
Iso boot debugging and fixes #1377
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,14 +21,20 @@ reset_entry() { | |
} | ||
|
||
filedir=`dirname $file` | ||
DEBUG "filedir= $filedir" | ||
bootdir="${bootdir%%/}" | ||
DEBUG "bootdir= $bootdir" | ||
bootlen="${#bootdir}" | ||
DEBUG "bootlen= $bootlen" | ||
appenddir="${filedir:$bootlen}" | ||
DEBUG "appenddir= $appenddir" | ||
|
||
fix_path() { | ||
path="$@" | ||
if [ "${path:0:1}" != "/" ]; then | ||
DEBUG "fix_path: path was $@" | ||
path="$appenddir/$path" | ||
DEBUG "fix_path: path is now $path" | ||
fi | ||
} | ||
|
||
|
@@ -38,7 +44,10 @@ check_path() { | |
local checkpath firstval | ||
checkpath="$1" | ||
firstval="$(echo "$checkpath" | cut -d\ -f1)" | ||
if ! [ -r "$bootdir$firstval" ]; then return 1; fi | ||
if ! [ -r "$bootdir$firstval" ]; then | ||
DEBUG "$bootdir$firstval doesn't exist" | ||
return 1; | ||
fi | ||
return 0 | ||
} | ||
|
||
|
@@ -111,26 +120,30 @@ grub_entry() { | |
# TODO: differentiate between Xen and other multiboot kernels | ||
kexectype="xen" | ||
kernel="$val" | ||
DEBUG " grub_entry multiboot kernel= $kernel" | ||
;; | ||
module*) | ||
case $val in | ||
--nounzip*) val=`echo $val | cut -d\ -f2-` ;; | ||
esac | ||
fix_path $val | ||
modules="$modules|module $path" | ||
DEBUG " grub_entry linux modules= $modules" | ||
;; | ||
linux*) | ||
# Some configs have a device specification in the kernel | ||
# or initrd path. Assume this would be /boot and remove | ||
# it. Keep the '/' following the device, since this | ||
# path is relative to the device root, not the config | ||
# location. | ||
DEBUG " grub_entry : linux trimcmd prior of kernel/append parsing: $trimcmd" | ||
kernel=`echo $trimcmd | sed "s/([^)]*)//g" | cut -d\ -f2` | ||
append=`echo $trimcmd | cut -d\ -f3-` | ||
;; | ||
initrd*) | ||
# Trim off device specification as above | ||
initrd="$(echo "$val" | sed "s/([^)]*)//g")" | ||
DEBUG " grub_entry: linux initrd= $initrd" | ||
;; | ||
esac | ||
} | ||
|
@@ -205,17 +218,20 @@ syslinux_entry() { | |
state="search" | ||
;; | ||
*) | ||
kernel="${val#"$bootdir"}" | ||
kernel="$val" | ||
DEBUG "kernel= $kernel" | ||
esac | ||
;; | ||
initrd* | INITRD* ) | ||
initrd="${val#"$bootdir"}" | ||
initrd="$val" | ||
Comment on lines
220
to
+226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're undoing part of a075347 - the concern in the commit was fixed right, kexec-boot now handles absolute paths correctly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will review that part. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interestingly, a075347 was supposed to be grub related. Without hack in my debugging PR to merge isolinux and grub entries to not have duplicate entries, Debian isolinux entries are exposed without "..." in labels. Will test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JonathonHall-Purism I haven't found a case where this PR caused issue following past comment There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current issue in master is that when isos are mounted under /boot, the actual bootdir is /boot/boot/ and that is getting stripped otherwise. I think a075347 was relevant until caller scripts fixed it themselves if needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tlaurion Sorry this sat in my inbox for a few days. Makes sense, agree it looks like the root issue was fixed. I see what you mean that stripping /boot from /boot/boot/... would cause this issue. Is this ready to merge then? |
||
DEBUG "initrd= $initrd" | ||
;; | ||
append* | APPEND* ) | ||
if [ "$kexectype" = "multiboot" -o "$kexectype" = "xen" ]; then | ||
syslinux_multiboot_append | ||
else | ||
append="$val" | ||
DEBUG "append= $append" | ||
fi | ||
;; | ||
esac | ||
|
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.
Wish there was a bit more of a standard here so we didn't have to spray all the variants we know about 😓 But it looks fine for our purposes
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.
Will add reference to webboot as comment prior of section. Since this is passed as kernel options down to user land init, passing all iso boot hacks for all linux distributions would be pretty harmless and probably should be done preventively prior if users opening issues... but that would require us to test them all individually :/