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

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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
@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.

2 participants