<fix>[volume]: <description#3291
Open
ZStack-Robot wants to merge 1 commit intofeature-zsv-5.0.0-zmigratefrom
Open
<fix>[volume]: <description#3291ZStack-Robot wants to merge 1 commit intofeature-zsv-5.0.0-zmigratefrom
ZStack-Robot wants to merge 1 commit intofeature-zsv-5.0.0-zmigratefrom
Conversation
APIImpact Resolves: ZSV-1 Change-Id: I656979726c736267776c7262716969726874716d
概览本次提交引入了备份存储文件操作的消息类、Ceph备份存储处理器实现、软件包升级相关的SDK操作类、ZMigrate功能的SDK操作，以及测试基础设施支持。涉及多个新的消息、处理逻辑、API操作和测试工具的添加。 变更
评估代码审查工作量🎯 4 (复杂) | ⏱️ ~50 分钟 庆祝诗
🚥 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: 7
🤖 Fix all issues with AI agents
In
`@header/src/main/java/org/zstack/header/storage/backup/CopyAndExecuteSoftwareUpgradePackageMsg.java`:
- Around line 6-79: Rename the abbreviated fields and their accessors in
CopyAndExecuteSoftwareUpgradePackageMsg to use full words (e.g. rename
dstFilePath -> destFilePath, dstHostSSHPort -> destHostSSHPort, dstHostSSHUser
-> destHostSSHUser, dstHostSSHPasswd -> destHostSSHPassword, dstHostIP ->
destHostIP) and update all corresponding getters/setters
(getDstFilePath/setDstFilePath -> getDestFilePath/setDestFilePath, etc.); then
update every call site that references these symbols (notably the usages in
CephBackupStorageBase around the reported lines) to use the new names, preserve
the `@NoLogging` annotation on the password field, and ensure any serialization or
RPC consumers are adjusted for the renamed fields.
In
`@header/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostReply.java`:
- Around line 8-14: The DeleteFilesOnBackupStorageHostReply contains fields
(md5sum, size, directUploadUrl, unzipInstallPath, filesSize) that don't match
the semantics of a delete operation; adjust the reply to reflect actual usage in
CephBackupStorageBase.handle(DeleteFilesOnBackupStorageHostMsg) by removing the
unused upload/download-related fields or replace the class body with an
empty/marker reply consistent with DeleteFilesResponse usage; ensure any callers
expecting those fields are updated to not access them and keep the class name
DeleteFilesOnBackupStorageHostReply and MessageReply inheritance unchanged.
In
`@header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.java`:
- Around line 55-57: isDownloadComplete currently treats 0-byte files as
incomplete because of the `actualSize > 0` check; update
GetFileDownloadProgressReply.isDownloadComplete to treat equality of actualSize
and downloadSize as the primary success condition (i.e., return actualSize ==
downloadSize) or, if there is an explicit completed flag (e.g., `completed`),
use that combined with the size check (e.g., return completed || actualSize ==
downloadSize) so zero-length files are correctly reported as complete; modify
the method accordingly and keep references to `actualSize`, `downloadSize`, and
`isDownloadComplete`.
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`:
- Around line 2267-2286: The code incorrectly constructs a
CephPrimaryStorageMonBase for a backup storage mon; replace the use of
CephPrimaryStorageMonBase with CephBackupStorageMonBase when transforming the
singleton list (the variable currently named mon and the resulting monBase) so
you create a CephBackupStorageMonBase instance and call its
httpCall(COPY_FILE_TO_HOST_PATH, ...) instead of using the primary-storage mon
base.
- Around line 2224-2248: The code is using CephPrimaryStorageMonBase for a
backup-storage mon; change the monBase construction to use the backup-storage
mon base type (CephBackupStorageMonVO -> its mon base helper class) so the http
call is made with the correct mon type: replace the transform that creates
CephPrimaryStorageMonBase with the appropriate CephBackupStorageMonBase (or the
class that wraps CephBackupStorageMonVO) when building monBase used for
FILE_UPLOAD_PROGRESS_PATH and invoking monBase.httpCall; ensure
GetDownloadFileProgressCmd, mon, FILE_UPLOAD_PROGRESS_PATH and monBase.httpCall
are left intact but monBase is instantiated from the backup-storage mon base
class.
- Around line 2196-2210: The code constructs a CephPrimaryStorageMonBase from a
CephBackupStorageMonVO and uses DownloadFileResponse incorrectly; replace the
incorrect type and response: when creating monBase from the existing mon
variable and transform call, use CephBackupStorageMonBase::new instead of
CephPrimaryStorageMonBase::new (so DeleteFilesCmd is sent via a
CephBackupStorageMonBase instance), and change the httpCall generic/response
class from DownloadFileResponse to DeleteFilesResponse so the callback
signatures match (keep existing fail/success handlers and reply logic).
In
`@sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.java`:
- Around line 14-20: Rename the misspelled field and its accessors in
GetZMigrateGatewayVmInstancesResult: change the field getawayVmInstances to
gatewayVmInstances and update the setter/getter method names from
setGetawayVmInstances/getGetawayVmInstances to
setGatewayVmInstances/getGatewayVmInstances; update all usages/references
(serialization keys, callers, tests) to the new name to keep the API consistent
and ensure any JSON/XML mapping/annotations reflect the corrected property name.
🧹 Nitpick comments (2)
sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateInfosResult.java (1)
1-4: 代码风格：多余的空行。包声明后有多余的空行，建议删除以保持代码整洁。
♻️ 建议的修改
package org.zstack.sdk.zmigrate.api; - - public class GetZMigrateInfosResult {plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java (1)
45-45: 导入可能是多余的。
CephPrimaryStorageMonBase的导入在修复上述 MonBase 类型错误后可能变为未使用。建议在修复后检查并移除此导入。
Comment on lines
+6
to
+79
| public class CopyAndExecuteSoftwareUpgradePackageMsg extends NeedReplyMessage implements BackupStorageMessage { | ||
| private String backupStorageUuid; | ||
| private String backupStorageHostUuid; | ||
| private String targetFilePath; | ||
| private String dstFilePath; | ||
| private int dstHostSSHPort; | ||
| private String dstHostSSHUser; | ||
| @NoLogging | ||
| private String dstHostSSHPasswd; | ||
| private String dstHostIP; | ||
|
|
||
| @Override | ||
| public String getBackupStorageUuid() { | ||
| return backupStorageUuid; | ||
| } | ||
|
|
||
| public void setBackupStorageUuid(String backupStorageUuid) { | ||
| this.backupStorageUuid = backupStorageUuid; | ||
| } | ||
|
|
||
| public String getBackupStorageHostUuid() { | ||
| return backupStorageHostUuid; | ||
| } | ||
|
|
||
| public void setBackupStorageHostUuid(String backupStorageHostUuid) { | ||
| this.backupStorageHostUuid = backupStorageHostUuid; | ||
| } | ||
|
|
||
| public String getDstFilePath() { | ||
| return dstFilePath; | ||
| } | ||
|
|
||
| public void setDstFilePath(String dstFilePath) { | ||
| this.dstFilePath = dstFilePath; | ||
| } | ||
|
|
||
| public String getTargetFilePath() { | ||
| return targetFilePath; | ||
| } | ||
|
|
||
| public void setTargetFilePath(String targetFilePath) { | ||
| this.targetFilePath = targetFilePath; | ||
| } | ||
|
|
||
| public int getDstHostSSHPort() { | ||
| return dstHostSSHPort; | ||
| } | ||
|
|
||
| public void setDstHostSSHPort(int dstHostSSHPort) { | ||
| this.dstHostSSHPort = dstHostSSHPort; | ||
| } | ||
|
|
||
| public String getDstHostSSHUser() { | ||
| return dstHostSSHUser; | ||
| } | ||
|
|
||
| public void setDstHostSSHUser(String dstHostSSHUser) { | ||
| this.dstHostSSHUser = dstHostSSHUser; | ||
| } | ||
|
|
||
| public String getDstHostSSHPasswd() { | ||
| return dstHostSSHPasswd; | ||
| } | ||
|
|
||
| public void setDstHostSSHPasswd(String dstHostSSHPasswd) { | ||
| this.dstHostSSHPasswd = dstHostSSHPasswd; | ||
| } | ||
|
|
||
| public String getDstHostIP() { | ||
| return dstHostIP; | ||
| } | ||
|
|
||
| public void setDstHostIP(String dstHostIP) { | ||
| this.dstHostIP = dstHostIP; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# 首先确认文件存在并查看实际代码
find . -type f -name "CopyAndExecuteSoftwareUpgradePackageMsg.java" | head -5Repository: MatheMatrix/zstack
Length of output: 162
🏁 Script executed:
# 搜索该类在代码库中的引用
rg "CopyAndExecuteSoftwareUpgradePackageMsg" -l --type javaRepository: MatheMatrix/zstack
Length of output: 244
🏁 Script executed:
# 搜索这些具体字段/方法的使用情况
rg "(getDstFilePath|setDstFilePath|getDstHostSSHPort|setDstHostSSHPort|getDstHostSSHUser|setDstHostSSHUser|getDstHostSSHPasswd|setDstHostSSHPasswd|getDstHostIP|setDstHostIP)" --type java -cRepository: MatheMatrix/zstack
Length of output: 249
🏁 Script executed:
# 检查代码库中是否存在类似的 dst 缩写模式
rg "dst[A-Z]" --type java -c | head -20Repository: MatheMatrix/zstack
Length of output: 1471
🏁 Script executed:
# 查看CephBackupStorageBase中该消息类的具体使用
rg -A 3 -B 3 "CopyAndExecuteSoftwareUpgradePackageMsg|getDstFilePath|setDstFilePath|getDstHostSSHPort|setDstHostSSHPort|getDstHostSSHUser|setDstHostSSHUser|getDstHostSSHPasswd|setDstHostSSHPasswd|getDstHostIP|setDstHostIP" plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.javaRepository: MatheMatrix/zstack
Length of output: 1368
🏁 Script executed:
# 检查其他消息类的命名模式，看是否存在类似的 dst* 缩写
rg "private String dst|public.*getDst|public.*setDst" --type java | head -15Repository: MatheMatrix/zstack
Length of output: 2069
🏁 Script executed:
# 检查代码库中其他XXXMsg类的命名规范
find . -type f -name "*Msg.java" -path "*/header/*" | xargs grep -l "private String" | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -50 {} | grep -E "private (String|int|boolean|long|double)" | head -10'Repository: MatheMatrix/zstack
Length of output: 1205
🏁 Script executed:
# 查看CopyAndExecuteSoftwareUpgradePackageMsg.java的完整内容
cat -n ./header/src/main/java/org/zstack/header/storage/backup/CopyAndExecuteSoftwareUpgradePackageMsg.java | head -85Repository: MatheMatrix/zstack
Length of output: 2725
🏁 Script executed:
# 查看CephBackupStorageBase.java中对这个消息的所有使用
rg "CopyAndExecuteSoftwareUpgradePackageMsg|msg\.getDst|msg\.get[DT]" plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java -nRepository: MatheMatrix/zstack
Length of output: 1149
🏁 Script executed:
# 确认是否还有其他地方引用这个消息类
rg "CopyAndExecuteSoftwareUpgradePackageMsg" --type javaRepository: MatheMatrix/zstack
Length of output: 741
🏁 Script executed:
# 检查CopyAndExecuteSoftwareUpgradePackageMsg的其他可能引用
rg "CopyAndExecuteSoftwareUpgradePackage" --type javaRepository: MatheMatrix/zstack
Length of output: 1111
需要避免使用缩写命名，请统一采用完整单词。
当前类中多个字段使用了缩写（dst、passwd 等），违反编码规范。需要重命名为完整单词，并同步更新 CephBackupStorageBase.java 中的调用代码（第 2268-2273 行）。
♻️ 建议命名调整
- private String dstFilePath;
- private int dstHostSSHPort;
- private String dstHostSSHUser;
- `@NoLogging`
- private String dstHostSSHPasswd;
- private String dstHostIP;
+ private String destinationFilePath;
+ private int destinationHostSshPort;
+ private String destinationHostSshUser;
+ `@NoLogging`
+ private String destinationHostSshPassword;
+ private String destinationHostIp;
- public String getDstFilePath() {
- return dstFilePath;
- }
- public void setDstFilePath(String dstFilePath) {
- this.dstFilePath = dstFilePath;
- }
+ public String getDestinationFilePath() {
+ return destinationFilePath;
+ }
+ public void setDestinationFilePath(String destinationFilePath) {
+ this.destinationFilePath = destinationFilePath;
+ }
- public int getDstHostSSHPort() {
- return dstHostSSHPort;
- }
- public void setDstHostSSHPort(int dstHostSSHPort) {
- this.dstHostSSHPort = dstHostSSHPort;
- }
+ public int getDestinationHostSshPort() {
+ return destinationHostSshPort;
+ }
+ public void setDestinationHostSshPort(int destinationHostSshPort) {
+ this.destinationHostSshPort = destinationHostSshPort;
+ }
- public String getDstHostSSHUser() {
- return dstHostSSHUser;
- }
- public void setDstHostSSHUser(String dstHostSSHUser) {
- this.dstHostSSHUser = dstHostSSHUser;
- }
+ public String getDestinationHostSshUser() {
+ return destinationHostSshUser;
+ }
+ public void setDestinationHostSshUser(String destinationHostSshUser) {
+ this.destinationHostSshUser = destinationHostSshUser;
+ }
- public String getDstHostSSHPasswd() {
- return dstHostSSHPasswd;
- }
- public void setDstHostSSHPasswd(String dstHostSSHPasswd) {
- this.dstHostSSHPasswd = dstHostSSHPasswd;
- }
+ public String getDestinationHostSshPassword() {
+ return destinationHostSshPassword;
+ }
+ public void setDestinationHostSshPassword(String destinationHostSshPassword) {
+ this.destinationHostSshPassword = destinationHostSshPassword;
+ }
- public String getDstHostIP() {
- return dstHostIP;
- }
- public void setDstHostIP(String dstHostIP) {
- this.dstHostIP = dstHostIP;
- }
+ public String getDestinationHostIp() {
+ return destinationHostIp;
+ }
+ public void setDestinationHostIp(String destinationHostIp) {
+ this.destinationHostIp = destinationHostIp;
+ }并同步更新 CephBackupStorageBase.java 的第 2268-2273 行调用。
编码规范明确要求："不允许使用不必要的缩写，如：AbsSchedulerJob、condi、Fu 等。应使用完整单词提升可读性。"
📝 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 class CopyAndExecuteSoftwareUpgradePackageMsg extends NeedReplyMessage implements BackupStorageMessage { | |
| private String backupStorageUuid; | |
| private String backupStorageHostUuid; | |
| private String targetFilePath; | |
| private String dstFilePath; | |
| private int dstHostSSHPort; | |
| private String dstHostSSHUser; | |
| @NoLogging | |
| private String dstHostSSHPasswd; | |
| private String dstHostIP; | |
| @Override | |
| public String getBackupStorageUuid() { | |
| return backupStorageUuid; | |
| } | |
| public void setBackupStorageUuid(String backupStorageUuid) { | |
| this.backupStorageUuid = backupStorageUuid; | |
| } | |
| public String getBackupStorageHostUuid() { | |
| return backupStorageHostUuid; | |
| } | |
| public void setBackupStorageHostUuid(String backupStorageHostUuid) { | |
| this.backupStorageHostUuid = backupStorageHostUuid; | |
| } | |
| public String getDstFilePath() { | |
| return dstFilePath; | |
| } | |
| public void setDstFilePath(String dstFilePath) { | |
| this.dstFilePath = dstFilePath; | |
| } | |
| public String getTargetFilePath() { | |
| return targetFilePath; | |
| } | |
| public void setTargetFilePath(String targetFilePath) { | |
| this.targetFilePath = targetFilePath; | |
| } | |
| public int getDstHostSSHPort() { | |
| return dstHostSSHPort; | |
| } | |
| public void setDstHostSSHPort(int dstHostSSHPort) { | |
| this.dstHostSSHPort = dstHostSSHPort; | |
| } | |
| public String getDstHostSSHUser() { | |
| return dstHostSSHUser; | |
| } | |
| public void setDstHostSSHUser(String dstHostSSHUser) { | |
| this.dstHostSSHUser = dstHostSSHUser; | |
| } | |
| public String getDstHostSSHPasswd() { | |
| return dstHostSSHPasswd; | |
| } | |
| public void setDstHostSSHPasswd(String dstHostSSHPasswd) { | |
| this.dstHostSSHPasswd = dstHostSSHPasswd; | |
| } | |
| public String getDstHostIP() { | |
| return dstHostIP; | |
| } | |
| public void setDstHostIP(String dstHostIP) { | |
| this.dstHostIP = dstHostIP; | |
| public class CopyAndExecuteSoftwareUpgradePackageMsg extends NeedReplyMessage implements BackupStorageMessage { | |
| private String backupStorageUuid; | |
| private String backupStorageHostUuid; | |
| private String targetFilePath; | |
| private String destinationFilePath; | |
| private int destinationHostSshPort; | |
| private String destinationHostSshUser; | |
| `@NoLogging` | |
| private String destinationHostSshPassword; | |
| private String destinationHostIp; | |
| `@Override` | |
| public String getBackupStorageUuid() { | |
| return backupStorageUuid; | |
| } | |
| public void setBackupStorageUuid(String backupStorageUuid) { | |
| this.backupStorageUuid = backupStorageUuid; | |
| } | |
| public String getBackupStorageHostUuid() { | |
| return backupStorageHostUuid; | |
| } | |
| public void setBackupStorageHostUuid(String backupStorageHostUuid) { | |
| this.backupStorageHostUuid = backupStorageHostUuid; | |
| } | |
| public String getDestinationFilePath() { | |
| return destinationFilePath; | |
| } | |
| public void setDestinationFilePath(String destinationFilePath) { | |
| this.destinationFilePath = destinationFilePath; | |
| } | |
| public String getTargetFilePath() { | |
| return targetFilePath; | |
| } | |
| public void setTargetFilePath(String targetFilePath) { | |
| this.targetFilePath = targetFilePath; | |
| } | |
| public int getDestinationHostSshPort() { | |
| return destinationHostSshPort; | |
| } | |
| public void setDestinationHostSshPort(int destinationHostSshPort) { | |
| this.destinationHostSshPort = destinationHostSshPort; | |
| } | |
| public String getDestinationHostSshUser() { | |
| return destinationHostSshUser; | |
| } | |
| public void setDestinationHostSshUser(String destinationHostSshUser) { | |
| this.destinationHostSshUser = destinationHostSshUser; | |
| } | |
| public String getDestinationHostSshPassword() { | |
| return destinationHostSshPassword; | |
| } | |
| public void setDestinationHostSshPassword(String destinationHostSshPassword) { | |
| this.destinationHostSshPassword = destinationHostSshPassword; | |
| } | |
| public String getDestinationHostIp() { | |
| return destinationHostIp; | |
| } | |
| public void setDestinationHostIp(String destinationHostIp) { | |
| this.destinationHostIp = destinationHostIp; |
🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/storage/backup/CopyAndExecuteSoftwareUpgradePackageMsg.java`
around lines 6 - 79, Rename the abbreviated fields and their accessors in
CopyAndExecuteSoftwareUpgradePackageMsg to use full words (e.g. rename
dstFilePath -> destFilePath, dstHostSSHPort -> destHostSSHPort, dstHostSSHUser
-> destHostSSHUser, dstHostSSHPasswd -> destHostSSHPassword, dstHostIP ->
destHostIP) and update all corresponding getters/setters
(getDstFilePath/setDstFilePath -> getDestFilePath/setDestFilePath, etc.); then
update every call site that references these symbols (notably the usages in
CephBackupStorageBase around the reported lines) to use the new names, preserve
the `@NoLogging` annotation on the password field, and ensure any serialization or
RPC consumers are adjusted for the renamed fields.
Comment on lines
+8
to
+14
| public class DeleteFilesOnBackupStorageHostReply extends MessageReply { | ||
| private String md5sum; | ||
| private long size; | ||
| @NoLogging(type = NoLogging.Type.Uri) | ||
| private String directUploadUrl; | ||
| private String unzipInstallPath; | ||
| private Map<String, Long> filesSize; |
There was a problem hiding this comment.
Reply 类字段与"删除文件"操作的语义不匹配。
类名 DeleteFilesOnBackupStorageHostReply 表明这是一个删除文件操作的响应，但包含的字段（md5sum、size、directUploadUrl、unzipInstallPath、filesSize）更适合下载或上传操作的响应。
查看 CephBackupStorageBase.java 中的 handle(DeleteFilesOnBackupStorageHostMsg) 方法，使用的 DeleteFilesResponse 是空类，且处理逻辑中并未设置这些字段。这些字段可能是从其他 Reply 类复制过来的，建议确认是否需要保留，或简化为空的 Reply 类以匹配实际用途。
🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/storage/backup/DeleteFilesOnBackupStorageHostReply.java`
around lines 8 - 14, The DeleteFilesOnBackupStorageHostReply contains fields
(md5sum, size, directUploadUrl, unzipInstallPath, filesSize) that don't match
the semantics of a delete operation; adjust the reply to reflect actual usage in
CephBackupStorageBase.handle(DeleteFilesOnBackupStorageHostMsg) by removing the
unused upload/download-related fields or replace the class body with an
empty/marker reply consistent with DeleteFilesResponse usage; ensure any callers
expecting those fields are updated to not access them and keep the class name
DeleteFilesOnBackupStorageHostReply and MessageReply inheritance unchanged.
Comment on lines
+55
to
+57
| public boolean isDownloadComplete() { | ||
| return actualSize > 0 && actualSize == downloadSize; | ||
| } |
There was a problem hiding this comment.
isDownloadComplete 对 0 字节文件会误判为未完成。
若存在空文件场景，actualSize > 0 会导致完成状态永远为 false。可考虑结合 completed 标志或放宽条件。
✅ 边界修正示例
- public boolean isDownloadComplete() {
- return actualSize > 0 && actualSize == downloadSize;
- }
+ public boolean isDownloadComplete() {
+ return completed || (downloadSize > 0 && actualSize == downloadSize);
+ }🤖 Prompt for AI Agents
In
`@header/src/main/java/org/zstack/header/storage/backup/GetFileDownloadProgressReply.java`
around lines 55 - 57, isDownloadComplete currently treats 0-byte files as
incomplete because of the `actualSize > 0` check; update
GetFileDownloadProgressReply.isDownloadComplete to treat equality of actualSize
and downloadSize as the primary success condition (i.e., return actualSize ==
downloadSize) or, if there is an explicit completed flag (e.g., `completed`),
use that combined with the size check (e.g., return completed || actualSize ==
downloadSize) so zero-length files are correctly reported as complete; modify
the method accordingly and keep references to `actualSize`, `downloadSize`, and
`isDownloadComplete`.
Comment on lines
+2196
to
+2210
| DeleteFilesCmd cmd = new DeleteFilesCmd(); | ||
| cmd.filesPath = msg.getFilesPath(); | ||
| CephPrimaryStorageMonBase monBase = transform(Collections.singletonList(mon), CephPrimaryStorageMonBase::new).get(0); | ||
| monBase.httpCall(DELETE_FILE_PATH, cmd, DownloadFileResponse.class, new ReturnValueCompletion<DownloadFileResponse>(msg) { | ||
| @Override | ||
| public void fail(ErrorCode err) { | ||
| reply.setError(err); | ||
| bus.reply(msg, reply); | ||
| } | ||
|
|
||
| @Override | ||
| public void success(DownloadFileResponse rsp) { | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); |
There was a problem hiding this comment.
严重错误：使用了错误的 MonBase 类型。
Line 2198 使用 CephPrimaryStorageMonBase 处理 CephBackupStorageMonVO，这是类型不匹配的错误：
CephPrimaryStorageMonBase的构造函数期望CephPrimaryStorageMonVO类型getSelf().getMons()返回的是CephBackupStorageMonVO列表- 这会导致运行时
ClassCastException或方法调用失败
应该使用 CephBackupStorageMonBase 而不是 CephPrimaryStorageMonBase。
🐛 修复建议
DeleteFilesCmd cmd = new DeleteFilesCmd();
cmd.filesPath = msg.getFilesPath();
- CephPrimaryStorageMonBase monBase = transform(Collections.singletonList(mon), CephPrimaryStorageMonBase::new).get(0);
- monBase.httpCall(DELETE_FILE_PATH, cmd, DownloadFileResponse.class, new ReturnValueCompletion<DownloadFileResponse>(msg) {
+ CephBackupStorageMonBase monBase = new CephBackupStorageMonBase(mon);
+ monBase.httpCall(DELETE_FILE_PATH, cmd, DeleteFilesResponse.class, new ReturnValueCompletion<DeleteFilesResponse>(msg) {
`@Override`
public void fail(ErrorCode err) {
reply.setError(err);
bus.reply(msg, reply);
}
`@Override`
- public void success(DownloadFileResponse rsp) {
+ public void success(DeleteFilesResponse rsp) {
bus.reply(msg, reply);
}
});另外，httpCall 的响应类型应该是 DeleteFilesResponse 而不是 DownloadFileResponse。
🤖 Prompt for AI Agents
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`
around lines 2196 - 2210, The code constructs a CephPrimaryStorageMonBase from a
CephBackupStorageMonVO and uses DownloadFileResponse incorrectly; replace the
incorrect type and response: when creating monBase from the existing mon
variable and transform call, use CephBackupStorageMonBase::new instead of
CephPrimaryStorageMonBase::new (so DeleteFilesCmd is sent via a
CephBackupStorageMonBase instance), and change the httpCall generic/response
class from DownloadFileResponse to DeleteFilesResponse so the callback
signatures match (keep existing fail/success handlers and reply logic).
Comment on lines
+2224
to
+2248
| GetDownloadFileProgressCmd cmd = new GetDownloadFileProgressCmd(); | ||
| cmd.taskUuid = msg.getTaskUuid(); | ||
|
|
||
| CephPrimaryStorageMonBase monBase = transform(Collections.singletonList(mon), CephPrimaryStorageMonBase::new).get(0); | ||
| 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.
严重错误：使用了错误的 MonBase 类型。
Line 2227 存在与上述相同的问题：使用 CephPrimaryStorageMonBase 处理 CephBackupStorageMonVO。
🐛 修复建议
GetDownloadFileProgressCmd cmd = new GetDownloadFileProgressCmd();
cmd.taskUuid = msg.getTaskUuid();
- CephPrimaryStorageMonBase monBase = transform(Collections.singletonList(mon), CephPrimaryStorageMonBase::new).get(0);
+ CephBackupStorageMonBase monBase = new CephBackupStorageMonBase(mon);
monBase.httpCall(FILE_UPLOAD_PROGRESS_PATH, cmd, GetDownloadFileProgressResponse.class, new ReturnValueCompletion<GetDownloadFileProgressResponse>(msg) {🤖 Prompt for AI Agents
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`
around lines 2224 - 2248, The code is using CephPrimaryStorageMonBase for a
backup-storage mon; change the monBase construction to use the backup-storage
mon base type (CephBackupStorageMonVO -> its mon base helper class) so the http
call is made with the correct mon type: replace the transform that creates
CephPrimaryStorageMonBase with the appropriate CephBackupStorageMonBase (or the
class that wraps CephBackupStorageMonVO) when building monBase used for
FILE_UPLOAD_PROGRESS_PATH and invoking monBase.httpCall; ensure
GetDownloadFileProgressCmd, mon, FILE_UPLOAD_PROGRESS_PATH and monBase.httpCall
are left intact but monBase is instantiated from the backup-storage mon base
class.
Comment on lines
+2267
to
+2286
| CopyFileProgressCmd cmd = new CopyFileProgressCmd(); | ||
| cmd.targetFilePath = msg.getTargetFilePath(); | ||
| cmd.dstPath = msg.getDstFilePath(); | ||
| cmd.dstHostSSHPort = msg.getDstHostSSHPort(); | ||
| cmd.dstHostSSHUser = msg.getDstHostSSHUser(); | ||
| cmd.dstHostSSHPasswd = msg.getDstHostSSHPasswd(); | ||
| cmd.dstHostIP = msg.getDstHostIP(); | ||
| CephPrimaryStorageMonBase monBase = transform(Collections.singletonList(mon), CephPrimaryStorageMonBase::new).get(0); | ||
| monBase.httpCall(COPY_FILE_TO_HOST_PATH, cmd, CopyFileProgressResponse.class, new ReturnValueCompletion<CopyFileProgressResponse>(msg) { | ||
| @Override | ||
| public void fail(ErrorCode err) { | ||
| reply.setError(err); | ||
| bus.reply(msg, reply); | ||
| } | ||
|
|
||
| @Override | ||
| public void success(CopyFileProgressResponse rsp) { | ||
| bus.reply(msg, reply); | ||
| } | ||
| }); |
There was a problem hiding this comment.
严重错误：使用了错误的 MonBase 类型。
Line 2274 存在与上述相同的问题：使用 CephPrimaryStorageMonBase 处理 CephBackupStorageMonVO。
🐛 修复建议
CopyFileProgressCmd cmd = new CopyFileProgressCmd();
cmd.targetFilePath = msg.getTargetFilePath();
cmd.dstPath = msg.getDstFilePath();
cmd.dstHostSSHPort = msg.getDstHostSSHPort();
cmd.dstHostSSHUser = msg.getDstHostSSHUser();
cmd.dstHostSSHPasswd = msg.getDstHostSSHPasswd();
cmd.dstHostIP = msg.getDstHostIP();
- CephPrimaryStorageMonBase monBase = transform(Collections.singletonList(mon), CephPrimaryStorageMonBase::new).get(0);
+ CephBackupStorageMonBase monBase = new CephBackupStorageMonBase(mon);
monBase.httpCall(COPY_FILE_TO_HOST_PATH, cmd, CopyFileProgressResponse.class, new ReturnValueCompletion<CopyFileProgressResponse>(msg) {📝 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
| CopyFileProgressCmd cmd = new CopyFileProgressCmd(); | |
| cmd.targetFilePath = msg.getTargetFilePath(); | |
| cmd.dstPath = msg.getDstFilePath(); | |
| cmd.dstHostSSHPort = msg.getDstHostSSHPort(); | |
| cmd.dstHostSSHUser = msg.getDstHostSSHUser(); | |
| cmd.dstHostSSHPasswd = msg.getDstHostSSHPasswd(); | |
| cmd.dstHostIP = msg.getDstHostIP(); | |
| CephPrimaryStorageMonBase monBase = transform(Collections.singletonList(mon), CephPrimaryStorageMonBase::new).get(0); | |
| monBase.httpCall(COPY_FILE_TO_HOST_PATH, cmd, CopyFileProgressResponse.class, new ReturnValueCompletion<CopyFileProgressResponse>(msg) { | |
| @Override | |
| public void fail(ErrorCode err) { | |
| reply.setError(err); | |
| bus.reply(msg, reply); | |
| } | |
| @Override | |
| public void success(CopyFileProgressResponse rsp) { | |
| bus.reply(msg, reply); | |
| } | |
| }); | |
| CopyFileProgressCmd cmd = new CopyFileProgressCmd(); | |
| cmd.targetFilePath = msg.getTargetFilePath(); | |
| cmd.dstPath = msg.getDstFilePath(); | |
| cmd.dstHostSSHPort = msg.getDstHostSSHPort(); | |
| cmd.dstHostSSHUser = msg.getDstHostSSHUser(); | |
| cmd.dstHostSSHPasswd = msg.getDstHostSSHPasswd(); | |
| cmd.dstHostIP = msg.getDstHostIP(); | |
| CephBackupStorageMonBase monBase = new CephBackupStorageMonBase(mon); | |
| monBase.httpCall(COPY_FILE_TO_HOST_PATH, cmd, CopyFileProgressResponse.class, new ReturnValueCompletion<CopyFileProgressResponse>(msg) { | |
| `@Override` | |
| public void fail(ErrorCode err) { | |
| reply.setError(err); | |
| bus.reply(msg, reply); | |
| } | |
| `@Override` | |
| public void success(CopyFileProgressResponse rsp) { | |
| bus.reply(msg, reply); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In
`@plugin/ceph/src/main/java/org/zstack/storage/ceph/backup/CephBackupStorageBase.java`
around lines 2267 - 2286, The code incorrectly constructs a
CephPrimaryStorageMonBase for a backup storage mon; replace the use of
CephPrimaryStorageMonBase with CephBackupStorageMonBase when transforming the
singleton list (the variable currently named mon and the resulting monBase) so
you create a CephBackupStorageMonBase instance and call its
httpCall(COPY_FILE_TO_HOST_PATH, ...) instead of using the primary-storage mon
base.
Comment on lines
+14
to
+20
| public java.util.List getawayVmInstances; | ||
| public void setGetawayVmInstances(java.util.List getawayVmInstances) { | ||
| this.getawayVmInstances = getawayVmInstances; | ||
| } | ||
| public java.util.List getGetawayVmInstances() { | ||
| return this.getawayVmInstances; | ||
| } |
There was a problem hiding this comment.
字段名拼写错误：getawayVmInstances 应为 gatewayVmInstances
当前字段名 getawayVmInstances 中 "getaway"（意为逃跑）与类名中的 "Gateway"（网关）不一致，缺少字母 't'。这会导致 API 命名不一致，影响 SDK 用户的使用体验。
🐛 建议修复拼写错误
- public java.util.List getawayVmInstances;
- public void setGetawayVmInstances(java.util.List getawayVmInstances) {
- this.getawayVmInstances = getawayVmInstances;
+ public java.util.List gatewayVmInstances;
+ public void setGatewayVmInstances(java.util.List gatewayVmInstances) {
+ this.gatewayVmInstances = gatewayVmInstances;
}
- public java.util.List getGetawayVmInstances() {
- return this.getawayVmInstances;
+ public java.util.List getGatewayVmInstances() {
+ return this.gatewayVmInstances;
}🤖 Prompt for AI Agents
In
`@sdk/src/main/java/org/zstack/sdk/zmigrate/api/GetZMigrateGatewayVmInstancesResult.java`
around lines 14 - 20, Rename the misspelled field and its accessors in
GetZMigrateGatewayVmInstancesResult: change the field getawayVmInstances to
gatewayVmInstances and update the setter/getter method names from
setGetawayVmInstances/getGetawayVmInstances to
setGatewayVmInstances/getGatewayVmInstances; update all usages/references
(serialization keys, callers, tests) to the new name to keep the API consistent
and ensure any JSON/XML mapping/annotations reflect the corrected property name.
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 !9112