hyperv_ic: implement SaveRestore for KVP & timesync - #4081
Conversation
There was a problem hiding this comment.
Pull request overview
Implements save/restore support for the Hyper-V IC KVP and timesync devices so they can be restored without revoking/reoffering their VMBus channels, working around a Linux kernel crash triggered by rapid offer/rescind patterns.
Changes:
- Add saved-state wire types for negotiated IC versions (
SavedVersions) and per-device channel state. - Enable
supports_save_restore()forTimesyncIcandKvpIc, with correspondingsave_open/restore_openimplementations. - On restore, handle in-flight states safely (e.g., timesync resends a sync message; KVP drains an outstanding response before accepting new requests).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| vm/devices/hyperv_ic/src/timesync.rs | Adds save/restore state capture and restore behavior for timesync channel state machine. |
| vm/devices/hyperv_ic/src/kvp.rs | Adds save/restore for KVP channel state machine, including response-drain handling after restore. |
| vm/devices/hyperv_ic/src/common.rs | Introduces SavedVersions and conversions to serialize negotiated version pairs in saved state. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
vm/devices/hyperv_ic/src/common.rs:196
test_with_tracing::testis a sync-only attribute macro (it rejects async fns) and it’s not used here (tests use#[async_test]). This import will be unused and the dev-dependency becomes unnecessary unless you explicitly calltest_with_tracing::init()in the async tests.
use pal_async::async_test;
use test_with_tracing::test;
use vmbus_channel::connected_async_channels;
vm/devices/hyperv_ic/Cargo.toml:38
test_with_tracingis only referenced via an unused import insrc/common.rstests. Once that import is removed, this dev-dependency should be dropped to avoid unnecessary workspace churn (and it will likely get removed by fmt anyway).
[dev-dependencies]
test_with_tracing.workspace = true
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
vm/devices/hyperv_ic/src/common.rs:109
- During version negotiation you currently ignore the response
statusreturned byread_message(_status). If the guest reports a failure status for the VERSION_NEGOTIATION transaction, we should treat negotiation as failed instead of proceeding to parse the body as if it succeeded.
let (message_type, _status, buf) = self.read_message().await?;
Backport of #4081 to `release/1.8.2607`. The cherry-pick of 788419a applied cleanly onto `release/1.8.2607` with no conflicts and no manual edits. Original PR: #4081 --- *This backport PR was created by an AI agent (GitHub Copilot) on behalf of @smalis-msft.*
|
Backported to release/1.8.2607 in #4127 |
We've discovered a Linux kernel bug that results in crashes when vmbus channels are rapidly offered and rescinded, a pattern we exercise in our vmm_tests. Work around this by implementing SaveRestore for the last two relevant devices that didn't already support it. Mimic Hyper-V's behavior by regenerating the devices on restore, rather than actually saving state.
A future change may make SaveRestore support non-optional, that's still under discussion.
This doesn't restabilize the broken tests yet, I'll let this bake for a bit first.