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 a test about boot option descriptions #788

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

iximeow
Copy link
Member

@iximeow iximeow commented Oct 10, 2024

ensuring we don't inflict Omicron#5112 on instances due to our own changes: boot option descriptions are load-bearing, so if we've changed them make sure we think through the consequences. i'm pleasantly surprised that EfiLoadOption just.. worked here. yay.

the procedure that gets us names we see for guest disks today is a bit subtle, but i've tried to document it fully to pair with the expectation in this test.

there are also specific conditions where failing this assertion is fine - say, if we've done #787, or changed how we're processing UEFI boot variables. just, as things are right now, we're surprisingly dependent on dancing with OVMF in exactly the same way every time a guest boots.

Copy link
Contributor

@gjcolombo gjcolombo left a comment

Choose a reason for hiding this comment

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

This is both a good test for us to have as we (or, well, I) change how NVMe device parameters work and a great description of how our firmware image actually constructs these IDs. Assuming this still passes with the latest HEAD we should merge it!

@iximeow
Copy link
Member Author

iximeow commented Jan 8, 2025

finally coming back to this: yep! gave this a run rebased on current master and the test still passes on Alpine, Debian, Ubuntu. ran it with a Windows guest on a lark and realized that this was missing a phd_skip!() for Windows guests, so i've added that. once CI's happy, in it goes.

uses efivarfs for guest info, so without efivarfs the test will fail
@iximeow iximeow force-pushed the ixi/boot-option-description branch from 392b4b4 to 2ff265b Compare January 8, 2025 21:05
@iximeow
Copy link
Member Author

iximeow commented Jan 8, 2025

the phd-build failure in 392b4b4 is because... after i added the is_linux() clause to the new test, i rebased that small diff back onto this old branch so i'd have a nice neat tiny diff between what Greg looked at and what i'm gonna merge. is_linux(), however, is newer than this test.

so i've just rebased onto master, and it'll actually pass now 😓

@iximeow iximeow merged commit bb01d5b into master Jan 8, 2025
11 checks passed
@iximeow iximeow deleted the ixi/boot-option-description branch January 8, 2025 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants