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

phd: gate OS-specific tests, make others more OS-agnostic #799

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

iximeow
Copy link
Member

@iximeow iximeow commented Oct 19, 2024

some smoketests instigated a graceful reboot by sending reboot\n to the serial console, but Windows guests are rewarded (via Cygwin) with

bash: reboot: command not found

as of #785 there is now a TestVm helper graceful_reboot that does this in whatever way the guest adapter desires, so use that instead.

unfortunately graceful_reboot is only mostly right for Windows guests, so fix that here too: shutdown immediately terminates the cmd.exe session, at which point the SAC redraws the previous screen, which happens to have all the sigils we look for to detect that Windows has freshly booted. we then log back into the imminently-shutdown Windows, shutdown takes effect, and PHD becomes fully desynchronized from the guest state.

so, look for "BdsDxe: loading " as an outside-the-guest-OS sigil that tells us we're watching a fresh boot. this comes from OVMF, so a booted guest will have clobbered the message and not know to redraw it even if it seeks to recreate a previous display.

other tests depend on Linux-specific features like efivarfs or mount -o ro, so skip them on non-Linux guests.

validated this with both a Debian guest and a Windows Server 2022 guest - Debian was and still is good, the 2k22 guest fail fewer tests and not any that it didn't fail before

some smoketests instigated a graceful reboot by sending `reboot\n` to
the serial console, but Windows guests are rewarded (via Cygwin) with

```
bash: reboot: command not found
```

as of #785 there is now a TestVm helper `graceful_reboot` that does this
in whatever way the guest adapter desires, so use that instead.

unfortunately `graceful_reboot` is only mostly right for Windows guests,
so fix that here too: `shutdown` immediately terminates the cmd.exe
session, at which point the SAC redraws the previous screen, which
happens to have all the sigils we look for to detect that Windows has
freshly booted. we then log back into the imminently-shutdown Windows,
shutdown takes effect, and PHD becomes fully desynchronized from the
guest state.

so, look for "BdsDxe: loading " as an outside-the-guest-OS sigil that
tells us we're watching a fresh boot. this comes from OVMF, so a booted
guest will have clobbered the message and not know to redraw it even if
it seeks to recreate a previous display.

other tests depend on Linux-specific features like `efivarfs` or
`mount -o ro`, so skip them on non-Linux guests.
@iximeow iximeow added the testing Related to testing and/or the PHD test framework. label Oct 19, 2024
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 had been on my "meant to do" list for a long while--thanks for patching this up!

Comment on lines +40 to +44
// Look for `BdsDxe:` as a sign that we're actually seeing a fresh boot.
// This is not terribly important in the case of a first boot, but in a
// case such as logging out and waiting for reboot, exiting a cmd.exe
// session causes Windows to redraw its previous screen - everything
// past `Computer is booting, ...` below.
Copy link
Contributor

Choose a reason for hiding this comment

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

Oooof. Nice catch here.

@iximeow iximeow merged commit 9ad7368 into master Oct 22, 2024
11 checks passed
@iximeow iximeow deleted the ixi/windows-graceful-reboot branch October 22, 2024 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Related to testing and/or the PHD test framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants