Skip to content

<feature>[kvm]: ZSTAC-82672 add iothread vq mapping data path#4071

Open
MatheMatrix wants to merge 1 commit into
feature-5.5.28-zbsfrom
sync/haidong.pang/feature/ZSTAC-82672-iothread-vq-mapping/rewrite@@3
Open

<feature>[kvm]: ZSTAC-82672 add iothread vq mapping data path#4071
MatheMatrix wants to merge 1 commit into
feature-5.5.28-zbsfrom
sync/haidong.pang/feature/ZSTAC-82672-iothread-vq-mapping/rewrite@@3

Conversation

@MatheMatrix
Copy link
Copy Markdown
Owner

Summary

Implement the mainline KVM data path needed by the unified IOThread VQ mapping feature.

Changes

  • Add qemu-kvm package version to KVM host facts and persist it as a host system tag.
  • Add VolumeTO.ioThreadMax so premium policy can pass target IOThread limits to KVM Agent.
  • Keep mainline changes limited to generic fact/tag and DTO data path support.

Resolves: ZSTAC-82672

sync from gitlab !9970

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

新增 HostFactResponse.qemuKvmPackageVersion(灰度 5.5.28),在 KVMSystemTags 中新增对应系统标签常量并在保存主机事实时写入或删除该标签;在 KVMConstant 中新增 iothread/vq 映射最低版本常量;在 VolumeTO 中增加 ioThreads 字段及其复制构造与访问器;新增集成测试验证标签清除逻辑。

Changes

QEMU/KVM 包版本跟踪

Layer / File(s) Summary
系统标签常量定义
plugin/kvm/src/main/java/org/zstack/kvm/KVMSystemTags.java
新增 QEMU_KVM_PACKAGE_VERSION_TOKENQEMU_KVM_PACKAGE_VERSION(pattern: qemu-kvm::package::version::{version},作用域 HostVO)。
代理响应数据模型
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
HostFactResponse 新增 private String qemuKvmPackageVersion(@GrayVersion 5.5.28)及其 getQemuKvmPackageVersion() / setQemuKvmPackageVersion(String) 方法。
主机事实数据持久化
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
在保存主机事实逻辑中,当 ret.getQemuKvmPackageVersion() 或 libvirt 版本字符串非空时,trim 后通过 createTagWithoutNonValue 写入对应系统标签(inherent=false);若为空则删除标签。

VolumeTO IO 线程配置与版本常量

Layer / File(s) Summary
iothread 最低版本常量与 VolumeTO 扩展
plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java, plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java
新增 MIN_QEMU_KVM_IOTHREAD_VQ_MAPPING_PACKAGE_VERSIONMIN_LIBVIRT_IOTHREAD_VQ_MAPPING_PACKAGE_VERSION 常量;在 VolumeTO 中新增私有字段 ioThreads(注:0 表示禁用自动 IOThread VQ 映射)、在复制构造中拷贝该字段,并增加 getIoThreads() / setIoThreads(int) 方法。

测试

Layer / File(s) Summary
AddHostCase 测试新增
test/src/test/groovy/org/zstack/test/integration/kvm/host/AddHostCase.groovy
新增 testPackageVersionTagsClearedWhenFactMissing() 测试,并在 test() 中加入调用;新增 KVMSystemTags import,用于断言包版本标签在主机事实缺失时被清除。

🎯 3 (Moderate) | ⏱️ ~20 minutes

"我是只小兔子,代码跳又轻,
包版本悄悄写,线程悄悄行,
主机回包带新名,卷对象多一声,
小改稳又短,审阅不费力 🐇"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题遵循规定格式 [scope]: ,包含JIRA键ZSTAC-82672,长度61字符,符合72字符以内的要求,准确概括了PR的主要变更。
Description check ✅ Passed PR描述清晰地说明了变更目的,包括添加QEMU-KVM包版本、VolumeTO.ioThreadMax字段等,与变更内容高度相关且提供了充分的背景信息。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/haidong.pang/feature/ZSTAC-82672-iothread-vq-mapping/rewrite@@3

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

