Skip to content

Conversation

@ZStack-Robot
Copy link
Collaborator

disable numa on vm based on embedded image

Resolves/Related: ZSTAC-80086

Change-Id: I777863666f636669726e786470696877756a736c

sync from gitlab !8964

@coderabbitai
Copy link

coderabbitai bot commented Dec 28, 2025

Walkthrough

startVm 方法中加入条件:若存在 image.spec 且其 inventory.platform == Embedded,则将 UseNuma 强制设为 false,以覆盖此前通过 rcf 配置的 NUMA 值(仅影响 Embedded 镜像的启动流程)。

Changes

Cohort / File(s) 变更摘要
NUMA 配置逻辑变更
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
startVm 方法中新增判断:当 image.spec 存在且 inventory.platform == Embedded 时,将 UseNuma 强制设为 false,覆盖之前的 rcf 配置。未修改任何公开 API 或导出声明。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 嵌入式镜像来报到,
NUMA 配置悄然搁浅,
启动虚机步履轻巧,
代码一改春意暖,
胡萝卜跳舞微笑。

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed 标题与PR的主要变更相关,但表述过于宽泛。标题声称"修复vm启动失败",而实际变更是针对Embedded镜像禁用NUMA的具体修复,标题没有准确体现这个关键细节。
Description check ✅ Passed 描述内容与变更集相关,准确说明了在基于embedded镜像时禁用NUMA的目的,并包含关联的issue编号和同步信息。
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sync/shan.wu/fix-ZSTAC-80086

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11a7fca and c3b0129.

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

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.40.3)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java

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

Copy link

@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: 0

🧹 Nitpick comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)

4266-4271: 针对 Embedded 镜像强制关闭 NUMA 的逻辑是正确且位置合适的

这里先按资源配置设置 cmd.setUseNuma(...),再在 spec.imageSpec.inventory.platform == Embedded 的前提下覆盖为 false,且做了空指针防御,实际影响范围被严格限定在基于 Embedded 镜像的 VM 上;并且覆盖发生在 setStartVmCpuTopology(spec, cmd, platform) 之前,保证后续 CPU 拓扑计算使用的是最终的 useNuma 值,整体实现符合预期修复目标,可以直接接受。

如果后续该策略需要在更多版本/分支长期存在,可以考虑用资源配置或系统标签来表达“Embedded 禁用 NUMA”策略,并在注释中引用需求或 JIRA 编号,替代当前对分支名的硬编码注释,以便统一运维和文档说明;就本次 4.8.30-jianchuan 的同步修复而言,保持现有最小改动也是符合项目既有约定的。(Based on learnings)

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5aa94f4 and f632b75.

📒 Files selected for processing (1)
  • plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java

⚙️ CodeRabbit configuration file

