Generate ACPI for newer QEMU versions instead of rejecting them - #1052
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates qemu-acpi’s QEMU version-to-compatibility mapping so that future QEMU major releases (beyond the newest modeled profile) still generate ACPI tables using the latest known profile, avoiding hard failures on upgrade and turning ABI changes into actionable blob mismatches.
Changes:
- Clamp QEMU versions newer than the newest modeled profile to
Compatibility::LATESTinstead of returningNone. - Add/extend documentation explaining the clamping behavior and the need to update
Compatibility::LATESTwhen adding a new profile. - Add tests covering version mapping and an end-to-end “newer QEMU still generates” case.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| dstack/crates/qemu-acpi/src/profile.rs | Adds Compatibility::LATEST, updates version mapping to clamp newer majors, and introduces mapping tests. |
| dstack/crates/qemu-acpi/src/tables.rs | Adds an end-to-end test ensuring unmodeled newer QEMU versions still generate identical blobs under the latest profile. |
| dstack/crates/qemu-acpi/src/lib.rs | Updates crate docs and improves the unsupported-version error message for pre-8.0 QEMU. |
| dstack/crates/qemu-acpi/fixtures/README.md | Documents the “newer than newest profile” behavior and the need to move Compatibility::LATEST when adding profiles. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
QemuVersion::compatibility()returnsNonefor any QEMU major past the newest modeled profile, andtables::buildturns that intoError::UnsupportedVersion. So the day QEMU 12.0 ships, every CVM that reports it stops producing expected ACPI blobs at all —dstack-mr'sMachine::build_tables()errors out, and with it the whole TDX measurement path that depends on it.That is the wrong default for a compatibility model. Most QEMU releases do not touch the Q35 ACPI ABI: the profiles here already collapse whole majors (
(9, _),(10, _),(11, 1..)) precisely because nothing observable changed across them. Refusing to generate assumes the opposite — that every unseen release broke the ABI — and the assumption costs more than it saves, because "cannot generate" and "generated something that does not match" both end in a failed verification. Only one of the two tells the operator what actually differs.Fix
Versions newer than the newest modeled profile now generate with that profile:
Versions older than 8.0 still return
None. Clamping downward would be a guess in the direction where QEMU's ACPI output is known to differ, anddstack-mrrejects< 8.0.0before it gets here anyway.Compatibility::LATESTis the single place that has to move when a profile is added; both theLayoutdoc comment andfixtures/README.mdnow say so, next to the existing "generate the four base fixtures" instructions.Verification
cargo test -p qemu-acpi— 18 passed. New tests:versions_map_to_their_own_profile— pins the existing mapping (8.2.2, 9.1.0, 9.2.1, 10.0.0, 11.0.3, 11.1.0) so the clamp arm cannot swallow a version that has its own profile.versions_newer_than_the_newest_profile_clamp_to_it— 11.9.0, 12.0.0, 99.4.1 resolve toLATEST.versions_older_than_the_oldest_profile_are_rejected— 7.2.0 and 0.0.0 stayNone.unmodeled_newer_versions_generate_with_the_latest_profile— end-to-end:build()with 12.0.0 returns blobs byte-identical to the 11.1 output, so the clamp reaches actual generation and is not just a mapping detail.No fixture changes; the byte-for-byte differential fixtures are untouched.