<feature>[vm]: optimize the statistics method of OS running time on virtual machines#4096
Open
zstack-robot-2 wants to merge 2 commits into
Open
<feature>[vm]: optimize the statistics method of OS running time on virtual machines#4096zstack-robot-2 wants to merge 2 commits into
zstack-robot-2 wants to merge 2 commits into
Conversation
…irtual machines DBImpact Resolves: ZSV-12297 Change-Id: I6a746a746b6b6a6172646470627a64636173656c
|
Warning Review limit reached
More reviews will be available in 32 minutes and 7 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Walkthrough新增 VmInstance.bootTime 列与视图暴露；新增 VmBootTimeUtils 提供格式化、读写、重置与条件回填；VmInstanceBase 在状态变更时同步 bootTime 并优先本地返回 uptime；KVM 重启事件触发重置。 变更内容VM 启动时间追踪功能
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 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 unit tests (beta)
Comment |
Owner
|
Comment from yaohua.wu: Review: MR !9994 — ZSV-12297Background (preserved across rounds)
🔴 P0 — Critical
🟡 P1 — High
🟢 P2 — Moderate
Pre-existing本轮无 pre-existing 类问题需单独列出。 Coverage
Verdict: REVISION_REQUIRED两条 P0 都属于"会让现网升级用户立即感知到错误"的级别（API 契约变更 + 存量 VM uptime 归零），合并前必须修复或与产品/SDK 团队达成"接受破坏 + 同步外部"的明确决策。P1 列表里的非原子/状态机覆盖问题与 P0-2 是同一类根因，建议一并处理。 🤖 Robot Reviewer |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java (1)
543-545: ⚡ Quick win建议为重启事件增加
vmUuid空值短路。当前新增了 bootTime 重置调用，先做空值防护可避免异常输入导致回调链路抖动。
建议修改
restf.registerSyncHttpCallHandler(KVMConstant.KVM_REPORT_VM_REBOOT_EVENT, ReportVmRebootEventCmd.class, cmd -> { + if (StringUtils.isBlank(cmd.vmUuid)) { + logger.warn("ignore vm reboot event because vmUuid is blank"); + return null; + } vmBootTimeUtils.resetBootTime(cmd.vmUuid); evf.fire(VmCanonicalEvents.VM_LIBVIRT_REPORT_REBOOT, cmd.vmUuid); return null; });🤖 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/KVMHostFactory.java` around lines 543 - 545, KVMHostFactory's handler for KVMConstant.KVM_REPORT_VM_REBOOT_EVENT calls vmBootTimeUtils.resetBootTime(cmd.vmUuid) and evf.fire(...) without guarding cmd.vmUuid; add a null/empty check for cmd.vmUuid at the start of the lambda (for ReportVmRebootEventCmd) and short-circuit (return) if missing, optionally logging a warning, so neither vmBootTimeUtils.resetBootTime nor evf.fire(VmCanonicalEvents.VM_LIBVIRT_REPORT_REBOOT, cmd.vmUuid) runs with a null/empty uuid.
🤖 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 `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java`:
- Around line 3742-3743: 当前把 APIGetVmUptimeMsg 在 VmInstanceBase 中统一改成读取
vmBootTimeUtils.getUptime(self.getUuid(), self.getState()) 会导致非 KVM 的虚拟机在 guest
重启但 host 状态仍为 Running 时返回错误的累计 uptime；请修改处理逻辑：在 VmInstanceBase 对该消息的处理里根据虚拟机所处
host 的 hypervisor 类型（或通过现有 host factory 判定）仍保留非 KVM 的 host-side 查询兜底（即对非 KVM
调用原来的从 Host/HostFactory 查询 uptime 的代码路径），仅在 KVM（或明确已同步 reboot 事件的
hypervisor）时才使用 vmBootTimeUtils.getUptime(self.getUuid(), self.getState());
参考符号：VmInstanceBase, APIGetVmUptimeMsg, vmBootTimeUtils.getUptime 和
KVMHostFactory 来定位并实现这个条件分支。
---
Nitpick comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java`:
- Around line 543-545: KVMHostFactory's handler for
KVMConstant.KVM_REPORT_VM_REBOOT_EVENT calls
vmBootTimeUtils.resetBootTime(cmd.vmUuid) and evf.fire(...) without guarding
cmd.vmUuid; add a null/empty check for cmd.vmUuid at the start of the lambda
(for ReportVmRebootEventCmd) and short-circuit (return) if missing, optionally
logging a warning, so neither vmBootTimeUtils.resetBootTime nor
evf.fire(VmCanonicalEvents.VM_LIBVIRT_REPORT_REBOOT, cmd.vmUuid) runs with a
null/empty uuid.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b4861025-3169-4860-b204-93e4a4042375
📒 Files selected for processing (6)
compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javaconf/db/zsv/V5.1.0__schema.sqlheader/src/main/java/org/zstack/header/vm/VmInstanceAO.javaheader/src/main/java/org/zstack/header/vm/VmInstanceAO_.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
| } | ||
| }); | ||
| reply.setUptime(vmBootTimeUtils.getUptime(self.getUuid(), self.getState())); | ||
| bus.reply(msg, reply); |
Collaborator
There was a problem hiding this comment.
Comment from taiyue.chen:
这个也和“当升级时，bootTime为null，所有虚拟机uptime被重置为0“有关。对于该问题，应当设置一个兜底策略，即当uptime为null时，应当从去从各个host上拿对应的时间。
Collaborator
|
Comment from taiyue.chen: re.getUptime() 这个返回的格式是 ”yyyy-MM-dd HH:mm:ss“，我已经在真机上测试过。所以，P0-1不成立。 |
MatheMatrix
force-pushed
the
sync/zstackio/ZSV-12297
branch
2 times, most recently
from
a84f8f4 to
3fa4d40
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.java (1)
43-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
resetBootTime存在 TOCTOU 竞态。Line 44 先
vmExists()再在 Line 49 更新，期间目标记录可能被删除或状态变化，导致返回值与真实写入结果不一致。建议改成单条条件更新并基于affected rows判断结果，避免检查与写入分离。可选修复示例
public String resetBootTime(String vmUuid) { - if (!vmExists(vmUuid)) { + if (StringUtils.isBlank(vmUuid)) { return STOPPED_UPTIME; } Timestamp bootTime = Timestamp.valueOf(LocalDateTime.now()); - setBootTime(vmUuid, bootTime); - return formatBootTime(bootTime); + int updated = SQL.New(VmInstanceVO.class) + .eq(VmInstanceVO_.uuid, vmUuid) + .set(VmInstanceVO_.bootTime, bootTime) + .update(); + return updated > 0 ? formatBootTime(bootTime) : STOPPED_UPTIME; }🤖 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 `@compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.java` around lines 43 - 50, The resetBootTime method has a TOCTOU race: it calls vmExists(vmUuid) then setBootTime(vmUuid, bootTime), which can race with deletes/updates; change resetBootTime to perform a single conditional update (e.g. "update ... where uuid = ? and <running/exists condition>") in one DB call and use the returned affected-rows to decide whether to return STOPPED_UPTIME or formatBootTime(bootTime); remove the separate vmExists check and ensure you call the same setBootTime/SQL logic (or refactor setBootTime into an update-returning-rows variant) so the method's return reflects the actual write result.
🧹 Nitpick comments (1)
compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.java (1)
106-109: ⚡ Quick win解析失败被静默吞掉，排障信号丢失。
Line 108-109 直接忽略
DateTimeParseException，会让 host uptime 格式漂移时难以及时定位。建议至少记录 debug/warn 日志（包含 vmUuid/原始 uptime）。🤖 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 `@compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.java` around lines 106 - 109, The DateTimeParseException in VmBootTimeUtils is currently swallowed; update the catch block where bootTime is parsed (using BOOT_TIME_FORMATTER and the bootTime variable) to log a warning or debug message that includes the vmUuid (if available) and the original bootTime string plus the exception details, then continue to return null; ensure the log uses the class logger (VmBootTimeUtils) and includes clear context ("vmUuid", "bootTime", and exception) so parsing failures are recorded for troubleshooting.
🤖 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 `@compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.java`:
- Around line 19-29: The method VmBootTimeUtils#getUptime(String vmUuid, boolean
isRunning) uses a boolean to convey VM state, which is unclear and error-prone;
change it to a clearer API by either (A) adding two semantic methods
getRunningUptime(String vmUuid) and getStoppedUptime(String vmUuid) that
encapsulate the current logic, or (B) replace the boolean signature with
getUptime(String vmUuid, VmInstanceState state) and branch on the enum (only
call getBootTime when state == VmInstanceState.Running); update all callers in
VmInstanceBase to use the new methods/enum and remove usages that pass raw
booleans. Ensure UNKNOWN_UPTIME/STOPPED_UPTIME behavior is preserved and
deprecate/remove the old boolean-based method.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java`:
- Around line 424-437: getBootTimeAfterStateChanged currently sets bootTime =
now for broad transitions (e.g., Stopped -> Running) causing incorrect
truncation of uptime and misses the first-run Resuming case; change the logic in
getBootTimeAfterStateChanged to only set Timestamp.valueOf(LocalDateTime.now())
when you can confirm a real start/reboot completion (e.g., oldState ==
VmInstanceState.Starting || oldState == VmInstanceState.Rebooting -> newState ==
Running) and do not unconditionally set now for Stopped -> Running so host-side
checkState/APIGetVmUptimeMsg can correct uptime; additionally, add a branch to
set now when oldState == VmInstanceState.Resuming && currentBootTime == null
(first resume -> Running) so first-time resumes get a bootTime written.
In `@conf/db/zsv/V5.1.0__schema.sql`:
- Line 42: The migration only adds a nullable bootTime column via INSERT_COLUMN
for VmInstanceEO but doesn't backfill historical running VMs, so post-upgrade
callers using VmBootTimeUtils will see unknown uptimes; add an idempotent
backfill step in the same migration that initializes VmInstanceEO.bootTime for
existing rows in running state (use the same source VmBootTimeUtils relies on —
e.g., existing uptime/lastPing/created_at or the calculated candidate from your
service — falling back to created_at or current_timestamp-uptime if available),
and document the approach; ensure the backfill is safe to re-run and preserves
NULL for VMs where no reasonable bootTime can be inferred.
---
Outside diff comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.java`:
- Around line 43-50: The resetBootTime method has a TOCTOU race: it calls
vmExists(vmUuid) then setBootTime(vmUuid, bootTime), which can race with
deletes/updates; change resetBootTime to perform a single conditional update
(e.g. "update ... where uuid = ? and <running/exists condition>") in one DB call
and use the returned affected-rows to decide whether to return STOPPED_UPTIME or
formatBootTime(bootTime); remove the separate vmExists check and ensure you call
the same setBootTime/SQL logic (or refactor setBootTime into an
update-returning-rows variant) so the method's return reflects the actual write
result.
---
Nitpick comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.java`:
- Around line 106-109: The DateTimeParseException in VmBootTimeUtils is
currently swallowed; update the catch block where bootTime is parsed (using
BOOT_TIME_FORMATTER and the bootTime variable) to log a warning or debug message
that includes the vmUuid (if available) and the original bootTime string plus
the exception details, then continue to return null; ensure the log uses the
class logger (VmBootTimeUtils) and includes clear context ("vmUuid", "bootTime",
and exception) so parsing failures are recorded for troubleshooting.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 23762618-a750-44ca-af90-8c5c9e15e6b4
📒 Files selected for processing (3)
compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javaconf/db/zsv/V5.1.0__schema.sql
|
|
||
| -- Feature: VM boot time | ZSV-12297 | ||
|
|
||
| CALL INSERT_COLUMN('VmInstanceEO', 'bootTime', 'timestamp NULL', 1, NULL, 'state'); |
There was a problem hiding this comment.
补齐升级场景的历史数据回填策略。
Line 42 仅新增了可空 bootTime 列，但迁移脚本里没有处理“已在运行中的历史 VM”初始化。结合 VmBootTimeUtils 在 Line 28 的行为，升级后这批 VM 在 bootTime 缺失时会返回未知 uptime，影响升级后可用性与一致性。建议在迁移阶段补充回填方案（或明确并保证可接受的惰性回填路径与行为）。
As per coding guidelines “Upgrading scene has been carefully handled”.
🤖 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 `@conf/db/zsv/V5.1.0__schema.sql` at line 42, The migration only adds a
nullable bootTime column via INSERT_COLUMN for VmInstanceEO but doesn't backfill
historical running VMs, so post-upgrade callers using VmBootTimeUtils will see
unknown uptimes; add an idempotent backfill step in the same migration that
initializes VmInstanceEO.bootTime for existing rows in running state (use the
same source VmBootTimeUtils relies on — e.g., existing
uptime/lastPing/created_at or the calculated candidate from your service —
falling back to created_at or current_timestamp-uptime if available), and
document the approach; ensure the backfill is safe to re-run and preserves NULL
for VMs where no reasonable bootTime can be inferred.
Collaborator
There was a problem hiding this comment.
Comment from taiyue.chen:
这个会在后面兜底时，自动去各个机器上查询其状态进行回填。所以，这边无需这么操作。
There was a problem hiding this comment.
@ZStack-Robot，感谢 taiyue.chen 的说明。
确认代码中存在兜底路径：当 APIGetVmUptimeMsg 被调用且 bootTime 为 null 时，VmInstanceBase 会回退到向 Host Agent 发送 GetVmUptimeMsg，成功后通过 backfillBootTimeIfMissing 将结果写回数据库。
需要注意的是，这是按需（request-triggered）的惰性回填，而非升级时的主动回填：
- 升级后，直到有调用方对某台 VM 发起
getUptime请求之前，其bootTime仍为null； - 每次未命中都会产生一次 Host Agent 查询，直到写入成功为止。
如果产品侧和 UI 侧对这个"首次查询延迟"行为可接受，则当前方案是合理的。
MatheMatrix
force-pushed
the
sync/zstackio/ZSV-12297
branch
2 times, most recently
from
d3c25a2 to
5f2d534
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.java`:
- Around line 36-43: The current resetBootTime performs a separate vmExists
check then calls setBootTime, creating a TOCTOU race where the VM may be removed
between the check and update; change resetBootTime to perform a single
conditional update by uuid (i.e. issue an UPDATE that sets boot_time = now()
WHERE uuid = :vmUuid) and treat zero rows affected as "not found" instead of
relying on vmExists, or adjust setBootTime to execute that single UPDATE and
return the affected-row count so resetBootTime can call it directly without a
prior vmExists check.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java`:
- Around line 3758-3776: The code mixes bootTime and uptime: both the local-path
(vmBootTimeUtils.getBootTime) and the host-path set reply.setUptime to a boot
timestamp, changing semantics; update VmInstanceBase so reply.setUptime always
contains the runtime duration (use GetVmUptimeReply#getUptime when handling the
host response) and do not overwrite it with vmBootTimeUtils.getBootTime; if you
need to persist or backfill boot timestamp keep
vmBootTimeUtils.backfillBootTimeIfMissing(self.getUuid(), re.getUptime()) but
store that value into a new field (e.g., reply.setBootTime or internal-only
variable) rather than reply.setUptime, and ensure the initial fast-path still
computes/returns a duration string rather than a boot timestamp.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f6788aa0-b5f9-4ad4-ad9b-7790249a32de
📒 Files selected for processing (3)
compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javaconf/db/zsv/V5.1.0__schema.sql
Comment on lines
3758
to
+3776
| @@ -3732,12 +3770,12 @@ private void handle(APIGetVmUptimeMsg msg) { | |||
| bus.send(gmsg, new CloudBusCallBack(msg) { | |||
| @Override | |||
| public void run(MessageReply r) { | |||
| if (!r.isSuccess()) { | |||
| reply.setSuccess(false); | |||
| reply.setError(r.getError()); | |||
| } else { | |||
| if (r.isSuccess()) { | |||
| GetVmUptimeReply re = (GetVmUptimeReply) r; | |||
| reply.setUptime(re.getUptime()); | |||
| vmBootTimeUtils.backfillBootTimeIfMissing(self.getUuid(), re.getUptime()); | |||
| reply.setUptime(StringUtils.defaultString(vmBootTimeUtils.getBootTime(self.getUuid()), VmBootTimeUtils.UNKNOWN_UPTIME)); | |||
There was a problem hiding this comment.
APIGetVmUptimeMsg 现在把 bootTime 塞进了 uptime 字段。
这里两条成功路径最终都执行 reply.setUptime(vmBootTimeUtils.getBootTime(...))，连 host 兜底成功后也不会直接返回 GetVmUptimeReply#getUptime()。这样 uptime 的语义已经变成了“启动时间”，并且返回格式会随本地是否已有 bootTime 而变化。建议保持 uptime 继续返回运行时长，bootTime 另起字段或仅作为内部计算值。
🤖 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 `@compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java` around lines
3758 - 3776, The code mixes bootTime and uptime: both the local-path
(vmBootTimeUtils.getBootTime) and the host-path set reply.setUptime to a boot
timestamp, changing semantics; update VmInstanceBase so reply.setUptime always
contains the runtime duration (use GetVmUptimeReply#getUptime when handling the
host response) and do not overwrite it with vmBootTimeUtils.getBootTime; if you
need to persist or backfill boot timestamp keep
vmBootTimeUtils.backfillBootTimeIfMissing(self.getUuid(), re.getUptime()) but
store that value into a new field (e.g., reply.setBootTime or internal-only
variable) rather than reply.setUptime, and ensure the initial fast-path still
computes/returns a duration string rather than a boot timestamp.
Collaborator
Author
There was a problem hiding this comment.
Comment from taiyue.chen:
这边upTime实际返回就是bootTime，没有问题。
There was a problem hiding this comment.
@zstack-robot-2 明白了，感谢说明。既然 APIGetVmUptimeReply.uptime 在当前约定里实际就是返回 bootTime，那我这条“语义从 uptime 变成 bootTime”的评论不成立；这里按现有实现返回 vmBootTimeUtils.getBootTime(...) 没问题。我会按这个约定看待后续相关改动。
🐰🤔
✏️ Learnings added
Learnt from: zstack-robot-2
Repo: MatheMatrix/zstack PR: 4096
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:3758-3776
Timestamp: 2026-05-26T09:38:38.804Z
Learning: In MatheMatrix/zstack, for the VM uptime API (`APIGetVmUptimeMsg` / `APIGetVmUptimeReply.uptime` in `compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java`), the `uptime` field is expected by maintainers to return the VM boot time value, not necessarily a duration. Do not flag `reply.setUptime(vmBootTimeUtils.getBootTime(...))` as an API semantic break solely because it returns bootTime.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 3826
File: compute/src/main/java/org/zstack/compute/allocator/HostAllocatorChain.java:261-265
Timestamp: 2026-04-22T04:38:00.864Z
Learning: 在 ZStack 的 `Platform.err()` / `Platform.toRawString()` 调用中，传入用于格式化的 `%s` 参数若实现了 `org.zstack.header.core.I18nMessage`，框架会自动通过 `getDetails()` 取得原始文本、并通过 `getI18nDetails()` 取得国际化文本。因此，在代码审查中不应将“直接把实现了 `I18nMessage` 的对象作为 `%s` 参数传入而可能打印出对象地址/不安全”当作问题；此用法是正确且应当被视为安全的（例如将 `HostCandidate.RejectedCandidate` 这类实现了 `I18nMessage` 的对象作为 `%s` 参数传入）。
Learnt from: ZStack-Robot
Repo: MatheMatrix/zstack PR: 3837
File: compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java:8789-8791
Timestamp: 2026-04-22T14:27:42.894Z
Learning: In ZStack compute code, calls to VmInstanceResourceMetadataManager.deleteVmResourceMetadata(...) should generally be treated as best-effort: do not fail/block the primary VM operation when deletion fails (for example, during detach/swap flows such as VmDetachNicFlow, VolumeManagerImpl, KVMHostFactory, and VmInstanceBase when swapping the default CD-ROM). Prefer logging the returned ErrorCode (and relevant context) rather than enforcing strict failure, unless maintainers explicitly request strict enforcement for that specific flow.
Learnt from: zstack-robot-1
Repo: MatheMatrix/zstack PR: 3906
File: compute/src/main/java/org/zstack/compute/zone/ZoneManagerImpl.java:162-164
Timestamp: 2026-05-09T02:01:09.283Z
Learning: In ZStack compute code, when invoking post-commit extension point hooks (for example `ZoneExtensionPointEmitter.afterCreate`, `ClusterExtensionPointEmitter.afterCreate`, and other `*ExtensionPointEmitter.after*` hooks), treat extension implementations as fail-loud by convention. Do not wrap these hook invocations in `try/catch` to suppress or convert exceptions—let exceptions thrown by the extension propagate to the caller so they are detected and handled. If you must do local cleanup, use `finally` for resource cleanup, but do not swallow or mask the thrown exception.
Resolves: ZSV-12297 Change-Id: I617664617a6f6c66736f6b717575636f676f7961
MatheMatrix
force-pushed
the
sync/zstackio/ZSV-12297
branch
from
5f2d534 to
53ad7b6
Compare
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
4 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.
DBImpact
Resolves: ZSV-12297
Change-Id: I6a746a746b6b6a6172646470627a64636173656c
sync from gitlab !9994