**/*.java: ## 1. API 设计要求

  • API 命名:
    • API 名称必须唯一,不能重复。
    • API 消息类需要继承 APIMessage;其返回类必须继承 APIReplyAPIEvent,并在注释中用 @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 类型类除外。
      • 抽象类采用 AbstractBase 前缀/后缀。
      • 异常类应以 Exception 结尾。
      • 测试类需要以 TestCase 结尾。
  • 方法名、参数名、成员变量和局部变量:

    • 使用 lowerCamelCase 风格。
  • 常量命名:

    • 全部大写,使用下划线分隔单词。
    • 要求表达清楚,避免使用含糊或不准确的名称。
  • 包名:

    • 统一使用小写,使用点分隔符,每个部分应是一个具有自然语义的英文单词(参考 Spring 框架的结构)。
  • 命名细节:

    • 避免在父子类或同一代码块中出现相同名字的成员或局部变量,防止混淆。
    • 命名缩写:
      • 不允许使用不必要的缩写,如:AbsSchedulerJobcondiFu 等。应使用完整单词提升可读性。

3. 编写自解释代码

  • 意图表达:

    • 避免使用布尔型参数造成含义不明确。例如:
      • 对于 stopAgent(boolean ignoreError),建议拆分为不同函数(如 stopAgentIgnoreError()),或使用枚举表达操作类型。
    • 命名应尽量用完整的单词组合表达意图,并在名称中体现数据类型或用途(例如在常量与变量名称中,将类型词放在末尾)。
  • 注释:

    • 代码应尽量做到自解释,对少于两行的说明可以直接写在代码中。
    • 对于较长的注释,需要仔细校对并随代码更新,确保内容正确。
    • 接口方法不应有多余的修饰符(例如 public),且必须配有有效的 Javadoc 注释。

4. 流程控制和结构优化

  • if...else 的使用:

    • 应尽量减少 if...else 结构的使用,建议:
      • 限制嵌套层级最多为两层,且内层不应再出现 else 分支。
      • 尽早返回(Early Return),将条件判断中的处理逻辑提前结束或抽成独立方法。
      • 使用 Java Stream 或 Lambda 表达式代替冗长的循环与条件判断。
  • 条件判断:

    • if 条件表达不宜过长或过于复杂,必要时可以将条件抽成 boolean 变量描述。
  • 代码块长度:

    • 单个 if 代码块不宜超过一屏显示,以提高可读性和后续维护性。

5. 异常处理与日志

  • 捕获异常的原则:
    • 对于可以通过预检查避免的 RuntimeException(如 NullPointerExceptionIndexOutOfBoundsException 等),不建议使用 try-catch 来进行处理。
    • 捕获异常应仅用于处理真正的意外情况,不应将异常逻辑当作正常流程控制。
    • 在必要时,应继续抛出异常,使上层业务处理者可以转换为用户友好的错误提示。
    • 使用 try-with-resources 语法管理资源,确保在 finally 块中正确关闭资源,并避免在 finally 中返回值。
      ...

Files:

  • plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
🧠 Learnings (2)
📓 Common learnings
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 暴露问题,与项目既有约定一致。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: network/src/main/java/org/zstack/network/l3/L3BasicNetwork.java:449-490
Timestamp: 2025-08-04T04:48:19.103Z
Learning: ZStack项目在cherry-pick操作中,即使发现了性能优化机会(如IP地址批量保存的内存优化),也严格遵循不做额外修改的政策,优先保证cherry-pick的完整性和一致性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2489
File: storage/src/main/java/org/zstack/storage/snapshot/VolumeTree.java:471-545
Timestamp: 2025-08-22T05:36:40.467Z
Learning: In ZStack项目中,对于核心逻辑相关的代码,团队倾向于暂时不进行重构,即使代码复杂度较高,优先保证核心功能的稳定性。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2325
File: compute/src/main/java/org/zstack/compute/vm/VmMigrateCallExtensionFlow.java:29-29
Timestamp: 2025-07-24T05:53:10.246Z
Learning: 在ZStack项目的重构过程中,团队采用务实的优先级策略:优先完成影响运行时功能的代码重构,对于注释中的旧引用等非功能性问题可以延后处理,因为这些不会影响系统的实际运行。
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 2360
File: compute/src/main/java/org/zstack/compute/vm/StaticIpOperator.java:364-501
Timestamp: 2025-08-04T03:19:07.454Z
Learning: In ZStack project, developers may prefer to keep related validation logic together in a single method (like validateStaticIpTagsInApiMessage with 130+ lines) when the architecture is clear and there's no code reuse, rather than splitting into smaller methods based purely on single responsibility principle.
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 2419
File: plugin/sdnController/src/main/java/org/zstack/sdnController/SdnControllerBase.java:986-1067
Timestamp: 2025-08-13T02:31:40.023Z
Learning: 在ZStack项目中,团队倾向于保持统一的业务流程方法不进行拆分,即使方法较长也会维持在单个方法中以保持业务逻辑的完整性。
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 2496
File: plugin/sharedMountPointPrimaryStorage/src/main/java/org/zstack/storage/primary/smp/KvmBackend.java:2545-2566
Timestamp: 2025-08-24T07:35:42.793Z
Learning: 在 ZStack 代码库中,当响应类包含数值字段(如 size、actualSize)时,优先使用原始类型(long)而不是包装类型(Long),以避免 NPE 风险和不必要的装箱/拆箱操作。如果 Agent 端可能不设置该字段,应在 Agent 端确保设置默认值,而不是在使用端做 null 检查。
📚 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

@MatheMatrix MatheMatrix force-pushed the sync/shan.wu/fix-ZSTAC-80086 branch from f632b75 to 11a7fca Compare December 28, 2025 07:50
disable numa on vm based on embedded image

Resolves/Related: ZSTAC-80086

Change-Id: I777863666f636669726e786470696877756a736c
@MatheMatrix MatheMatrix force-pushed the sync/shan.wu/fix-ZSTAC-80086 branch from 11a7fca to c3b0129 Compare December 28, 2025 08:30
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