vmm_test: test keepalive servicing with a namespace change after save #2184
Conversation
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
…er restore with keepaliveg
gurasinghMS
force-pushed
the
servicing-with-namespace-change
branch
from
5058934 to
08e73ea
Compare
gurasinghMS
commented
Oct 16, 2025
mattkur
reviewed
Oct 16, 2025
… be cloneable anymore
gurasinghMS
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds a test that verifies NVMe keepalive functionality during OpenHCL servicing when a namespace change occurs. The test ensures that the controller properly handles namespace changes during servicing by verifying that appropriate asynchronous event requests (AER) and GET_LOG_PAGE commands are observed after restoration.
Key changes:
- Adds a new test
servicing_keepalive_with_namespace_updatethat triggers a namespace change during servicing and verifies the controller's response - Introduces a
Verifyvariant toQueueFaultBehaviorthat signals when a matching command is observed - Removes
Clonetrait fromQueueFaultBehaviorandAdminQueueFaultConfigto support the new verification mechanism
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs | Adds test for namespace change during servicing with keepalive enabled, extracts vtl2_nsid constant |
| vm/devices/storage/nvme_test/src/workers/admin.rs | Updates fault handling to use mutable references and adds Verify behavior support |
| vm/devices/storage/nvme_resources/src/fault.rs | Adds Verify variant to QueueFaultBehavior and removes Clone trait |
Co-authored-by: Copilot <[email protected]>
gurasinghMS
changed the title
mattkur
approved these changes
Oct 21, 2025
| vm.restore_openhcl().await?; | ||
|
|
||
| let _ = CancelContext::new() | ||
| .with_timeout(Duration::from_secs(10)) |
There was a problem hiding this comment.
nit: this is reasonable, but you may find that this is flakey if machines are under load. Let's see ...
There was a problem hiding this comment.
Will keep an eye out for flakiness from this! New test viewer should help :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR adds a new queue fault for the admin queue called verify which is modeled as a Oneshot sender. When the fault controller observes a matching command, it completes the oneshot channel. This indicates to the test that a matching command was observed. The test controller at this point disposes off the sender. Signal is only sent for the first matching command.
This also adds a new vmm test that leverages the
Verifyfunctionality to check for AER and GET_LOG_PAGE admin commands after restore is complete. The test waits 10s (max) for each command, failing the test if no command is seen in that time frame.