Skip to content

Conversation

xiagao
Copy link
Contributor

@xiagao xiagao commented Jul 5, 2025

Create a test case about windows updatete performance, and complete the first scenario of vbs support.
ID: 3338

Summary by CodeRabbit

  • New Features
    • Added a Windows virtualization performance test validating Secure Boot, Device Guard/VBS readiness, nested virtualization, Hyper‑V/IOMMU checks, and WSL2 setup with a RHEL distribution.
  • Tests
    • Introduced configurable IOMMU variants (enabled/disabled), including an Intel‑only path with advanced virtio/IOMMU settings.
    • Automated setup, reboots, and verification steps with clearer logging and failure reporting.
    • Supports recent Windows Server versions and UEFI/q35 environments for broader coverage.

Create a test case about windows updatete performance, and
complete the first scenario of vbs support.

Assisted-by: Claude Code.Approximately 50% of the content
was generated using it.

Signed-off-by: Xiaoling Gao <[email protected]>
Copy link

coderabbitai bot commented Sep 25, 2025

Walkthrough

Adds a new QEMU Windows virtualization performance test: a configuration file defining parameters and variants, and a Python test module implementing the test flow, including secure boot/VBS checks, Device Guard readiness tool execution, WSL2 installation with a RHEL distro, nested virtualization/IOMMU options, and reboot/session orchestration.

Changes

Cohort / File(s) Summary of Changes
QEMU test configuration
qemu/tests/cfg/win_virtio_perf_test.cfg
Introduces a Windows performance test config with constraints (q35, OVMF, Win2022/Win2025), runtime controls, nested virtualization flags, Hyper-V/IOMMU checks, DG Readiness Tool paths/commands, VBS-related commands, WSL2 enablement and RHEL WSL distro setup, and parameters for no_iommu vs iommu_enable (Intel-restricted) with device/IOMMU options.
Windows virtio perf test module
qemu/tests/win_virtio_perf_test.py
Adds a new test module exposing run(test, params, env). Implements sequence: secure boot check, copy/run DG Readiness Tool, VBS readiness verification, WSL2 and RHEL distro installation, controlled reboots, and cleanup. Includes helper functions, logging, error handling with test.fail, and session/VM management.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Tester as Avocado/Test Runner
  participant QEMU as QEMU VM
  participant Guest as Windows Guest
  participant PS as PowerShell/Tools
  participant WSL as WSL2/RHEL Distro

  Tester->>QEMU: Boot VM (params: CPU/IOMMU/nesting)
  QEMU->>Guest: Start guest OS
  Tester->>Guest: Open session
  Note over Guest,PS: Set execution policy, prepare paths
  Tester->>PS: Check Secure Boot / VBS readiness
  Tester->>Guest: Copy DG Readiness Tool
  Tester->>PS: Run DG commands (dg_command)
  alt VBS enabled/required
    Tester->>QEMU: Reboot VM (sync)
    QEMU->>Guest: Guest up
    Tester->>Guest: Reconnect session
  end
  Tester->>PS: Enable WSL + VM Platform, set default v2
  Tester->>WSL: Install RHEL distro
  Note right of WSL: Validate distro availability
  Tester-->>QEMU: Optional VBS disable and reboot
  Tester->>Guest: Final checks and cleanup
  Tester-->>Tester: Report result (pass/fail)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I tapped my paws on the hypervisor’s door,
Spun up a VM—then a warren of more.
Device Guard nodded, WSL2 cheered,
RHEL hopped in, exactly as geared.
With IOMMU trails and VBS bright,
This bun benchmarks through the night. 🐇⚙️

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title “windows_update_perf: support VBS” does not accurately summarize the main change, which is the addition of a Windows virtio performance test (win_virtio_perf_test) with VBS support rather than a Windows “update” performance scenario. It mislabels the test suite and may mislead reviewers about the nature of the changes. Rename the title to clearly reference the new virtio performance test and VBS support, for example “tests: add Windows virtio performance test with VBS support.”
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
qemu/tests/cfg/win_virtio_perf_test.cfg (1)

21-23: Keep the ExecutionPolicy relaxation scoped to the session

Set-ExecutionPolicy without -Scope defaults to LocalMachine, permanently dropping the host’s PowerShell policy to Unrestricted. That weakens security for subsequent scenarios on the same worker. Please either scope it to the current process (-Scope Process -ExecutionPolicy Bypass) or restore the original policy once the readiness script finishes so the machine isn’t left wide open.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 870c1ea and a65f937.

📒 Files selected for processing (2)
  • qemu/tests/cfg/win_virtio_perf_test.cfg (1 hunks)
  • qemu/tests/win_virtio_perf_test.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
qemu/tests/win_virtio_perf_test.py (1)
qemu/deps/win_driver_install/win_driver_install.py (1)
  • cmd_output (12-28)

Comment on lines +24 to +35
"""
Set PowerShell execution policy using the provided session.
It is used when creating a new session.

:param cmd: The PowerShell command to set execution policy.
"""
error_context.context("Setting PowerShell execution policy.")
status, output = session.cmd_status_output(executionPolicy_command)
if status != 0:
test.fail("Failed to set PowerShell execution policy: %s" % output)

def check_secure_boot_enabled():
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix PowerShell policy helper to use the active session

After the WSL reboot we create new_session, but set_powershell_execute_policy() still dereferences the old session captured from the outer scope. That handle is already invalid once the guest reboots, so the execution-policy command either fails outright or silently never updates the new session, breaking the rest of the workflow. Pass the live session object into the helper so that every call (including the one right after the reboot) runs against the correct connection.

@@
-    def set_powershell_execute_policy():
-        """
-        Set PowerShell execution policy using the provided session.
-        It is used when creating a new session.
-
-        :param cmd: The PowerShell command to set execution policy.
-        """
+    def set_powershell_execute_policy(current_session):
+        """
+        Set PowerShell execution policy on the given session.
+        """
@@
-        status, output = session.cmd_status_output(executionPolicy_command)
+        status, output = current_session.cmd_status_output(executionPolicy_command)
@@
-        set_powershell_execute_policy()
+        set_powershell_execute_policy(session)
@@
-            set_powershell_execute_policy()
+            set_powershell_execute_policy(session)
@@
-        set_powershell_execute_policy()
+        set_powershell_execute_policy(new_session)

Also applies to: 111-114, 174-183

🤖 Prompt for AI Agents
In qemu/tests/win_virtio_perf_test.py around lines 24-35 (also apply same change
at 111-114 and 174-183), the helper set_powershell_execute_policy() closes over
the old session variable so after WSL reboot it uses a stale/invalid session;
change the helper signature to accept a session parameter and use that passed-in
session for session.cmd_status_output(executionPolicy_command) (and update all
callers to pass the current/live session, including immediately after creating
new_session) so the execution policy runs against the active connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant