Skip to content

<feature>[vm]: optimize the statistics method of OS running time on virtual machines #4096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zstack-robot-2 wants to merge 2 commits into
base: zsv_5.1.0
Choose a base branch
from
Open

<feature>[vm]: optimize the statistics method of OS running time on virtual machines #4096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
88ff08e
<feature>[vm]: optimize the statistics method of OS running time on v…
May 25, 2026
53ad7b6
<feature>[compute]: fix all issues mentioned by ai
May 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions compute/src/main/java/org/zstack/compute/vm/VmBootTimeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package org.zstack.compute.vm;

import org.apache.commons.lang.StringUtils;
import org.zstack.core.db.Q;
import org.zstack.core.db.SQL;
import org.zstack.header.vm.VmInstanceState;
import org.zstack.header.vm.VmInstanceVO;
import org.zstack.header.vm.VmInstanceVO_;

import java.sql.Timestamp;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;

public class VmBootTimeUtils {
public static final String STOPPED_UPTIME = "0";
public static final String UNKNOWN_UPTIME = "";
private static final DateTimeFormatter BOOT_TIME_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss");

public static boolean isBootTimeValidState(VmInstanceState state) {
return state != null && state != VmInstanceState.Stopped && state != VmInstanceState.Destroyed;
}

public String getBootTime(String vmUuid) {
if (StringUtils.isBlank(vmUuid)) {
return null;
}

Timestamp bootTime = Q.New(VmInstanceVO.class)
.select(VmInstanceVO_.bootTime)
.eq(VmInstanceVO_.uuid, vmUuid)
.findValue();
return formatBootTime(bootTime);
}

public void resetBootTime(String vmUuid) {
if (StringUtils.isBlank(vmUuid)) {
return;
}

SQL.New(VmInstanceVO.class)
.eq(VmInstanceVO_.uuid, vmUuid)
.set(VmInstanceVO_.bootTime, Timestamp.valueOf(LocalDateTime.now()))
.update();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

public void clearBootTime(String vmUuid) {
if (StringUtils.isBlank(vmUuid)) {
return;
}

SQL.New(VmInstanceVO.class)
.eq(VmInstanceVO_.uuid, vmUuid)
.set(VmInstanceVO_.bootTime, (Timestamp) null)
.update();
}

public void backfillBootTimeIfMissing(String vmUuid, String hostUptime) {
Timestamp bootTime = parseBootTime(hostUptime);
if (StringUtils.isBlank(vmUuid) || bootTime == null) {
return;
}

SQL.New(VmInstanceVO.class)
.eq(VmInstanceVO_.uuid, vmUuid)
.isNull(VmInstanceVO_.bootTime)
.set(VmInstanceVO_.bootTime, bootTime)
.update();
}

private static String formatBootTime(Timestamp bootTime) {
if (bootTime == null) {
return null;
}

return bootTime.toLocalDateTime().format(BOOT_TIME_FORMATTER);
}

private static Timestamp parseBootTime(String bootTime) {
if (StringUtils.isBlank(bootTime)) {
return null;
}

try {
return Timestamp.valueOf(LocalDateTime.parse(bootTime, BOOT_TIME_FORMATTER));
} catch (DateTimeParseException ignored) {
return null;
}
}
}
48 changes: 43 additions & 5 deletions compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ public class VmInstanceBase extends AbstractVmInstance {
protected String syncThreadName;
private final static StaticIpOperator ipOperator = new StaticIpOperator();
private final static VmConfigSyncHelper vmConfigSyncHelper = new VmConfigSyncHelper();
private final static VmBootTimeUtils vmBootTimeUtils = new VmBootTimeUtils();

private void detachTpmKeyProviderBestEffort(String tpmUuid) {
if (tpmUuid == null) {
Expand Down Expand Up @@ -381,6 +382,7 @@ protected void scripts() {
}

self.setState(state);
self.setBootTime(getBootTimeAfterStateChanged(bs, state, getCurrentBootTimeForStateChange()));
self = merge(self);
}
};
Expand Down Expand Up @@ -419,6 +421,35 @@ public void run(VmStateChangedExtensionPoint ext) {
return self;
}

private Timestamp getCurrentBootTimeForStateChange() {
if (self.getBootTime() != null) {
return self.getBootTime();
}

return Q.New(VmInstanceVO.class)
.select(VmInstanceVO_.bootTime)
.eq(VmInstanceVO_.uuid, self.getUuid())
.findValue();
}

private Timestamp getBootTimeAfterStateChanged(VmInstanceState oldState, VmInstanceState newState, Timestamp currentBootTime) {
if (!VmBootTimeUtils.isBootTimeValidState(newState)) {
return null;
}

if (newState != VmInstanceState.Running) {
return currentBootTime;
}

if (oldState == VmInstanceState.Starting
|| oldState == VmInstanceState.Rebooting
|| (oldState == VmInstanceState.Resuming && currentBootTime == null)) {
return Timestamp.valueOf(LocalDateTime.now());
}

return currentBootTime;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

@Override
@MessageSafe
public void handleMessage(final Message msg) {
Expand Down Expand Up @@ -3724,6 +3755,13 @@ public void done(ErrorCodeList errorCodeList) {
private void handle(APIGetVmUptimeMsg msg) {
APIGetVmUptimeReply reply = new APIGetVmUptimeReply();

String uptime = StringUtils.defaultString(vmBootTimeUtils.getBootTime(self.getUuid()), VmBootTimeUtils.UNKNOWN_UPTIME);
if (StringUtils.isNotEmpty(uptime)) {
reply.setUptime(uptime);
bus.reply(msg, reply);
return;
}

GetVmUptimeMsg gmsg = new GetVmUptimeMsg();
gmsg.setVmInstanceUuid(self.getUuid());
gmsg.setHostUuid(self.getHostUuid());
Expand All @@ -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));
Comment on lines 3758 to +3776
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 26, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

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.

Copy link
Copy Markdown
Collaborator Author

@zstack-robot-2 zstack-robot-2 May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from taiyue.chen:

这边upTime实际返回就是bootTime，没有问题。

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} else {
reply.setUptime(VmBootTimeUtils.UNKNOWN_UPTIME);
}
bus.reply(msg, reply);
}
Expand Down
34 changes: 34 additions & 0 deletions conf/db/zsv/V5.1.0__schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,37 @@ SET src.`type` = IF(ldap.serverType IN ('OpenLdap', 'WindowsAD'), ldap.serverTyp
WHERE src.`type` = 'ldap';

CALL INSERT_COLUMN('ThirdPartyAccountSourceVO', 'updateAccountStrategies', 'varchar(255)', 0, '', 'createAccountStrategy');

-- Feature: VM boot time | ZSV-12297

CALL INSERT_COLUMN('VmInstanceEO', 'bootTime', 'timestamp NULL', 1, NULL, 'state');
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 26, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

补齐升级场景的历史数据回填策略。

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.

Copy link
Copy Markdown
Collaborator

@ZStack-Robot ZStack-Robot May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment from taiyue.chen:

这个会在后面兜底时，自动去各个机器上查询其状态进行回填。所以，这边无需这么操作。

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZStack-Robot，感谢 taiyue.chen 的说明。

确认代码中存在兜底路径：当 APIGetVmUptimeMsg 被调用且 bootTime 为 null 时，VmInstanceBase 会回退到向 Host Agent 发送 GetVmUptimeMsg，成功后通过 backfillBootTimeIfMissing 将结果写回数据库。

需要注意的是，这是按需（request-triggered）的惰性回填，而非升级时的主动回填：

  • 升级后，直到有调用方对某台 VM 发起 getUptime 请求之前，其 bootTime 仍为 null；
  • 每次未命中都会产生一次 Host Agent 查询，直到写入成功为止。

如果产品侧和 UI 侧对这个"首次查询延迟"行为可接受，则当前方案是合理的。


CREATE OR REPLACE VIEW `zstack`.`VmInstanceVO` AS
SELECT uuid,
name,
description,
zoneUuid,
clusterUuid,
imageUuid,
hostUuid,
internalId,
lastHostUuid,
instanceOfferingUuid,
rootVolumeUuid,
defaultL3NetworkUuid,
type,
hypervisorType,
cpuNum,
cpuSpeed,
memorySize,
reservedMemorySize,
platform,
guestOsType,
allocatorStrategy,
createDate,
lastOpDate,
state,
architecture,
bootTime
FROM `zstack`.`VmInstanceEO`
WHERE deleted IS NULL;
12 changes: 12 additions & 0 deletions header/src/main/java/org/zstack/header/vm/VmInstanceAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ public class VmInstanceAO extends ResourceVO {
@Enumerated(EnumType.STRING)
private VmInstanceState state;

@Column
private Timestamp bootTime;

public VmInstanceAO() {
}

Expand All @@ -125,6 +128,7 @@ public VmInstanceAO(VmInstanceAO other) {
this.createDate = other.createDate;
this.lastOpDate = other.lastOpDate;
this.state = other.state;
this.bootTime = other.bootTime;
this.platform = other.platform;
this.guestOsType = other.guestOsType;
this.architecture = other.architecture;
Expand Down Expand Up @@ -279,6 +283,14 @@ public void setState(VmInstanceState state) {
this.state = state;
}

public Timestamp getBootTime() {
return bootTime;
}

public void setBootTime(Timestamp bootTime) {
this.bootTime = bootTime;
}

public String getRootVolumeUuid() {
return rootVolumeUuid;
}
Expand Down
1 change: 1 addition & 0 deletions header/src/main/java/org/zstack/header/vm/VmInstanceAO_.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,6 @@ public class VmInstanceAO_ extends ResourceVO_ {
public static volatile SingularAttribute<VmInstanceAO, Long> cpuSpeed;
public static volatile SingularAttribute<VmInstanceAO, Timestamp> createDate;
public static volatile SingularAttribute<VmInstanceAO, Timestamp> lastOpDate;
public static volatile SingularAttribute<VmInstanceAO, Timestamp> bootTime;
public static volatile SingularAttribute<VmInstanceAO, VmInstanceState> state;
}
3 changes: 3 additions & 0 deletions plugin/kvm/src/main/java/org/zstack/kvm/KVMHostFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.zstack.compute.vm.CrashStrategy;
import org.zstack.compute.vm.VmGlobalConfig;
import org.zstack.compute.vm.VmNicManager;
import org.zstack.compute.vm.VmBootTimeUtils;
import org.zstack.core.CoreGlobalProperty;
import org.zstack.core.Platform;
import org.zstack.core.ansible.AnsibleFacade;
Expand Down Expand Up @@ -96,6 +97,7 @@
public class KVMHostFactory extends AbstractService implements HypervisorFactory, Component,
ManagementNodeReadyExtensionPoint, MaxDataVolumeNumberExtensionPoint, HypervisorMessageFactory, ProxyHardwareFactory {
private static final CLogger logger = Utils.getLogger(KVMHostFactory.class);
private static final VmBootTimeUtils vmBootTimeUtils = new VmBootTimeUtils();

public static final HypervisorType hypervisorType = new HypervisorType(KVMConstant.KVM_HYPERVISOR_TYPE);
public static final VolumeFormat QCOW2_FORMAT = new VolumeFormat(VolumeConstant.VOLUME_FORMAT_QCOW2, hypervisorType);
Expand Down Expand Up @@ -539,6 +541,7 @@ public String handleSyncHttpCall(TransmitVmOperationToMnCmd cmd) {
});

restf.registerSyncHttpCallHandler(KVMConstant.KVM_REPORT_VM_REBOOT_EVENT, ReportVmRebootEventCmd.class, cmd -> {
vmBootTimeUtils.resetBootTime(cmd.vmUuid);
evf.fire(VmCanonicalEvents.VM_LIBVIRT_REPORT_REBOOT, cmd.vmUuid);

return null;
Expand Down