initrd/etc/functions: Refine list_usb_storage logic to enable booting from a USB device with an isohybrid-created image #1540
Conversation
|
New requirement from linuxboot org: commits need to be --sign-off Sorry about that @P5vc . Reading from phone will check code tomorrow |
|
@P5vc testing https://app.circleci.com/pipelines/github/linuxboot/heads/701/workflows/90e1bf0d-f35a-43fe-ae46-f1c9975a7b73/jobs/13647 's https://output.circle-artifacts.com/output/job/521ebb13-af2b-469e-8eed-31954831d3dc/artifacts/0/build/x86/x230-hotp-maximized/heads-x230-hotp-maximized-v0.2.0-1919-g73c790e.zip against Q4.2 rc5 dd'ed to usb thumb drive. Will edit with results and comment on code afterward. Note that I never use dd'ed iso to dedicated usb thumb drive or dvd anymore. It boots successfully:
Enabling debug+tracing through config option. Redoing:
|
@tlaurion No worries! Would you like me to force push signed versions of those commits? If so, I don't mind, just let me know!
Just a heads up, it appears that the Qubes OS devs reverted the switch to using isohybrid just before Qubes OS 4.2 Release Candidate 5 was built, in response to this issue. Therefore, using one of the two previous RC builds (Qubes-R4.2.0-rc3-x86_64.iso or Qubes-R4.2.0-rc4-x86_64.iso) may be a better test of the bug fix. 👍
This is objectively a much better method that I will certainly need to start using more often!! Thanks for the tip! |
|
Well I'm happy regression tests worked. Can do one last test with older release on test laptop.
@P5vc yes please sign-off. Info is availle under failed test |
Signed-off-by: Priveasy Admin <[email protected]>
… function Main changes to the list_usb_storage function include: - Modifying the logic to include in its output block devices that have filesystems written directly to them. This is primarily designed fix a bug preventing some bootable media (such as those with an isohybrid-created image written to them) from being included in the USB boot options - Simplifying some of the logic to better-conform to the POSIX standard, and remove dependence on more-complex utilities (such as sed and awk), when simpler utilities will do the trick - Adding comments to better-document the purpose of each command; especially helpful given the long chain of piped output Signed-off-by: Priveasy Admin <[email protected]>
P5vc
force-pushed
the
isohybrid-boot-fix
branch
from
73c790e to
91b1191
Compare
|
The commits have been signed off on, as requested! |
|
@P5vc Thanks a lot for the cleanup and consistency, and for identifying the problem here. Thanks also for the context from Qubes. TL;DR - Unfortunately, using the device iso9660 filesystem creates problems for a number of install media I tried today. I do not think it is correct to boot isohybrid from flash media using the device iso9660 filesystem, we should use the partition table - that's the entire reason isohybrid exists, otherwise it would not exist and we'd just use iso9660 everywhere. The real issue here was that Qubes 4.2 RC media had a broken partition table, as mentioned in the linked change reverting it. Using the device iso9660 filesystem was a workaround to ignore the broken partition table. They've fixed it, so we should not do this. The cleanup is great though, I would like to merge that. Below I listed the problems I ran into, so if we can revert the behavior of allowing the device to mount when partitions exist, this would be a great improvement. I liked the idea of doing a test mount of the device to see if it contains a filesystem, the test before was a bit fragile. However, since we need to favor the partitions if both exist, I don't think we can go that direction. Images offering both disk and partitionsSeveral images that used to offer two partition choices now offer both the disk itself and the partitions: memtest86+ GRUB, Debian 12 netinstall, PureOS byzantium Plasma, Debian 11 GNOME live, Librem EC update ISO. That makes the partition selection more complex. In a few cases it used to be able to select the partition automatically. Old remnant ISO9660 foundI have one flash drive in particular that behaves strangely with this change. It has a GPT partition table, one ext4 partition, and that partition contains a bunch of ISOs I use to test Heads/PureBoot. Current master works fine, finds the partition automatically and asks to select an ISO. Strangely, with the MR it now finds remnants of an old iso9660 filesystem and mounts that automatically - it doesn't even offer me a choice between that or the legitimate ext4 partition. Apparently I had a Fedora 36 image on this at one point, because those are the boot options I get. I don't think we can blame the partitioning here, we can't require zeroing the entire disk to repartition it. Restricting the device mount to iso9660 won't solve this (tried it, that's what it's finding). |
| test_mntpt='/tmp/direct-mount-test' | ||
| mkdir -p "$test_mntpt" | ||
| blkdev_included='false' | ||
| if mount -r "$blkdev" "$test_mntpt" >/dev/null 2>/dev/null |
There was a problem hiding this comment.
Per my other comment, we need to go back to the strategy of checking if the device has a partition table first, and using only the partitions if it does.
I liked the idea of doing a mount test - maybe we could apply that to the partitions or to unpartitioned media in the future - but since it can apparently find remnant filesystems after repartitioning I don't think we can go this route.
|
@JonathonHall-Purism Thank you so much for the feedback! I spent the latter half of my day tweaking these commits per your comments, and then conducting extensive testing. I also managed to go down an embarrassing number of rabbit holes in the process, but hey, it made for a fascinating and educational day! :) I should hopefully be ready to push these new changes in the next couple of days, schedule permitting, and will likely have a couple of follow up questions for you as well. Thanks again! |
|
Main question on my side is: what other OS is doing hybrid ISOs that would not boot from heads if user guided into detach signing them. That would be #1438 (comment) instead. The reasoning being: should ISOs dd'ed or prepared by external tools be deprecated and replaced by booting ISOs that don't show the bug in question? Is supporting raw iso over USB thumb drive simply a need of users not aware that booting from ISOs is better and would be improved by Heads trying to find iso.asc iso.sig for same iso and if not found, prompt user to verify sha256sum once and then detach sign that iso? And support checksum mechanisms and signing that are done from other distributions today? Maybe that is where the time should be invested instead? |
No! Practically all instructions for installing any Linux distribution tell users to prepare a bootable USB flash drive this way. It's great to support additional means, but if that most common method does not work in Heads, it will bring a lot of pain to users. IMO, we should work toward closing the gap between "things that boot with BIOS/UEFI" and "things that boot with Heads", not widening it. |
|
@tlaurion I'll go ahead and close this pull request. The original premise was mistaken, and although I ended up making some good changes, I got wrapped up in a bunch of other projects and haven't had a chance to revisit that work and update this pull request accordingly. In any event, those changes would probably be more appropriate to propose in separate pull requests. Thank you both again for your feedback and suggestions! |
This pull request modifies the
list_usb_storagefunction within/initrd/etc/functionswith the primary goal of correcting a bug that prevents a user from booting from certain USB media.For example, here's a post from the Qubes OS forum of users encountering this bug, and here's an article outlining, in depth, the root cause of this issue.
In addition to correcting the aforementioned bug, this pull request takes advantage of having to rewrite a chunk of the core logic anyways, to:
fat32workaround to apply to theoretically any filesystem causing the same type of errorsedandawk), when simpler utilities will do the trickAs a final note, you may notice that main addition to the pre-existing logic was to add a mount test for all detected USB block storage devices (but not for their partitions). Although perhaps not the prettiest way to address this issue, I was unfortunately unable to find a better solution within the confines of the busybox environment. For example, I initially attempted to simply add logic looking for the ISO 9660 filesystem on the block devices, however utilities that can easily do this on most systems (such as
blkid), do not support this functionality in their busybox versions. In fact, the only busybox utility I could find with this functionality wasmount. Therefore, this pull request could theoretically be modified to add the-t iso9660option to themountcommand, and as a result only end up mounting (and including) devices with the ISO 9660 filesystem (fixing the specific error from the links above). However, adding this option would not significantly impact efficiency (at least it didn't during my limited testing), and may unnecessarily limit the fix by making it overly-specific to the above issues. That being said, if you think that adding this option is preferred, or if you know of a better approach that doesn't require usingmount, please let me know!