<feature>[kvm]: ZSTAC-82672 add iothread vq capability#4025
Open
zstack-robot-2 wants to merge 1 commit into
Open
<feature>[kvm]: ZSTAC-82672 add iothread vq capability#4025zstack-robot-2 wants to merge 1 commit into
zstack-robot-2 wants to merge 1 commit into
Conversation
Walkthrough新增 HostFactResponse 中的 qemuKvmPackageVersion 字段与 IOTHREAD_VQ_MAPPING 系统标签常量，新增 KvmHostIothreadVqMappingCapability 进行版本判定，并在保存主机事实时依据判定创建或删除该标签，含单元测试覆盖。 sequenceDiagram
participant saveKvmHost as saveKvmHostRelatedFacts
participant refresh as refreshIothreadVqMappingCapability
participant capability as KvmHostIothreadVqMappingCapability
participant tag as KVMSystemTags.IOTHREAD_VQ_MAPPING
saveKvmHost->>refresh: HostFactResponse (qemuKvmPackageVersion, libvirtPackageVersion)
refresh->>capability: supported(qemuVersion, libvirtVersion)
capability-->>refresh: boolean
alt 版本满足
refresh->>tag: 重建非固有标签
else 版本不满足
refresh->>tag: 删除标签
end
🎯 2 (Simple) | ⏱️ ~12 分钟 变更说明IO线程VQ映射能力检查
总体描述此 PR 为 KVM 主机添加了 IO 线程 VQ 映射能力的版本检查机制。在主机事实采集时，根据 QEMU-KVM 和 libvirt 包版本的支持情况，动态管理 诗歌
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.2)plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaComment |
MatheMatrix
force-pushed
the
sync/haidong.pang/feature/ZSTAC-82672-iothread-vq-mapping@@3
branch
from
4e5a91e to
a18fd36
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KvmHostIothreadVqMappingCapability.java (1)
27-38: ⚡ Quick win建议为版本规范化逻辑添加注释说明。
normalizeVersion方法的处理逻辑较为复杂，涉及空白符分割、按-拆段、过滤非数字开头段等操作。建议添加注释说明其设计意图，例如：
- 为何取首个空白分隔的 token（处理可能包含多余空格或其他信息的版本字符串）
- 为何过滤掉非数字开头的段（移除 "rc"、"beta" 等非版本号的限定符）
- 边界情况处理（当所有段都被过滤后返回原始 trimmed 值）
这将有助于后续维护者理解代码意图，避免误改。
💡 建议的注释示例
private static String normalizeVersion(String version) { if (StringUtils.isBlank(version)) { return null; } + // Extract first token and remove non-numeric prefixes (e.g., "rc", "beta", "el") + // to get a clean version string for comparison String trimmed = version.trim().split("\\s+")[0]; String normalized = Arrays.stream(trimmed.split("-")) .filter(it -> !it.isEmpty()) .filter(it -> Character.isDigit(it.charAt(0))) .collect(Collectors.joining("-")); + // If all segments are filtered out, fall back to original trimmed version return StringUtils.isBlank(normalized) ? trimmed : normalized; }🤖 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/KvmHostIothreadVqMappingCapability.java` around lines 27 - 38, Add explanatory JavaDoc and inline comments to the normalizeVersion method: document that it first returns null for blank input, then trims and takes the first whitespace-separated token to ignore trailing metadata, explain splitting on '-' and filtering out empty segments and segments that don't start with a digit (to remove qualifiers like "rc"/"beta"), and note the fallback behavior that if all segments are filtered out it returns the original trimmed token; reference the method name normalizeVersion and its internal steps (trimmed token, split on "-", filter by Character.isDigit) so future maintainers understand the intent and edge-case handling.
🤖 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.
Nitpick comments:
In
`@plugin/kvm/src/main/java/org/zstack/kvm/KvmHostIothreadVqMappingCapability.java`:
- Around line 27-38: Add explanatory JavaDoc and inline comments to the
normalizeVersion method: document that it first returns null for blank input,
then trims and takes the first whitespace-separated token to ignore trailing
metadata, explain splitting on '-' and filtering out empty segments and segments
that don't start with a digit (to remove qualifiers like "rc"/"beta"), and note
the fallback behavior that if all segments are filtered out it returns the
original trimmed token; reference the method name normalizeVersion and its
internal steps (trimmed token, split on "-", filter by Character.isDigit) so
future maintainers understand the intent and edge-case handling.
ℹ️ 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: e61731a1-b927-4b2a-a268-6bc961019810
📒 Files selected for processing (5)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMSystemTags.javaplugin/kvm/src/main/java/org/zstack/kvm/KvmHostIothreadVqMappingCapability.javatest/src/test/java/org/zstack/kvm/KvmHostIothreadVqMappingCapabilityTest.java
Owner
|
Comment from yaohua.wu: Review: MR !9920 — ZSTAC-82672关联 MR
整体评价方案清晰：将 iothread-vq-mapping 能力检测下沉到 cloud 端，根据 KVM agent 上报的 qemu-kvm 与 libvirt 软件包版本动态创建/删除 SystemTag。抽出 P1 — High
触发场景：以 baseline P2 — Moderate
P3 — Low / 优点
Coverage
VerdictAPPROVED with comments — P1 为加固建议而非阻塞；版本比较在已验证的 RHEL g-hash 包格式下能正常工作，但跨打包格式风险建议在合入前评估。 🤖 Robot Reviewer |
Resolves: ZSTAC-82672 Change-Id: I4c48ef8c9d16eac758ccefcb03fbd33c4c412bfe
MatheMatrix
force-pushed
the
sync/haidong.pang/feature/ZSTAC-82672-iothread-vq-mapping@@3
branch
from
a18fd36 to
5949dd4
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/src/test/java/org/zstack/kvm/KvmHostIothreadVqMappingCapabilityTest.java (1)
10-14: ⚡ Quick win建议补一条“跨打包风格排序”回归用例，锁定比较语义。
当前正向用例已覆盖多种格式，但还缺少“不同 qualifier 风格混用时，数值发布号优先”的断言（例如
.g<hash>.el8vs.module+el8...的边界比较）。这条用例能直接防回归到词典序误判。🤖 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 `@test/src/test/java/org/zstack/kvm/KvmHostIothreadVqMappingCapabilityTest.java` around lines 10 - 14, Add a regression test in supportsBaselineVersions that covers "mixed packaging qualifier" ordering to lock the comparison semantics: call KvmHostIothreadVqMappingCapability.supported with two versions that are identical up to the numeric release but differ in qualifier style (e.g. one with .g<hash>.el8 and the other with .module+el8...) and assert the result matches numeric-release precedence (the expected boolean). Place this new assert alongside the existing asserts in supportsBaselineVersions so it catches regressions from lexicographic ordering mistakes in KvmHostIothreadVqMappingCapability.supported.
🤖 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.
Nitpick comments:
In
`@test/src/test/java/org/zstack/kvm/KvmHostIothreadVqMappingCapabilityTest.java`:
- Around line 10-14: Add a regression test in supportsBaselineVersions that
covers "mixed packaging qualifier" ordering to lock the comparison semantics:
call KvmHostIothreadVqMappingCapability.supported with two versions that are
identical up to the numeric release but differ in qualifier style (e.g. one with
.g<hash>.el8 and the other with .module+el8...) and assert the result matches
numeric-release precedence (the expected boolean). Place this new assert
alongside the existing asserts in supportsBaselineVersions so it catches
regressions from lexicographic ordering mistakes in
KvmHostIothreadVqMappingCapability.supported.
ℹ️ 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: 11dc98af-87f8-45b0-bb02-b9123499995e
📒 Files selected for processing (5)
plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMSystemTags.javaplugin/kvm/src/main/java/org/zstack/kvm/KvmHostIothreadVqMappingCapability.javatest/src/test/java/org/zstack/kvm/KvmHostIothreadVqMappingCapabilityTest.java
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
2 participants
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.
Summary
Add the backend host capability foundation for IOThread VQ mapping. The capability is derived from qemu-kvm and libvirt package versions reported by KVM host facts.
Changes
capability::iothread-vq-mappinghost SystemTag.qemuKvmPackageVersiontoHostFactResponse.Testing
git diff --checkopenspec validate --changes zbs-iothread-vq-mappingmvnis unavailable on this host.Resolves: ZSTAC-82672
sync from gitlab !9920