@MatheMatrix MatheMatrix force-pushed the sync/haidong.pang/feature/ZSTAC-82672-iothread-vq-mapping/rewrite@@3 branch from 7241a03 to 5da32cb Compare May 24, 2026 09:47
Copy link
Copy Markdown

@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)
plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java (1)

84-84: ⚡ Quick win

澄清 VolumeTO.ioThreads 默认值 0 的语义:当前代码无外部依赖,建议补充字段说明

  • plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.javaioThreads 仅存在字段声明、复制构造 this.ioThreads = other.ioThreads;、以及 getIoThreads/setIoThreads,全仓库未发现任何 getIoThreads() 的调用点或基于 ioThreads(含与 0 的比较)的业务判断逻辑。
  • 因此目前不存在因默认 0 产生行为差异的依赖;但建议在字段/注释或相关参数说明中明确 0 表示的语义(未设置/等价默认限制/无限制等),避免后续接入误用。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java` at line 84, The
VolumeTO.ioThreads field's default value of 0 has no external use now but its
semantics should be explicit to avoid future misuse: update VolumeTO by adding a
JavaDoc comment on the ioThreads field (and matching javadoc on
getIoThreads/setIoThreads) that states what 0 means (e.g., "0 = unset / use host
default / no limit" — choose the correct semantics for your design), and ensure
the copy constructor assignment (this.ioThreads = other.ioThreads;) preserves
that meaning; reference the VolumeTO class, the ioThreads field, and the
getIoThreads/setIoThreads methods when making the clarification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java`:
- Line 54: The PR text references a field named ioThreadMax but the VolumeTO
class actually declares private int ioThreads; decide whether to align code or
docs: either rename the field in VolumeTO from ioThreads to ioThreadMax (update
any getters/setters, constructors, serialization uses and references to
VolumeTO) or update the PR description and any related docs/tests to mention
ioThreads; locate VolumeTO and its accessor methods to apply the rename or
update references elsewhere in the diff so names remain consistent across the
codebase and PR description.

---

