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

lib: don't send CPUID settings in vCPU device state payload #848

Merged
merged 3 commits into from
Feb 5, 2025

Conversation

gjcolombo
Copy link
Contributor

@gjcolombo gjcolombo commented Feb 4, 2025

Remove the CPUID portion of the vCPU device state payload that gets sent during live migration. #847 prevented this payload from being constructed correctly. With #844 in place, however, the instance specs that targets receive during live migrations will always contain the source's explicitly-applied CPUID settings, even if the VM's "original" instance spec (the one used to create it) contained no explicit CPUID values. This means there's no longer any need to transfer these settings in the device state phase.

Upgrade PHD's cpuid_boot_test to detect this issue and verify the fix.

This change breaks the migrate-from-base tests because migration targets with the fix no longer accept all of the payload offers from sources without the fix. (In the future, when this sort of change is no longer allowed, targets that no longer care about a segment of a source device's payload offering will have to consume that segment explicitly but then ignore its contents.)

Fixes #847.

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Looks good to me! I left a couple comments on the test, but they're not blockers.

phd-tests/tests/src/cpuid.rs Outdated Show resolved Hide resolved
phd-tests/tests/src/cpuid.rs Show resolved Hide resolved
@gjcolombo gjcolombo merged commit 1061b1e into master Feb 5, 2025
10 of 11 checks passed
@gjcolombo gjcolombo deleted the gjcolombo/cpuid-migration branch February 5, 2025 00:50
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.

Vcpu::get_cpuid always returns an empty CpuidSet
2 participants