-
Notifications
You must be signed in to change notification settings - Fork 0
<fix>[kvm]: force virtio driver for windows vhostuser nic to fix boot failure #3213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough在 Windows 平台启动虚拟机时新增运行时网卡驱动检查:若镜像平台为 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🔍 Remote MCP AtlassianMCPRelevant context for reviewing PR
|
There was a problem hiding this 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
🤖 Fix all issues with AI agents
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 4520-4535: The code currently reads platform from
spec.getVmInventory().getPlatform() into currentPlatform, which misses cases
where platform is null but an earlier-resolved variable platform (with image
fallback) exists; update the check to reuse the previously resolved platform
variable (use platform instead of
currentPlatform/spec.getVmInventory().getPlatform()) in the conditional that
enforces vhostuser NIC driver to virtio (affecting cmd.getNics(), NicTO,
VmNicType.VmNicSubType.VHOSTUSER and VmNicDriverType.VIRTIO logic) so Windows
images detected via the image fallback are correctly handled.
📜 Review details
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: - 代码里不应当有有中文,包括报错、注释等都应当使用正确的、无拼写错误的英文来写
Files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: ## 1. API 设计要求
- API 命名:
- API 名称必须唯一,不能重复。
- API 消息类需要继承
APIMessage;其返回类必须继承APIReply或APIEvent,并在注释中用@RestResponse进行标注。- API 消息上必须添加注解
@RestRequest,并满足如下规范:
path:
- 针对资源使用复数形式。
- 当 path 中引用消息类变量时,使用
{variableName}格式。- HTTP 方法对应:
- 查询操作 →
HttpMethod.GET- 更新操作 →
HttpMethod.PUT- 创建操作 →
HttpMethod.POST- 删除操作 →
HttpMethod.DELETE- API 类需要实现
__example__方法以便生成 API 文档,并确保生成对应的 Groovy API Template 与 API Markdown 文件。
2. 命名与格式规范
类名:
- 使用 UpperCamelCase 风格。
- 特殊情况:
- VO/AO/EO 类型类除外。
- 抽象类采用
Abstract或Base前缀/后缀。- 异常类应以
Exception结尾。- 测试类需要以
Test或Case结尾。方法名、参数名、成员变量和局部变量:
- 使用 lowerCamelCase 风格。
常量命名:
- 全部大写,使用下划线分隔单词。
- 要求表达清楚,避免使用含糊或不准确的名称。
包名:
- 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
命名细节:
- 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
- 命名缩写:
- 不允许使用不必要的缩写,如:
AbsSchedulerJob、condi、Fu等。应使用完整单词提升可读性。
3. 编写自解释代码
意图表达:
- 避免使用布尔型参数造成含义不明确。例如:
- 对于
stopAgent(boolean ignoreError),建议拆分为不同函数(如stopAgentIgnoreError()),或使用枚举表达操作类型。- 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
- 避免使用魔法值(Magic Value):
直接使用未经定义的数值或字符串(如 if (status == 5))应替换为枚举或常量。
示例:
// 错误示例:魔法值
if (user.getStatus() == 5) { ... }
// 正确示例:常量或枚举
public static final int STATUS_ACTIVE = 5;
if (user.getStatus() == STATUS_ACTIVE) { ... }
// 或使用枚举
enum UserStatus { ACTIVE, INACTIVE }
注释:
- 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
- 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
- 接口方法不应有多余的修饰符(例如
public),且必须配有有效的 Javadoc 注释。
4. 流程控制和结构优化
if...else 的使用:
- 应尽量减少 if...else 结构的使用,建议:
- 限制嵌套层级最多为两层,且内层不应再出现
else分支。- 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
- 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
条件判断:
- if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
代码块长度:
...
Files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
🧠 Learnings (3)
📚 Learning: 2025-10-20T11:27:25.928Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2763
File: sdk/src/main/java/org/zstack/sdk/UpdateHostKernelInterfaceAction.java:31-31
Timestamp: 2025-10-20T11:27:25.928Z
Learning: UpdateHostKernelInterface API (`sdk/src/main/java/org/zstack/sdk/UpdateHostKernelInterfaceAction.java`) 在 PR `#2763` 时尚未被实际使用,因此对其参数约束的变更(如 name 字段从必需改为可选)不会造成破坏性影响。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 Learning: 2025-08-12T05:39:14.846Z
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2418
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:3856-3865
Timestamp: 2025-08-12T05:39:14.846Z
Learning: 在 compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java 的 setNoIpAllocationStaticIp 流程中,从数据库加载的 VmNicVO 实例其 getUsedIps() 为空时也不会为 null(返回空集合)。因此无需对 getUsedIps() 再做 Optional/空列表归一化的空指针保护;若找不到对应 NIC,使用 orElse(new VmNicVO()) 的约定允许后续逻辑通过 NPE 暴露问题,与项目既有约定一致。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
📚 Learning: 2025-08-10T13:42:01.027Z
Learnt from: MatheMatrix
Repo: MatheMatrix/zstack PR: 2410
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceHelper.java:334-356
Timestamp: 2025-08-10T13:42:01.027Z
Learning: 在 ZStack 的 `VmInstanceHelper.validateVmNicParams` 方法中调用 `VmNicParamValidator` 时,不需要对 msg.getType() 返回 null 的情况进行兼容处理,因为 vmType 为 null 的情况已在内部(VmNicParamValidator 或其他地方)得到处理。
Applied to files:
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
🧬 Code graph analysis (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
header/src/main/java/org/zstack/header/vm/VmNicType.java (1)
VmNicType(5-65)
🔍 Remote MCP AtlassianMCP
Summary of Additional Context Found,,
Jira Issue ZSTAC-76013 - Core Problem Context
Issue Summary: "【5.5.6必须解】创建windows云主机加载ovs-dpdk网络,创建失败" (Creating Windows VMs with OVS-DPDK networks fails - Critical for 5.5.6)
Key Details:
- Priority: P1 (Critical)
- Severity: Critical
- Status: In Progress
- Assignee: 刘德靖 (Dejing Liu) - same as PR author
- Created: June 26, 2025
- Issue Type: Bug
Problem Statement:
The issue occurs when creating Windows VMs connected to OVS-DPDK networks. OVS-DPDK relies on vhost-user interface mode, which requires the 'virtio-net' driver on the frontend (guest side) in libvirt/QEMU, but ZStack defaults Windows NICs to 'e1000' or other emulated models, resulting in libvirt errors like "vhostuser requires the virtio-net* frontend".
Reproduce Steps:
Creating a Windows VM using an OVS-DPDK network with a Windows 2019 image (http://minio.zstack.io:9001/download/guest_os/windows2019_qcow2_x86_apps.qcow2) that does not have virtio drivers enabled.
Version Target: 5.5.6 (marked as must-resolve for this release as of January 4, 2026)
Related Vhost-user Issues Context
The search revealed multiple related vhost-user/vhost-user-blk issues in the codebase:
- ZSTAC-44058: vdpa and vhostuser resource cleanup issues after host restart
- ZBS-677: vhost-user-blk single-threaded performance issues
- ZSV-7560: VM import failure with message "vhostuser disk only support virtio mode"
- ZSTAC-60637: VM startup fails with "iotune is not supported with vhostuser disk"
This demonstrates that vhost-user compatibility is a critical infrastructure concern affecting multiple device types (network and disk).
Technical Background from Confluence
Documentation confirms that vhost-user architecture requires:
- Virtio frontend driver in the guest for vhost-user interfaces to function
- DPDK PMD drivers for high-performance data paths in both host user-space and guest space
- Proper kernel-space acceleration through vhost backend modules
The fix aligns with established vhost-user/virtio requirements that are documented throughout ZStack's knowledge base.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
f9b3c16 to
903219e
Compare
The vhost-user interface (used by OVS-DPDK) strictly requires the 'virtio-net' frontend driver in Libvirt/QEMU. However, ZStack defaults Windows NIC drivers to 'e1000' (or other emulated models) when the image platform is 'Windows', causing VM start failure with Libvirt error: "vhostuser requires the virtio-net* frontend". This fix forces the NIC driver to 'virtio' specifically when the interface type is 'vhostuser' on Windows platforms, regardless of the default platform settings. Satisfying both the legacy boot requirements of Windows and the vhost-user network requirements. Resolves: ZSTAC-76013 Change-Id: I737a6e637273796d776e726e7969786c7a6a6478
903219e to
c25f4ea
Compare
The vhost-user interface (used by OVS-DPDK) strictly requires the 'virtio-net' frontend driver in Libvirt/QEMU. However, ZStack defaults Windows NIC drivers to 'e1000' (or other emulated models) when the image platform is 'Windows', causing VM start failure with Libvirt error: "vhostuser requires the virtio-net* frontend".
This fix forces the NIC driver to 'virtio' specifically when the interface type is 'vhostuser' on Windows platforms, regardless of the default platform settings. Satisfying both the legacy boot requirements of Windows and the vhost-user network requirements.
Resolves: ZSTAC-76013
Change-Id: I737a6e637273796d776e726e7969786c7a6a6478
sync from gitlab !9032