WIP: Integrate VMM.Perf into the OpenVMM pipeline - #4184
Open
ayusharora221204 wants to merge 8 commits into
Open
WIP: Integrate VMM.Perf into the OpenVMM pipeline#4184ayusharora221204 wants to merge 8 commits into
ayusharora221204 wants to merge 8 commits into
Conversation
Add VMM.Perf to the existing VMM-test pipeline with Petri and nextest integration. Run the supported benchmark profiles on Linux x64 KVM and publish their results through the existing CI artifacts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1b288be-8b12-4b27-9ab6-345919473d6d
Contributor
There was a problem hiding this comment.
Pull request overview
Integrates VMM.Perf profiles into the existing Petri/nextest-driven VMM test pipeline, including a new vmm_perf test binary, runtime artifact plumbing, and CI scheduling/nextest resource controls so the perf runs are isolated to the intended Linux x64 KVM job.
Changes:
- Added a new Petri-based
vmm_perftest binary that executes FIO, IPERF3, and boot-time profiles via VirtualClient and writes results into existing VMM-test artifacts. - Introduced a new “VMM.Perf runtime” blob artifact and extended artifact download plumbing to support artifacts coming from different Azure storage account/container pairs.
- Updated nextest configuration and CI gate logic to give VMM.Perf exclusive host resources and to enable it only for
x64-linux-amd-kvmin GitHub CI.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vmm_tests/vmm_tests/tests/vmm_perf.rs | New Petri/nextest test binary that extracts the VMM.Perf runtime, runs VirtualClient-driven profiles, and collects logs/results. |
| vmm_tests/vmm_tests/Cargo.toml | Registers the new vmm_perf test binary with harness = false. |
| vmm_tests/vmm_test_images/src/lib.rs | Adds VmmPerfRuntimeLinuxX64 and extends metadata to include per-artifact storage account/container. |
| vmm_tests/petri_artifacts_vmm_test/src/lib.rs | Declares the VMM.Perf runtime blob artifact and adds a tag trait for that blob store. |
| Guide/src/dev_guide/dev_tools/xflowey.md | Documents how to run VMM.Perf via cargo xflowey vmm-tests-run filters. |
| flowey/flowey_lib_hvlite/src/download_openvmm_vmm_tests_artifacts.rs | Updates artifact download logic to group downloads by (storage account, container) and download each group separately. |
| flowey/flowey_hvlite/src/pipelines/vmm_tests_run.rs | Maps the new runtime artifact ID into the download set for VMM test runs. |
| flowey/flowey_hvlite/src/pipelines/checkin_gates.rs | Enables VMM.Perf only for the GitHub CI x64-linux-amd-kvm job and excludes it elsewhere. |
| .config/nextest.toml | Adds per-binary overrides so vmm_perf requires all threads and gets extended slow timeouts. |
Suppressed comments (1)
Guide/src/dev_guide/dev_tools/xflowey.md:29
- This is a shell command example; per the Guide style guide, shell command fences should be labeled
bashrather thantext.
To run one profile, add its test name:
```text
cargo xflowey vmm-tests-run --filter "binary(vmm_perf) & test(fio)"
</details>
Comment on lines
+358
to
+362
| let status = Command::new("tar") | ||
| .args(["-xf"]) | ||
| .arg(archive) | ||
| .arg("-C") | ||
| .arg(&staging) |
Comment on lines
+323
to
+326
| let status = Command::new("sudo") | ||
| .args(["-n", "chown", "-R", "--"]) | ||
| .arg(format!("{uid}:{gid}")) | ||
| .args(paths) |
Comment on lines
+388
to
+390
| if directory.join(virtual_client_name).is_file() { | ||
| candidates.push(directory.clone()); | ||
| } |
|
|
||
| Run all Linux x64 VMM.Perf profiles with: | ||
|
|
||
| ```text |
Comment on lines
+1708
to
+1722
| let vmm_perf_runtime = match (backend_hint, config, label) { | ||
| ( | ||
| PipelineBackendHint::Github, | ||
| PipelineConfig::Ci, | ||
| "x64-linux-amd-kvm", | ||
| ) => Some(VmmPerfRuntimeLinuxX64), | ||
| _ => None, | ||
| }; | ||
|
|
||
| if let Some(runtime) = vmm_perf_runtime { | ||
| test_artifacts.push(runtime); | ||
| } else { | ||
| nextest_filter_expr = | ||
| format!("({nextest_filter_expr}) & !binary(vmm_perf)"); | ||
| } |
Contributor
There was a problem hiding this comment.
This shouldn't be done as a match and modification after parameter declaration, it should be done in the parameters somehow.
| filter = 'package(~vmm_tests) and binary(vmm_perf)' | ||
| # Performance profiles must own the host while VirtualClient drives OpenVMM. | ||
| threads-required = "num-cpus" | ||
| slow-timeout = { period = "60m", terminate-after = 4 } |
Contributor
There was a problem hiding this comment.
If this is going to be running in CI on every commit then 4 hours feels way too long.
Author
There was a problem hiding this comment.
Agreed. The four-hour limit was a temporary setting used during initial profile validation. I have reduced it to a 15-minute slow period with termination after two periods.
Contributor
There was a problem hiding this comment.
Steven Malis (@smalis-msft) we will not be running this in PR pipeline, we will add it initially in PR pipeline to test it out, but before merge we want this to only run on CI merges.
Comment on lines
+3
to
+4
|
|
||
| //! VMM.Perf profiles executed through the Petri/nextest test harness. |
Contributor
There was a problem hiding this comment.
What gap does this solve that the burette crate doesn't? See here
Contributor
There was a problem hiding this comment.
Daman Mulye (@damanm24) we want to make this change so that we have a single harness across multiple VMMs, we have an internal initiative on it, we had some discussions around it, we could sync offline to discuss this more.
Limit each VMM.Perf profile to 30 minutes by marking it slow after 15 minutes and terminating it after two periods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1b288be-8b12-4b27-9ab6-345919473d6d
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (5)
vmm_tests/vmm_test_images/src/lib.rs:153
container()is documented as returning the blob container, but for VMM.Perf the value is a container + prefix (e.g.vmmperf/latest). Please clarify the API contract in the doc comment so callers don’t treat this as a strict container name.
/// Get the Azure Blob container containing the artifact.
pub fn container(self) -> &'static str {
self.meta().container
}
Guide/src/dev_guide/dev_tools/xflowey.md:23
- These are shell commands; using a
bashcode fence (instead oftext) matches the Guide style guidance and improves readability/syntax highlighting. Suggested change: switch the fence fromtexttobashfor this block.
```text
cargo xflowey vmm-tests-run --filter "binary(vmm_perf)"
**Guide/src/dev_guide/dev_tools/xflowey.md:29**
* Same as above: this block is a shell command, so a `bash` code fence is more appropriate than `text`. Suggested change: replace the opening fence language `text` with `bash`.
cargo xflowey vmm-tests-run --filter "binary(vmm_perf) & test(fio)"
**vmm_tests/vmm_test_images/src/lib.rs:119**
* `vmm_perf_meta` hard-codes a different storage account/container path than the rest of the VMM test images. However, at least some fallback download paths still assume the global `STORAGE_ACCOUNT`/`CONTAINER` (e.g. `xtask guest-test download-image` and `OpenvmmKnownPathsTestArtifactResolver`’s MissingCommand). If `VmmPerfRuntimeLinuxX64` isn’t already present in the cache, those code paths will try to fetch it from the wrong location and fail.
Consider updating the shared download helpers to use `KnownTestArtifacts::storage_account()` / `container()` when resolving *any* `KnownTestArtifacts`, so non-HvLite-hosted artifacts work end-to-end.
download_name: T::DOWNLOAD_NAME,
supports_blob_disk: false,
storage_account: "vmmperf",
container: "vmmperf/latest",
}
**vmm_tests/vmm_test_images/src/lib.rs:148**
* The `storage_account()` doc comment implies a single Azure Storage account “containing the artifact”, but the code now supports multiple sources. It would be clearer to document that this value is the storage account name used by download tooling for this specific artifact.
This issue also appears on line 150 of the same file.
/// Get the Azure Storage account containing the artifact.
pub fn storage_account(self) -> &'static str {
self.meta().storage_account
}
</details>
|
|
||
| declare_blob_artifacts! { | ||
| /// VMM.Perf runtime for Linux x86_64 hosts. | ||
| RUNTIME_LINUX_X64, |
Contributor
There was a problem hiding this comment.
We ship the linux arm64 bin as well can you add that as well?
| .stderr(Stdio::from(stderr)) | ||
| .status() | ||
| .with_context(|| { | ||
| format!( |
Contributor
There was a problem hiding this comment.
can we make a VirtualClientCommandBuilder here which will provide abstraction over these?
VirtualClientCommandBuilder::new()
.profile(Enum)
.parameter(..)
.parameter(...)
.logger()
.log_to_file(true)
.... etc.
Comment on lines
+85
to
+202
| fn run_profile( | ||
| params: petri::PetriTestParams<'_>, | ||
| artifacts: VmmPerfArtifacts, | ||
| profile: Profile, | ||
| ) -> anyhow::Result<()> { | ||
| validate_host()?; | ||
|
|
||
| let openvmm = artifacts.openvmm.get(); | ||
| let firmware = artifacts.firmware.get(); | ||
| let runtime_archive = artifacts.runtime_archive.get(); | ||
| let output_dir = artifacts.log_dir.get(); | ||
| ensure_file(openvmm, "OpenVMM executable")?; | ||
| ensure_file(firmware, "MSVM firmware")?; | ||
| ensure_file(runtime_archive, "VMM.Perf runtime archive")?; | ||
| fs_err::create_dir_all(output_dir)?; | ||
|
|
||
| let virtual_client_name = "VirtualClient"; | ||
| let runtime_dir = prepare_runtime(runtime_archive, virtual_client_name)?; | ||
| register_package_file(&runtime_dir, "openvmm", "openvmm", openvmm)?; | ||
| register_package_file( | ||
| &runtime_dir, | ||
| "msvm-firmware", | ||
| Path::new("FV").join("MSVM.fd"), | ||
| firmware, | ||
| )?; | ||
| ensure_runtime_executables(&runtime_dir, virtual_client_name)?; | ||
|
|
||
| let profile_path = runtime_dir.join("profiles").join(profile.file); | ||
| ensure_file(&profile_path, "VMM.Perf profile")?; | ||
|
|
||
| let work_parent = std::env::var_os("VMM_PERF_WORK_DIR") | ||
| .map(PathBuf::from) | ||
| .unwrap_or_else(std::env::temp_dir); | ||
| fs_err::create_dir_all(&work_parent)?; | ||
| let work = tempfile::Builder::new() | ||
| .prefix(&format!("vmm-perf-{}-", profile.name)) | ||
| .tempdir_in(work_parent)?; | ||
| let data_dir = work.path().join("data"); | ||
| let temp_dir = work.path().join("temp"); | ||
| let control_dir = work.path().join("control"); | ||
| fs_err::create_dir_all(&data_dir)?; | ||
| fs_err::create_dir_all(&temp_dir)?; | ||
| fs_err::create_dir_all(&control_dir)?; | ||
|
|
||
| let patched_profile = control_dir.join(profile.file); | ||
| patch_profile_work_dir(&profile_path, &patched_profile, &data_dir)?; | ||
|
|
||
| let virtual_client_logs = output_dir.join("virtual-client"); | ||
| let results_dir = output_dir.join("results"); | ||
| let openvmm_logs_dir = output_dir.join("openvmm-logs"); | ||
| fs_err::create_dir_all(&virtual_client_logs)?; | ||
| fs_err::create_dir_all(&results_dir)?; | ||
| fs_err::create_dir_all(&openvmm_logs_dir)?; | ||
|
|
||
| let runtime_logs = runtime_dir.join("logs"); | ||
| if runtime_logs.exists() { | ||
| fs_err::remove_dir_all(&runtime_logs)?; | ||
| } | ||
|
|
||
| let experiment_id = format!( | ||
| "{}-{}", | ||
| profile.name, | ||
| SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() | ||
| ); | ||
| let console_log_path = output_dir.join("console.log"); | ||
| let console_log = File::create(&console_log_path)?; | ||
| let started = Instant::now(); | ||
| let status = run_virtual_client( | ||
| &runtime_dir, | ||
| virtual_client_name, | ||
| &patched_profile, | ||
| &data_dir, | ||
| &temp_dir, | ||
| &virtual_client_logs, | ||
| &experiment_id, | ||
| console_log, | ||
| )?; | ||
| let duration_ms = started.elapsed().as_millis().min(u64::MAX as u128) as u64; | ||
|
|
||
| copy_profile_diagnostics( | ||
| [("data", data_dir.as_path()), ("temp", temp_dir.as_path())], | ||
| output_dir, | ||
| )?; | ||
| if runtime_logs.exists() { | ||
| copy_directory(&runtime_logs, &virtual_client_logs.join("runtime"))?; | ||
| } | ||
|
|
||
| let exit_code = status.code().unwrap_or(-1); | ||
| fs_err::write( | ||
| output_dir.join("run-summary.json"), | ||
| serde_json::to_vec_pretty(&serde_json::json!({ | ||
| "profile": profile.file, | ||
| "success": status.success(), | ||
| "exit_code": exit_code, | ||
| "experiment_id": experiment_id, | ||
| "duration_ms": duration_ms, | ||
| "runtime_rid": "linux-x64", | ||
| "runtime_version": "3.0.21", | ||
| "runtime_source": "public-blob", | ||
| }))?, | ||
| )?; | ||
|
|
||
| tracing::info!( | ||
| test = params.test_name, | ||
| profile = profile.file, | ||
| exit_code, | ||
| duration_ms, | ||
| "VMM.Perf profile completed" | ||
| ); | ||
|
|
||
| anyhow::ensure!( | ||
| status.success(), | ||
| "VMM.Perf profile {} failed with exit code {}", | ||
| profile.file, | ||
| exit_code | ||
| ); | ||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
Can you add a light state machine like abstraction to this? We may benefit from some smaller module and structs.
Contributor
There was a problem hiding this comment.
I would think treat this module as the vmm test orchestrator which invokes into the VirtualClient Lifecycle.
added 2 commits
August 11, 2026 20:56
Add VMM.Perf runtime integration, configurable VM shapes, host validation, diagnostic collection, and metrics publication for Linux KVM and MSHV PR jobs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
flowey/flowey_hvlite/src/pipelines/checkin_gates.rs:1680
- The PR description says VMM.Perf is enabled only in the GitHub CI
x64-linux-amd-kvmjob for now, but this change also enables it forx64-linux-intel-mshvviapr_vmm_perf_filter/pr_vmm_perf_artifactsin this job definition.
nextest_filter_expr: pr_vmm_perf_filter(format!(
"{standard_filter} & !test(pcat_x64)"
)),
test_artifacts: pr_vmm_perf_artifacts(standard_x64_test_artifacts.clone()),
vmm_tests/vmm_tests/tests/vmm_perf/virtual_client.rs:96
WorkDircan be overridden via configuration parameters, andprofile_work_diris resolved/validated from that parameter, but theVC_VMM_WORK_DIRenvironment passed to VirtualClient is still hard-coded todirectories.data_dir. This means a customWorkDiris validated but not actually used by VirtualClient, which is likely to break non-default setups.
.log_to_file(true)
.work_dir(&directories.data_dir)
.temp_dir(&directories.temp_dir);
Comment on lines
+151
to
+155
| self.runtime.root(), | ||
| &directories.data_dir, | ||
| &directories.temp_dir, | ||
| &directories.config_output_dir, | ||
| &directories.profile_work_dir, |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
flowey/flowey_hvlite/src/pipelines/checkin_gates.rs:1679
- The PR description says VMM.Perf is enabled only for the GitHub CI
x64-linux-amd-kvmjob, but this job block also applies the VMM.Perf-enabling filter/artifact logic tox64-linux-intel-mshv. This expands CI scope (and downloads the runtime) beyond what the PR description states.
// - No legal way to obtain gen1 pcat blobs on non-msft linux machines
nextest_filter_expr: pr_vmm_perf_filter(format!(
"{standard_filter} & !test(pcat_x64)"
)),
test_artifacts: pr_vmm_perf_artifacts(standard_x64_test_artifacts.clone()),
prep_steps_variants: standard_x64_prep_variants.clone(),
flowey/flowey_hvlite/src/pipelines/vmm_tests_run.rs:528
parse_parameter_settrims parameter names but not values. This makes common inputs likeCpuCount=2, MemoryMB=4096(note the space after the comma) serialize values with leading whitespace, which then fail later numeric parsing in the VMM.Perf runner/config logic. Trimming (and rejecting empty) values here would make the CLI more robust.
anyhow::ensure!(
parameters
.insert(name.to_owned(), value.to_owned())
.is_none(),
vmm_tests/vmm_tests/tests/vmm_perf/config.rs:153
stringify_parametersclones thenamestring even though it can be moved into the result after computing the value string. Avoiding the clone removes an allocation per parameter (and should satisfy clippy in this hot-ish parsing path).
anyhow::ensure!(
!name.trim().is_empty(),
"{context} contains an empty parameter name"
);
Ok((name.clone(), scalar_to_string(&name, &value)?))
vmm_tests/vmm_tests/tests/vmm_perf/virtual_client.rs:345
experiment_iduses the current time truncated to milliseconds. When running multiple configurations back-to-back, it’s plausible for two runs to be prepared within the same millisecond, producing identical experiment IDs and potentially colliding/overwriting VirtualClient outputs for those runs.
Ok(format!(
"{}-{config_name}-{}",
profile.name(),
SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis()
))
vmm_tests/vmm_test_images/src/lib.rs:157
KnownTestArtifacts::container()is documented as returning the Azure Blob container, but some artifacts (e.g. VMM.Perf) appear to encode a container plus virtual-directory prefix (e.g.perfpackage/latest). This doc comment is misleading and may cause future callers to use it withaz storage blob ... --container-name, which would fail.
/// Get the Azure Blob container containing the artifact.
pub fn container(self) -> &'static str {
self.meta().container
Comment on lines
+86
to
+90
| let status = Command::new("tar") | ||
| .args(["-xf"]) | ||
| .arg(archive) | ||
| .arg("-C") | ||
| .arg(&staging) |
ayusharora221204
marked this pull request as ready for review
August 12, 2026 06:52
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (3)
vmm_tests/vmm_tests/tests/vmm_perf/runtime.rs:93
tarextraction of an externally downloaded runtime archive should avoid restoring ownership/permissions (especially if the test ever runs as root) and should reject absolute paths. Adding defensive tar flags reduces the risk of a malicious or corrupted archive writing unexpected metadata or paths during extraction.
let status = Command::new("tar")
.args(["-xf"])
.arg(archive)
.arg("-C")
.arg(&staging)
.status()
.context("failed to launch tar for VMM.Perf runtime extraction")?;
anyhow::ensure!(status.success(), "failed to extract VMM.Perf runtime");
vmm_tests/vmm_tests/tests/vmm_perf/virtual_client.rs:103
- When a config supplies a custom "WorkDir" (used as the base directory),
directories.profile_work_dircan differ fromdirectories.data_dir, butVirtualClientCommandBuilder::work_diris currently set todata_dirwhile theWorkDirparameter is set toprofile_work_dir. This mismatch means VirtualClient/OpenVMM may write to different locations, and diagnostics collection (which readsprofile_work_dir) can miss data when an explicit WorkDir base is used.
.logger("csv")
.logger("summary")
.log_to_file(true)
.work_dir(&directories.data_dir)
.temp_dir(&directories.temp_dir);
vmm_tests/vmm_test_images/src/lib.rs:154
KnownTestArtifacts::container()is documented as returning the Azure Blob container, but for VMM.Perf it is set to"perfpackage/latest"(container path + virtual directory prefix). This doc comment is misleading given how the value is used to build blob URLs.
/// Get the Azure Blob container containing the artifact.
|
ayusharora221204 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
Guide/src/dev_guide/dev_tools/xflowey.md:38
- These blocks are shell commands; using a
bash-labeled code fence improves readability and matches the repo doc style expectations for command snippets (instead oftext).
```text
cargo xflowey vmm-tests-run --filter "binary(vmm_perf)"
To run one profile, add its test name:
cargo xflowey vmm-tests-run --filter "binary(vmm_perf) & test(fio)"
To run explicit VM sizes:
cargo xflowey vmm-tests-run \
--filter "binary(vmm_perf) & test(fio)" \
--vmm-perf-vmsizes 'CpuCount=2,MemoryMB=4096' \
--vmm-perf-vmsizes 'CpuCount=8,MemoryMB=16384'
**vmm_tests/vmm_tests/tests/vmm_perf/virtual_client.rs:103**
* When a custom WorkDir base is provided, `profile_work_dir` may differ from `data_dir`, but `VC_VMM_WORK_DIR` is currently set from `data_dir`. This means the selected/validated WorkDir can be ignored and host disk-space validation may be checking the wrong directory.
.log_to_file(true)
.work_dir(&directories.data_dir)
.temp_dir(&directories.temp_dir);
**flowey/flowey_hvlite/src/pipelines/checkin_gates.rs:1678**
* The PR description says VMM.Perf is enabled only in the GitHub CI `x64-linux-amd-kvm` job for now, but this job configuration also enables it for `x64-linux-intel-mshv` via `pr_vmm_perf_filter`/`pr_vmm_perf_artifacts`. If this should stay kvm-only, exclude `binary(vmm_perf)` and don’t add the runtime artifact for the mshv job.
nextest_filter_expr: pr_vmm_perf_filter(format!(
"{standard_filter} & !test(pcat_x64)"
)),
test_artifacts: pr_vmm_perf_artifacts(standard_x64_test_artifacts.clone()),
</details>
| } | ||
|
|
||
| fn prepare(request: VirtualClientRunRequest<'a>) -> anyhow::Result<Self> { | ||
| let mut custom_parameters = request.config.parameters; |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec48d4d4-a0ec-4ddb-958e-e2646e09f96d
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.
Add VMM.Perf to the existing Petri and nextest VMM-test pipeline.
This change:
x64-linux-amd-kvmjob for now.