Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,14 @@ public String getHostUuid() {
public void setHostUuid(String hostUuid) {
this.hostUuid = hostUuid;
}

private boolean cleanAllVmMetadata;

public boolean isCleanAllVmMetadata() {
return cleanAllVmMetadata;
}

public void setCleanAllVmMetadata(boolean cleanAllVmMetadata) {
this.cleanAllVmMetadata = cleanAllVmMetadata;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,13 @@
import org.zstack.header.message.MessageReply;

public class CleanupVmInstanceMetadataOnPrimaryStorageReply extends MessageReply {
private int cleanedCount;

public int getCleanedCount() {
return cleanedCount;
}

public void setCleanedCount(int cleanedCount) {
this.cleanedCount = cleanedCount;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ public void setFailedVmUuids(List<String> failedVmUuids) {
this.failedVmUuids = failedVmUuids;
}

private List<String> failedPrimaryStorageUuids;

public List<String> getFailedPrimaryStorageUuids() {
return failedPrimaryStorageUuids;
}

public void setFailedPrimaryStorageUuids(List<String> failedPrimaryStorageUuids) {
this.failedPrimaryStorageUuids = failedPrimaryStorageUuids;
}

public static APICleanupVmInstanceMetadataEvent __example__() {
APICleanupVmInstanceMetadataEvent evt = new APICleanupVmInstanceMetadataEvent();
evt.totalCleaned = 5;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@
isAction = true
)
public class APICleanupVmInstanceMetadataMsg extends APIMessage {
@APIParam(resourceType = VmInstanceVO.class, nonempty = true)
@APIParam(resourceType = VmInstanceVO.class, required = false)
private List<String> vmUuids;

@APIParam(required = false)
private boolean cleanAllVmMetadata;

Comment on lines +17 to +22
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 搜索 APICleanupVmInstanceMetadataMsg 相关校验逻辑 =="
rg -n -C3 --type=java 'APICleanupVmInstanceMetadataMsg|cleanAllVmMetadata|getVmUuids|vmUuids|intercept|validate'

echo
echo "== 定位 VmInstanceApiInterceptor 并查看上下文 =="
fd -t f 'VmInstanceApiInterceptor.java' -x sh -c '
  echo ">>> {}"
  rg -n -C6 "APICleanupVmInstanceMetadataMsg|cleanAllVmMetadata|getVmUuids|vmUuids|validate|intercept" "{}"
'

Repository: MatheMatrix/zstack

Length of output: 50376


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 搜索 VmInstanceApiInterceptor 中 APICleanupVmInstanceMetadataMsg 的处理 =="
fd -type f -name "VmInstanceApiInterceptor.java" -exec rg -n -A 20 "APICleanupVmInstanceMetadataMsg" {} +

echo ""
echo "== 在计算模块中搜索 cleanAllVmMetadata 的验证逻辑 =="
rg -n -B 3 -A 10 "cleanAllVmMetadata" --type=java compute/ 2>/dev/null | head -100

echo ""
echo "== 搜索 APICleanupVmInstanceMetadataMsg 的完整定义 =="
fd -type f -name "APICleanupVmInstanceMetadataMsg.java" -exec cat {} +

Repository: MatheMatrix/zstack

Length of output: 321


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 搜索 VmInstanceApiInterceptor 中 APICleanupVmInstanceMetadataMsg 的处理 =="
fd -name "VmInstanceApiInterceptor.java" -exec rg -n -A 20 "APICleanupVmInstanceMetadataMsg" {} +

echo ""
echo "== 在计算模块中搜索 cleanAllVmMetadata 的验证逻辑 =="
rg -n -B 3 -A 10 "cleanAllVmMetadata" compute/ 2>/dev/null | head -100

echo ""
echo "== 搜索 APICleanupVmInstanceMetadataMsg 的完整定义 =="
fd -name "APICleanupVmInstanceMetadataMsg.java" -exec cat {} +

Repository: MatheMatrix/zstack

Length of output: 381


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 搜索 VmInstanceApiInterceptor.java 文件 =="
fd "VmInstanceApiInterceptor.java" 

echo ""
echo "== 在计算模块中查找 APICleanupVmInstanceMetadataMsg 的处理 =="
rg "APICleanupVmInstanceMetadataMsg" compute/ --type java -A 10 -B 2

echo ""
echo "== 搜索 cleanAllVmMetadata 的验证逻辑 =="
rg "cleanAllVmMetadata" compute/ --type java -B 5 -A 5

Repository: MatheMatrix/zstack

Length of output: 229


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 查看 VmInstanceApiInterceptor 的完整内容,检查是否有 APICleanupVmInstanceMetadataMsg 处理 =="
cat compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java | grep -n -i "cleanup\|intercept" | head -30

echo ""
echo "== 搜索整个仓库中 APICleanupVmInstanceMetadataMsg 的使用 =="
rg "APICleanupVmInstanceMetadataMsg" --type java

echo ""
echo "== 查看 APICleanupVmInstanceMetadataMsg 的定义 =="
fd "APICleanupVmInstanceMetadataMsg.java" -exec wc -l {} + && fd "APICleanupVmInstanceMetadataMsg.java" -exec cat {} +

Repository: MatheMatrix/zstack

Length of output: 4193


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 查看 APICleanupVmInstanceMetadataMsg 的定义 =="
cat header/src/main/java/org/zstack/header/vm/APICleanupVmInstanceMetadataMsg.java

echo ""
echo "== 搜索整个仓库中 APICleanupVmInstanceMetadataMsg 在 intercept 方法中的处理 =="
rg "intercept|APICleanupVmInstanceMetadataMsg" compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java --type java -B 2 -A 5

Repository: MatheMatrix/zstack

Length of output: 1841


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 在 VmInstanceApiInterceptor 的 intercept 方法中查找完整列表,确认是否缺少 APICleanupVmInstanceMetadataMsg 的处理 =="
rg "instanceof API" compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java --type java | grep -i "cleanup\|metadata" || echo "Not found in intercept"

echo ""
echo "== 查看 VmInstanceApiInterceptor intercept 方法的完整代码范围 =="
sed -n '94,600p' compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java | head -200

echo ""
echo "== 搜索整个 compute 模块中是否有其他拦截器处理 APICleanupVmInstanceMetadataMsg =="
rg "APICleanupVmInstanceMetadataMsg" compute/ --type java

Repository: MatheMatrix/zstack

Length of output: 11401


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 检查是否有其他地方处理 APICleanupVmInstanceMetadataMsg(如消息处理器) =="
rg "APICleanupVmInstanceMetadataMsg\|cleanAllVmMetadata" --type java | head -50

echo ""
echo "== 查找 VmInstanceApiInterceptor 完整的 intercept 方法结束位置 =="
grep -n "APIRegisterVmInstanceFromMetadataMsg" compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java
grep -n "setServiceId" compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java | head -5

echo ""
echo "== 统计 VmInstanceApiInterceptor 中 instanceof 检查的个数 =="
grep -c "instanceof API" compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java || echo "0"

Repository: MatheMatrix/zstack

Length of output: 119


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== 搜索 CleanupVmInstanceMetadata 相关的任何处理逻辑(包括消息处理器) =="
rg -i "cleanup.*metadata\|metadata.*cleanup" compute/ --type java -B 2 -A 5

echo ""
echo "== 查找 VmInstanceApiInterceptor 是否在其他地方处理过此消息 =="
grep -n "APICleanupVmInstanceMetadataMsg" compute/src/main/java/org/zstack/compute/vm/VmInstanceApiInterceptor.java || echo "Not found in VmInstanceApiInterceptor"

echo ""
echo "== 确认 __example__ 方法的完整内容 =="
tail -10 header/src/main/java/org/zstack/header/vm/APICleanupVmInstanceMetadataMsg.java

Repository: MatheMatrix/zstack

Length of output: 115


需要添加参数组合验证。

现在 vmUuidscleanAllVmMetadata 都是可选的,但代码中没有验证至少一个参数被设置。建议在 VmInstanceApiInterceptor 中为 APICleanupVmInstanceMetadataMsg 添加验证方法,确保:vmUuids 非空或 cleanAllVmMetadata=true(至少满足其一),避免发送空清理请求。

同时,__example__() 方法应覆盖新增的 cleanAllVmMetadata 参数,以提升 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/APICleanupVmInstanceMetadataMsg.java`
around lines 17 - 22, Add a parameter-combination validation for
APICleanupVmInstanceMetadataMsg: implement a validate method in
VmInstanceApiInterceptor that checks the message's vmUuids and
cleanAllVmMetadata fields and rejects the request unless vmUuids is non-empty OR
cleanAllVmMetadata is true; reference APICleanupVmInstanceMetadataMsg.vmUuids
and APICleanupVmInstanceMetadataMsg.cleanAllVmMetadata in the check and return a
clear API error when neither is provided. Also update
APICleanupVmInstanceMetadataMsg.__example__() to populate the new
cleanAllVmMetadata field (e.g., set it to true or include a sample vmUuids list)
so the API docs show the new option.

public List<String> getVmUuids() {
return vmUuids;
}
Expand All @@ -25,9 +28,18 @@ public void setVmUuids(List<String> vmUuids) {
this.vmUuids = vmUuids;
}

public boolean isCleanAllVmMetadata() {
return cleanAllVmMetadata;
}

public void setCleanAllVmMetadata(boolean cleanAllVmMetadata) {
this.cleanAllVmMetadata = cleanAllVmMetadata;
}

public static APICleanupVmInstanceMetadataMsg __example__() {
APICleanupVmInstanceMetadataMsg msg = new APICleanupVmInstanceMetadataMsg();
msg.vmUuids = java.util.Arrays.asList(uuid(), uuid());
msg.cleanAllVmMetadata = false;
return msg;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import java.io.File;
import java.util.*;
import java.util.concurrent.Callable;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -3558,6 +3559,11 @@ public void done(ErrorCodeList errorCodeList) {

@Override
protected void handle(final CleanupVmInstanceMetadataOnPrimaryStorageMsg msg) {
if (msg.isCleanAllVmMetadata()) {
handleCleanAllVmMetadataOnLocalStorage(msg);
return;
}

thdf.chainSubmit(new ChainTask(msg) {
@Override
public String getSyncSignature() {
Expand Down Expand Up @@ -3623,4 +3629,64 @@ public String getName() {
}
});
}

private void handleCleanAllVmMetadataOnLocalStorage(final CleanupVmInstanceMetadataOnPrimaryStorageMsg msg) {
CleanupVmInstanceMetadataOnPrimaryStorageReply reply = new CleanupVmInstanceMetadataOnPrimaryStorageReply();

List<String> connectedHostUuids = SQL.New(
"select h.hostUuid from LocalStorageHostRefVO h, HostVO host" +
" where h.primaryStorageUuid = :psUuid" +
" and h.hostUuid = host.uuid" +
" and host.status = :hstatus", String.class)
.param("psUuid", self.getUuid())
.param("hstatus", HostStatus.Connected)
.list();
if (connectedHostUuids.isEmpty()) {
logger.warn(String.format("[MetadataCleanup] cleanAll: no connected host found for local ps[uuid:%s]", self.getUuid()));
reply.setCleanedCount(0);
bus.reply(msg, reply);
return;
}

AtomicInteger totalCleaned = new AtomicInteger(0);
new While<>(connectedHostUuids).all((hostUuid, com) -> {
final LocalStorageHypervisorBackend bkd;
try {
LocalStorageHypervisorFactory f = getHypervisorBackendFactoryByHostUuid(hostUuid);
bkd = f.getHypervisorBackend(self);
} catch (Exception e) {
logger.warn(String.format("[MetadataCleanup] cleanAll: failed to prepare backend for host[uuid:%s] on ps[uuid:%s]: %s",
hostUuid, self.getUuid(), e.getMessage()));
com.addError(operr("failed to prepare backend for host[uuid:%s]: %s", hostUuid, e.getMessage()));
com.done();
return;
}
bkd.handle(msg, hostUuid, new ReturnValueCompletion<CleanupVmInstanceMetadataOnPrimaryStorageReply>(com) {
@Override
public void success(CleanupVmInstanceMetadataOnPrimaryStorageReply returnValue) {
totalCleaned.addAndGet(returnValue.getCleanedCount());
com.done();
}

@Override
public void fail(ErrorCode errorCode) {
logger.warn(String.format("[MetadataCleanup] cleanAll: failed on host[uuid:%s] on ps[uuid:%s]: %s",
hostUuid, self.getUuid(), errorCode));
com.addError(errorCode);
com.done();
}
});
}).run(new WhileDoneCompletion(msg) {
@Override
public void done(ErrorCodeList errorCodeList) {
if (!errorCodeList.getCauses().isEmpty() && errorCodeList.getCauses().size() == connectedHostUuids.size()) {
reply.setError(operr("failed to cleanup all vm metadata from all hosts on local primary storage[uuid:%s], causes: %s",
self.getUuid(), errorCodeList));
} else {
reply.setCleanedCount(totalCleaned.get());
}
bus.reply(msg, reply);
Comment on lines +3636 to +3688
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

不要把断连主机默认为“已全部清理”。

这里仅遍历 Connected 主机。只要 local primary storage 上还挂着断连主机,cleanAllVmMetadata=true 就可能在实际未清完的情况下返回成功;尤其是 Line 3644-3648,当前会在“没有任何可连主机”时直接返回 cleanedCount=0。这会让调用方误以为“全部 VM metadata 已清理”,但断连主机上的 metadata 其实还保留着。

建议和本类的 handle(ScanVmInstanceMetadataFromPrimaryStorageMsg) 保持一致:先拿到全部 host,再把断连 host 计入失败集合;只要存在未处理 host,就向上返回明确失败,或者至少把失败主机列表带回去。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@plugin/localstorage/src/main/java/org/zstack/storage/primary/local/LocalStorageBase.java`
around lines 3636 - 3688, The code only queries Connected hosts
(connectedHostUuids) and treats “no connected host” as a successful
cleanedCount=0, which hides metadata left on disconnected hosts; change to query
all host UUIDs attached to this primary storage, separate them into connected vs
disconnected, and treat disconnected hosts as failures (add errors into the
While's ErrorCodeList or immediately add to com via com.addError) instead of
returning cleanedCount=0; keep using getHypervisorBackendFactoryByHostUuid and
LocalStorageHypervisorBackend.handle for connected hosts, accumulate
totalCleaned from successful callbacks, and if any host (including disconnected
ones) failed or was skipped then set reply.setError(...) (or include failed host
list in the reply) rather than reporting success via reply.setCleanedCount.

}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -946,9 +946,11 @@ public static class ScanVmMetadataRsp extends AgentResponse {

public static class CleanupVmMetadataCmd extends AgentCommand {
public String metadataPath;
public boolean cleanAllVmMetadata;
}

public static class CleanupVmMetadataRsp extends AgentResponse {
public int cleanedCount;
}

public static class PrefixRebaseBackingFilesCmd extends LocalStorageKvmBackend.AgentCommand {
Expand Down Expand Up @@ -3945,11 +3947,13 @@ public void fail(ErrorCode errorCode) {
void handle(CleanupVmInstanceMetadataOnPrimaryStorageMsg msg, String hostUuid, ReturnValueCompletion<CleanupVmInstanceMetadataOnPrimaryStorageReply> completion) {
CleanupVmMetadataCmd cmd = new CleanupVmMetadataCmd();
cmd.metadataPath = msg.getMetadataPath();
cmd.cleanAllVmMetadata = msg.isCleanAllVmMetadata();

httpCall(CLEANUP_VM_METADATA_PATH, hostUuid, cmd, CleanupVmMetadataRsp.class, new ReturnValueCompletion<CleanupVmMetadataRsp>(completion) {
@Override
public void success(CleanupVmMetadataRsp rsp) {
CleanupVmInstanceMetadataOnPrimaryStorageReply reply = new CleanupVmInstanceMetadataOnPrimaryStorageReply();
reply.setCleanedCount(rsp.cleanedCount);
completion.success(reply);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2170,6 +2170,7 @@ public void handle(CleanupVmInstanceMetadataOnPrimaryStorageMsg msg, String host
CleanupVmMetadataCmd cmd = new CleanupVmMetadataCmd();
cmd.setUuid(msg.getPrimaryStorageUuid());
cmd.metadataPath = msg.getMetadataPath();
cmd.cleanAllVmMetadata = msg.isCleanAllVmMetadata();

KVMHostAsyncHttpCallMsg hmsg = new KVMHostAsyncHttpCallMsg();
hmsg.setCommand(cmd);
Expand All @@ -2191,6 +2192,7 @@ public void run(MessageReply reply) {
}

CleanupVmInstanceMetadataOnPrimaryStorageReply r = new CleanupVmInstanceMetadataOnPrimaryStorageReply();
r.setCleanedCount(rsp.cleanedCount);
completion.success(r);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -990,9 +990,11 @@ public static class ScanVmMetadataRsp extends NfsPrimaryStorageAgentResponse {

public static class CleanupVmMetadataCmd extends NfsPrimaryStorageAgentCommand {
public String metadataPath;
public boolean cleanAllVmMetadata;
}

public static class CleanupVmMetadataRsp extends NfsPrimaryStorageAgentResponse {
public int cleanedCount;
}

public static class PrefixRebaseBackingFilesCmd extends NfsPrimaryStorageAgentCommand {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ public Result throwExceptionIfError() {
}
}

@Param(required = true, nonempty = true, nullElements = false, emptyString = true, noTrim = false)
@Param(required = false, nonempty = false, nullElements = false, emptyString = true, noTrim = false)
public java.util.List vmUuids;

@Param(required = false)
public boolean cleanAllVmMetadata;

@Param(required = false)
public java.util.List systemTags;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,12 @@ public java.util.List getFailedVmUuids() {
return this.failedVmUuids;
}

public java.util.List failedPrimaryStorageUuids;
public void setFailedPrimaryStorageUuids(java.util.List failedPrimaryStorageUuids) {
this.failedPrimaryStorageUuids = failedPrimaryStorageUuids;
}
public java.util.List getFailedPrimaryStorageUuids() {
return this.failedPrimaryStorageUuids;
}

}