<feature>[vm_local_volume_cache]: vm local volume cache imp#3406
Open
zstack-robot-2 wants to merge 1 commit intofeature-5.5.6-local-cachefrom
Open
<feature>[vm_local_volume_cache]: vm local volume cache imp#3406zstack-robot-2 wants to merge 1 commit intofeature-5.5.6-local-cachefrom
zstack-robot-2 wants to merge 1 commit intofeature-5.5.6-local-cachefrom
Conversation
Walkthrough引入本地卷缓存（Local Volume Cache）功能：新增API与消息、数据库表与实体、KVM agent 命令与托管器整合，以及在VM创建与主机分配/卷附加流程中传播与启用缓存配置。 Changes
Sequence Diagram(s)sequenceDiagram
participant API as API Server
participant HostBase as HostBase (controller)
participant HostSvc as Host Service (bus)
participant KVMHost as KVMHost (agent proxy)
participant Agent as KVM Agent
API->>HostBase: APIGetBlockDevicesMsg
HostBase->>HostSvc: GetBlockDevicesOnHostMsg (hostUuid)
HostSvc->>KVMHost: deliver message
KVMHost->>Agent: HTTP POST KVM_GET_BLOCK_DEVICES_PATH
Agent-->>KVMHost: GetBlockDevicesRsp (blockDevices / error)
KVMHost-->>HostSvc: GetBlockDevicesOnHostReply (blockDevices / error)
HostSvc-->>HostBase: reply
HostBase-->>API: APIGetBlockDevicesEvent (blockDevices / error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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.41.0)compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceManagerImpl.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (12)
plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolMessage.java (1)
3-5: 建议添加 Javadoc 注释以说明接口用途。与
VmLocalVolumeCacheMessage接口相比，此接口缺少文档注释。根据编码规范，接口方法应配有有效的 Javadoc 注释。📝 建议的修改
package org.zstack.kvm.localvolumecache; +/** + * Marker interface for messages routed to VmLocalVolumeCachePoolBase via passThrough. + */ public interface VmLocalVolumeCachePoolMessage { String getPoolUuid(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolMessage.java` around lines 3 - 5, 在 VmLocalVolumeCachePoolMessage 接口上添加 Javadoc 注释以说明接口用途和方法含义；具体在接口声明处为 VmLocalVolumeCachePoolMessage 添加一段简短描述（功能、使用场景），并为其方法 getPoolUuid() 添加 `@return` 描述说明返回值含义，必要时参考相似接口 VmLocalVolumeCacheMessage 的注释风格以保持一致性，并确保注释遵循项目的 Javadoc 规范。compute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.java (1)
415-417: 建议补充透传 cache pool/mode，避免缓存配置在此链路被忽略。当前
EnableVolumeCacheMsg仅设置了volumeUuid。若上游已携带池与模式配置，这里未写入会导致策略退化为默认行为。♻️ 可选改进示例
EnableVolumeCacheMsg emsg = new EnableVolumeCacheMsg(); emsg.setVolumeUuid(volumeInventory.getUuid()); +if (diskAO.getCachePoolUuid() != null) { + emsg.setPoolUuid(diskAO.getCachePoolUuid()); +} +if (diskAO.getCacheMode() != null) { + emsg.setCacheMode(diskAO.getCacheMode()); +} bus.makeLocalServiceId(emsg, VolumeConstant.SERVICE_ID);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.java` around lines 415 - 417, EnableVolumeCacheMsg 目前只设置了 volumeUuid（在 EnableVolumeCacheMsg 和 bus.makeLocalServiceId(..., VolumeConstant.SERVICE_ID) 处），会丢失上游传来的缓存池/模式配置；请在构造 EnableVolumeCacheMsg 时把上游的 cache pool 与 cache mode 字段透传到该消息（例如从调用上下文或 volumeInventory/请求对象中读取对应的缓存配置字段并通过 EnableVolumeCacheMsg 的对应 setter 赋值），确保发送前包含 cache pool 与 cache mode，保持原有 bus.makeLocalServiceId 调用不变。plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java (1)
68-90: 拷贝构造函数浅拷贝cache的潜在风险较低。Line 89 的
this.cache = other.cache;确实存在浅拷贝问题。但由于CacheTO的所有字段均为String类型（不可变），即使两个VolumeTO共享同一个CacheTO引用，通过 setter 修改的也都是不可变的String对象，不会产生实际的状态污染。此外，代码中未发现.getCache().setXxx()这样的修改模式，该风险在实际应用中未被触发。若要从代码规范角度完整修复，可考虑深拷贝（对应字段逐个复制）或使用不可变对象设计，但当前实际影响有限。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java` around lines 68 - 90, The copy constructor VolumeTO(VolumeTO other) currently performs a shallow copy of the cache field (this.cache = other.cache), which can lead to shared-reference issues; change it to perform a deep copy of CacheTO by constructing a new CacheTO instance and copying its String fields (e.g., via new CacheTO(other.getCache()) or by setting each field from other.getCache()) so VolumeTO and other do not share the same CacheTO reference; update the constructor to use the new CacheTO copy and ensure getters/setters used are CacheTO-specific (getCache(), setCache(...)).plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolVO.java (1)
43-47: 容量字段建议使用long类型
totalCapacity和availableCapacity使用double类型可能导致浮点精度问题。存储容量通常以字节为单位，建议使用long类型以避免精度损失，这也是存储系统中的常见做法。♻️ 建议修改
`@Column` - private double totalCapacity; + private long totalCapacity; `@Column` - private double availableCapacity; + private long availableCapacity;同时需要更新相应的 getter/setter 方法：
- public double getTotalCapacity() { + public long getTotalCapacity() { return totalCapacity; } - public void setTotalCapacity(double totalCapacity) { + public void setTotalCapacity(long totalCapacity) { this.totalCapacity = totalCapacity; } - public double getAvailableCapacity() { + public long getAvailableCapacity() { return availableCapacity; } - public void setAvailableCapacity(double availableCapacity) { + public void setAvailableCapacity(long availableCapacity) { this.availableCapacity = availableCapacity; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolVO.java` around lines 43 - 47, The two capacity fields totalCapacity and availableCapacity in VmLocalVolumeCachePoolVO are typed as double and should be changed to long to avoid floating-point precision issues for byte-sized storage; update the field declarations in class VmLocalVolumeCachePoolVO, change their types to long, and adjust the corresponding getter/setter methods (getTotalCapacity/setTotalCapacity and getAvailableCapacity/setAvailableCapacity) to use long; also check any places that read/write these fields (ORM mapping annotations on the fields and any code that expects double) and update conversions or database column type mappings if necessary to keep persistence consistent.plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/CachePoolHostAllocatorFilter.java (2)
69-71: 简化返回空列表的写法使用
filter(h -> false)来返回空列表是不必要的复杂写法。♻️ 建议修改
- return candidates.stream() - .filter(h -> false) - .collect(Collectors.toList()); + return Collections.emptyList();需要添加导入：
import java.util.Collections;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/CachePoolHostAllocatorFilter.java` around lines 69 - 71, The code in CachePoolHostAllocatorFilter returns an empty list via candidates.stream().filter(h -> false).collect(...), which is unnecessarily complex; replace that expression with Collections.emptyList() and add the import java.util.Collections; update the return in the relevant method inside class CachePoolHostAllocatorFilter to return Collections.emptyList() instead of using the stream/filter pattern.
93-94: 简化 Set 转换
hostsWithPool已经是List<String>，可以直接使用new HashSet<>(hostsWithPool)或者使用Set.copyOf()(Java 10+)，无需再次 stream。♻️ 建议修改
- Set<String> qualifiedHostUuids = hostsWithPool.stream().collect(Collectors.toSet()); + Set<String> qualifiedHostUuids = new HashSet<>(hostsWithPool);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/CachePoolHostAllocatorFilter.java` around lines 93 - 94, The code creates qualifiedHostUuids by streaming hostsWithPool which is already a List<String>; replace the stream-based conversion with a direct Set construction to simplify and improve readability: in CachePoolHostAllocatorFilter replace the hostsWithPool.stream().collect(Collectors.toSet()) expression used to assign qualifiedHostUuids with either new HashSet<>(hostsWithPool) or Set.copyOf(hostsWithPool) (Java 10+) so qualifiedHostUuids is built directly from hostsWithPool.compute/src/main/java/org/zstack/compute/vm/VmInstantiateCacheFlow.java (1)
54-63: 考虑使用批量查询优化性能当前实现在循环中为每个 volume UUID 执行单独的数据库查询。当有多个数据卷时，这会产生 N+1 查询问题。建议使用批量查询。
♻️ 建议修改
- List<VmLocalVolumeCacheVO> toInstantiate = new ArrayList<>(); - for (String volUuid : volumeUuids) { - VmLocalVolumeCacheVO cacheVO = Q.New(VmLocalVolumeCacheVO.class) - .eq(VmLocalVolumeCacheVO_.volumeUuid, volUuid) - .find(); - if (cacheVO != null - && cacheVO.getState() == VmLocalVolumeCacheState.Uninstantiated - && cacheVO.getPoolUuid() != null) { - toInstantiate.add(cacheVO); - } - } + List<VmLocalVolumeCacheVO> toInstantiate = Q.New(VmLocalVolumeCacheVO.class) + .in(VmLocalVolumeCacheVO_.volumeUuid, volumeUuids) + .eq(VmLocalVolumeCacheVO_.state, VmLocalVolumeCacheState.Uninstantiated) + .notNull(VmLocalVolumeCacheVO_.poolUuid) + .list();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/vm/VmInstantiateCacheFlow.java` around lines 54 - 63, The loop in VmInstantiateCacheFlow issues one DB query per volumeUuid causing N+1 queries; replace it with a single batch query against VmLocalVolumeCacheVO using an IN filter on volumeUuid plus conditions for state == VmLocalVolumeCacheState.Uninstantiated and poolUuid != null, then iterate the returned list to populate toInstantiate (matching by volumeUuid or simply adding all results); update references to volumeUuids, VmLocalVolumeCacheVO, VmLocalVolumeCacheState and toInstantiate accordingly to remove per-UUID Q.New calls.storage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.java (2)
694-708: 建议对 volumeUuid 进行 trim 处理根据编码规范，API 参数可能包含用户从浏览器复制粘贴时带入的空格或换行符。建议对
volumeUuid进行 trim 处理。♻️ 建议修改
private void validate(APIEnableVolumeCacheMsg msg) { - String volumeUuid = msg.getVolumeUuid(); + String volumeUuid = msg.getVolumeUuid(); + if (volumeUuid != null) { + volumeUuid = volumeUuid.trim(); + msg.setVolumeUuid(volumeUuid); + } if (!dbf.isExist(volumeUuid, VolumeVO.class)) { throw new ApiMessageInterceptionException(argerr("volume[uuid:%s] does not exist", volumeUuid)); }As per coding guidelines: "注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@storage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.java` around lines 694 - 708, In validate(APIEnableVolumeCacheMsg msg) trim the incoming volumeUuid (e.g., String volumeUuid = msg.getVolumeUuid().trim()) before using it in dbf.isExist(...) and the Q.New(...).eq(...) lookup and in exception messages; ensure you handle null safely (check msg.getVolumeUuid() for null) so the validation uses the trimmed value for existence checks and comparisons (refer to validate, volumeUuid, dbf.isExist, Q.New(VmLocalVolumeCacheVO.class) and VmLocalVolumeCacheVO_.volumeUuid).
710-723: 同样建议对 APIDisableVolumeCacheMsg 的 volumeUuid 进行 trim 处理与
validate(APIEnableVolumeCacheMsg)相同的问题。♻️ 建议修改
private void validate(APIDisableVolumeCacheMsg msg) { - String volumeUuid = msg.getVolumeUuid(); + String volumeUuid = msg.getVolumeUuid(); + if (volumeUuid != null) { + volumeUuid = volumeUuid.trim(); + msg.setVolumeUuid(volumeUuid); + } if (!dbf.isExist(volumeUuid, VolumeVO.class)) { throw new ApiMessageInterceptionException(argerr("volume[uuid:%s] does not exist", volumeUuid)); }As per coding guidelines: "注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@storage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.java` around lines 710 - 723, The validate(APIDisableVolumeCacheMsg msg) method uses volumeUuid directly; trim the input like in validate(APIEnableVolumeCacheMsg) by obtaining a trimmed variable (e.g., String volumeUuid = msg.getVolumeUuid() == null ? null : msg.getVolumeUuid().trim()), validate it for null/empty before calling dbf.isExist(…, VolumeVO.class), and then use that trimmed variable in the Q.New(VmLocalVolumeCacheVO.class).eq(VmLocalVolumeCacheVO_.volumeUuid, volumeUuid).find() and subsequent checks against VmLocalVolumeCacheState.None to ensure whitespace from APIDisableVolumeCacheMsg is handled.plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIExtendVmLocalVolumeCachePoolEvent.java (1)
25-27:__example__()方法应填充示例 inventory参考
APICreateVmLocalVolumeCachePoolEvent.__example__()的实现，该方法设置了 inventory 示例数据。为保持一致性并生成更完整的 API 文档，建议在此方法中也设置 inventory。♻️ 建议修改
public static APIExtendVmLocalVolumeCachePoolEvent __example__() { - return new APIExtendVmLocalVolumeCachePoolEvent(); + APIExtendVmLocalVolumeCachePoolEvent event = new APIExtendVmLocalVolumeCachePoolEvent(); + VmLocalVolumeCachePoolInventory inventory = new VmLocalVolumeCachePoolInventory(); + event.setInventory(inventory); + return event; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIExtendVmLocalVolumeCachePoolEvent.java` around lines 25 - 27, The __example__() in class APIExtendVmLocalVolumeCachePoolEvent currently returns an empty event; update APIExtendVmLocalVolumeCachePoolEvent.__example__ to construct and return an event with a populated inventory (mirroring APICreateVmLocalVolumeCachePoolEvent.__example__), e.g., create a VmLocalVolumeCachePoolInventory instance, fill its fields with representative sample values (uuid, name, description, type, url, zoneUuid, primaryStorageUuid, createDate, lastOpDate, etc.), call event.setInventory(populatedInventory), and then return the event so the API docs include a concrete inventory example.storage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.java (1)
1516-1518: 使用dbf.getCurrentSqlTime()替代new java.util.Date()直接使用
new java.util.Date().getTime()创建时间戳可能导致时间不一致问题，特别是在分布式环境中。建议使用dbf.getCurrentSqlTime()以确保与数据库时间同步。♻️ 建议修改
- cacheVO.setCreateDate(new Timestamp(new java.util.Date().getTime())); - cacheVO.setLastOpDate(new Timestamp(new java.util.Date().getTime())); + Timestamp now = dbf.getCurrentSqlTime(); + cacheVO.setCreateDate(now); + cacheVO.setLastOpDate(now);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@storage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.java` around lines 1516 - 1518, Replace the two Timestamp constructions using new java.util.Date().getTime() for cacheVO timestamps with the database-synchronized time from dbf.getCurrentSqlTime(); in VolumeManagerImpl where cacheVO.setCreateDate(...) and cacheVO.setLastOpDate(...) are called (just before dbf.persist(cacheVO)), call new Timestamp(dbf.getCurrentSqlTime().getTime()) or otherwise use dbf.getCurrentSqlTime() directly to set both create and last-op dates so the persisted timestamps are consistent with DB time.header/src/main/java/org/zstack/header/vm/CreateVmInstanceMsg.java (1)
412-421: 建议在新字符串字段写入前做 trim 规整Line 412 和 Line 420 直接保存原始字符串，来自消息链路时若包含前后空格/换行，后续按 UUID 或 mode 匹配时容易出现隐式失败。建议在 setter 中做
trim()（可选再把空串归一化为null）。💡 建议修改
public void setCachePoolUuid(String cachePoolUuid) { - this.cachePoolUuid = cachePoolUuid; + if (cachePoolUuid == null) { + this.cachePoolUuid = null; + return; + } + String trimmed = cachePoolUuid.trim(); + this.cachePoolUuid = trimmed.isEmpty() ? null : trimmed; } public void setCacheMode(String cacheMode) { - this.cacheMode = cacheMode; + if (cacheMode == null) { + this.cacheMode = null; + return; + } + String trimmed = cacheMode.trim(); + this.cacheMode = trimmed.isEmpty() ? null : trimmed; }As per coding guidelines “注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等”。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/CreateVmInstanceMsg.java` around lines 412 - 421, The setters setCachePoolUuid and setCacheMode in CreateVmInstanceMsg currently assign raw input strings; update both to trim incoming values (e.g., String v = input == null ? null : input.trim()) and normalize empty strings to null before assigning to this.cachePoolUuid and this.cacheMode so leading/trailing whitespace or newlines from messages won’t break UUID/mode matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java`:
- Around line 154-161: The root-cache-size calculation currently uses
image.getSize() whenever an ImageInventory exists, which treats ISOs as having
cache size and mismatches the prepareMsg logic; update the block in
VmAllocateHostFlow where enableRoot is handled so that you only use
image.getSize() when the image is not an ISO and size != 0, otherwise fall back
to spec.getRootDiskOffering().getDiskSize() (i.e., change the condition around
ImageInventory image = spec.getImageSpec().getInventory() to check image media
type/ISO status before using image.getSize()); keep totalCacheSize accumulation
and ensure this logic mirrors the root-size calculation in prepareMsg.
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 7-8: The capacity columns `totalCapacity` and `availableCapacity`
are currently defined as DOUBLE; change their types to BIGINT UNSIGNED to avoid
floating-point precision issues and match prior schema versions (use
`totalCapacity` BIGINT UNSIGNED NOT NULL DEFAULT 0 and `availableCapacity`
BIGINT UNSIGNED NOT NULL DEFAULT 0). Update the ALTER/CREATE statement in
V5.5.12__schema.sql to use BIGINT UNSIGNED and ensure any migration path handles
existing rows (e.g., cast/validate current values to integers and apply DEFAULT
0 for NULLs) so upgrades don’t lose or corrupt capacity data.
In `@header/src/main/java/org/zstack/header/host/HostBlockDeviceStruct.java`:
- Around line 22-34: The equals/hashCode pair in HostBlockDeviceStruct currently
treats all instances with wwid == null as equal; change the logic in
equals(Object o) and hashCode() so that if wwid is non-null you compare/hash by
wwid, but if wwid is null you fall back to a stable secondary key composed of
path and hctl (e.g., compare path.equals(...) and hctl.equals(...) and compute
hash from path and hctl). Update both HostBlockDeviceStruct.equals and
HostBlockDeviceStruct.hashCode to use the same wwid-first else path+hctl
strategy so null-wwid devices are distinguished consistently.
In
`@header/src/main/java/org/zstack/header/volume/localcache/APIDisableVolumeCacheEvent.java`:
- Around line 5-6: APIDisableVolumeCacheEvent is missing the `@RestResponse`
annotation for its inventory field; update the class APIDisableVolumeCacheEvent
to add the `@RestResponse`(allTo = "inventory") annotation (same as
APIEnableVolumeCacheEvent) so the VmLocalVolumeCacheInventory inventory is
exposed correctly in REST responses.
In
`@header/src/main/java/org/zstack/header/volume/localcache/APIEnableVolumeCacheEvent.java`:
- Around line 5-6: The APIEnableVolumeCacheEvent class declares a
VmLocalVolumeCacheInventory field named inventory but is missing the REST
response annotation; add the annotation `@RestResponse`(allTo = "inventory") to
the APIEnableVolumeCacheEvent class (same pattern used in
APIExtendVmLocalVolumeCachePoolEvent and APIUpdateVmLocalVolumeCachePoolEvent)
so the inventory is correctly serialized in REST responses.
In
`@header/src/main/java/org/zstack/header/volume/localcache/APIEnableVolumeCacheMsg.java`:
- Around line 29-31: In APIEnableVolumeCacheMsg update the setVolumeUuid(String
volumeUuid) setter to trim incoming values before assignment (e.g., assign
this.volumeUuid = volumeUuid == null ? null : volumeUuid.trim()) so pasted UUIDs
with leading/trailing spaces or newlines are normalized; ensure you reference
the setter method setVolumeUuid and the field volumeUuid in class
APIEnableVolumeCacheMsg and preserve null-safety when trimming.
In
`@header/src/main/java/org/zstack/header/volume/localcache/DisableVolumeCacheMsg.java`:
- Around line 12-13: 在 DisableVolumeCacheMsg 的 setVolumeUuid 方法中对传入的 volumeUuid
做规范化：在赋值前调用 trim()（并处理 null 安全），例如在 setVolumeUuid(String volumeUuid) 中将
this.volumeUuid 设为 volumeUuid == null ? null : volumeUuid.trim()，确保后续使用该字段（如
UUID 比对/查询）不会因为首尾空白或换行导致匹配失败。
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMApiInterceptor.java`:
- Around line 190-191: In KVMApiInterceptor (class KVMApiInterceptor) remove the
pointless "%s", "" placeholder from the error message construction so the
message reads without the trailing format placeholder; locate the string that
currently reads "cachePoolUuid or cacheMode is specified but neither
enableRootVolumeCache nor enableDataVolumeCache is set to true%s", "" and
replace it with the plain message "cachePoolUuid or cacheMode is specified but
neither enableRootVolumeCache nor enableDataVolumeCache is set to true" so no
formatting arguments are passed.
- Around line 106-110: The validate method
(validate(APICreateVmLocalVolumeCachePoolMsg msg)) currently uses msg.getName(),
msg.getHostUuid(), and msg.getUuid() directly when checking uniqueness and
ownership; update the code to trim incoming message fields first (e.g., apply
StringUtils.trimToNull or similar to msg.getName(), msg.getHostUuid(),
msg.getUuid() at the start of validate and any other interceptor methods) and
then use the trimmed values for DB queries and equality checks (the Q.New(...)
name/hostUuid comparisons and any subsequent ownership checks), ensuring
null/empty is handled consistently after trimming to prevent bypass by
leading/trailing whitespace.
In `@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/CacheTO.java`:
- Around line 44-50: The valueOf method in CacheTO calls
inv.getCacheMode().name() which can NPE if cacheMode is null; update
CacheTO.valueOf to null-check inv.getCacheMode() (or use Optional) and only call
name() when non-null, otherwise setCacheMode to null (or a safe default/empty
string) so legacy/absent cacheMode values do not throw; reference the
CacheTO.valueOf method and VmLocalVolumeCacheInventory.getCacheMode() when
making the change and preserve existing behavior for non-null values.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCacheManagerImpl.java`:
- Around line 261-264: The code in VmLocalVolumeCacheManagerImpl parses
spec.getCacheMode() with VmLocalVolumeCacheMode.valueOf(...) without validating
or trimming input and does not handle IllegalArgumentException; update the
parsing in the method that contains specPoolUuid and cacheMode so you first
sanitize the string (null-check and trim), attempt to map it to
VmLocalVolumeCacheMode inside a try/catch, and on IllegalArgumentException call
completion.fail(...) with a clear error message (fall back to
VmLocalVolumeCacheMode.WriteBack only if intended), ensuring the same defensive
handling is applied where cacheMode is parsed later (the other valueOf usage
around lines 281-283).
- Around line 151-160: The cache injection queries in
VmLocalVolumeCacheManagerImpl use only volumeUuid and Instantiated state and
must also ensure the cache belongs to the target host; update the Q.New(...)
lookup (the places that create VmLocalVolumeCacheVO via
Q.New(...).eq(VmLocalVolumeCacheVO_.volumeUuid, volumeUuid).find() right before
volumeTO.setCache(...)) to also filter by the current host (e.g.
.eq(VmLocalVolumeCacheVO_.hostUuid, hostUuid)) or, if the VO stores poolUuid
only, resolve the pool's host and verify it matches hostUuid before using the
cache; apply this change consistently to all occurrences that set
CacheTO.valueOf(VmLocalVolumeCacheInventory.valueOf(cacheVO)) so only same-host
(or pool->host equal) instantiated caches are injected.
- Around line 299-317: The current early-return on existing cache
(VmLocalVolumeCacheVO existing with existing.getState() !=
VmLocalVolumeCacheState.None) is too broad and allows an Instantiated cache from
a different host to remain unchanged; change the logic to check host affinity:
if existing != null then compare existing.getHostUuid() with the current
hostUuid and only skip/return when the existing record is on the same host and
in a reusable state; otherwise treat the record as non-reusable by setting
existing.setState(VmLocalVolumeCacheState.Uninstantiated), update
existing.setCacheMode(cacheMode) and setPoolUuid (determine poolUuid via
selectBestPoolOnHost(hostUuid, volumeSize) when needed), then persist with
dbf.updateAndRefresh(existing) so cross-host stale records get reset and
reassigned.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolBase.java`:
- Around line 405-439: The two handlers
handle(APISyncVmLocalVolumeCachePoolCapacityMsg) and
handle(SyncCachePoolCapacityMsg) call syncPoolCapacity(...) directly and must be
serialized on the same queue as other pool ops; wrap the syncPoolCapacity
invocation inside a ChainTask submitted to the pool's syncThreadName (use the
existing ChainTask pattern used elsewhere in this class) so capacity sync runs
on the same serial thread as delete/reconnect/extend; inside the ChainTask's
run/next callback call syncPoolCapacity with the same ReturnValueCompletion
implementation and publish/reply the event/response from the completion
callbacks, preserving error handling.
- Around line 155-166: When deleting a pool in
deletePool(APIDeleteVmLocalVolumeCachePoolMsg msg, Completion completion) you
must first handle VmLocalVolumeCacheVO records that reference this poolUuid
instead of directly calling dbf.remove(self); query for VmLocalVolumeCacheVO
where poolUuid == self.getUuid(), then either block the delete by returning an
error if any active caches exist or atomically update those rows to clear/mark
their pool reference (e.g. set poolUuid=null or set status=ORPHAN) inside the
same DB transaction before calling dbf.remove(self); ensure you use the existing
DB transaction APIs and update the VM cache records and pool removal atomically
so no dangling VmLocalVolumeCacheVO.poolUuid references remain.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolCanonicalEvents.java`:
- Around line 50-55: The getters/setters for Date in
VmLocalVolumeCachePoolCanonicalEvents leak the mutable Date reference; change
getDate() to return a defensive copy (e.g., new Date(date.getTime()) or
null-safe equivalent) and change setDate(Date date) to store a copy of the
incoming Date (or null if input is null) so the internal field cannot be mutated
by external callers; update both methods (getDate and setDate) to handle null
safely.
In `@storage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.java`:
- Around line 1569-1577: In instantiateAndAttachCache, when the
AttachVolumeCacheMsg reply fails, add rollback cleanup for the cache created by
InstantiateLocalVolumeCacheMsg: detect the failure branch (where
reply.isSuccess() is false), extract the created cache identity from
ir.getInventory() (or the InstantiateLocalVolumeCacheMsg result), and issue a
cleanup operation (e.g., send a DeleteLocalVolumeCacheMsg or call the existing
delete/remove-cache routine with the cache UUID/path) before replying to
originMsg; ensure errors from the cleanup are logged but do not suppress
replying with originReply.setError(reply.getError()).
---
Nitpick comments:
In `@compute/src/main/java/org/zstack/compute/vm/VmInstantiateCacheFlow.java`:
- Around line 54-63: The loop in VmInstantiateCacheFlow issues one DB query per
volumeUuid causing N+1 queries; replace it with a single batch query against
VmLocalVolumeCacheVO using an IN filter on volumeUuid plus conditions for state
== VmLocalVolumeCacheState.Uninstantiated and poolUuid != null, then iterate the
returned list to populate toInstantiate (matching by volumeUuid or simply adding
all results); update references to volumeUuids, VmLocalVolumeCacheVO,
VmLocalVolumeCacheState and toInstantiate accordingly to remove per-UUID Q.New
calls.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.java`:
- Around line 415-417: EnableVolumeCacheMsg 目前只设置了 volumeUuid（在
EnableVolumeCacheMsg 和 bus.makeLocalServiceId(..., VolumeConstant.SERVICE_ID)
处），会丢失上游传来的缓存池/模式配置；请在构造 EnableVolumeCacheMsg 时把上游的 cache pool 与 cache mode
字段透传到该消息（例如从调用上下文或 volumeInventory/请求对象中读取对应的缓存配置字段并通过 EnableVolumeCacheMsg 的对应
setter 赋值），确保发送前包含 cache pool 与 cache mode，保持原有 bus.makeLocalServiceId 调用不变。
In `@header/src/main/java/org/zstack/header/vm/CreateVmInstanceMsg.java`:
- Around line 412-421: The setters setCachePoolUuid and setCacheMode in
CreateVmInstanceMsg currently assign raw input strings; update both to trim
incoming values (e.g., String v = input == null ? null : input.trim()) and
normalize empty strings to null before assigning to this.cachePoolUuid and
this.cacheMode so leading/trailing whitespace or newlines from messages won’t
break UUID/mode matching.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIExtendVmLocalVolumeCachePoolEvent.java`:
- Around line 25-27: The __example__() in class
APIExtendVmLocalVolumeCachePoolEvent currently returns an empty event; update
APIExtendVmLocalVolumeCachePoolEvent.__example__ to construct and return an
event with a populated inventory (mirroring
APICreateVmLocalVolumeCachePoolEvent.__example__), e.g., create a
VmLocalVolumeCachePoolInventory instance, fill its fields with representative
sample values (uuid, name, description, type, url, zoneUuid, primaryStorageUuid,
createDate, lastOpDate, etc.), call event.setInventory(populatedInventory), and
then return the event so the API docs include a concrete inventory example.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/CachePoolHostAllocatorFilter.java`:
- Around line 69-71: The code in CachePoolHostAllocatorFilter returns an empty
list via candidates.stream().filter(h -> false).collect(...), which is
unnecessarily complex; replace that expression with Collections.emptyList() and
add the import java.util.Collections; update the return in the relevant method
inside class CachePoolHostAllocatorFilter to return Collections.emptyList()
instead of using the stream/filter pattern.
- Around line 93-94: The code creates qualifiedHostUuids by streaming
hostsWithPool which is already a List<String>; replace the stream-based
conversion with a direct Set construction to simplify and improve readability:
in CachePoolHostAllocatorFilter replace the
hostsWithPool.stream().collect(Collectors.toSet()) expression used to assign
qualifiedHostUuids with either new HashSet<>(hostsWithPool) or
Set.copyOf(hostsWithPool) (Java 10+) so qualifiedHostUuids is built directly
from hostsWithPool.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolMessage.java`:
- Around line 3-5: 在 VmLocalVolumeCachePoolMessage 接口上添加 Javadoc
注释以说明接口用途和方法含义；具体在接口声明处为 VmLocalVolumeCachePoolMessage 添加一段简短描述（功能、使用场景），并为其方法
getPoolUuid() 添加 `@return` 描述说明返回值含义，必要时参考相似接口 VmLocalVolumeCacheMessage
的注释风格以保持一致性，并确保注释遵循项目的 Javadoc 规范。
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolVO.java`:
- Around line 43-47: The two capacity fields totalCapacity and availableCapacity
in VmLocalVolumeCachePoolVO are typed as double and should be changed to long to
avoid floating-point precision issues for byte-sized storage; update the field
declarations in class VmLocalVolumeCachePoolVO, change their types to long, and
adjust the corresponding getter/setter methods
(getTotalCapacity/setTotalCapacity and
getAvailableCapacity/setAvailableCapacity) to use long; also check any places
that read/write these fields (ORM mapping annotations on the fields and any code
that expects double) and update conversions or database column type mappings if
necessary to keep persistence consistent.
In `@plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java`:
- Around line 68-90: The copy constructor VolumeTO(VolumeTO other) currently
performs a shallow copy of the cache field (this.cache = other.cache), which can
lead to shared-reference issues; change it to perform a deep copy of CacheTO by
constructing a new CacheTO instance and copying its String fields (e.g., via new
CacheTO(other.getCache()) or by setting each field from other.getCache()) so
VolumeTO and other do not share the same CacheTO reference; update the
constructor to use the new CacheTO copy and ensure getters/setters used are
CacheTO-specific (getCache(), setCache(...)).
In `@storage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.java`:
- Around line 694-708: In validate(APIEnableVolumeCacheMsg msg) trim the
incoming volumeUuid (e.g., String volumeUuid = msg.getVolumeUuid().trim())
before using it in dbf.isExist(...) and the Q.New(...).eq(...) lookup and in
exception messages; ensure you handle null safely (check msg.getVolumeUuid() for
null) so the validation uses the trimmed value for existence checks and
comparisons (refer to validate, volumeUuid, dbf.isExist,
Q.New(VmLocalVolumeCacheVO.class) and VmLocalVolumeCacheVO_.volumeUuid).
- Around line 710-723: The validate(APIDisableVolumeCacheMsg msg) method uses
volumeUuid directly; trim the input like in validate(APIEnableVolumeCacheMsg) by
obtaining a trimmed variable (e.g., String volumeUuid = msg.getVolumeUuid() ==
null ? null : msg.getVolumeUuid().trim()), validate it for null/empty before
calling dbf.isExist(…, VolumeVO.class), and then use that trimmed variable in
the Q.New(VmLocalVolumeCacheVO.class).eq(VmLocalVolumeCacheVO_.volumeUuid,
volumeUuid).find() and subsequent checks against VmLocalVolumeCacheState.None to
ensure whitespace from APIDisableVolumeCacheMsg is handled.
In `@storage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.java`:
- Around line 1516-1518: Replace the two Timestamp constructions using new
java.util.Date().getTime() for cacheVO timestamps with the database-synchronized
time from dbf.getCurrentSqlTime(); in VolumeManagerImpl where
cacheVO.setCreateDate(...) and cacheVO.setLastOpDate(...) are called (just
before dbf.persist(cacheVO)), call new
Timestamp(dbf.getCurrentSqlTime().getTime()) or otherwise use
dbf.getCurrentSqlTime() directly to set both create and last-op dates so the
persisted timestamps are consistent with DB time.
ℹ️ Review info
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
conf/globalConfig/kvm.xmlis excluded by!**/*.xmlconf/persistence.xmlis excluded by!**/*.xmlconf/serviceConfig/vmLocalVolumeCachePool.xmlis excluded by!**/*.xmlconf/serviceConfig/volume.xmlis excluded by!**/*.xmlconf/springConfigXml/Kvm.xmlis excluded by!**/*.xmlconf/springConfigXml/VmInstanceManager.xmlis excluded by!**/*.xml
📒 Files selected for processing (88)
compute/src/main/java/org/zstack/compute/host/HostBase.javacompute/src/main/java/org/zstack/compute/vm/InstantiateVmFromNewCreatedStruct.javacompute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceManagerImpl.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceUtils.javacompute/src/main/java/org/zstack/compute/vm/VmInstantiateCacheFlow.javacompute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.javaconf/db/upgrade/V5.5.12__schema.sqlheader/src/main/java/org/zstack/header/host/APIGetBlockDevicesEvent.javaheader/src/main/java/org/zstack/header/host/APIGetBlockDevicesMsg.javaheader/src/main/java/org/zstack/header/host/GetBlockDevicesOnHostMsg.javaheader/src/main/java/org/zstack/header/host/GetBlockDevicesOnHostReply.javaheader/src/main/java/org/zstack/header/host/HostBlockDeviceStruct.javaheader/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/CreateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/InstantiateNewCreatedVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/VmInstanceSpec.javaheader/src/main/java/org/zstack/header/volume/localcache/APIDisableVolumeCacheEvent.javaheader/src/main/java/org/zstack/header/volume/localcache/APIDisableVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/volume/localcache/APIEnableVolumeCacheEvent.javaheader/src/main/java/org/zstack/header/volume/localcache/APIEnableVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/volume/localcache/AttachVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/volume/localcache/AttachVolumeCacheReply.javaheader/src/main/java/org/zstack/header/volume/localcache/DeleteLocalVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/volume/localcache/DeleteLocalVolumeCacheReply.javaheader/src/main/java/org/zstack/header/volume/localcache/DisableVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/volume/localcache/DisableVolumeCacheReply.javaheader/src/main/java/org/zstack/header/volume/localcache/EnableVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/volume/localcache/EnableVolumeCacheReply.javaheader/src/main/java/org/zstack/header/volume/localcache/InstantiateLocalVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/volume/localcache/InstantiateLocalVolumeCacheReply.javaheader/src/main/java/org/zstack/header/volume/localcache/LiveDetachVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/volume/localcache/LiveDetachVolumeCacheReply.javaheader/src/main/java/org/zstack/header/volume/localcache/OfflineDetachVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/volume/localcache/OfflineDetachVolumeCacheReply.javaheader/src/main/java/org/zstack/header/volume/localcache/VmLocalVolumeCacheConstant.javaheader/src/main/java/org/zstack/header/volume/localcache/VmLocalVolumeCacheInventory.javaheader/src/main/java/org/zstack/header/volume/localcache/VmLocalVolumeCacheMessage.javaheader/src/main/java/org/zstack/header/volume/localcache/VmLocalVolumeCacheMode.javaheader/src/main/java/org/zstack/header/volume/localcache/VmLocalVolumeCacheState.javaheader/src/main/java/org/zstack/header/volume/localcache/VmLocalVolumeCacheVO.javaheader/src/main/java/org/zstack/header/volume/localcache/VmLocalVolumeCacheVO_.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMApiInterceptor.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIChangeVmLocalVolumeCachePoolStateEvent.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIChangeVmLocalVolumeCachePoolStateMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APICreateVmLocalVolumeCachePoolEvent.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APICreateVmLocalVolumeCachePoolMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIDeleteVmLocalVolumeCachePoolEvent.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIDeleteVmLocalVolumeCachePoolMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIExtendVmLocalVolumeCachePoolEvent.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIExtendVmLocalVolumeCachePoolMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIQueryVmLocalVolumeCachePoolMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIQueryVmLocalVolumeCachePoolReply.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIReconnectVmLocalVolumeCachePoolEvent.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIReconnectVmLocalVolumeCachePoolMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APISyncVmLocalVolumeCachePoolCapacityEvent.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APISyncVmLocalVolumeCachePoolCapacityMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIUpdateVmLocalVolumeCachePoolEvent.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/APIUpdateVmLocalVolumeCachePoolMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/CachePoolHostAllocatorFilter.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/CacheTO.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/RBACInfo.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/SyncCachePoolCapacityMsg.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/SyncCachePoolCapacityReply.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCacheBase.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCacheConstant.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCacheManager.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCacheManagerImpl.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolBase.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolCanonicalEvents.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolGlobalConfig.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolInventory.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolManager.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolManagerImpl.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolMessage.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolState.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolStatus.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolVO.javaplugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolVO_.javastorage/src/main/java/org/zstack/storage/volume/DisableVolumeCacheLongJob.javastorage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.javastorage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.javautils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
Comment on lines
+154
to
+161
| if (enableRoot) { | ||
| // Root volume size: from image size or root disk offering | ||
| ImageInventory image = spec.getImageSpec().getInventory(); | ||
| if (image != null && image.getSize() != 0) { | ||
| totalCacheSize += image.getSize(); | ||
| } else if (spec.getRootDiskOffering() != null) { | ||
| totalCacheSize += spec.getRootDiskOffering().getDiskSize(); | ||
| } |
There was a problem hiding this comment.
根卷缓存大小计算缺少 ISO 分支，可能导致主机分配误判。
当前只要 image 存在就优先用 image.getSize()。但 ISO 场景根卷大小应来自 root disk offering（与本文件 prepareMsg 的根盘计算逻辑不一致），否则会把 ISO 体积当作缓存需求，放大 requiredSize 约束。
🔧 建议修改
if (enableRoot) {
- // Root volume size: from image size or root disk offering
- ImageInventory image = spec.getImageSpec().getInventory();
- if (image != null && image.getSize() != 0) {
- totalCacheSize += image.getSize();
- } else if (spec.getRootDiskOffering() != null) {
- totalCacheSize += spec.getRootDiskOffering().getDiskSize();
- }
+ // Root volume size: ISO场景取root disk offering，其它镜像优先取image size
+ ImageInventory image = spec.getImageSpec() == null ? null : spec.getImageSpec().getInventory();
+ boolean nonIsoImage = image != null && !ImageMediaType.ISO.toString().equals(image.getMediaType());
+ if (nonIsoImage && image.getSize() > 0) {
+ totalCacheSize += image.getSize();
+ } else if (spec.getRootDiskOffering() != null) {
+ totalCacheSize += spec.getRootDiskOffering().getDiskSize();
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java` around
lines 154 - 161, The root-cache-size calculation currently uses image.getSize()
whenever an ImageInventory exists, which treats ISOs as having cache size and
mismatches the prepareMsg logic; update the block in VmAllocateHostFlow where
enableRoot is handled so that you only use image.getSize() when the image is not
an ISO and size != 0, otherwise fall back to
spec.getRootDiskOffering().getDiskSize() (i.e., change the condition around
ImageInventory image = spec.getImageSpec().getInventory() to check image media
type/ISO status before using image.getSize()); keep totalCacheSize accumulation
and ensure this logic mirrors the root-size calculation in prepareMsg.
conf/db/upgrade/V5.5.12__schema.sql
Outdated
Comment on lines
+7
to
+8
| `totalCapacity` DOUBLE NOT NULL DEFAULT 0, | ||
| `availableCapacity` DOUBLE NOT NULL DEFAULT 0, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how capacity-related fields are defined across the codebase
rg -n "Capacity.*DOUBLE|Capacity.*BIGINT|Capacity.*INT|capacity.*DOUBLE|capacity.*BIGINT|capacity.*INT" --type sql -C 2Repository: MatheMatrix/zstack
Length of output: 3173
容量字段应使用 BIGINT 而非 DOUBLE 类型。
totalCapacity 和 availableCapacity 目前使用 DOUBLE 类型，但根据代码库中的其他版本（如 V5.5.0、V3.9.0、V3.4.2），容量字段统一使用 BIGINT 或 BIGINT UNSIGNED。使用 DOUBLE 会引入浮点数精度问题，不适合存储容量值（通常为字节数）。建议改为 BIGINT UNSIGNED 以保持一致性。
另外，注意这两个字段添加了 NOT NULL DEFAULT 0，在升级场景中需要确认现有数据的处理方式。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 7 - 8, The capacity columns
`totalCapacity` and `availableCapacity` are currently defined as DOUBLE; change
their types to BIGINT UNSIGNED to avoid floating-point precision issues and
match prior schema versions (use `totalCapacity` BIGINT UNSIGNED NOT NULL
DEFAULT 0 and `availableCapacity` BIGINT UNSIGNED NOT NULL DEFAULT 0). Update
the ALTER/CREATE statement in V5.5.12__schema.sql to use BIGINT UNSIGNED and
ensure any migration path handles existing rows (e.g., cast/validate current
values to integers and apply DEFAULT 0 for NULLs) so upgrades don’t lose or
corrupt capacity data.
Comment on lines
+22
to
+34
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
|
|
||
| HostBlockDeviceStruct that = (HostBlockDeviceStruct) o; | ||
| return wwid != null ? wwid.equals(that.wwid) : that.wwid == null; | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return wwid != null ? wwid.hashCode() : 0; | ||
| } |
There was a problem hiding this comment.
避免把 wwid 为空的不同设备判定为相等
Line 28-33 当前逻辑会把所有 wwid == null 的对象视为相等，这会在集合去重或比对时吞掉真实设备条目。建议在 wwid 为空时回退到稳定次级键（如 path + hctl）比较与哈希。
建议修改
+import java.util.Objects;
@@
`@Override`
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
HostBlockDeviceStruct that = (HostBlockDeviceStruct) o;
- return wwid != null ? wwid.equals(that.wwid) : that.wwid == null;
+ if (wwid != null && that.wwid != null) {
+ return wwid.equals(that.wwid);
+ }
+ return Objects.equals(path, that.path) && Objects.equals(hctl, that.hctl);
}
`@Override`
public int hashCode() {
- return wwid != null ? wwid.hashCode() : 0;
+ return wwid != null ? wwid.hashCode() : Objects.hash(path, hctl);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@header/src/main/java/org/zstack/header/host/HostBlockDeviceStruct.java`
around lines 22 - 34, The equals/hashCode pair in HostBlockDeviceStruct
currently treats all instances with wwid == null as equal; change the logic in
equals(Object o) and hashCode() so that if wwid is non-null you compare/hash by
wwid, but if wwid is null you fall back to a stable secondary key composed of
path and hctl (e.g., compare path.equals(...) and hctl.equals(...) and compute
hash from path and hctl). Update both HostBlockDeviceStruct.equals and
HostBlockDeviceStruct.hashCode to use the same wwid-first else path+hctl
strategy so null-wwid devices are distinguished consistently.
Comment on lines
+5
to
+6
| public class APIDisableVolumeCacheEvent extends APIEvent { | ||
| private VmLocalVolumeCacheInventory inventory; |
There was a problem hiding this comment.
缺少 @RestResponse 注解
与 APIEnableVolumeCacheEvent 相同，此类包含 inventory 字段但缺少 @RestResponse(allTo = "inventory") 注解。建议添加以保持与其他 API Event 类的一致性。
🔧 建议修复
package org.zstack.header.volume.localcache;
import org.zstack.header.message.APIEvent;
+import org.zstack.header.rest.RestResponse;
+@RestResponse(allTo = "inventory")
public class APIDisableVolumeCacheEvent extends APIEvent {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@header/src/main/java/org/zstack/header/volume/localcache/APIDisableVolumeCacheEvent.java`
around lines 5 - 6, APIDisableVolumeCacheEvent is missing the `@RestResponse`
annotation for its inventory field; update the class APIDisableVolumeCacheEvent
to add the `@RestResponse`(allTo = "inventory") annotation (same as
APIEnableVolumeCacheEvent) so the VmLocalVolumeCacheInventory inventory is
exposed correctly in REST responses.
Comment on lines
+5
to
+6
| public class APIEnableVolumeCacheEvent extends APIEvent { | ||
| private VmLocalVolumeCacheInventory inventory; |
There was a problem hiding this comment.
缺少 @RestResponse 注解
参考其他类似的 API Event 类（如 APIExtendVmLocalVolumeCachePoolEvent、APIUpdateVmLocalVolumeCachePoolEvent 等），当 Event 类包含 inventory 字段时，应添加 @RestResponse(allTo = "inventory") 注解以确保 REST 响应正确序列化。
🔧 建议修复
package org.zstack.header.volume.localcache;
import org.zstack.header.message.APIEvent;
+import org.zstack.header.rest.RestResponse;
+@RestResponse(allTo = "inventory")
public class APIEnableVolumeCacheEvent extends APIEvent {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@header/src/main/java/org/zstack/header/volume/localcache/APIEnableVolumeCacheEvent.java`
around lines 5 - 6, The APIEnableVolumeCacheEvent class declares a
VmLocalVolumeCacheInventory field named inventory but is missing the REST
response annotation; add the annotation `@RestResponse`(allTo = "inventory") to
the APIEnableVolumeCacheEvent class (same pattern used in
APIExtendVmLocalVolumeCachePoolEvent and APIUpdateVmLocalVolumeCachePoolEvent)
so the inventory is correctly serialized in REST responses.
Comment on lines
+299
to
+317
| VmLocalVolumeCacheVO existing = Q.New(VmLocalVolumeCacheVO.class) | ||
| .eq(VmLocalVolumeCacheVO_.volumeUuid, volumeUuid) | ||
| .find(); | ||
| if (existing != null && existing.getState() != VmLocalVolumeCacheState.None) { | ||
| logger.debug(String.format("volume[uuid:%s] already has cache in state[%s], skip creating", | ||
| volumeUuid, existing.getState())); | ||
| return; | ||
| } | ||
|
|
||
| String poolUuid = specPoolUuid; | ||
| if (poolUuid == null) { | ||
| poolUuid = selectBestPoolOnHost(hostUuid, volumeSize); | ||
| } | ||
|
|
||
| if (existing != null) { | ||
| existing.setState(VmLocalVolumeCacheState.Uninstantiated); | ||
| existing.setCacheMode(cacheMode); | ||
| existing.setPoolUuid(poolUuid); | ||
| dbf.updateAndRefresh(existing); |
There was a problem hiding this comment.
已有缓存记录的复用条件过宽，跨主机场景不会重置记录
Line 302 只要 existing.getState() != None 就直接跳过。若该记录是旧主机上的 Instantiated 缓存，本次不会更新 poolUuid/state，会长期保留过期上下文。建议仅在“当前主机可复用”时跳过，否则重置为 Uninstantiated 并重新分配池。
建议修复（增加主机一致性判定）
- if (existing != null && existing.getState() != VmLocalVolumeCacheState.None) {
- logger.debug(String.format("volume[uuid:%s] already has cache in state[%s], skip creating",
- volumeUuid, existing.getState()));
- return;
- }
+ if (existing != null && existing.getState() != VmLocalVolumeCacheState.None) {
+ String existingPoolHostUuid = existing.getPoolUuid() == null ? null
+ : Q.New(VmLocalVolumeCachePoolVO.class)
+ .select(VmLocalVolumeCachePoolVO_.hostUuid)
+ .eq(VmLocalVolumeCachePoolVO_.uuid, existing.getPoolUuid())
+ .findValue();
+ if (hostUuid.equals(existingPoolHostUuid)) {
+ logger.debug(String.format("volume[uuid:%s] already has reusable cache in state[%s], skip creating",
+ volumeUuid, existing.getState()));
+ return;
+ }
+ existing.setState(VmLocalVolumeCacheState.Uninstantiated);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCacheManagerImpl.java`
around lines 299 - 317, The current early-return on existing cache
(VmLocalVolumeCacheVO existing with existing.getState() !=
VmLocalVolumeCacheState.None) is too broad and allows an Instantiated cache from
a different host to remain unchanged; change the logic to check host affinity:
if existing != null then compare existing.getHostUuid() with the current
hostUuid and only skip/return when the existing record is on the same host and
in a reusable state; otherwise treat the record as non-reusable by setting
existing.setState(VmLocalVolumeCacheState.Uninstantiated), update
existing.setCacheMode(cacheMode) and setPoolUuid (determine poolUuid via
selectBestPoolOnHost(hostUuid, volumeSize) when needed), then persist with
dbf.updateAndRefresh(existing) so cross-host stale records get reset and
reassigned.
Comment on lines
+155
to
+166
| private void deletePool(APIDeleteVmLocalVolumeCachePoolMsg msg, Completion completion) { | ||
| KVMAgentCommands.DeletePoolCmd cmd = new KVMAgentCommands.DeletePoolCmd(); | ||
| cmd.poolUuid = self.getUuid(); | ||
| cmd.mountPoint = self.getMetadata(); | ||
|
|
||
| httpCall(KVMConstant.LOCAL_VOLUME_CACHE_DELETE_POOL_PATH, self.getHostUuid(), cmd, | ||
| KVMAgentCommands.PoolRsp.class, new ReturnValueCompletion<KVMAgentCommands.PoolRsp>(completion) { | ||
| @Override | ||
| public void success(KVMAgentCommands.PoolRsp rsp) { | ||
| dbf.remove(self); | ||
| completion.success(); | ||
| } |
There was a problem hiding this comment.
删除池时未处理关联缓存记录，可能产生悬挂引用
Line 164 直接 dbf.remove(self)，但仍引用该池的 VmLocalVolumeCacheVO.poolUuid 记录未被阻断或清理。后续缓存侧按 poolUuid 查池时会触发空指针路径，并且这些缓存记录也会脱离正常容量/GC管理。
建议在删除前先校验并阻断（或原子清理关联缓存状态）再删除池。
💡 建议修改（阻断删除示例）
private void deletePool(APIDeleteVmLocalVolumeCachePoolMsg msg, Completion completion) {
+ long activeCacheCount = org.zstack.core.db.Q.New(org.zstack.header.volume.localcache.VmLocalVolumeCacheVO.class)
+ .eq(org.zstack.header.volume.localcache.VmLocalVolumeCacheVO_.poolUuid, self.getUuid())
+ .eq(org.zstack.header.volume.localcache.VmLocalVolumeCacheVO_.state,
+ org.zstack.header.volume.localcache.VmLocalVolumeCacheState.Instantiated)
+ .count();
+ if (activeCacheCount > 0) {
+ completion.fail(operr("pool[uuid:%s] still has %s instantiated caches, detach/delete caches first",
+ self.getUuid(), activeCacheCount));
+ return;
+ }
+
KVMAgentCommands.DeletePoolCmd cmd = new KVMAgentCommands.DeletePoolCmd();
cmd.poolUuid = self.getUuid();
cmd.mountPoint = self.getMetadata();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolBase.java`
around lines 155 - 166, When deleting a pool in
deletePool(APIDeleteVmLocalVolumeCachePoolMsg msg, Completion completion) you
must first handle VmLocalVolumeCacheVO records that reference this poolUuid
instead of directly calling dbf.remove(self); query for VmLocalVolumeCacheVO
where poolUuid == self.getUuid(), then either block the delete by returning an
error if any active caches exist or atomically update those rows to clear/mark
their pool reference (e.g. set poolUuid=null or set status=ORPHAN) inside the
same DB transaction before calling dbf.remove(self); ensure you use the existing
DB transaction APIs and update the VM cache records and pool removal atomically
so no dangling VmLocalVolumeCacheVO.poolUuid references remain.
Comment on lines
+405
to
+439
| private void handle(APISyncVmLocalVolumeCachePoolCapacityMsg msg) { | ||
| APISyncVmLocalVolumeCachePoolCapacityEvent event = new APISyncVmLocalVolumeCachePoolCapacityEvent(msg.getId()); | ||
|
|
||
| syncPoolCapacity(new ReturnValueCompletion<VmLocalVolumeCachePoolInventory>(msg) { | ||
| @Override | ||
| public void success(VmLocalVolumeCachePoolInventory inv) { | ||
| event.setInventory(inv); | ||
| bus.publish(event); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| event.setError(errorCode); | ||
| bus.publish(event); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| private void handle(SyncCachePoolCapacityMsg msg) { | ||
| syncPoolCapacity(new ReturnValueCompletion<VmLocalVolumeCachePoolInventory>(msg) { | ||
| @Override | ||
| public void success(VmLocalVolumeCachePoolInventory inv) { | ||
| SyncCachePoolCapacityReply reply = new SyncCachePoolCapacityReply(); | ||
| reply.setInventory(inv); | ||
| bus.reply(msg, reply); | ||
| } | ||
|
|
||
| @Override | ||
| public void fail(ErrorCode errorCode) { | ||
| SyncCachePoolCapacityReply reply = new SyncCachePoolCapacityReply(); | ||
| reply.setError(errorCode); | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
容量同步入口未串行化，存在并发覆盖风险
Line 405 和 Line 423 的两个 sync handler 直接调用 syncPoolCapacity(...)，没有进入 syncThreadName 对应的 ChainTask。这会与 delete/reconnect/extend 并发，带来状态回写覆盖或对已删除对象执行更新的风险。
建议将这两个入口也纳入同一串行队列，和其它 pool 操作保持一致的同步语义。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolBase.java`
around lines 405 - 439, The two handlers
handle(APISyncVmLocalVolumeCachePoolCapacityMsg) and
handle(SyncCachePoolCapacityMsg) call syncPoolCapacity(...) directly and must be
serialized on the same queue as other pool ops; wrap the syncPoolCapacity
invocation inside a ChainTask submitted to the pool's syncThreadName (use the
existing ChainTask pattern used elsewhere in this class) so capacity sync runs
on the same serial thread as delete/reconnect/extend; inside the ChainTask's
run/next callback call syncPoolCapacity with the same ReturnValueCompletion
implementation and publish/reply the event/response from the completion
callbacks, preserving error handling.
Comment on lines
+50
to
+55
| public Date getDate() { | ||
| return date; | ||
| } | ||
|
|
||
| public void setDate(Date date) { | ||
| this.date = date; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 搜索是否有对 getDate() 返回值进行修改的操作
rg -nP --type=java '\.getDate\s*\(\)\s*\.\s*set(?:Time|Year|Month|Date|Hours|Minutes|Seconds|Full)\s*\(' plugin/kvmRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 搜索是否有调用 setDate(null) 的代码
rg -nP --type=java 'setDate\s*\(\s*null\s*\)' plugin/kvmRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 查看 PoolDisconnectedData 类的完整内容和使用情况
cat -n plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolCanonicalEvents.java | head -100Repository: MatheMatrix/zstack
Length of output: 1829
🏁 Script executed:
# 搜索在其他文件中 PoolDisconnectedData 的使用情况
rg -n 'PoolDisconnectedData' plugin/kvm --type=java -A3 -B1Repository: MatheMatrix/zstack
Length of output: 1722
Date 可变引用泄漏，应添加防御性复制。
getDate() 直接返回内部 Date 引用，setDate() 直接持有外部引用，调用方可在类外修改该日期对象。虽然当前代码中未发现此类修改，但为防止数据被意外改变，建议采用防御性复制方案。
建议修改
public Date getDate() {
- return date;
+ return date == null ? null : new Date(date.getTime());
}
public void setDate(Date date) {
- this.date = date;
+ this.date = date == null ? null : new Date(date.getTime());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public Date getDate() { | |
| return date; | |
| } | |
| public void setDate(Date date) { | |
| this.date = date; | |
| public Date getDate() { | |
| return date == null ? null : new Date(date.getTime()); | |
| } | |
| public void setDate(Date date) { | |
| this.date = date == null ? null : new Date(date.getTime()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin/kvm/src/main/java/org/zstack/kvm/localvolumecache/VmLocalVolumeCachePoolCanonicalEvents.java`
around lines 50 - 55, The getters/setters for Date in
VmLocalVolumeCachePoolCanonicalEvents leak the mutable Date reference; change
getDate() to return a defensive copy (e.g., new Date(date.getTime()) or
null-safe equivalent) and change setDate(Date date) to store a copy of the
incoming Date (or null if input is null) so the internal field cannot be mutated
by external callers; update both methods (getDate and setDate) to handle null
safely.
Comment on lines
+1569
to
+1577
| if (!reply.isSuccess()) { | ||
| originReply.setError(reply.getError()); | ||
| } else { | ||
| originReply.setInventory(ir.getInventory()); | ||
| } | ||
| bus.reply(originMsg, originReply); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
热挂载失败时缺少回滚清理逻辑
在 instantiateAndAttachCache 方法中，如果 AttachVolumeCacheMsg 失败（Line 1569），之前已经通过 InstantiateLocalVolumeCacheMsg 创建的缓存文件不会被清理。这可能导致磁盘上残留孤立的缓存文件。
🐛 建议添加回滚逻辑
bus.send(amsg, new CloudBusCallBack(originMsg) {
`@Override`
public void run(MessageReply reply) {
if (!reply.isSuccess()) {
+ // Rollback: delete the instantiated cache file
+ DeleteLocalVolumeCacheMsg delMsg = new DeleteLocalVolumeCacheMsg();
+ delMsg.setCacheUuid(cacheUuid);
+ bus.makeTargetServiceIdByResourceUuid(delMsg, VmLocalVolumeCacheConstant.CACHE_SERVICE_ID, cacheUuid);
+ bus.send(delMsg, new CloudBusCallBack(originMsg) {
+ `@Override`
+ public void run(MessageReply delReply) {
+ if (!delReply.isSuccess()) {
+ logger.warn(String.format("failed to rollback cache[uuid:%s]: %s",
+ cacheUuid, delReply.getError()));
+ }
+ originReply.setError(reply.getError());
+ bus.reply(originMsg, originReply);
+ }
+ });
+ return;
- originReply.setError(reply.getError());
} else {
originReply.setInventory(ir.getInventory());
}
bus.reply(originMsg, originReply);
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@storage/src/main/java/org/zstack/storage/volume/VolumeManagerImpl.java`
around lines 1569 - 1577, In instantiateAndAttachCache, when the
AttachVolumeCacheMsg reply fails, add rollback cleanup for the cache created by
InstantiateLocalVolumeCacheMsg: detect the failure branch (where
reply.isSuccess() is false), extract the created cache identity from
ir.getInventory() (or the InstantiateLocalVolumeCacheMsg result), and issue a
cleanup operation (e.g., send a DeleteLocalVolumeCacheMsg or call the existing
delete/remove-cache routine with the cache UUID/path) before replying to
originMsg; ensure errors from the cleanup are logged but do not suppress
replying with originReply.setError(reply.getError()).
Owner
|
Comment from yaohua.wu: Review: MR !9255 — ZSTAC-81715 (Java 侧：VM 本地卷缓存)本 MR 实现 VM 本地卷缓存功能（CachePool 管理 + 卷缓存生命周期 + VM 启动集成），共 94 个文件变更。以下结合 Jira ZSTAC-81715 需求逐文件审查。 Critical
Warning
Suggestion
Verdict: REVISION_REQUIREDCritical 项（尤其是 C-1 容量精度、C-3 缓存注入忽略 host、C-5 schema 缺列）必须修复后才能合入。 🤖 Robot Reviewer |
MatheMatrix
force-pushed
the
sync/haidong.pang/feature/ZSTAC-81715
branch
from
b7f8bd3 to
959fe16
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java (1)
67-89:⚠️ Potential issue | 🟡 Minor拷贝构造函数中的
cache字段应进行深拷贝。Line 88 直接赋值
this.cache = other.cache会使两个VolumeTO实例共享同一个CacheTO对象。由于CacheTO是可变的（提供了setCacheUuid、setPoolUuid等 setter 方法），任一实例对cache的修改都会影响另一实例，导致数据一致性问题。建议：
- 为
CacheTO添加拷贝构造函数，在此处进行深拷贝：this.cache = other.cache == null ? null : new CacheTO(other.cache);- 或在
CacheTO中实现clone()方法- 或在注释中明确说明该共享语义是有意设计
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java` around lines 67 - 89, The VolumeTO copy constructor currently shallow-copies the cache field (this.cache = other.cache), causing two VolumeTO instances to share a mutable CacheTO; update the copy logic to perform a deep copy: add a copy constructor or clone method to CacheTO (e.g., CacheTO(CacheTO other) or CacheTO.clone()) and change VolumeTO(VolumeTO other) to set this.cache = other.cache == null ? null : new CacheTO(other.cache) (or use other.cache.clone()), so each VolumeTO gets its own CacheTO instance; alternatively, if sharing is intentional, add a clear comment in VolumeTO and CacheTO documenting the shared, mutable semantics.header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java (1)
74-75:⚠️ Potential issue | 🟡 MinorJavadoc 中包含中文文本
Line 74 的
@summary 创建云主机包含中文，违反编码规范要求"代码里不应当有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文"。建议修改
-* `@summary` 创建云主机 +* `@summary` Create a VM instanceAs per coding guidelines: "代码里不应当有有中文，包括报错、注释等都应当使用正确的、无拼写错误的英文来写"。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java` around lines 74 - 75, The Javadoc `@summary` contains Chinese text; update the class Javadoc for APICreateVmInstanceMsg to use English (e.g., replace "@summary 创建云主机" with a clear English summary like "@summary Create VM instance" or "@summary Create virtual machine") ensuring the comment contains no Chinese characters and follows existing project wording style.
🧹 Nitpick comments (5)
plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java (1)
724-728: 建议对blockDevices做空值归一化，避免向上游透传 null。当前成功分支直接
reply.setBlockDevices(rsp.getBlockDevices())，当 agent 返回 null 时会把判空成本转移给调用方。建议统一返回空集合。可选修改示例
public void success(KVMAgentCommands.GetBlockDevicesRsp rsp) { if (!rsp.isSuccess()) { reply.setError(operr("operation error, because:%s", rsp.getError())); } else { - reply.setBlockDevices(rsp.getBlockDevices()); + reply.setBlockDevices(rsp.getBlockDevices() == null + ? Collections.emptyList() + : rsp.getBlockDevices()); } bus.reply(msg, reply); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java` around lines 724 - 728, The success branch currently calls reply.setBlockDevices(rsp.getBlockDevices()) which can propagate a null to callers; change it to normalize null to an empty collection (e.g. if rsp.getBlockDevices() == null then set an empty List) before calling reply.setBlockDevices so callers always receive a non-null list; update the success branch in the method where reply.setBlockDevices and rsp.getBlockDevices are used (KVMHost related handling).header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolVO.java (1)
42-46: 建议：容量计算时考虑溢出保护
totalCapacity和availableCapacity使用long类型是正确的。但在进行容量加减计算时，建议使用Math.addExact()或Math.subtractExact()来检测溢出，避免在极端场景（>8PB 容量累加）下产生静默溢出。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolVO.java` around lines 42 - 46, The capacity fields totalCapacity and availableCapacity in VmLocalVolumeCachePoolVO are declared as long but arithmetic on them can silently overflow; update any methods that modify these fields (e.g., code paths that add or subtract capacity deltas such as capacity allocation/release helpers) to use Math.addExact(...) and Math.subtractExact(...) (or wrap those calls to catch ArithmeticException) when computing new totals so overflows are detected and handled; ensure exceptions are translated to a clear error/rollback path rather than letting incorrect values be written.header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java (2)
241-243: 建议为DiskAO.cacheMode添加有效值约束
APICreateVmInstanceMsg的顶层cacheMode字段有validValues = {"WriteBack"}约束，但DiskAO内部类的cacheMode字段没有相应的约束。为保持一致性和数据校验，建议添加相同的约束或通过文档说明有效值。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java` around lines 241 - 243, The DiskAO inner class inside APICreateVmInstanceMsg defines a cacheMode field without the same allowed-value validation as the top-level APICreateVmInstanceMsg.cacheMode; add the same validation annotation (e.g., `@APIParam`(validValues = {"WriteBack"}) or the project's equivalent) to DiskAO.cacheMode to enforce consistent valid values, or alternatively document the allowed value in DiskAO if annotations are not used.
366-385: 为VolumeCacheConfig添加@PythonClassInventory注解同一文件中的
DiskAO类已标注@PythonClassInventory，而新增的VolumeCacheConfig缺少此注解。两个类均是 API 消息中的静态内部类，用于配置数据，应保持一致。建议添加该注解以确保 Python SDK 生成的完整性。建议修改
+ `@PythonClassInventory` public static class VolumeCacheConfig { private String cachePoolUuid; private String cacheMode;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java` around lines 366 - 385, VolumeCacheConfig is missing the `@PythonClassInventory` annotation that DiskAO already has; add the `@PythonClassInventory` annotation to the VolumeCacheConfig static inner class declaration (and import the annotation if not present) so the Python SDK generator treats it like DiskAO; ensure the annotation is placed immediately above "public static class VolumeCacheConfig" and run any existing codegen/tests that validate Python inventory generation.header/src/main/java/org/zstack/header/vm/InstantiateNewCreatedVmInstanceMsg.java (1)
76-94: 建议在消息层对字符串参数做 trim，降低脏输入风险
cacheMode/rootVolumeCachePoolUuid/rootVolumeCacheMode直接透传，若上游带空白字符，后续匹配和解析容易失败。建议在 setter 中统一规整。建议修改
public void setCacheMode(String cacheMode) { - this.cacheMode = cacheMode; + this.cacheMode = cacheMode == null ? null : cacheMode.trim(); } public void setRootVolumeCachePoolUuid(String rootVolumeCachePoolUuid) { - this.rootVolumeCachePoolUuid = rootVolumeCachePoolUuid; + this.rootVolumeCachePoolUuid = rootVolumeCachePoolUuid == null ? null : rootVolumeCachePoolUuid.trim(); } public void setRootVolumeCacheMode(String rootVolumeCacheMode) { - this.rootVolumeCacheMode = rootVolumeCacheMode; + this.rootVolumeCacheMode = rootVolumeCacheMode == null ? null : rootVolumeCacheMode.trim(); }As per coding guidelines: “注意检查来自 Message 的参数是否做过 trim，用户可能在浏览器上复制粘贴的数据带有空格、换行符等”。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/InstantiateNewCreatedVmInstanceMsg.java` around lines 76 - 94, Update the setters to normalize incoming string parameters by trimming whitespace: modify setCacheMode, setRootVolumeCachePoolUuid, and setRootVolumeCacheMode to assign value == null ? null : value.trim() (i.e., perform null-safe trim) so that leading/trailing spaces/newlines from user input are removed before storing in the message fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@conf/db/upgrade/V5.5.12__schema.sql`:
- Around line 19-35: The CREATE TABLE for VmLocalVolumeCacheVO is missing the
installPath column required by the Java VO; update the schema in
V5.5.12__schema.sql to add an installPath column (e.g. a VARCHAR with the same
length/nullability as the VO expects) to the VmLocalVolumeCacheVO definition and
ensure any ALTER/upgrade path adds the same installPath column for existing DBs
so the table structure matches the Java class VmLocalVolumeCacheVO and avoids
mapping/runtime errors.
In
`@header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolInventory.java`:
- Around line 35-37: The code in VmLocalVolumeCachePoolInventory currently calls
JSONObjectUtil.toObject on vo.getMetadata() after only a null check, which can
throw when metadata is an empty or whitespace string; update the conditional
around inv.setMetadata to also guard against blank/empty strings (e.g., use
vo.getMetadata().trim().isEmpty() or StringUtils.isNotBlank(vo.getMetadata()))
before calling JSONObjectUtil.toObject to avoid deserialization errors.
In
`@header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheVO.java`:
- Around line 32-35: Update the Javadoc in VmLocalVolumeCacheVO that currently
says "in kvm plugin" to avoid the incorrect module claim: either replace that
phrase with the correct module/package that actually contains
VmLocalVolumeCachePoolVO or remove the module reference entirely so the comment
simply states it references VmLocalVolumeCachePoolVO and therefore omits an
`@ForeignKey` to avoid cross-module dependency; make this change in the comment
block that documents the nullable pool reference in VmLocalVolumeCacheVO.
- Around line 39-40: The VmLocalVolumeCacheVO entity declares a field
installPath (`@Column`(length = 2048) private String installPath;) but the
V5.5.12__schema.sql migration does not create that column, causing JPA mapping
failures; update the migration (or add a new incremental migration) to add the
installPath column to the VmLocalVolumeCacheVO table (a VARCHAR(2048) NULL
column) so the database schema matches the VmLocalVolumeCacheVO entity mapping.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java`:
- Around line 5193-5350: The new command/response classes (e.g., InitPoolCmd,
ConnectPoolCmd, ExtendPoolCmd, DeletePoolCmd, CheckPoolCmd, GetPoolCapacityCmd,
GCPoolCmd, AllocateCacheCmd, DeleteCacheCmd, FlushCacheCmd, GetCacheCapacityCmd,
AttachVolumeCacheCmd, DetachVolumeCacheCmd, and response classes like PoolRsp,
PoolHealthRsp, CacheRsp) expose required fields (poolUuid, mountPoint,
instanceUuid, volume, etc.) without null/blank validation; add validation by
annotating required fields with `@NotNull` or `@NotBlank` where appropriate (e.g.,
poolUuid, instanceUuid, volume, mountPoint) or add simple validation checks in
constructors/factory methods (throw IllegalArgumentException on missing required
values) so agents fail fast with clear errors before
serialization/deserialization.
---
Outside diff comments:
In `@header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java`:
- Around line 74-75: The Javadoc `@summary` contains Chinese text; update the
class Javadoc for APICreateVmInstanceMsg to use English (e.g., replace "@summary
创建云主机" with a clear English summary like "@summary Create VM instance" or
"@summary Create virtual machine") ensuring the comment contains no Chinese
characters and follows existing project wording style.
In `@plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java`:
- Around line 67-89: The VolumeTO copy constructor currently shallow-copies the
cache field (this.cache = other.cache), causing two VolumeTO instances to share
a mutable CacheTO; update the copy logic to perform a deep copy: add a copy
constructor or clone method to CacheTO (e.g., CacheTO(CacheTO other) or
CacheTO.clone()) and change VolumeTO(VolumeTO other) to set this.cache =
other.cache == null ? null : new CacheTO(other.cache) (or use
other.cache.clone()), so each VolumeTO gets its own CacheTO instance;
alternatively, if sharing is intentional, add a clear comment in VolumeTO and
CacheTO documenting the shared, mutable semantics.
---
Nitpick comments:
In
`@header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolVO.java`:
- Around line 42-46: The capacity fields totalCapacity and availableCapacity in
VmLocalVolumeCachePoolVO are declared as long but arithmetic on them can
silently overflow; update any methods that modify these fields (e.g., code paths
that add or subtract capacity deltas such as capacity allocation/release
helpers) to use Math.addExact(...) and Math.subtractExact(...) (or wrap those
calls to catch ArithmeticException) when computing new totals so overflows are
detected and handled; ensure exceptions are translated to a clear error/rollback
path rather than letting incorrect values be written.
In `@header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java`:
- Around line 241-243: The DiskAO inner class inside APICreateVmInstanceMsg
defines a cacheMode field without the same allowed-value validation as the
top-level APICreateVmInstanceMsg.cacheMode; add the same validation annotation
(e.g., `@APIParam`(validValues = {"WriteBack"}) or the project's equivalent) to
DiskAO.cacheMode to enforce consistent valid values, or alternatively document
the allowed value in DiskAO if annotations are not used.
- Around line 366-385: VolumeCacheConfig is missing the `@PythonClassInventory`
annotation that DiskAO already has; add the `@PythonClassInventory` annotation to
the VolumeCacheConfig static inner class declaration (and import the annotation
if not present) so the Python SDK generator treats it like DiskAO; ensure the
annotation is placed immediately above "public static class VolumeCacheConfig"
and run any existing codegen/tests that validate Python inventory generation.
In
`@header/src/main/java/org/zstack/header/vm/InstantiateNewCreatedVmInstanceMsg.java`:
- Around line 76-94: Update the setters to normalize incoming string parameters
by trimming whitespace: modify setCacheMode, setRootVolumeCachePoolUuid, and
setRootVolumeCacheMode to assign value == null ? null : value.trim() (i.e.,
perform null-safe trim) so that leading/trailing spaces/newlines from user input
are removed before storing in the message fields.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMHost.java`:
- Around line 724-728: The success branch currently calls
reply.setBlockDevices(rsp.getBlockDevices()) which can propagate a null to
callers; change it to normalize null to an empty collection (e.g. if
rsp.getBlockDevices() == null then set an empty List) before calling
reply.setBlockDevices so callers always receive a non-null list; update the
success branch in the method where reply.setBlockDevices and rsp.getBlockDevices
are used (KVMHost related handling).
ℹ️ Review info
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (59)
conf/globalConfig/kvm.xmlis excluded by!**/*.xmlconf/persistence.xmlis excluded by!**/*.xmlconf/serviceConfig/vmLocalVolumeCache.xmlis excluded by!**/*.xmlconf/serviceConfig/vmLocalVolumeCachePool.xmlis excluded by!**/*.xmlconf/springConfigXml/Kvm.xmlis excluded by!**/*.xmlconf/springConfigXml/VmInstanceManager.xmlis excluded by!**/*.xmlconf/springConfigXml/localVolumeCache.xmlis excluded by!**/*.xmlconf/zstack.xmlis excluded by!**/*.xmlsdk/src/main/java/SourceClassMap.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ApplicationDevelopmentServiceInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CacheTO.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeVmLocalVolumeCachePoolStateAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeVmLocalVolumeCachePoolStateResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateOvnControllerVmAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateSchedulerJobAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateSchedulerJobGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateVmInstanceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeployAppDevelopmentServiceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeployModelEvalServiceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeployModelServiceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DisableVolumeCacheAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DisableVolumeCacheResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/EnableVolumeCacheAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/EnableVolumeCacheResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ExtendVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ExtendVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GetBlockDevicesAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GetBlockDevicesResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GetMdevDeviceCandidatesAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GpuAllocateStatus.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GpuDeviceSpecInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/HostBlockDeviceStruct.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/MatchModelServiceTemplateWithModelAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/MdevDeviceSpecInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ModelService.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ModelServiceInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ReconnectVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ReconnectVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SyncVmLocalVolumeCachePoolCapacityAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SyncVmLocalVolumeCachePoolCapacityResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateMdevDeviceSpecAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateModelServiceInstanceGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdatePciDeviceSpecAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCacheInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCacheMode.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCachePoolInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCachePoolState.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCachePoolStatus.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCacheState.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VolumeBackupInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VolumeTO.javais excluded by!sdk/**
📒 Files selected for processing (38)
compute/src/main/java/org/zstack/compute/host/HostBase.javacompute/src/main/java/org/zstack/compute/vm/InstantiateVmFromNewCreatedStruct.javacompute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceManagerImpl.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceUtils.javacompute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.javaconf/db/upgrade/V5.5.12__schema.sqlheader/src/main/java/org/zstack/header/host/APIGetBlockDevicesEvent.javaheader/src/main/java/org/zstack/header/host/APIGetBlockDevicesMsg.javaheader/src/main/java/org/zstack/header/host/GetBlockDevicesOnHostMsg.javaheader/src/main/java/org/zstack/header/host/GetBlockDevicesOnHostReply.javaheader/src/main/java/org/zstack/header/host/HostBlockDeviceStruct.javaheader/src/main/java/org/zstack/header/localVolumeCache/EnableVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/localVolumeCache/EnableVolumeCacheReply.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheConstant.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheInventory.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheMode.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolInventory.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolState.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolStatus.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolVO.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolVO_.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheState.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheVO.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheVO_.javaheader/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/CreateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/InstantiateNewCreatedVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/VmInstanceSpec.javaplugin/kvm/src/main/java/org/zstack/kvm/CacheTO.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMApiInterceptor.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.javastorage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.javautils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
💤 Files with no reviewable changes (2)
- storage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.java
- plugin/kvm/src/main/java/org/zstack/kvm/KVMApiInterceptor.java
🚧 Files skipped from review as they are similar to previous changes (7)
- header/src/main/java/org/zstack/header/host/GetBlockDevicesOnHostReply.java
- header/src/main/java/org/zstack/header/host/GetBlockDevicesOnHostMsg.java
- plugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.java
- compute/src/main/java/org/zstack/compute/vm/VmInstanceManagerImpl.java
- compute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.java
- compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java
- compute/src/main/java/org/zstack/compute/vm/VmInstanceUtils.java
Comment on lines
+19
to
+35
| CREATE TABLE IF NOT EXISTS `VmLocalVolumeCacheVO` ( | ||
| `uuid` VARCHAR(32) NOT NULL, | ||
| `volumeUuid` VARCHAR(32) NOT NULL, | ||
| `poolUuid` VARCHAR(32) DEFAULT NULL, | ||
| `cacheMode` VARCHAR(32) NOT NULL, | ||
| `state` VARCHAR(32) NOT NULL, | ||
| `createDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| `lastOpDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, | ||
| PRIMARY KEY (`uuid`), | ||
| UNIQUE KEY `uniVmLocalVolumeCacheVOVolumeUuid` (`volumeUuid`), | ||
| CONSTRAINT `fkVmLocalVolumeCacheVOVolumeEO` | ||
| FOREIGN KEY (`volumeUuid`) REFERENCES `VolumeEO` (`uuid`) | ||
| ON DELETE CASCADE, | ||
| CONSTRAINT `fkVmLocalVolumeCacheVOPoolUuid` | ||
| FOREIGN KEY (`poolUuid`) REFERENCES `VmLocalVolumeCachePoolVO` (`uuid`) | ||
| ON DELETE SET NULL | ||
| ) ENGINE = InnoDB DEFAULT CHARSET = utf8; |
There was a problem hiding this comment.
缺少 installPath 列会导致表结构与 Java VO 不一致。
当前 VmLocalVolumeCacheVO 未包含 installPath，这会在实体映射/运行期触发错误（尤其在严格校验环境下）。建议在本次升级脚本中补齐该列。
🛠 建议修复
CREATE TABLE IF NOT EXISTS `VmLocalVolumeCacheVO` (
`uuid` VARCHAR(32) NOT NULL,
`volumeUuid` VARCHAR(32) NOT NULL,
`poolUuid` VARCHAR(32) DEFAULT NULL,
+ `installPath` VARCHAR(2048) DEFAULT NULL,
`cacheMode` VARCHAR(32) NOT NULL,
`state` VARCHAR(32) NOT NULL,
`createDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
`lastOpDate` TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@conf/db/upgrade/V5.5.12__schema.sql` around lines 19 - 35, The CREATE TABLE
for VmLocalVolumeCacheVO is missing the installPath column required by the Java
VO; update the schema in V5.5.12__schema.sql to add an installPath column (e.g.
a VARCHAR with the same length/nullability as the VO expects) to the
VmLocalVolumeCacheVO definition and ensure any ALTER/upgrade path adds the same
installPath column for existing DBs so the table structure matches the Java
class VmLocalVolumeCacheVO and avoids mapping/runtime errors.
Comment on lines
+35
to
+37
| if (vo.getMetadata() != null) { | ||
| inv.setMetadata(JSONObjectUtil.toObject(vo.getMetadata(), LinkedHashMap.class)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 查找目标文件及其上下文
fd 'VmLocalVolumeCachePoolInventory.java' -t f --exec cat -n {} \;Repository: MatheMatrix/zstack
Length of output: 8214
🏁 Script executed:
# 查找 metadata 的写入路径和 JSONObjectUtil 的使用方式
rg -n 'setMetadata|metadata.*=' --type java -B 2 -A 2 | head -60Repository: MatheMatrix/zstack
Length of output: 5621
🏁 Script executed:
# 查找数据库 schema 中 VmLocalVolumeCachePool 的定义
fd '__schema\.sql$|migration.*sql$' -t f --exec grep -l 'VmLocalVolumeCachePool' {} \;Repository: MatheMatrix/zstack
Length of output: 99
🏁 Script executed:
# 查找 JSONObjectUtil.toObject 的实现和使用示例
rg 'JSONObjectUtil\.toObject' --type java -B 1 -A 1 | head -40Repository: MatheMatrix/zstack
Length of output: 3971
🏁 Script executed:
# 检查类似的 metadata 处理模式
rg 'toObject.*metadata|metadata.*toObject' --type java -B 2 -A 2Repository: MatheMatrix/zstack
Length of output: 2593
🏁 Script executed:
# 查找 JSONObjectUtil 的实现
fd 'JSONObjectUtil.java' -t f --exec cat {} \;Repository: MatheMatrix/zstack
Length of output: 4412
🏁 Script executed:
# 查找 VmLocalVolumeCachePoolVO 的定义和写入逻辑
fd 'VmLocalVolumeCachePoolVO.java' -t f --exec cat {} \;Repository: MatheMatrix/zstack
Length of output: 3598
🏁 Script executed:
# 查找该新增文件是否在 PR 中有相关的写入/保存逻辑
rg 'VmLocalVolumeCachePool.*DAO|VmLocalVolumeCachePoolVO.*save|VmLocalVolumeCachePoolVO.*insert' --type java -B 2 -A 5Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 查找其他 toObject 对 metadata 的使用，检查是否存在 blank 检查的最佳实践
rg 'toObject.*metadata|metadata.*toObject' --type java -B 3 -A 3 | head -80Repository: MatheMatrix/zstack
Length of output: 3442
🏁 Script executed:
# 验证 JSONUtil 在其他代码中是否有类似的 blank 防护
rg 'StringUtils\.isBlank\(.*metadata\)|StringUtils\.isEmpty\(.*metadata\)' --type java -B 2 -A 2Repository: MatheMatrix/zstack
Length of output: 44
在反序列化 metadata 之前需要检查空字符串
当前代码仅判断 null，但若 metadata 为空字符串或纯空白字符，JSONObjectUtil.toObject 会触发 Gson 反序列化异常。建议在调用前添加 blank 判断：
if (vo.getMetadata() != null && !vo.getMetadata().trim().isEmpty()) {
inv.setMetadata(JSONObjectUtil.toObject(vo.getMetadata(), LinkedHashMap.class));
}或使用 StringUtils.isNotBlank(vo.getMetadata())。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolInventory.java`
around lines 35 - 37, The code in VmLocalVolumeCachePoolInventory currently
calls JSONObjectUtil.toObject on vo.getMetadata() after only a null check, which
can throw when metadata is an empty or whitespace string; update the conditional
around inv.setMetadata to also guard against blank/empty strings (e.g., use
vo.getMetadata().trim().isEmpty() or StringUtils.isNotBlank(vo.getMetadata()))
before calling JSONObjectUtil.toObject to avoid deserialization errors.
Comment on lines
+32
to
+35
| /** | ||
| * Nullable: null means this cache entry has not been assigned to a pool yet. | ||
| * References VmLocalVolumeCachePoolVO in kvm plugin — no @ForeignKey to avoid cross-module dependency. | ||
| */ |
There was a problem hiding this comment.
注释中的模块归属描述不准确，建议修正
这里写的是 “in kvm plugin”，但 VmLocalVolumeCachePoolVO 并不在该模块，建议改为准确路径/模块描述，避免后续误判依赖边界。
As per coding guidelines: “对于较长的注释，需要仔细校对并随代码更新，确保内容正确。”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheVO.java`
around lines 32 - 35, Update the Javadoc in VmLocalVolumeCacheVO that currently
says "in kvm plugin" to avoid the incorrect module claim: either replace that
phrase with the correct module/package that actually contains
VmLocalVolumeCachePoolVO or remove the module reference entirely so the comment
simply states it references VmLocalVolumeCachePoolVO and therefore omits an
`@ForeignKey` to avoid cross-module dependency; make this change in the comment
block that documents the nullable pool reference in VmLocalVolumeCacheVO.
Comment on lines
+39
to
+40
| @Column(length = 2048) | ||
| private String installPath; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[Step 1] Finding all schema migration files"
fd 'V.*__schema\.sql$' -t f
echo ""
echo "[Step 2] Checking for VmLocalVolumeCache table and installPath column in migrations"
fd 'V.*__schema\.sql$' -t f | while read -r f; do
echo "=== Checking: $f ==="
# Look for table creation/modification with context
rg -i '(create|alter).*table.*local.*volume|installpath|install_path' "$f" -C 5 || echo " No matches found"
done
echo ""
echo "[Step 3] Search for any file mentioning VmLocalVolumeCache"
rg 'VmLocalVolumeCache' --type java -l 2>/dev/null || echo "No VmLocalVolumeCache references found"Repository: MatheMatrix/zstack
Length of output: 40971
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[Step 1] Read V5.5.12__schema.sql and extract VmLocalVolumeCacheVO table definition"
# Find the VmLocalVolumeCacheVO table creation and show surrounding lines
rg -n 'CREATE TABLE.*VmLocalVolumeCacheVO' conf/db/upgrade/V5.5.12__schema.sql -A 30Repository: MatheMatrix/zstack
Length of output: 1045
数据库迁移脚本缺少 installPath 列定义，会导致实体映射失败
在 V5.5.12__schema.sql 中创建的 VmLocalVolumeCacheVO 表不包含 installPath 列，而实体类在第 39-40 行声明了该字段的映射（@Column(length = 2048) private String installPath;）。启动时 Hibernate/JPA 会尝试映射该字段到不存在的数据库列，导致映射异常，属于上线阻塞项。
需在 V5.5.12__schema.sql 或后续升级脚本中补充该列定义：
ALTER TABLE `VmLocalVolumeCacheVO` ADD COLUMN `installPath` VARCHAR(2048) DEFAULT NULL;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheVO.java`
around lines 39 - 40, The VmLocalVolumeCacheVO entity declares a field
installPath (`@Column`(length = 2048) private String installPath;) but the
V5.5.12__schema.sql migration does not create that column, causing JPA mapping
failures; update the migration (or add a new incremental migration) to add the
installPath column to the VmLocalVolumeCacheVO table (a VARCHAR(2048) NULL
column) so the database schema matches the VmLocalVolumeCacheVO entity mapping.
Comment on lines
+5193
to
+5350
| public static class InitPoolCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public String mountPoint; | ||
| public boolean force; | ||
| public List<String> pvs; | ||
| } | ||
|
|
||
| public static class ConnectPoolCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public String mountPoint; | ||
| public boolean force; | ||
| } | ||
|
|
||
| public static class ExtendPoolCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public String mountPoint; | ||
| public boolean force; | ||
| public List<String> pvs; | ||
| } | ||
|
|
||
| public static class DeletePoolCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public String mountPoint; | ||
| public boolean force; | ||
| } | ||
|
|
||
| public static class CheckPoolCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public String mountPoint; | ||
| public boolean force; | ||
| } | ||
|
|
||
| public static class GetPoolCapacityCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public String mountPoint; | ||
| public boolean force; | ||
| } | ||
|
|
||
| public static class GCPoolCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public String mountPoint; | ||
| public boolean force; | ||
| public List<VolumeTO> volumes; | ||
| } | ||
|
|
||
| public static class AllocateCacheCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public VolumeTO volume; | ||
| } | ||
|
|
||
| public static class DeleteCacheCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public VolumeTO volume; | ||
| } | ||
|
|
||
| public static class FlushCacheCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public VolumeTO volume; | ||
| } | ||
|
|
||
| public static class GetCacheCapacityCmd extends AgentCommand { | ||
| public String poolUuid; | ||
| public VolumeTO volume; | ||
| } | ||
|
|
||
| public static class AttachVolumeCacheCmd extends AgentCommand { | ||
| public String instanceUuid; | ||
| public VolumeTO volume; | ||
| } | ||
|
|
||
| public static class DetachVolumeCacheCmd extends AgentCommand implements HasThreadContext { | ||
| public String instanceUuid; | ||
| public VolumeTO volume; | ||
| } | ||
|
|
||
| // ======================================================================== | ||
| // Local Volume Cache — Ref Structures | ||
| // ======================================================================== | ||
|
|
||
| public static class PVRef { | ||
| public String pvUuid; | ||
| public String pvName; | ||
| public String pvDevicePath; | ||
| } | ||
|
|
||
| public static class PVHealthRef extends PVRef { | ||
| public Boolean healthy; | ||
| } | ||
|
|
||
| public static class VGRef { | ||
| public String vgUuid; | ||
| public String vgName; | ||
| } | ||
|
|
||
| public static class LVRef { | ||
| public String lvUuid; | ||
| public String lvName; | ||
| public String lvPath; | ||
| } | ||
|
|
||
| public static class FileSystemRef { | ||
| public String fsUuid; | ||
| public String fsType; | ||
| } | ||
|
|
||
| public static class VolumeRef { | ||
| public String volumeUuid; | ||
| public String installPath; | ||
| public String deviceType; | ||
| public String format; | ||
| public Long size; | ||
| } | ||
|
|
||
| // ======================================================================== | ||
| // Local Volume Cache — Response Definitions | ||
| // ======================================================================== | ||
|
|
||
| public static class PoolRsp extends AgentResponse { | ||
| public String poolUuid; | ||
| public String mountPoint; | ||
| public List<PVRef> pvs; | ||
| public VGRef vg; | ||
| public LVRef lv; | ||
| public FileSystemRef filesystem; | ||
| } | ||
|
|
||
| public static class PoolHealthRsp extends AgentResponse { | ||
| public Boolean healthy; | ||
| public List<PVHealthRef> pvs; | ||
| public Boolean vg; | ||
| public Boolean lv; | ||
| public Boolean filesystem; | ||
| } | ||
|
|
||
| public static class PoolCapacityRsp extends AgentResponse { | ||
| public Long total; | ||
| public Long used; | ||
| public Long available; | ||
| public Long allocated; | ||
| public Long dirty; | ||
| } | ||
|
|
||
| public static class CacheRsp extends AgentResponse { | ||
| public String installPath; | ||
| public Long virtualSize; | ||
| public Long actualSize; | ||
| } | ||
|
|
||
| public static class GCPoolRsp extends AgentResponse { | ||
| public List<String> gcFiles; | ||
| public Integer gcCount; | ||
| } | ||
|
|
||
| public static class AttachVolumeCacheRsp extends AgentResponse { | ||
| } | ||
|
|
||
| public static class DetachVolumeCacheRsp extends AgentResponse { | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 首先检查文件是否存在以及获取其基本信息
ls -lh plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java
wc -l plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaRepository: MatheMatrix/zstack
Length of output: 239
🏁 Script executed:
# 检查这些命令对象在文件中的构造位置
rg -n "new.*InitPoolCmd|new.*ConnectPoolCmd|new.*ExtendPoolCmd|new.*DeletePoolCmd|new.*CheckPoolCmd|new.*GetPoolCapacityCmd|new.*GCPoolCmd|new.*AllocateCacheCmd|new.*DeleteCacheCmd|new.*FlushCacheCmd|new.*GetCacheCapacityCmd|new.*AttachVolumeCacheCmd|new.*DetachVolumeCacheCmd" plugin/kvm/src/main/java/org/zstack/kvm/Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 检查同文件中其他 DTO 的访问风格（查找其他继承 AgentCommand 或 AgentResponse 的类）
rg -n "extends AgentCommand\|extends AgentResponse" plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java | head -20Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 查看这些新增 DTO 之前的代码样式（查看行号 5100-5193 之间的内容）
sed -n '5100,5193p' plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java | tail -50Repository: MatheMatrix/zstack
Length of output: 1421
🏁 Script executed:
# 检查这些命令是否在其他文件中被构造使用
rg -l "InitPoolCmd|ConnectPoolCmd|ExtendPoolCmd|DeletePoolCmd" plugin/kvm/src --type java | grep -v KVMAgentCommands.javaRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 获取行号 5193-5350 的完整代码
sed -n '5193,5350p' plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java | head -100Repository: MatheMatrix/zstack
Length of output: 2823
🏁 Script executed:
# 搜索这些类在整个 kvm 目录中的使用（不仅是构造）
rg "InitPoolCmd|ConnectPoolCmd|ExtendPoolCmd" plugin/kvm/src --type java -A 2 | head -50Repository: MatheMatrix/zstack
Length of output: 1005
🏁 Script executed:
# 查看 AgentCommand 基类的定义
rg -n "class AgentCommand" plugin/kvm/src --type java -A 5Repository: MatheMatrix/zstack
Length of output: 667
🏁 Script executed:
# 检查文件中其他 DTO 的访问模式，看是否存在两种混合使用的情况
sed -n '1,500p' plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java | grep -E "public (String|List|Boolean|Integer|Long|VolumeTO)" | head -20Repository: MatheMatrix/zstack
Length of output: 891
🏁 Script executed:
# 查看整个文件的访问风格统计（public 字段 vs private+getter/setter）
grep -n "public String\|public List\|public Boolean\|public Integer\|public Long" plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java | wc -l
grep -n "private String\|private List\|private Boolean\|private Integer\|private Long" plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java | wc -lRepository: MatheMatrix/zstack
Length of output: 69
🏁 Script executed:
# 查看更多 AgentCommand 的子类实现，了解整体风格
sed -n '70,500p' plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java | grep -E "class.*extends AgentCommand" -A 3 | head -30Repository: MatheMatrix/zstack
Length of output: 1149
🏁 Script executed:
# 检查是否有地方对这些字段进行了验证或非空检查
rg "poolUuid|instanceUuid" plugin/kvm/src --type java -B 2 -A 2 | grep -E "Objects.requireNonNull|if.*==|assert" | head -20Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
# 查看 Response 类是否也有相同的访问风格
sed -n '5250,5350p' plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaRepository: MatheMatrix/zstack
Length of output: 2871
🏁 Script executed:
# 检查这些命令类在何处被创建或序列化
rg "InitPoolCmd\|PoolRsp" plugin/kvm/src/main/java --type java | head -20Repository: MatheMatrix/zstack
Length of output: 44
新增命令/响应类缺少关键字段的非空验证。
行 5193-5350 新增的 Command、Reference 和 Response 类（如 InitPoolCmd、AttachVolumeCacheCmd、PoolRsp 等）都使用 public 字段直接暴露。虽然这与文件中 AgentCommand 基类及其他大多数子类的风格一致，但这些必填字段（poolUuid、instanceUuid、volume 等）缺少非空校验，容易因上游未正确初始化而导致后续 agent 端晚失败。建议至少对必填字段添加 @NotNull 或 @NotBlank 注解，或在构造/序列化前进行显式验证。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.java` around lines
5193 - 5350, The new command/response classes (e.g., InitPoolCmd,
ConnectPoolCmd, ExtendPoolCmd, DeletePoolCmd, CheckPoolCmd, GetPoolCapacityCmd,
GCPoolCmd, AllocateCacheCmd, DeleteCacheCmd, FlushCacheCmd, GetCacheCapacityCmd,
AttachVolumeCacheCmd, DetachVolumeCacheCmd, and response classes like PoolRsp,
PoolHealthRsp, CacheRsp) expose required fields (poolUuid, mountPoint,
instanceUuid, volume, etc.) without null/blank validation; add validation by
annotating required fields with `@NotNull` or `@NotBlank` where appropriate (e.g.,
poolUuid, instanceUuid, volume, mountPoint) or add simple validation checks in
constructors/factory methods (throw IllegalArgumentException on missing required
values) so agents fail fast with clear errors before
serialization/deserialization.
Related: ZSTAC-81715 Change-Id: I6d747a786b73746a6f756a6c72787479726e6765
MatheMatrix
force-pushed
the
sync/haidong.pang/feature/ZSTAC-81715
branch
from
959fe16 to
e037533
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java (1)
67-89:⚠️ Potential issue | 🟠 Major拷贝构造函数对
cache使用浅拷贝，存在状态串扰风险Line 88 直接执行
this.cache = other.cache;，会让两个VolumeTO实例共享同一个CacheTO引用。后续任一处修改cache都可能污染另一个对象，建议改为深拷贝（或显式复制字段）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java` around lines 67 - 89, The VolumeTO copy constructor currently assigns the cache field by reference (this.cache = other.cache) which causes shared mutable state; modify the VolumeTO(VolumeTO other) constructor to deep-copy the CacheTO by creating a new CacheTO instance (or invoking its copy/clone method) and assigning that to this.cache, or explicitly copy CacheTO's fields into a new object so each VolumeTO has an independent cache; update any constructor calls or helpers accordingly and ensure CacheTO's copy mechanism (e.g., a copy constructor or clone method) is used if available.
♻️ Duplicate comments (2)
plugin/kvm/src/main/java/org/zstack/kvm/CacheTO.java (1)
43-49:⚠️ Potential issue | 🟠 Major补齐
valueOf的空值防护，避免运行时 NPE当前映射在
inv或cacheMode为空时会直接中断，影响缓存信息下发流程。建议修复
public static CacheTO valueOf(VmLocalVolumeCacheInventory inv) { + if (inv == null) { + return null; + } CacheTO to = new CacheTO(); to.setCacheUuid(inv.getUuid()); to.setPoolUuid(inv.getPoolUuid()); to.setInstallPath(inv.getInstallPath()); - to.setCacheMode(inv.getCacheMode().name()); + to.setCacheMode(inv.getCacheMode() == null ? null : inv.getCacheMode().name()); return to; }As per coding guidelines "向后兼容原则：之前的代码产生的行为不要直接去改动... 应当对原状态做好记录，做好二次确认，做好回退准备"。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/kvm/src/main/java/org/zstack/kvm/CacheTO.java` around lines 43 - 49, In CacheTO.valueOf, add null checks to prevent NPEs: if the input VmLocalVolumeCacheInventory inv is null, return null immediately; otherwise set cacheUuid, poolUuid and installPath from inv as before but guard cacheMode by checking inv.getCacheMode() != null — call setCacheMode(inv.getCacheMode().name()) only when non-null and setCacheMode(null) or leave it unset when cacheMode is null so no .name() is invoked; reference the CacheTO.valueOf method and VmLocalVolumeCacheInventory.getCacheMode()/getUuid()/getPoolUuid()/getInstallPath() when applying these checks.header/src/main/java/org/zstack/header/host/HostBlockDeviceStruct.java (1)
22-34:⚠️ Potential issue | 🟠 Major修正
equals/hashCode：wwid为空时不能把所有设备都视为同一个当前实现会把
wwid == null的设备全部判等，可能导致设备列表被错误去重。建议修复
+import java.util.Objects; @@ `@Override` public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; HostBlockDeviceStruct that = (HostBlockDeviceStruct) o; - return wwid != null ? wwid.equals(that.wwid) : that.wwid == null; + if (wwid != null && that.wwid != null) { + return wwid.equals(that.wwid); + } + return Objects.equals(path, that.path) && Objects.equals(hctl, that.hctl); } `@Override` public int hashCode() { - return wwid != null ? wwid.hashCode() : 0; + return wwid != null ? wwid.hashCode() : Objects.hash(path, hctl); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/host/HostBlockDeviceStruct.java` around lines 22 - 34, The current equals/hashCode treat two HostBlockDeviceStruct instances with null wwid as equal; change HostBlockDeviceStruct.equals to return true only for same reference when wwid is null (i.e., if (this==o) true; if (o==null || getClass()!=o.getClass()) false; cast to HostBlockDeviceStruct then if (this.wwid == null || that.wwid == null) return false; return this.wwid.equals(that.wwid);), and update hashCode to return wwid != null ? wwid.hashCode() : System.identityHashCode(this) so hashCode is consistent with the new equals behavior.
🧹 Nitpick comments (4)
compute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.java (1)
405-434: 建议把“缓存启用失败”提升为可观测结果，而不只写日志。当前降级策略是继续流程，但调用链上层无法直接感知该盘缓存未生效。建议记录失败卷并透传到流程上下文/事件，便于排障与运维审计。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.java` around lines 405 - 434, The flow currently swallows cache-enable failures (inside the NoRollbackFlow anonymous flow in VmInstantiateOtherDiskFlow) by only logging them; change it to record failures into the flow context so upstream callers can observe them: when EnableVolumeCacheMsg reply.isSuccess() is false, add an entry (e.g. append volumeInventory.getUuid() plus reply.getError() to a List/Map stored in the flow data/context under a well-known key like "failedCacheVolumes" or "volumeCacheEnableFailures") before calling innerTrigger.next(); ensure the data structure is created if absent and use the same key so later flows or the outer caller can read the failures and/or emit an event.header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java (2)
215-223: 建议收敛cacheMode与rootVolumeCacheMode的语义边界。当前两者都表达根盘缓存模式，建议尽早明确优先级或收敛为单一来源，避免后续新增模式时出现配置分歧。
Also applies to: 635-657
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java` around lines 215 - 223, The two fields cacheMode and rootVolumeCacheMode in APICreateVmInstanceMsg both represent root disk cache mode and must be reconciled: pick a single canonical field (preferably rootVolumeCacheMode) or define a clear precedence rule (e.g., rootVolumeCacheMode overrides cacheMode) and implement it everywhere; update the APIParam annotations and validation logic around cacheMode and rootVolumeCacheMode, add deprecation notes/compat handling if you keep both (map legacy cacheMode into rootVolumeCacheMode at message construction), and adjust any consumers that read either field so they consistently use the chosen canonical value.
667-679: 建议在__example__中补充缓存配置示例。新增 API 字段未在示例中体现，会降低调用方对新能力的可发现性。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java` around lines 667 - 679, Update the APICreateVmInstanceMsg.__example__ to include a representative cache configuration so the new API fields are discoverable; inside the method (where msg is built and methods like msg.setRootVolumeSystemTags(...) and msg.setDataVolumeSystemTags(...) are called) add example calls to the new cache-related setters (e.g., the cache configuration setters added to APICreateVmInstanceMsg such as setCacheMode / setDiskCacheMode / setVolumeCachePolicy or whatever exact field names were introduced) with realistic sample values so callers can see how to enable/configure caching when creating a VM.compute/src/main/java/org/zstack/compute/vm/InstantiateVmFromNewCreatedStruct.java (1)
150-154: 可选：抽取公共方法减少重复赋值代码。两处缓存字段赋值完全同构，建议抽成私有 helper，后续改字段时不易漏改。
Also applies to: 174-178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@compute/src/main/java/org/zstack/compute/vm/InstantiateVmFromNewCreatedStruct.java` around lines 150 - 154, Extract the repeated cache-field assignments into a private helper to avoid duplication: create a method (e.g. applyCacheFieldsToStruct or copyCacheConfig) that accepts the target struct (InstantiateVmFromNewCreatedStruct or whatever type of "struct") and the source message/object (the msg being used) and moves the five setters (setEnableRootVolumeCache, setCacheMode, setRootVolumeCachePoolUuid, setRootVolumeCacheMode, setDataDiskCacheConfigOnIndex) into that helper; then replace both original code blocks (the lines around setEnableRootVolumeCache.. at 150-154 and the same block at 174-178) with calls to this new helper so future changes to those fields only need to be made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@plugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.java`:
- Around line 67-89: The VolumeTO copy constructor currently assigns the cache
field by reference (this.cache = other.cache) which causes shared mutable state;
modify the VolumeTO(VolumeTO other) constructor to deep-copy the CacheTO by
creating a new CacheTO instance (or invoking its copy/clone method) and
assigning that to this.cache, or explicitly copy CacheTO's fields into a new
object so each VolumeTO has an independent cache; update any constructor calls
or helpers accordingly and ensure CacheTO's copy mechanism (e.g., a copy
constructor or clone method) is used if available.
---
Duplicate comments:
In `@header/src/main/java/org/zstack/header/host/HostBlockDeviceStruct.java`:
- Around line 22-34: The current equals/hashCode treat two HostBlockDeviceStruct
instances with null wwid as equal; change HostBlockDeviceStruct.equals to return
true only for same reference when wwid is null (i.e., if (this==o) true; if
(o==null || getClass()!=o.getClass()) false; cast to HostBlockDeviceStruct then
if (this.wwid == null || that.wwid == null) return false; return
this.wwid.equals(that.wwid);), and update hashCode to return wwid != null ?
wwid.hashCode() : System.identityHashCode(this) so hashCode is consistent with
the new equals behavior.
In `@plugin/kvm/src/main/java/org/zstack/kvm/CacheTO.java`:
- Around line 43-49: In CacheTO.valueOf, add null checks to prevent NPEs: if the
input VmLocalVolumeCacheInventory inv is null, return null immediately;
otherwise set cacheUuid, poolUuid and installPath from inv as before but guard
cacheMode by checking inv.getCacheMode() != null — call
setCacheMode(inv.getCacheMode().name()) only when non-null and
setCacheMode(null) or leave it unset when cacheMode is null so no .name() is
invoked; reference the CacheTO.valueOf method and
VmLocalVolumeCacheInventory.getCacheMode()/getUuid()/getPoolUuid()/getInstallPath()
when applying these checks.
---
Nitpick comments:
In
`@compute/src/main/java/org/zstack/compute/vm/InstantiateVmFromNewCreatedStruct.java`:
- Around line 150-154: Extract the repeated cache-field assignments into a
private helper to avoid duplication: create a method (e.g.
applyCacheFieldsToStruct or copyCacheConfig) that accepts the target struct
(InstantiateVmFromNewCreatedStruct or whatever type of "struct") and the source
message/object (the msg being used) and moves the five setters
(setEnableRootVolumeCache, setCacheMode, setRootVolumeCachePoolUuid,
setRootVolumeCacheMode, setDataDiskCacheConfigOnIndex) into that helper; then
replace both original code blocks (the lines around setEnableRootVolumeCache..
at 150-154 and the same block at 174-178) with calls to this new helper so
future changes to those fields only need to be made in one place.
In `@compute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.java`:
- Around line 405-434: The flow currently swallows cache-enable failures (inside
the NoRollbackFlow anonymous flow in VmInstantiateOtherDiskFlow) by only logging
them; change it to record failures into the flow context so upstream callers can
observe them: when EnableVolumeCacheMsg reply.isSuccess() is false, add an entry
(e.g. append volumeInventory.getUuid() plus reply.getError() to a List/Map
stored in the flow data/context under a well-known key like "failedCacheVolumes"
or "volumeCacheEnableFailures") before calling innerTrigger.next(); ensure the
data structure is created if absent and use the same key so later flows or the
outer caller can read the failures and/or emit an event.
In `@header/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.java`:
- Around line 215-223: The two fields cacheMode and rootVolumeCacheMode in
APICreateVmInstanceMsg both represent root disk cache mode and must be
reconciled: pick a single canonical field (preferably rootVolumeCacheMode) or
define a clear precedence rule (e.g., rootVolumeCacheMode overrides cacheMode)
and implement it everywhere; update the APIParam annotations and validation
logic around cacheMode and rootVolumeCacheMode, add deprecation notes/compat
handling if you keep both (map legacy cacheMode into rootVolumeCacheMode at
message construction), and adjust any consumers that read either field so they
consistently use the chosen canonical value.
- Around line 667-679: Update the APICreateVmInstanceMsg.__example__ to include
a representative cache configuration so the new API fields are discoverable;
inside the method (where msg is built and methods like
msg.setRootVolumeSystemTags(...) and msg.setDataVolumeSystemTags(...) are
called) add example calls to the new cache-related setters (e.g., the cache
configuration setters added to APICreateVmInstanceMsg such as setCacheMode /
setDiskCacheMode / setVolumeCachePolicy or whatever exact field names were
introduced) with realistic sample values so callers can see how to
enable/configure caching when creating a VM.
ℹ️ 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: da8e7de6-017b-474a-80f2-ed1b38cb4ce8
⛔ Files ignored due to path filters (58)
conf/globalConfig/kvm.xmlis excluded by!**/*.xmlconf/persistence.xmlis excluded by!**/*.xmlconf/serviceConfig/vmLocalVolumeCache.xmlis excluded by!**/*.xmlconf/serviceConfig/vmLocalVolumeCachePool.xmlis excluded by!**/*.xmlconf/springConfigXml/VmInstanceManager.xmlis excluded by!**/*.xmlconf/springConfigXml/localVolumeCache.xmlis excluded by!**/*.xmlconf/zstack.xmlis excluded by!**/*.xmlsdk/src/main/java/SourceClassMap.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ApplicationDevelopmentServiceInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CacheTO.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeVmLocalVolumeCachePoolStateAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ChangeVmLocalVolumeCachePoolStateResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateOvnControllerVmAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateSchedulerJobAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateSchedulerJobGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateVmInstanceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/CreateVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeleteVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeployAppDevelopmentServiceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeployModelEvalServiceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DeployModelServiceAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DisableVolumeCacheAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/DisableVolumeCacheResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/EnableVolumeCacheAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/EnableVolumeCacheResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ExtendVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ExtendVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GetBlockDevicesAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GetBlockDevicesResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GetMdevDeviceCandidatesAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GpuAllocateStatus.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/GpuDeviceSpecInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/HostBlockDeviceStruct.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/MatchModelServiceTemplateWithModelAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/MdevDeviceSpecInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ModelService.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ModelServiceInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/QueryVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ReconnectVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/ReconnectVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SyncVmLocalVolumeCachePoolCapacityAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/SyncVmLocalVolumeCachePoolCapacityResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateMdevDeviceSpecAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateModelServiceInstanceGroupAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdatePciDeviceSpecAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateVmLocalVolumeCachePoolAction.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/UpdateVmLocalVolumeCachePoolResult.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCacheInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCacheMode.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCachePoolInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCachePoolState.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCachePoolStatus.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VmLocalVolumeCacheState.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VolumeBackupInventory.javais excluded by!sdk/**sdk/src/main/java/org/zstack/sdk/VolumeTO.javais excluded by!sdk/**
📒 Files selected for processing (38)
compute/src/main/java/org/zstack/compute/host/HostBase.javacompute/src/main/java/org/zstack/compute/vm/InstantiateVmFromNewCreatedStruct.javacompute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceBase.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceManagerImpl.javacompute/src/main/java/org/zstack/compute/vm/VmInstanceUtils.javacompute/src/main/java/org/zstack/compute/vm/VmInstantiateOtherDiskFlow.javaconf/db/upgrade/V5.5.12__schema.sqlheader/src/main/java/org/zstack/header/host/APIGetBlockDevicesEvent.javaheader/src/main/java/org/zstack/header/host/APIGetBlockDevicesMsg.javaheader/src/main/java/org/zstack/header/host/GetBlockDevicesOnHostMsg.javaheader/src/main/java/org/zstack/header/host/GetBlockDevicesOnHostReply.javaheader/src/main/java/org/zstack/header/host/HostBlockDeviceStruct.javaheader/src/main/java/org/zstack/header/localVolumeCache/EnableVolumeCacheMsg.javaheader/src/main/java/org/zstack/header/localVolumeCache/EnableVolumeCacheReply.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheConstant.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheInventory.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheMode.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolInventory.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolState.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolStatus.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolVO.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCachePoolVO_.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheState.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheVO.javaheader/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheVO_.javaheader/src/main/java/org/zstack/header/vm/APICreateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/CreateVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/InstantiateNewCreatedVmInstanceMsg.javaheader/src/main/java/org/zstack/header/vm/VmInstanceSpec.javaplugin/kvm/src/main/java/org/zstack/kvm/CacheTO.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMAgentCommands.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMApiInterceptor.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMConstant.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/kvm/src/main/java/org/zstack/kvm/VolumeTO.javastorage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.javautils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
💤 Files with no reviewable changes (2)
- storage/src/main/java/org/zstack/storage/volume/VolumeApiInterceptor.java
- plugin/kvm/src/main/java/org/zstack/kvm/KVMApiInterceptor.java
🚧 Files skipped from review as they are similar to previous changes (11)
- conf/db/upgrade/V5.5.12__schema.sql
- header/src/main/java/org/zstack/header/localVolumeCache/EnableVolumeCacheMsg.java
- header/src/main/java/org/zstack/header/localVolumeCache/EnableVolumeCacheReply.java
- header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheVO_.java
- header/src/main/java/org/zstack/header/vm/VmInstanceSpec.java
- header/src/main/java/org/zstack/header/vm/CreateVmInstanceMsg.java
- compute/src/main/java/org/zstack/compute/vm/VmAllocateHostFlow.java
- compute/src/main/java/org/zstack/compute/vm/VmInstanceBase.java
- header/src/main/java/org/zstack/header/localVolumeCache/VmLocalVolumeCacheMode.java
- compute/src/main/java/org/zstack/compute/vm/VmInstanceManagerImpl.java
- header/src/main/java/org/zstack/header/host/GetBlockDevicesOnHostReply.java
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
2 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.
Related: ZSTAC-81715
Change-Id: I6d747a786b73746a6f756a6c72787479726e6765
sync from gitlab !9255