Skip to content

<fix>[kvm]: trigger nested config deploy#3989

Closed
zstack-robot-2 wants to merge 1 commit into
5.5.22from
sync/zstackio/fix/5.5.22/ZSTAC-84199@@2
Closed

<fix>[kvm]: trigger nested config deploy#3989
zstack-robot-2 wants to merge 1 commit into
5.5.22from
sync/zstackio/fix/5.5.22/ZSTAC-84199@@2

Conversation

@zstack-robot-2
Copy link
Copy Markdown
Collaborator

修复逻辑:

  1. 在 KVM Host reconnect 的 AnsibleRunner 中增加 /etc/modprobe.d/kvm-nested.conf 文件存在性检查。
  2. 仅对 x86_64 Host 安装该 checker,避免非 x86 架构因为没有 nested KVM 配置而反复触发 kvm ansible。
  3. 当该配置缺失时,checker 返回 needDeploy=true,使 API/CLI 重连 Host 时不会因为 kvmagent 端口正常而跳过 kvm.py

配套 zstack-utility MR 会负责 kvm.py 执行后写入并校验 nested KVM 持久化配置。

Jira: ZSTAC-84199

sync from gitlab !9882

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent 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: 360447fa-7b28-4e24-ad3a-d12e26017f34

📥 Commits

Reviewing files that changed from the base of the PR and between 6631a9a and 31d53d9.

📒 Files selected for processing (2)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
✅ Files skipped from review due to trivial changes (1)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java

Walkthrough

在 KVM 宿主机连接的 ansible 阶段新增对 /etc/modprobe.d/kvm-nested.conf 的存在性检测,并仅在主机架构为 x86_64 时将该检查器安装到 AnsibleRunner。

变更说明

嵌套 KVM 配置检测

层 / 文件 说明
嵌套 KVM 配置文件检查
plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java, plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
新增常量 NESTED_KVM_CONF_FILE_PATH 指向 /etc/modprobe.d/kvm-nested.conf;在 apply-ansible-playbook 中创建 SshFileExistChecker nestedKvmConfChecker 检查该文件,并仅在 self.getArchitecture()CpuArchitecture.x86_64 时调用 runner.installChecker(nestedKvmConfChecker)

Sequence Diagram(s)

sequenceDiagram
  participant KVMHost
  participant AnsibleRunner
  participant SshFileExistChecker
  KVMHost->>AnsibleRunner: create runner & provide SSH credentials and target IP
  AnsibleRunner->>AnsibleRunner: read getSelf().getArchitecture()
  alt architecture == CpuArchitecture.x86_64
    AnsibleRunner->>SshFileExistChecker: install checker with path /etc/modprobe.d/kvm-nested.conf
    SshFileExistChecker->>TargetHost: check file existence /etc/modprobe.d/kvm-nested.conf
  else architecture == null
    AnsibleRunner->>AnsibleRunner: log debug and skip installing checker
  end
Loading

预估审查工作量

🎯 2 (Simple) | ⏱️ ~8 minutes

🐰 小兔说:架构识新巧,
x86 特别照,
嵌套虚机梦起高,
配置檔静候探道,
检查一行心安心.

🚥 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]: 格式,长度为 40 个字符(在 72 字符限制内),准确描述了主要变更内容。
Description check ✅ Passed 描述清楚地说明了修复逻辑、目的和关联的 Jira 号,与代码变更内容高度相关。
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync/zstackio/fix/5.5.22/ZSTAC-84199@@2

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

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

@MatheMatrix MatheMatrix force-pushed the sync/zstackio/fix/5.5.22/ZSTAC-84199@@2 branch from dc578dc to 924a7b6 Compare May 15, 2026 09:01
@MatheMatrix
Copy link
Copy Markdown
Owner

Comment from yaohua.wu:

Review: MR !9882 — ZSTAC-84199

变更概述:在 KVM Host 连接流程的 AnsibleRunner 中新增 SshFileExistChecker,检查宿主机 /etc/modprobe.d/kvm-nested.conf 是否存在;仅对 x86_64 架构主机安装该 checker。文件缺失时触发 needDeploy,使 reconnect 不会因 kvmagent 端口正常而跳过 kvm.py

总体评价:修复方向正确。与配套 utility MR 形成闭环——存量主机(运行态 nested 已启用但配置文件缺失)首次 reconnect 时会被 checker 命中并补齐持久化配置;下次 reconnect 文件已存在则不再触发。仅 x86_64 安装 checker 与 kvm.py@with_arch(todo_list=['x86_64']) 的约束一致,aarch64 不会被无谓触发。路径常量 NESTED_KVM_CONF_FILE_PATH 抽取得当。未发现 Critical / Warning 问题。

🟢 Suggestion

# 位置 说明
1 KVMHost.java ~5844 架构判断用 self.getArchitecture(),而同段其他 checker(dhcpChecker 等 setter)统一用 getSelf()。建议统一为 getSelf().getArchitecture(),消除 selfgetSelf() 混用带来的可读性歧义。
2 KVMHost.java ~5844 CpuArchitecture.x86_64.name().equals(...) 已是 null-safe(常量在前);但若存量主机 architecture 字段为 null,则不会安装该 checker,其 nested 配置漂移不会被此机制修复。正常添加的 KVM 主机均有 architecture,影响面窄——可视情况在此分支补一条 trace 日志,便于现场排查。

关联 MR

  • zstack-utility !7066kvm.pycheck_nested_kvm 重构:每次 kvmagent 部署都写入 /etc/modprobe.d/kvm-nested.conf,并按 CPU 类型选择 kvm_intel/kvm_amd

跨 repo 一致性:本 MR 的 checker 依赖 utility 侧 kvm.py「每次部署都写文件」才能真正闭环。旧版 kvm.py 仅在运行态 nested 未启用时写文件——若本 MR 先于 utility !7066 发版,存量「运行态已启用 + 文件缺失」的主机会出现 checker needDeploy 恒为 true、每次 reconnect 都重跑整套 kvm 部署的情况。两 MR 源分支同为 fix/5.5.22/ZSTAC-84199@@2须联合合并、同包发版。两侧路径字符串一致 ✓。

结论

APPROVED — 可合并。两条 Suggestion 不阻塞,建议合并前顺手统一 #1getSelf() 用法。


🤖 Robot Reviewer

Run KVM ansible when nested KVM config is missing on x86_64 hosts.

Resolves: ZSTAC-84199

Change-Id: I12a0a80b91cb2f78b505a12cfa112cb2279f33ea
@MatheMatrix MatheMatrix force-pushed the sync/zstackio/fix/5.5.22/ZSTAC-84199@@2 branch from 6631a9a to 31d53d9 Compare May 15, 2026 10:04
@MatheMatrix MatheMatrix deleted the sync/zstackio/fix/5.5.22/ZSTAC-84199@@2 branch May 18, 2026 07:12
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