<fix>[volume]: <description#3299
Open
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-zmigratefrom
Open
<fix>[volume]: <description#3299zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-zmigratefrom
zstack-robot-2 wants to merge 1 commit intofeature-zsv-5.0.0-zmigratefrom
Conversation
Walkthrough新增备份存储文件操作消息/回复与 SDK 接口；Ceph 备份存储实现上传/下载/进度/删除/拷贝/取消的远程调用逻辑与 REST 路径；BackupStorage 基类与测试模拟器添加相应路由与封装调用。 Changes
Sequence DiagramsequenceDiagram
participant Client
participant CephBackupStorageBase
participant HttpCaller
participant CephMON as Ceph<br/>Monitor<br/>Host
participant Target as Target<br/>Host
rect rgba(200, 150, 255, 0.5)
Note over Client,Target: 上传文件到备份存储
Client->>CephBackupStorageBase: UploadFileToBackupStorageHostMsg
CephBackupStorageBase->>CephBackupStorageBase: 解析 URL/scheme/taskUuid
CephBackupStorageBase->>HttpCaller: POST UploadFileCmd 到 MON
HttpCaller->>CephMON: UploadFileCmd (url, taskUuid, installPath)
CephMON->>Target: 从 URL 拉取或通过 SSH 拷贝
Target-->>CephMON: UploadFileResponse (md5,size,路径)
CephMON-->>HttpCaller: 返回 UploadFileResponse
HttpCaller-->>CephBackupStorageBase: 映射响应为 Reply
CephBackupStorageBase-->>Client: UploadFileToBackupStorageHostReply
end
rect rgba(150, 200, 255, 0.5)
Note over Client,CephMON: 取消下载
Client->>CephBackupStorageBase: CancelDownloadFileMsg
CephBackupStorageBase->>HttpCaller: POST CancelCommand (taskUuid)
HttpCaller->>CephMON: CancelCommand
CephMON-->>HttpCaller: ACK/结果
HttpCaller-->>CephBackupStorageBase: 映射响应
CephBackupStorageBase-->>Client: CancelDownloadFileReply
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
🤖 Fix all issues with AI agents
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`:
- Line 34: The class CephBackupStorageBase imports the wrong
GetFileDownloadProgressReply
(org.zstack.header.host.GetFileDownloadProgressReply) for handling
GetFileDownloadProgressMsg; replace that import with
org.zstack.header.storage.backup.GetFileDownloadProgressReply so the reply type
matches BackupStorageMessage semantics and includes backup-specific fields (e.g.
format, unzipInstallPath) when used in CephBackupStorageBase.
- Around line 2208-2245: The code in CephBackupStorageBase.handle(...) is using
the wrong GetFileDownloadProgressReply type (the host module variant) which
lacks backup-specific fields; replace the incorrect import with import
org.zstack.header.storage.backup.GetFileDownloadProgressReply (or remove the
specific import and rely on import org.zstack.header.storage.backup.*;) so the
CephBackupStorageBase class constructs and returns the backup-storage version
that includes format, unzipInstallPath and unzipFiles.
🧹 Nitpick comments (2)
sdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadSoftwarePackageToBackupStorageResult.java (1)
3-3: 冗余的导入语句。
SoftwarePackageInventory与当前类位于同一包org.zstack.sdk.softwarePackage.header中，无需显式导入。♻️ 建议移除冗余导入
package org.zstack.sdk.softwarePackage.header; -import org.zstack.sdk.softwarePackage.header.SoftwarePackageInventory; - public class UploadSoftwarePackageToBackupStorageResult {testlib/src/main/java/org/zstack/testlib/CephBackupStorageSpec.groovy (1)
249-252: 建议：FILE_UPLOAD_PROGRESS_PATH 应返回进度信息当前
FILE_UPLOAD_PROGRESS_PATH模拟器返回通用的AgentResponse，但进度查询通常需要返回具体的进度数据（如已完成百分比、已传输字节数等）。如果存在专门的进度响应类型（如GetDownloadFileProgressResponse），建议使用该类型并填充模拟进度数据，以便更真实地测试进度查询流程。
plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java
Show resolved
Hide resolved
Comment on lines
+2208
to
+2245
| @Override | ||
| protected void handle(GetFileDownloadProgressMsg msg) { | ||
| GetFileDownloadProgressReply reply = new GetFileDownloadProgressReply(); | ||
|
|
||
| CephBackupStorageMonVO mon = getSelf().getMons().stream() | ||
| .filter(m -> m.getUuid().equals(msg.getBackupStorageHostUuid())).findAny().orElse(null); | ||
| if (mon == null) { | ||
| reply.setError(operr("failed to find mon with uuid [%s]", msg.getBackupStorageHostUuid())); | ||
| bus.reply(msg, reply); | ||
| return; | ||
| } | ||
|
|
||
| GetDownloadFileProgressCmd cmd = new GetDownloadFileProgressCmd(); | ||
| cmd.taskUuid = msg.getTaskUuid(); | ||
|
|
||
| CephBackupStorageMonBase monBase = new CephBackupStorageMonBase(mon); | ||
| monBase.httpCall(FILE_UPLOAD_PROGRESS_PATH, cmd, GetDownloadFileProgressResponse.class, new ReturnValueCompletion<GetDownloadFileProgressResponse>(msg) { | ||
| @Override | ||
| public void fail(ErrorCode err) { | ||
| reply.setError(err); | ||
| bus.reply(msg, reply); | ||
| } | ||
|
|
||
| @Override | ||
| public void success(GetDownloadFileProgressResponse rsp) { | ||
| reply.setCompleted(rsp.completed); | ||
| reply.setProgress(rsp.progress); | ||
| reply.setActualSize(rsp.actualSize); | ||
| reply.setSize(rsp.size); | ||
| reply.setInstallPath(rsp.installPath); | ||
| reply.setDownloadSize(rsp.downloadSize); | ||
| reply.setLastOpTime(rsp.lastOpTime); | ||
| reply.setMd5sum(rsp.md5sum); | ||
| reply.setSupportSuspend(rsp.supportSuspend); | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find GetFileDownloadProgressReply classes in both packages
echo "=== Searching for GetFileDownloadProgressReply in host package ==="
find . -type f -name "*.java" | xargs grep -l "class GetFileDownloadProgressReply" | grep "header/host"
echo -e "\n=== Searching for GetFileDownloadProgressReply in storage.backup package ==="
find . -type f -name "*.java" | xargs grep -l "class GetFileDownloadProgressReply" | grep "storage.backup"
echo -e "\n=== Searching for all GetFileDownloadProgressReply classes ==="
find . -type f -name "*.java" | xargs grep -l "class GetFileDownloadProgressReply"Repository: MatheMatrix/zstack
Length of output: 612
🏁 Script executed:
#!/bin/bash
# Check imports in CephBackupStorageBase.java
echo "=== Imports related to GetFileDownloadProgressReply ==="
head -50 plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java | grep -i "GetFileDownloadProgress"
echo -e "\n=== All imports in the file ==="
head -100 plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java | grep "^import"Repository: MatheMatrix/zstack
Length of output: 3191
🏁 Script executed:
#!/bin/bash
# Find GetFileDownloadProgressReply class definitions and their fields
echo "=== Finding GetFileDownloadProgressReply definitions ==="
find . -type f -name "*.java" -exec grep -l "class GetFileDownloadProgressReply" {} \;
echo -e "\n=== Check field definitions in each class ==="
for file in $(find . -type f -name "*.java" -exec grep -l "class GetFileDownloadProgressReply" {} \;); do
echo "File: $file"
grep -A 30 "class GetFileDownloadProgressReply" "$file" | head -40
echo "---"
doneRepository: MatheMatrix/zstack
Length of output: 1966
使用了错误的 GetFileDownloadProgressReply 类，应该导入存储备份模块的版本。
当前代码在 org.zstack.header.host.GetFileDownloadProgressReply 和 org.zstack.header.storage.backup.GetFileDownloadProgressReply 两个类之间存在混淆。由于 CephBackupStorageBase 位于备份存储模块中，应导入 org.zstack.header.storage.backup.GetFileDownloadProgressReply，后者包含额外的备份相关字段（format、unzipInstallPath、unzipFiles）。当前导入的主机模块版本缺少这些字段，可能导致备份存储操作的数据不完整。
请将第 34 行的导入改为：import org.zstack.header.storage.backup.GetFileDownloadProgressReply;（或者移除此导入，利用已有的 import org.zstack.header.storage.backup.*; 通配符导入）。
🤖 Prompt for AI Agents
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`
around lines 2208 - 2245, The code in CephBackupStorageBase.handle(...) is using
the wrong GetFileDownloadProgressReply type (the host module variant) which
lacks backup-specific fields; replace the incorrect import with import
org.zstack.header.storage.backup.GetFileDownloadProgressReply (or remove the
specific import and rely on import org.zstack.header.storage.backup.*;) so the
CephBackupStorageBase class constructs and returns the backup-storage version
that includes format, unzipInstallPath and unzipFiles.
MatheMatrix
force-pushed
the
sync/tao.gan/zmigrate-ZSV-1@@3
branch
4 times, most recently
from
3e02cc7 to
4ba3d68
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`:
- Around line 2232-2242: In CephBackupStorageBase's anonymous callback
success(GetDownloadFileProgressResponse rsp) you forgot to copy the format field
into the reply; update the code that populates GetFileDownloadProgressReply (the
reply object created in that method) to set reply.setFormat(rsp.format) (or the
appropriate setter name) so the format from GetDownloadFileProgressResponse is
forwarded before bus.reply(msg, reply).
🧹 Nitpick comments (1)
testlib/src/main/java/org/zstack/testlib/CephBackupStorageSpec.groovy (1)
245-252: 模拟器返回类型可能不正确。
FILE_UPLOAD_PATH应返回UploadFileResponse（包含directUploadPath字段），FILE_UPLOAD_PROGRESS_PATH应返回GetDownloadFileProgressResponse（包含进度相关字段）。当前返回通用的AgentResponse可能导致测试无法覆盖实际业务逻辑中对这些字段的使用。♻️ 建议的修复
simulator(CephBackupStorageBase.FILE_UPLOAD_PATH) { HttpEntity<String> entity -> - def rsp = new CephBackupStorageBase.AgentResponse() + def rsp = new CephBackupStorageBase.UploadFileResponse() + rsp.directUploadPath = "/upload/direct/path" return rsp } simulator(CephBackupStorageBase.FILE_UPLOAD_PROGRESS_PATH) { HttpEntity<String> entity -> - def rsp = new CephBackupStorageBase.AgentResponse() + def rsp = new CephBackupStorageBase.GetDownloadFileProgressResponse() + rsp.completed = true + rsp.progress = 100 + rsp.size = 1024L return rsp }
Comment on lines
+2232
to
+2242
| public void success(GetDownloadFileProgressResponse rsp) { | ||
| reply.setCompleted(rsp.completed); | ||
| reply.setProgress(rsp.progress); | ||
| reply.setActualSize(rsp.actualSize); | ||
| reply.setSize(rsp.size); | ||
| reply.setInstallPath(rsp.installPath); | ||
| reply.setDownloadSize(rsp.downloadSize); | ||
| reply.setLastOpTime(rsp.lastOpTime); | ||
| reply.setMd5sum(rsp.md5sum); | ||
| reply.setSupportSuspend(rsp.supportSuspend); | ||
| bus.reply(msg, reply); |
There was a problem hiding this comment.
GetFileDownloadProgressReply 缺少 format 字段的设置。
GetDownloadFileProgressResponse 包含 format 字段，但在设置 reply 时未将该字段传递给 GetFileDownloadProgressReply。
🔧 建议的修复
public void success(GetDownloadFileProgressResponse rsp) {
reply.setCompleted(rsp.completed);
reply.setProgress(rsp.progress);
reply.setActualSize(rsp.actualSize);
reply.setSize(rsp.size);
reply.setInstallPath(rsp.installPath);
reply.setDownloadSize(rsp.downloadSize);
reply.setLastOpTime(rsp.lastOpTime);
reply.setMd5sum(rsp.md5sum);
reply.setSupportSuspend(rsp.supportSuspend);
+ reply.setFormat(rsp.format);
bus.reply(msg, reply);
}🤖 Prompt for AI Agents
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`
around lines 2232 - 2242, In CephBackupStorageBase's anonymous callback
success(GetDownloadFileProgressResponse rsp) you forgot to copy the format field
into the reply; update the code that populates GetFileDownloadProgressReply (the
reply object created in that method) to set reply.setFormat(rsp.format) (or the
appropriate setter name) so the format from GetDownloadFileProgressResponse is
forwarded before bus.reply(msg, reply).
APIImpact Resolves: ZSV-1 Change-Id: I656979726c736267776c7262716969726874716d
MatheMatrix
force-pushed
the
sync/tao.gan/zmigrate-ZSV-1@@3
branch
from
4ba3d68 to
51550a9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.java`:
- Around line 14-20: The field and its accessors in
GetZMigrateGatewayVmInstancesResult use a raw java.util.List; change the
declaration of gatewayVmInstances and the signatures of
setGatewayVmInstances/getGatewayVmInstances to use a parameterized List of the
correct inventory type (e.g., List<VmInstanceInventory> or the appropriate
subclass), update imports accordingly, and ensure any callers/assignments are
adjusted to the typed list to restore compile-time type safety.
🧹 Nitpick comments (4)
sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosAction.java (2)
9-11: 常量命名建议使用大写下划线风格根据编码规范，
private static final字段属于常量，应使用全大写下划线分隔的命名风格（如PARAMETER_MAP、NON_API_PARAMETER_MAP）。但考虑到这是 SDK 自动生成的代码，且本次 PR 是同步操作，如果此命名风格与其他 SDK Action 类保持一致，可忽略此建议。
28-32: 建议使用泛型参数
systemTags和userTags使用了原始类型List，缺少泛型参数。建议使用List<String>以获得更好的类型安全性。不过，如果这是 SDK 自动生成的代码且与其他 Action 类保持一致，可作为后续统一优化的事项。
sdk/src/main/java/org/zstack/sdk/softwarePackage/header/UploadAndExecuteSoftwareUpgradePackageAction.java (1)
43-47: 建议使用泛型类型而非原始类型。
systemTags和userTags使用了原始的java.util.List类型，建议使用List<String>以提高类型安全性。♻️ 建议的修改
`@Param`(required = false) - public java.util.List systemTags; + public java.util.List<String> systemTags; `@Param`(required = false) - public java.util.List userTags; + public java.util.List<String> userTags;header/src/main/java/org/zstack/header/storage/backup/CopyAndExecuteSoftwareUpgradePackageMsg.java (1)
10-15:dstHostSSHPort使用int类型可能导致默认值问题。当
dstHostSSHPort未设置时，其默认值为0，这可能导致下游代码难以区分"未设置"和"显式设置为0"的情况。建议考虑使用Integer类型以支持null值检测。♻️ 建议的修改
- private int dstHostSSHPort; + private Integer dstHostSSHPort;同时更新对应的 getter/setter 方法。
Comment on lines
+14
to
+20
| public java.util.List gatewayVmInstances; | ||
| public void setGatewayVmInstances(java.util.List gatewayVmInstances) { | ||
| this.gatewayVmInstances = gatewayVmInstances; | ||
| } | ||
| public java.util.List getGatewayVmInstances() { | ||
| return this.gatewayVmInstances; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 读取完整的文件内容
cat -n sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.javaRepository: MatheMatrix/zstack
Length of output: 916
🏁 Script executed:
#!/bin/bash
# 查找与 GatewayVmInstance 相关的类
rg -n "class.*GatewayVmInstance" --type java | head -20Repository: MatheMatrix/zstack
Length of output: 364
🏁 Script executed:
#!/bin/bash
# 检查 zmigrate 包中的其他 SDK 结果类，看它们是否使用泛型
fd "Result\.java$" sdk/src/main/java/org/zstack/sdk/zmigrate/ -x cat -n {} \; | head -100Repository: MatheMatrix/zstack
Length of output: 3094
🏁 Script executed:
#!/bin/bash
# 检查其他 SDK Result 类是否也使用原始类型
rg -n "public java\.util\.(List|Map|Set)" sdk/src/main/java/org/zstack/sdk/ --type java -A 1 | head -40Repository: MatheMatrix/zstack
Length of output: 2386
🏁 Script executed:
#!/bin/bash
# 查看是否有文件头注释表明这是自动生成的代码
head -20 sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.javaRepository: MatheMatrix/zstack
Length of output: 759
🏁 Script executed:
#!/bin/bash
# 检查其他包中的 Result 类
fd "Result\.java$" sdk/src/main/java/org/zstack/sdk/ -not -path "*/zmigrate/*" | head -5 -exec head -30 {} \;Repository: MatheMatrix/zstack
Length of output: 360
🏁 Script executed:
#!/bin/bash
# 查找 SDK 代码生成的迹象
rg -i "auto.*generated|generated.*code|do not edit" sdk/src/main/java/org/zstack/sdk/ --type java | head -10Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# 查找 Result 类的模式 - 检查其他包中是否也都是原始类型
find sdk/src/main/java/org/zstack/sdk -name "*Result.java" -not -path "*/zmigrate/*" | head -3 | xargs -I {} cat {}Repository: MatheMatrix/zstack
Length of output: 1231
🏁 Script executed:
#!/bin/bash
# 查看 GetZMigrateGatewayVmInstancesAction 以了解预期的返回类型
cat sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesAction.javaRepository: MatheMatrix/zstack
Length of output: 2664
🏁 Script executed:
#!/antml/bash
# 搜索是否存在相关的 Inventory 类
rg -n "class.*GatewayVmInstance.*Inventory" --type javaRepository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# 搜索在 zmigrate 相关的代码中对 gatewayVmInstances 的使用方式
rg -B 3 -A 3 "gatewayVmInstances" --type java | head -40Repository: MatheMatrix/zstack
Length of output: 1440
🏁 Script executed:
#!/bin/bash
# 查找所有与 GatewayVmInstance 相关的类定义
find . -name "*GatewayVmInstance*.java" -type f | head -20Repository: MatheMatrix/zstack
Length of output: 239
🏁 Script executed:
#!/bin/bash
# 在 zmigrate 模块的核心代码中查找相关的类定义
find compute/src -o -find storage/src -o -find core/src 2>/dev/null | xargs grep -l "GatewayVmInstance" | head -10Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# 更广泛地搜索 GatewayVmInstance 的定义
rg "class.*GatewayVmInstance" --type java | head -20Repository: MatheMatrix/zstack
Length of output: 360
🏁 Script executed:
#!/bin/bash
# 查找 VmInstance 相关的 Inventory 或数据类
rg "class.*VmInstance.*Inventory|class.*VmInstanceInventory" --type java | head -10Repository: MatheMatrix/zstack
Length of output: 1390
🏁 Script executed:
#!/bin/bash
# 查看 zmigrate 相关的核心业务代码，看 gatewayVmInstances 应该包含什么类型
find . -path "*/zmigrate/*" -name "*.java" -not -path "*/sdk/*" -type f | head -10Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# 搜索是否有任何地方使用或创建 gatewayVmInstances
rg -B 5 -A 5 "gatewayVmInstances" --type java -i | grep -v "^sdk/src" | head -50Repository: MatheMatrix/zstack
Length of output: 67
🏁 Script executed:
#!/bin/bash
# 查看 zmigrate 模块的目录结构，找相关的业务代码
find . -path "*/zmigrate/*" -type f -name "*.java" | grep -v "/sdk/" | head -20Repository: MatheMatrix/zstack
Length of output: 44
🏁 Script executed:
#!/bin/bash
# 查看 GetZMigrateImagesResult 如何使用 Map 类型
cat sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateImagesResult.javaRepository: MatheMatrix/zstack
Length of output: 335
需要为 List 添加泛型类型参数以保持类型安全
gatewayVmInstances 字段使用了原始类型 java.util.List，缺少泛型类型参数。这与其他 SDK Result 类的标准做法不一致——其他 Result 类均使用明确的类型化字段（如 AccessControlListEntryInventory inventory）。原始类型会导致编译器无法进行类型检查，降低代码的类型安全性。
建议确认列表元素的具体类型（如是否为 VmInstanceInventory 或相关子类），并相应更新字段声明及对应的 getter/setter 方法。
🤖 Prompt for AI Agents
In
`@sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.java`
around lines 14 - 20, The field and its accessors in
GetZMigrateGatewayVmInstancesResult use a raw java.util.List; change the
declaration of gatewayVmInstances and the signatures of
setGatewayVmInstances/getGatewayVmInstances to use a parameterized List of the
correct inventory type (e.g., List<VmInstanceInventory> or the appropriate
subclass), update imports accordingly, and ensure any callers/assignments are
adjusted to the typed list to restore compile-time type safety.
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.
APIImpact
Resolves: ZSV-1
Change-Id: I656979726c736267776c7262716969726874716d
sync from gitlab !9118