Skip to content

<fix>[kvm]: prevent metadata deletion when DVD returns empty#3642

Open
zstack-robot-1 wants to merge 1 commit into5.5.12from
sync/yaohua.wu/bugfix/ZSTAC-83682@@2
Open

<fix>[kvm]: prevent metadata deletion when DVD returns empty#3642
zstack-robot-1 wants to merge 1 commit into5.5.12from
sync/yaohua.wu/bugfix/ZSTAC-83682@@2

Conversation

@zstack-robot-1
Copy link
Copy Markdown
Collaborator

ZSTAC-83682

Root Cause

When hypervisor metadata collection from DVD returns an empty list (e.g., DVD not mounted), saveHostOsCategoryList first deleted all existing metadata for the management node, then checked if the input was empty. This wiped all metadata, causing matchTargetVersion to become null and matchState to become Unknown, which suppressed hypervisor mismatch alarms.

Changes

Module Change
kvm Move empty-list check before metadata delete in KvmHypervisorInfoManagerImpl.saveHostOsCategoryList(), preserving existing metadata when DVD collection returns empty

Test

  • Test cases in premium repo (HypervisorAlarmCase): testMultiClusterAlarmContinue, testUnknownStateTriggersAlarm

Related MRs

  • premium: bugfix/ZSTAC-83682@@25.5.12

Related: ZSTAC-83682

sync from gitlab !9504

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 31, 2026

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: 8c16b369-0b01-49ff-bde1-0d8d84f4f243

📥 Commits

Reviewing files that changed from the base of the PR and between 8f78f0b and 179dfbf.

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

Walkthrough

KvmHypervisorInfoManagerImpl.saveHostOsCategoryList 中将空的 categoryVOS 检查提前到方法开始:当输入为空时立即记录警告并返回 false,避免随后执行对 KvmHostHypervisorMetadataVO 的删除操作或触发管理节点元数据刷新/删除流程(行为顺序变更,功能不变)。

Changes

Cohort / File(s) Summary
KVM 虚拟机管理器元数据处理
plugin/kvm/src/main/java/org/zstack/kvm/hypervisor/KvmHypervisorInfoManagerImpl.java
将空 categoryVOS 检查上移至方法开头;当为空时记录警告并立即返回 false,从而避免随后执行对 KvmHostHypervisorMetadataVO 的删除或触发管理节点元数据刷新/删除。

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

庆祝诗

🐰 早晨跳来见变更,
空值先判不忙刨,
少了无谓的删与刷,
代码轻快又安好。

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)
Check name Status Explanation
Title check ✅ Passed 标题遵循了 [scope]: 格式要求,长度为 60 个字符,不超过 72 个字符限制;内容准确反映了变更的主要目的——防止在 DVD 返回空列表时删除元数据。
Description check ✅ Passed 描述包含了 JIRA 工单号、根本原因分析、具体修改内容、测试用例和相关 MR 信息,与变更集内容高度相关;提供了充分的上下文来理解修改的目的和影响。

✏️ 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/yaohua.wu/bugfix/ZSTAC-83682@@2

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

@ZStack-Robot
Copy link
Copy Markdown
Collaborator

Comment from yaohua.wu:

Review: MR !9504 — ZSTAC-83682 (zstack repo)

Scope: KvmHypervisorInfoManagerImpl.saveHostOsCategoryList() — 防御性 metadata 保护

业务背景

仅升级 MN 的 qemu 并重启后,物理机 matchState 变为 Unknown(因 metadata 被清空),且报警检查器只筛选 Unmatched 状态,导致 qemu 版本不一致报警缺失。本 MR 修复根因之一:DVD 采集返回空列表时先删后查导致 metadata 被清空。

变更分析

CollectionUtils.isEmpty(categoryVOS) 检查从 DELETE 操作之后移到之前,空列表时直接 return false 跳过刷新,保留已有 metadata。

验证链路

  • start()refreshMetadata()saveMetadataList()saveHostOsCategoryList()
  • 该方法标注 @Transactional,空列表提前返回时无 DB 操作,事务无副作用 ✅
  • saveHostOsCategoryListprotected,仅 saveMetadataList 调用,调用链唯一 ✅
  • 后续 refreshHostMatchState() 依赖 metadata 表数据(通过 collectExpectedHypervisorInfoForHosts 实时查询),metadata 保留后 matchTargetVersion 不会变为 null ✅

Upstream Freshness

分支基于 upstream/5.5.12 最新提交 (47efb851),无冲突风险。✅

Verdict: APPROVED ✅

修复精准,改动最小化(仅移动代码顺序 + 增加 warn 日志),无副作用风险。日志信息 "no hypervisor metadata collected from DVD, skip refresh to preserve existing metadata" 清晰描述了行为和原因。

Suggestion

  1. [KvmHypervisorInfoManagerImpl.java] 测试覆盖 — 当前 premium 仓库的测试覆盖了 Unknown 状态触发报警的场景,但未直接测试"空列表不删除 metadata"这一防御逻辑。建议补充一个测试:模拟 saveHostOsCategoryList([]) 后验证已有 metadata 记录未被删除,防止未来重构回归。

🤖 Robot Reviewer

When hypervisor metadata collection from DVD returns an empty
list, the existing metadata was deleted before the empty check,
causing all hosts to lose their hypervisor version metadata and
matchState to become Unknown.

1. Why is this change necessary?
saveHostOsCategoryList first deleted all existing metadata for
the management node, then checked if the input was empty. When
DVD collection returned empty, this wiped all metadata, causing
matchTargetVersion to be null and matchState to become Unknown.

2. How does it address the problem?
Move the empty-list check before the delete operation so that
an empty input preserves existing metadata. A warning is logged
to indicate the skip.

3. Are there any side effects?
None. Non-empty input behavior is unchanged.

# Summary of changes (by module):
- kvm: move empty check before metadata delete in
  KvmHypervisorInfoManagerImpl.saveHostOsCategoryList()

Related: ZSTAC-83682
Change-Id: I81f9baacac7fce9af2363a0ce5c960532d383890
@MatheMatrix MatheMatrix force-pushed the sync/yaohua.wu/bugfix/ZSTAC-83682@@2 branch from 8f78f0b to 179dfbf Compare April 7, 2026 11:08
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