Nitpick comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java`:
- Line 84: The VolumeTO.ioThreads field's default value of 0 has no external use
now but its semantics should be explicit to avoid future misuse: update VolumeTO
by adding a JavaDoc comment on the ioThreads field (and matching javadoc on
getIoThreads/setIoThreads) that states what 0 means (e.g., "0 = unset / use host
default / no limit" — choose the correct semantics for your design), and ensure
the copy constructor assignment (this.ioThreads = other.ioThreads;) preserves
that meaning; reference the VolumeTO class, the ioThreads field, and the
getIoThreads/setIoThreads methods when making the clarification.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)

Review profile: CHILL

Plan: Pro

Run ID: 06c45ba1-3c8d-4f24-8e90-66017521253e

📥 Commits

Reviewing files that changed from the base of the PR and between 7241a03 and 5da32cb.

📒 Files selected for processing (4)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMSystemTags.java
  • plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java

Comment thread plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java
@zstack-robot-1
Copy link
Copy Markdown
Collaborator

Comment from yaohua.wu:

Review: MR !9970 — ZSTAC-82672

Background

  • Jira: ZSTAC-82672 — ZSTAC-82319 引入底层组件升级,需Cloud侧结合现有功能进行适配(IOThread VQ Mapping)
  • Bug summary: 引入底层 qemu-kvm / libvirt 升级以提升 ZBS 读写性能,需要 Cloud 侧支持 IOThread VQ Mapping(按 queue 自动分配多个 iothread);本 MR 是主线 KVM 数据通路改造。
  • Intent & scope: 主线只动通用的 fact 上报、System Tag 定义、DTO 字段(VolumeTO.ioThreads)。具体策略(何时启用、计算上限)在 premium,运行时 XML 生成与生命周期管理在 zstack-utility。Scope 极小(4 文件 / ~25 行)。

关联 MR

  • premium !14049 — 策略层:VolumeIothreadVqMappingPolicy/Manager/ResolverKvmHostIothreadVqMappingCapability(基于 qemu-kvm 包版本判定 host 能力)、Host Allocator filter、迁移 ExtensionPoint,并通过 to.setIoThreads(...) 写入本 MR 新增的 VolumeTO.ioThreads
  • zstack-utility !7116 — KVM Agent 侧:上报 qemuKvmPackageVersion(仅 yum),IothreadVqMappingAllocator 生成 <iothreads><iothread id=... ><queue id=N-M/></iothread></iothreads> libvirt XML,热插拔/迁移期的 iothread 生命周期清理

P3 — Low / Suggestion

# File:Line Issue Fix
1 plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java:54 private int ioThreads; 默认值 0 的语义未在字段处注明。从 premium / agent 代码看,0 表示"不启用自动 mapping"(needs_automatic_mapping 要求 ioThreads > 0),但读这个 DTO 的人无法在本地直接确认这一点 加一行 Javadoc:// 0 表示未启用自动 IOThread VQ Mapping;非 0 时 KVM Agent 据此从 101 起分配 iothread 并按 queue 数分摊
2 plugin/kvm/src/main/java/org/zstack/kvm/KVMSystemTags.java:21-22 QEMU_KVM_PACKAGE_VERSION_TOKEN = "version" 与上一行 QEMU_IMG_VERSION_TOKEN = "version" 同名(仅常量名不同)。当前未引发冲突,但如果将来用 getTokenByResourceUuid(uuid, "version") 之类的纯字符串调用会有歧义 现状可接受。已有 codebase 都按常量引用,无需变更,仅注意

Coverage

  • 跨仓核对:VolumeTO.ioThreads 的唯一 producer 是 premium VolumeIothreadVqMappingManager.setVolumeIothreads(在 beforeStartVmOnKvm / beforeAttachVolume / convertVolumeIfNeed 三处调用 + 迁移时 KvmStorageLiveMigrationFlowChain.inheritRuntimeVolumeConfig 复制),唯一 consumer 是 zstack-utility IothreadVqMappingAllocator.needs_automatic_mapping。链路完整。
  • qemu-kvm::package::version System Tag 的写入路径:本 MR KVMHost.java:6333-6335if (ret.getQemuKvmPackageVersion() != null) 守卫与 zstack-utility 在 apt-based 主机上 qemuKvmPackageVersionNone 的事实一致 —— Tag 在 apt 主机上不会写入,从而 premium 的 KvmHostIothreadVqMappingCapability.supported(...) 返回 false,apt 主机被 allocator 静默过滤。该问题已在 premium / utility 两个 MR 的 review 中详细提出,本 MR 不需变更。
  • @GrayVersion("5.5.28") 与 premium grayUpgrade.json 一致。

Verdict: APPROVED

主线侧改动极小、纯 DTO/Tag 拓展,无副作用,可合入。真正的复杂度集中在 premium 与 zstack-utility 两个关联 MR,本 MR 仅作为数据通路存在。


🤖 Robot Reviewer

@MatheMatrix MatheMatrix force-pushed the sync/haidong.pang/feature/ZSTAC-82672-iothread-vq-mapping/rewrite@@3 branch from 5da32cb to fc63a59 Compare May 25, 2026 04:01
@ZStack-Robot
Copy link
Copy Markdown
Collaborator

Comment from haidong.pang:

感谢 review,已按意见处理:

  1. VolumeTO.ioThreads:已补充字段注释,明确 0 表示不启用 automatic IOThread VQ Mapping。
  2. QEMU_KVM_PACKAGE_VERSION_TOKENQEMU_IMG_VERSION_TOKEN 都叫 version:这里保持不改。两个 token 分别挂在不同的 PatternedSystemTag pattern 下,当前代码也都通过各自常量读取,不存在实际解析冲突。

主线侧仍保持只提供 fact/tag/DTO 数据通路,策略和行为边界放在 premium / utility。

@MatheMatrix MatheMatrix force-pushed the sync/haidong.pang/feature/ZSTAC-82672-iothread-vq-mapping/rewrite@@3 branch 4 times, most recently from 35e3eae to 3d99feb Compare May 29, 2026 09:57
Resolves: ZSTAC-82672

Change-Id: Ib43a7a02a0528cc40cd8aa370b93b7fba7dfc051
@MatheMatrix MatheMatrix force-pushed the sync/haidong.pang/feature/ZSTAC-82672-iothread-vq-mapping/rewrite@@3 branch from 3d99feb to cff7e71 Compare May 29, 2026 10:04
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.

3 participants