Integrate VMM.Perf into the OpenVMM pipeline - #4184
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
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>
| let status = Command::new("tar") | ||
| .args(["-xf"]) | ||
| .arg(archive) | ||
| .arg("-C") | ||
| .arg(&staging) |
| let status = Command::new("sudo") | ||
| .args(["-n", "chown", "-R", "--"]) | ||
| .arg(format!("{uid}:{gid}")) | ||
| .args(paths) |
| if directory.join(virtual_client_name).is_file() { | ||
| candidates.push(directory.clone()); | ||
| } |
|
|
||
| Run all Linux x64 VMM.Perf profiles with: | ||
|
|
||
| ```text |
| 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)"); | ||
| } |
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 } |
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.
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.
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.
|
|
||
| //! VMM.Perf profiles executed through the Petri/nextest test harness. |
There was a problem hiding this comment.
What gap does this solve that the burette crate doesn't? See here
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.
There was a problem hiding this comment.
We talked about this before, and I think it is ok to use VMM.Perf for now as long as it is separate from the existing infrastructure. But in the future I think we should consider adding more VMM backends to Petri (currently it supports openvmm and hyper-v) and use that for in-repo perf testing.
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
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, |
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!( |
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.
| 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(()) | ||
| } |
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.
There was a problem hiding this comment.
I would think treat this module as the vmm test orchestrator which invokes into the VirtualClient Lifecycle.
Add VMM.Perf runtime integration, configurable VM shapes, host validation, diagnostic collection, and metrics publication for Linux KVM and MSHV PR jobs.
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);
| self.runtime.root(), | ||
| &directories.data_dir, | ||
| &directories.temp_dir, | ||
| &directories.config_output_dir, | ||
| &directories.profile_work_dir, |
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
| let status = Command::new("tar") | ||
| .args(["-xf"]) | ||
| .arg(archive) | ||
| .arg("-C") | ||
| .arg(&staging) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
vmm_tests/vmm_perf/src/virtual_client.rs:64
WorkDiris treated as a base directory (it is removed fromcustom_parametersand used to create a new temp work dir under it), which makes it impossible to pass an explicit VirtualClientWorkDirand is confusing given the parameter name. Consider introducing a separate parameter key (e.g.WorkDirBase) for the base directory and leavingWorkDiras the actual work dir passed to VirtualClient (or documenting thatWorkDiris reserved/repurposed here).
let mut custom_parameters = request.config.parameters;
let work_dir_base = custom_parameters.remove("WorkDir");
let directories = RunDirectories::prepare(
request.profile,
&request.config.name,
request.output_dir,
request.temp_dir,
request.runtime.root(),
work_dir_base.as_deref(),
)?;
vmm_tests/vmm_perf/src/runtime.rs:143
archive_signature()is based only on(size, mtime), so it can miss archive updates that keep the same size and have insufficient timestamp resolution (leading to reusing a stale extracted runtime). For a “stable” URL that can change over time, consider incorporating a content digest (e.g. SHA-256) into the signature, or otherwise verifying freshness (ETag/Last-Modified) so cache correctness doesn’t depend on filesystem timestamp granularity.
pub(crate) fn archive_signature(archive: &Path) -> anyhow::Result<String> {
let metadata = fs_err::metadata(archive)?;
let modified = metadata
.modified()
.context("failed to query VMM.Perf archive modification time")?
.duration_since(std::time::UNIX_EPOCH)
.context("VMM.Perf archive modification time predates the Unix epoch")?;
Ok(format!(
"size={};modified_nanos={}",
metadata.len(),
modified.as_nanos()
))
}
| for (arch, outputs) in requests_by_arch { | ||
| let filename = match arch { | ||
| CommonArch::X86_64 => "vmm-perf-linux-x64.tar.gz", | ||
| CommonArch::Aarch64 => "vmm-perf-linux-arm64.tar.gz", | ||
| }; | ||
| let url = format!( | ||
| "https://vmmperfartifactpublic.blob.core.windows.net/perfpackage/stable/{filename}" | ||
| ); | ||
|
|
|
|
||
| #![forbid(unsafe_code)] | ||
|
|
||
| #[cfg(any(target_os = "linux", target_os = "windows"))] |
There was a problem hiding this comment.
What is the point of all of these cfg's? linux and windows is all we test on currently.
There was a problem hiding this comment.
These guards were added after the macOS Clippy check failed. Although VMM.Perf only executes on Linux and Windows, the repository runs cargo clippy for all targets, so every workspace member including vmm_perf must compile for that target. The original unsupported-platform compile_error! failed the check, and the full host implementation cannot compile on macOS. I agree the repeated attributes are noisy, so I’ll consolidate them behind one supported-platform gate.
There was a problem hiding this comment.
What is the error on macOS? If it is relatively simple to fix, we should strive to make our code clippy-ok on macOS even if we don't build for it yet. If you can't fix the issue, then I think it would be better to add this crate to our clippy exclusions list for MacOS (Darwin) in flowey/flowey_lib_hvlite/src/_jobs/check_clippy.rs:171, and add to the comment above with why it can't be supported.
There was a problem hiding this comment.
The issue I was getting was the explicit compile_error! for non-Linux/Windows targets, along with several functions in vmm_perf/host.rs that only had Linux and Windows implementations. native_hypervisor_backend also had no return path for Darwin. This should be fixable by adding unsupported-platform implementations for those host-specific functions that return a clear error. The complete crate can then be checked by Clippy on macOS, so we would not need to add vmm_perf to the Darwin exclusion list.
There was a problem hiding this comment.
Yeah if you can just stub out the missing implementations, that would be great.
| (backend_hint, config), | ||
| (PipelineBackendHint::Github, PipelineConfig::Pr) | ||
| ); | ||
| let (mut pub_vmm_perf_gnu, use_vmm_perf_gnu) = if enable_vmm_perf { |
There was a problem hiding this comment.
I don't think it would hurt to always build the vmm-perf-runner even on PRs, it only takes about 30s to build both. Plus it would make this code a bit simpler.
There was a problem hiding this comment.
Yeah, that makes sense. I originally made the runner build conditional to avoid building and publishing an artifact when the corresponding VMM.Perf job was not enabled. I can enable it to download always.
| // exercising its dedicated jobs on the GitHub PR pipeline. | ||
| let enable_vmm_perf = matches!( | ||
| (backend_hint, config), | ||
| (PipelineBackendHint::Github, PipelineConfig::Pr) |
There was a problem hiding this comment.
This is what I meant here: (PipelineBackendHint::Github, PipelineConfig::Ci | PipelineConfig::PrRelease)
Then if you want to run the perf tests on a PR you manually add the "release-ci-required" tag to your PR and it will run.
There was a problem hiding this comment.
Ohh,Got it, thanks for clarifying. I’ll enable the VMM.Perf jobs for GitHub Ci and PrRelease instead of the normal Pr pipeline.
| Some(use_openvmm_vhost_musl.clone()); | ||
| vmm_tests_artifacts_linux_musl_x86.use_prep_steps = | ||
| Some(use_prep_steps.clone()); | ||
| if enable_vmm_perf { |
There was a problem hiding this comment.
This can just be unconditional even if you don't use it.
There was a problem hiding this comment.
Agreed. This condition was only there because the VMM.Perf jobs and their artifact handles were optional. But yeah having it unconditional makes sense.
| }); | ||
|
|
||
| if build_only { | ||
| ctx.emit_rust_step("finish VMM.Perf build", |ctx| { |
There was a problem hiding this comment.
You can use ctx.emit_side_effect_step for this
There was a problem hiding this comment.
Ah, I see emit_side_effect_step can express that the two builds must complete before resolving done, without emitting a no-op runtime Rust step. I can use that.
| @@ -0,0 +1,84 @@ | |||
| // Copyright (c) Microsoft Corporation. | |||
There was a problem hiding this comment.
maybe rename this module local_build_and_run_vmm_perf
There was a problem hiding this comment.
Yeah that seems accurate with the functionality.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec48d4d4-a0ec-4ddb-958e-e2646e09f96d
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
vmm_tests/vmm_perf/src/diagnostics.rs:108
copy_virtual_client_resultscan end up copying symlinks because it only treatsis_dir()specially. Since these logs/results come from an external runtime, ensure the entry is a regular file beforefs_err::copyto avoid following symlinks into arbitrary host paths.
for entry in fs_err::read_dir(&directory)? {
let entry = entry?;
let path = entry.path();
if entry.file_type()?.is_dir() {
pending.push_back(path);
continue;
}
| for entry in fs_err::read_dir(&directory)? { | ||
| let entry = entry?; | ||
| let path = entry.path(); | ||
| if entry.file_type()?.is_dir() { | ||
| pending.push_back(path); | ||
| continue; | ||
| } |
Preserve the upstream VMM-test pipeline refactor while building GNU and MUSL VMM.Perf runners for both Linux architectures. Benchmark execution remains limited to the supported x64 AMD KVM and Intel MSHV pools. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec48d4d4-a0ec-4ddb-958e-e2646e09f96d
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Guide/src/dev_guide/dev_tools/xflowey.md:24
- The new command examples are fenced as
text, but the Guide style expects command snippets to be labeled with an appropriate language (e.g.bash) for consistent rendering/highlighting. Update these code fences fromtexttobashthroughout this new VMM.Perf section.
```text
cargo xflowey vmm-perf
</details>
| fn prepare(request: VirtualClientRunRequest<'a>) -> anyhow::Result<Self> { | ||
| let mut custom_parameters = request.config.parameters; | ||
| let work_dir_base = custom_parameters.remove("WorkDir"); |
Enable the standalone x64 KVM and MSHV VMM.Perf jobs in the normal GitHub PR workflow so the integration can be validated without the release label. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec48d4d4-a0ec-4ddb-958e-e2646e09f96d
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
flowey/flowey_lib_hvlite/src/download_vmm_perf_runtime.rs:83
- The VMM.Perf runtime archive is downloaded from a public blob URL without any integrity verification (checksum/signature). That makes the CI pipeline vulnerable to corrupted or tampered downloads.
flowey::shell_cmd!(
rt,
"{azcopy} copy
{url}
{archive}
| .map(|(name, value)| { | ||
| anyhow::ensure!( | ||
| !name.trim().is_empty(), | ||
| "{context} contains an empty parameter name" | ||
| ); | ||
| Ok((name.clone(), scalar_to_string(&name, &value)?)) | ||
| }) |
| let staging = archive_parent.join(format!( | ||
| ".vmm-perf-runtime-extracting-{}", | ||
| std::process::id() | ||
| )); | ||
| if staging.exists() { | ||
| fs_err::remove_dir_all(&staging)?; | ||
| } | ||
| fs_err::create_dir_all(&staging)?; |
Normal PR benchmark execution is removed after validation, while CI and PR-release remain enabled. Parameter names are trimmed before VirtualClient lookup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec48d4d4-a0ec-4ddb-958e-e2646e09f96d
| const MIB: u64 = 1024 * 1024; | ||
| const MIB_PER_GIB: u64 = 1024; | ||
| const DEFAULT_CAPACITY_PERCENT: u64 = 80; | ||
|
|
| let mut log_file = log_file.lock(); | ||
| writeln!(log_file, "[{stream_name}] {line}") | ||
| .with_context(|| format!("failed to write VMM.Perf {stream_name} console output"))?; | ||
| log_file.flush().with_context(|| { | ||
| format!("failed to flush VMM.Perf {stream_name} console log output") | ||
| })?; |
|
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”),
|
Add VMM.Perf to the existing Petri and nextest VMM-test pipeline.
This change:
x64-linux-amd-kvmandx64-linux-intel-mshvjob for now.