Add OpenVMM virtio device support - #4621
Open
vyadavmsft wants to merge 5 commits into
Open
Conversation
Allow OpenVMM runbooks to select virtio-blk disks and virtio-net NICs over PCIe while retaining existing synthetic defaults. Add command, schema, and launch propagation tests.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds virtio device selection support for OpenVMM guests, allowing runbooks to choose virtio-blk disks and virtio-net NICs over PCIe while keeping the existing synthetic/SCSI defaults. This extends the OpenVMM launch command generation, updates the OpenVMM runbook schema to accept the new options, and adds selftests to validate command composition and runbook propagation.
Changes:
- Add
disk_deviceandnetwork_deviceoptions (with validation) and propagate them into OpenVMM launch configuration. - Extend OpenVMM command-line generation to emit PCIe + virtio device arguments when requested, while retaining current defaults.
- Add/extend selftests covering command output, schema loading, and launch propagation behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| selftests/test_openvmm_tool.py | Validates default vs virtio+PCIe command-line generation in OpenVmm.build_command(). |
| selftests/test_openvmm_schema.py | Ensures OpenVMM guest schema accepts virtio device selections. |
| selftests/test_openvmm_node.py | Verifies launch config propagation for disk/network device selections. |
| lisa/tools/openvmm.py | Adds device-selection constants, launch config fields, validation, and virtio/PCIe argument composition. |
| lisa/sut_orchestrator/openvmm/schema.py | Adds disk_device and network.device schema fields with validation and defaults. |
| lisa/sut_orchestrator/openvmm/node.py | Propagates runbook device selections into OpenVmmLaunchConfig. |
Collaborator
AI Test Case SelectionSelected 2 test case(s): verify_boot_with_debug_kernel,verify_serial_console Marketplace image: Result: Failed |
1 similar comment
Collaborator
AI Test Case SelectionSelected 2 test case(s): verify_boot_with_debug_kernel,verify_serial_console Marketplace image: Result: Failed |
Expose none, Intel VT-d, and AMD IOMMU selections for OpenVMM virtio PCIe guests while preserving the existing disabled default. Add an opt-in shared TAP subnet mode that keeps per-guest TAP interfaces and addresses while reusing one bridge, DHCP service, and lease database across guests. Track shared host networking resources with synchronized reference counts, serialize bridge netfilter state changes, roll back partial setup failures, and resolve DHCP leases through each TAP interface's learned MAC address. Extend focused command and network derivation coverage for the new configuration.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (4)
lisa/tools/openvmm.py:156
- Error message doesn’t list the supported network device values, which makes it harder to fix invalid configurations. Consider including the supported values in the exception message (consistent with the schema validation errors).
raise LisaException(
f"Unsupported OpenVMM network device: {config.network_device}"
)
lisa/tools/openvmm.py:162
- Error message doesn’t list the supported IOMMU values, which makes it harder to fix invalid configurations. Consider including the supported values in the exception message (consistent with the schema validation errors).
raise LisaException(f"Unsupported OpenVMM IOMMU: {config.iommu}")
lisa/tools/openvmm.py:149
- Error message doesn’t list the supported disk device values, which makes it harder to fix invalid configurations. Consider including the supported values in the exception message (consistent with the schema validation errors).
This issue also appears in the following locations of the same file:
- line 154
- line 162
raise LisaException(
f"Unsupported OpenVMM disk device: {config.disk_device}"
)
lisa/sut_orchestrator/openvmm/schema.py:113
- This PR introduces shared TAP subnet/resource-sharing behavior (shared_subnet) in addition to the virtio device selection described in the PR title/description. Please update the PR description to explicitly cover this new behavior (or split it into a separate PR) so reviewers can evaluate scope and risk appropriately.
class OpenVmmNetworkSchema:
mode: str = OPENVMM_NETWORK_MODE_USER
device: str = OPENVMM_NETWORK_DEVICE_SYNTHETIC
shared_subnet: bool = False
connection_mode: str = OPENVMM_CONNECTION_MODE_FORWARDED_PORT
Collaborator
AI Test Case SelectionSelected 1 test case(s): smoke_test Marketplace image: Result: Succeeded |
Expose a validated network.queue_count option for OpenVMM guests and pass it through the controller launch configuration. Emit the OpenVMM queues=N NIC prefix while preserving the existing default when omitted. Cover schema validation, command generation, and controller mapping with focused selftests.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
selftests/test_openvmm_node.py:177
- With schema defaults,
queue_countis omitted (None) unless explicitly configured. This assertion should match the default behavior for synthetic networking.
self.assertEqual(1, launch_config.network_queue_count)
lisa/sut_orchestrator/openvmm/node.py:2133
- When
shared_subnetsetup fails after iptables rules/dnsmasq state is created but before it’s copied intoSharedTapNetworkContext,_teardown_tap_networkskips cleanup becauseshared_tap_network_keyis set. This can leak host iptables rules and/or a dnsmasq process on setup errors.
Consider making teardown remove per-node iptables rules/dnsmasq when the shared context hasn’t yet recorded them (e.g., shared_context.input_rules_added / shared_context.dnsmasq_pid_file are empty).
if not shared_network_key and node_context.tap_input_rules_added:
rules_to_remove = node_context.tap_input_rules_added
for rule in rules_to_remove:
self.host_node.execute(
f"iptables -D {rule} || true",
selftests/test_openvmm_node.py:145
- This test configures a synthetic NIC but sets
queue_count=1. In the real schema,queue_countdefaults toNone, and forcing a default of 1 here makes the test validate a behavior that users won’t actually get by default.
This issue also appears on line 177 of the same file.
queue_count=1,
Add validated vps-per-socket and SMT launch controls. Default OpenVMM guests to one socket containing all requested virtual processors with SMT disabled, matching a 1x4x1 topology for four-vCPU performance runs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
lisa/sut_orchestrator/openvmm/node.py:755
- Major: vps_per_socket defaults to processor_count when unspecified, which forces --vps-per-socket to always be passed (changing the default OpenVMM command line for existing runbooks). If the intent is to keep existing behavior unless the runbook explicitly opts in, pass through runbook.vps_per_socket directly and let it remain None by default.
),
processors=processor_count,
vps_per_socket=(
runbook.vps_per_socket
if runbook.vps_per_socket is not None
else processor_count
),
smt=runbook.smt,
lisa/sut_orchestrator/openvmm/node.py:421
- Major: In create_effective_network(), the shared_subnet branch returns before validate_tap_interface_names() is called. Since tap_name is still suffixed per guest, shared_subnet can silently produce invalid interface names (e.g., exceeding max length or using invalid characters) without the usual validation/error context.
base_tap_host_cidr = effective_network.tap_host_cidr
effective_network.tap_name = _increment_name_suffix(
effective_network.tap_name, guest_index
)
if effective_network.shared_subnet:
if effective_network.address_mode == OPENVMM_ADDRESS_MODE_STATIC:
effective_network.guest_address = _increment_ip_address(
effective_network.guest_address,
effective_network.tap_host_cidr,
guest_index,
)
if effective_network.forward_ssh_port:
effective_network.forwarded_port += guest_index
if effective_network.forwarded_port > 65535:
raise LisaException(
"cannot derive OpenVMM forwarded SSH port from "
f"'{network.forwarded_port}' for guest index {guest_index}: "
"derived port exceeds 65535. Use a lower base forwarded_port."
)
return effective_network
lisa/tools/openvmm.py:168
- Major: _validate_processor_topology() doesn't validate the relationship between processors and vps_per_socket. As written, vps_per_socket can be greater than processors or not divide evenly into processors, which is likely to produce an invalid or surprising CPU topology at runtime.
def _validate_processor_topology(self, config: OpenVmmLaunchConfig) -> None:
if config.vps_per_socket is not None and config.vps_per_socket < 1:
raise LisaException(
"OpenVMM vps_per_socket must be at least 1. "
"Set it to the number of virtual processors in each socket."
)
if config.smt and config.smt not in [
OPENVMM_SMT_AUTO,
OPENVMM_SMT_FORCE,
OPENVMM_SMT_OFF,
]:
raise LisaException(
f"OpenVMM SMT mode '{config.smt}' is not supported. "
f"Use {OPENVMM_SMT_AUTO}, {OPENVMM_SMT_FORCE}, or "
f"{OPENVMM_SMT_OFF}."
)
Mark the scheduled parent environment as used when dispatching a transient guest wrapper. This prevents use_new_environment cases from reusing the same parent while preserving normal environment reuse and retry resets.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (4)
lisa/sut_orchestrator/openvmm/schema.py:369
- Major:
OpenVmmGuestNodeSchema.__post_init__rejects an empty SMT value. Ifsmtis intended to be optional (empty meaning "use OpenVMM default"), gate the validation onself.smtbeing non-empty (mirrorsOpenVmmLaunchConfigvalidation).
if self.smt not in [OPENVMM_SMT_AUTO, OPENVMM_SMT_FORCE, OPENVMM_SMT_OFF]:
lisa/sut_orchestrator/openvmm/schema.py:317
- Major:
OpenVmmGuestNodeSchema.smtdefaults toOPENVMM_SMT_OFF, which means OpenVMM launches will always emit--smt offeven when the runbook doesn't specify SMT. To preserve existing OpenVMM defaults unless explicitly configured, make the schema default empty and rely on the tool layer to validate non-empty values.
This issue also appears on line 369 of the same file.
smt: str = OPENVMM_SMT_OFF
lisa/sut_orchestrator/openvmm/node.py:754
- Major:
launch()always forcesvps_per_sockettoprocessor_countwhen the runbook value is unset, which makes the generated OpenVMM command always include--vps-per-socketand changes default behavior. If the intent is to preserve existing defaults unless explicitly configured, pass throughrunbook.vps_per_socketas-is (None) and letOpenVmmLaunchConfigomit the flag.
vps_per_socket=(
runbook.vps_per_socket
if runbook.vps_per_socket is not None
else processor_count
),
lisa/sut_orchestrator/openvmm/schema.py:122
- Minor: This PR adds
shared_subnetsupport and shared TAP resource management, but the PR title/description focuses on virtio device selection. Please update the PR description (and/or title) to mention the shared-subnet feature and runner behavior change so reviewers understand the full scope and can validate appropriately.
shared_subnet: bool = False
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.
Allow OpenVMM runbooks to select virtio-blk disks and virtio-net NICs over PCIe while retaining existing synthetic defaults. Add command, schema, and launch propagation tests.
Description
Related Issue
Type of Change
Checklist
Test Validation
Key Test Cases:
Impacted LISA Features:
Tested Azure Marketplace Images:
Test Results