Skip to content

<fix>[compute]: hard delete host file if created state vm destroyed #3889

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
zstack-robot-2 wants to merge 1 commit into
base: zsv_5.0.0
Choose a base branch
from
+142 −8
Open

<fix>[compute]: hard delete host file if created state vm destroyed #3889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
75 changes: 71 additions & 4 deletions compute/src/main/java/org/zstack/compute/vm/devices/VmHostBackupFileCascadeExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,21 @@
import org.zstack.header.vm.VmDeletionStruct;
import org.zstack.header.vm.VmInstanceConstant;
import org.zstack.header.vm.VmInstanceDeletionPolicyManager.VmInstanceDeletionPolicy;
import org.zstack.header.vm.VmInstanceInventory;
import org.zstack.header.vm.VmInstanceState;
import org.zstack.header.vm.VmInstanceVO;
import org.zstack.header.vm.VmInstanceVO_;
import org.zstack.header.vm.additions.VmHostBackupFileDeletionMsg;
import org.zstack.header.vm.additions.VmHostBackupFileVO;
import org.zstack.header.vm.additions.VmHostBackupFileVO_;
import org.zstack.utils.CollectionUtils;
import org.zstack.utils.Utils;
import org.zstack.utils.logging.CLogger;

import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import static org.zstack.core.Platform.operr;
import static org.zstack.utils.CollectionDSL.list;
Expand All @@ -44,8 +50,9 @@ public class VmHostBackupFileCascadeExtension extends AbstractAsyncCascadeExtens
public void asyncCascade(CascadeAction action, Completion completion) {
if (action.isActionCode(CascadeConstant.DELETION_CHECK_CODE)) {
handleDeletionCheck(action, completion);
} else if (action.isActionCode(CascadeConstant.DELETION_DELETE_CODE, CascadeConstant.DELETION_FORCE_DELETE_CODE)
|| action.isActionCode(CascadeConstant.VM_INSTANCE_EXPUNGE_CODE)) {
} else if (action.isActionCode(CascadeConstant.VM_INSTANCE_EXPUNGE_CODE)) {
handleDeletion(action, completion);
} else if (action.isActionCode(CascadeConstant.DELETION_DELETE_CODE, CascadeConstant.DELETION_FORCE_DELETE_CODE)) {
if (shouldDeferVmAssociatedDeletion(action)) {
completion.success();
return;
Expand All @@ -65,6 +72,13 @@ private boolean shouldDeferVmAssociatedDeletion(CascadeAction action) {
if (!VmInstanceVO.class.getSimpleName().equals(action.getParentIssuer())) {
return false;
}
if (hasCreatedVmInDeletionContext(action)) {
logger.info(String.format(
"VmHostBackupFileCascadeExtension: skip deferring backup-file deletion for Created VM(s): %s; "
+ "destroy uses DBOnly hard-delete without expunge, backup rows must be removed in this cascade",
formatCreatedVmUuidsFromContext(action)));
return false;
Comment on lines +75 to +80
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 1, 2026

Choose a reason for hiding this comment

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

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

不要让一个 Created VM 带着整批 backup file 一起绕过 defer。

这里同样是 action 级别的开关：只要这批 VmDeletionStruct 里出现 Created VM，就会让整批 action 继续走 handleDeletion(...)，后面会给所有 resourceUuid 对应的 VmHostBackupFileVO 发删除消息。若同批还包含删除策略仍应 Delay/Never 的非 Created VM，它们的 backup file 也会被提前清理。建议只对 Created VM 对应的 backup rows 放开 defer，其余资源继续沿用原策略。

🤖 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/devices/VmHostBackupFileCascadeExtension.java`
around lines 75 - 80, The current logic in VmHostBackupFileCascadeExtension uses
hasCreatedVmInDeletionContext(action) to skip deferring backup-file deletion for
the entire action, which causes backup rows for non-Created VMs to be
prematurely removed; change handleDeletion(...) so that you only bypass defer
for backup rows belonging to Created VMs: use
hasCreatedVmInDeletionContext(action) and
formatCreatedVmUuidsFromContext(action) (or iterate VmDeletionStructs) to
collect Created VM UUIDs, and when generating deletion messages for
VmHostBackupFileVO resources only mark those whose resourceUuid matches a
Created VM UUID as non-deferred while preserving the original defer semantics
for all other resourceUuid entries.

}
Object raw = action.getParentIssuerContext();
if (!(raw instanceof List)) {
return false;
Expand All @@ -82,6 +96,37 @@ private boolean shouldDeferVmAssociatedDeletion(CascadeAction action) {
return false;
}

private boolean hasCreatedVmInDeletionContext(CascadeAction action) {
Object raw = action.getParentIssuerContext();
if (!(raw instanceof List)) {
return false;
}
for (Object o : (List<?>) raw) {
if (!(o instanceof VmDeletionStruct)) {
continue;
}
VmInstanceInventory inv = ((VmDeletionStruct) o).getInventory();
if (inv != null && VmInstanceState.Created.toString().equals(inv.getState())) {
return true;
}
}
return false;
}

private String formatCreatedVmUuidsFromContext(CascadeAction action) {
Object raw = action.getParentIssuerContext();
if (!(raw instanceof List)) {
return "[]";
}
return ((List<?>) raw).stream()
.filter(VmDeletionStruct.class::isInstance)
.map(VmDeletionStruct.class::cast)
.map(VmDeletionStruct::getInventory)
.filter(inv -> inv != null && VmInstanceState.Created.toString().equals(inv.getState()))
.map(VmInstanceInventory::getUuid)
.collect(Collectors.joining(", "));
}

private List<VmHostBackupFileVO> voFromAction(CascadeAction action) {
if (VmInstanceVO.class.getSimpleName().equals(action.getParentIssuer())) {
List<VmDeletionStruct> vmDeletionStructs = action.getParentIssuerContext();
Expand Down Expand Up @@ -114,11 +159,22 @@ private void handleDeletion(CascadeAction action, Completion completion) {
return;
}

Set<String> createdVmUuidsAsResource = findVmUuidsInCreatedState(
voList.stream().map(VmHostBackupFileVO::getResourceUuid).collect(Collectors.toSet()));
if (!createdVmUuidsAsResource.isEmpty()) {
logger.info(String.format(
"VmHostBackupFileCascadeExtension: deleting VmHostBackupFile row(s) tied to Created VM(s) %s with forceDelete=true "
+ "(same as expunge path; avoids leftovers when VM row is hard-deleted)",
String.join(", ", createdVmUuidsAsResource)));
}

new While<>(voList).each((vo, whileCompletion) -> {
VmHostBackupFileDeletionMsg msg = new VmHostBackupFileDeletionMsg();
msg.setUuid(vo.getUuid());
msg.setForceDelete(action.isActionCode(CascadeConstant.DELETION_FORCE_DELETE_CODE)
|| CascadeConstant.VM_INSTANCE_EXPUNGE_CODE.equals(action.getActionCode()));
boolean force = action.isActionCode(CascadeConstant.DELETION_FORCE_DELETE_CODE)
|| CascadeConstant.VM_INSTANCE_EXPUNGE_CODE.equals(action.getActionCode())
|| createdVmUuidsAsResource.contains(vo.getResourceUuid());
msg.setForceDelete(force);
bus.makeLocalServiceId(msg, VmInstanceConstant.SECURE_BOOT_SERVICE_ID);
bus.send(msg, new CloudBusCallBack(whileCompletion) {
@Override
Expand Down Expand Up @@ -146,6 +202,17 @@ public void done(ErrorCodeList errorCodeList) {
});
}

private Set<String> findVmUuidsInCreatedState(Set<String> candidateVmUuids) {
if (candidateVmUuids.isEmpty()) {
return new HashSet<>();
}
return new HashSet<>(Q.New(VmInstanceVO.class)
.in(VmInstanceVO_.uuid, candidateVmUuids)
.eq(VmInstanceVO_.state, VmInstanceState.Created)
.select(VmInstanceVO_.uuid)
.listValues());
}

private void handleDeletionCleanup(CascadeAction action, Completion completion) {
completion.success();
}
Expand Down
75 changes: 71 additions & 4 deletions compute/src/main/java/org/zstack/compute/vm/devices/VmHostFileCascadeExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,21 @@
import org.zstack.header.vm.VmDeletionStruct;
import org.zstack.header.vm.VmInstanceConstant;
import org.zstack.header.vm.VmInstanceDeletionPolicyManager.VmInstanceDeletionPolicy;
import org.zstack.header.vm.VmInstanceInventory;
import org.zstack.header.vm.VmInstanceState;
import org.zstack.header.vm.VmInstanceVO;
import org.zstack.header.vm.VmInstanceVO_;
import org.zstack.header.vm.additions.VmHostFileDeletionMsg;
import org.zstack.header.vm.additions.VmHostFileVO;
import org.zstack.header.vm.additions.VmHostFileVO_;
import org.zstack.utils.CollectionUtils;
import org.zstack.utils.Utils;
import org.zstack.utils.logging.CLogger;

import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import static org.zstack.core.Platform.operr;
import static org.zstack.utils.CollectionDSL.list;
Expand All @@ -44,8 +50,9 @@ public class VmHostFileCascadeExtension extends AbstractAsyncCascadeExtension {
public void asyncCascade(CascadeAction action, Completion completion) {
if (action.isActionCode(CascadeConstant.DELETION_CHECK_CODE)) {
handleDeletionCheck(action, completion);
} else if (action.isActionCode(CascadeConstant.DELETION_DELETE_CODE, CascadeConstant.DELETION_FORCE_DELETE_CODE)
|| action.isActionCode(CascadeConstant.VM_INSTANCE_EXPUNGE_CODE)) {
} else if (action.isActionCode(CascadeConstant.VM_INSTANCE_EXPUNGE_CODE)) {
handleDeletion(action, completion);
} else if (action.isActionCode(CascadeConstant.DELETION_DELETE_CODE, CascadeConstant.DELETION_FORCE_DELETE_CODE)) {
if (shouldDeferVmAssociatedDeletion(action)) {
completion.success();
return;
Expand All @@ -65,6 +72,13 @@ private boolean shouldDeferVmAssociatedDeletion(CascadeAction action) {
if (!VmInstanceVO.class.getSimpleName().equals(action.getParentIssuer())) {
return false;
}
if (hasCreatedVmInDeletionContext(action)) {
logger.info(String.format(
"VmHostFileCascadeExtension: skip deferring host-file deletion for Created VM(s): %s; "
+ "destroy uses DBOnly hard-delete without expunge, host files must be removed in this cascade",
formatCreatedVmUuidsFromContext(action)));
return false;
Comment on lines +75 to +80
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 1, 2026

Choose a reason for hiding this comment

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

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

不要因为一个 Created VM 就对整批级联关闭 defer。

这里是按 action 维度放开 defer：只要 hasCreatedVmInDeletionContext(action) 命中一个 Created VM，后面的 voFromAction(action) / handleDeletion(...) 就会处理这批 List<VmDeletionStruct> 里的全部 VmHostFileVO。如果同一个 action 里还混有删除策略仍应 Delay/Never 的非 Created VM，它们的 host file 也会被提前删掉。这里需要按 VM 维度而不是按 action 维度放开 defer。

🤖 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/devices/VmHostFileCascadeExtension.java`
around lines 75 - 80, The current code aborts deferring for the whole action
when any Created VM is present; change this to per-VM handling: remove the early
"return false" triggered by hasCreatedVmInDeletionContext(action), instead
iterate the VmDeletionStruct list from voFromAction(action) and separate Created
VMs from others, log created UUIDs using
formatCreatedVmUuidsFromContext(action), skip/force immediate deletion of host
files for Created VMs (VmHostFileVO entries), and only call handleDeletion(...)
(or pass the filtered list) for VMs whose deletion policy still allows
Delay/Never. Adjust logic in VmHostFileCascadeExtension to operate on each
VmDeletionStruct (or a filtered collection) rather than on the whole action.

}
Object raw = action.getParentIssuerContext();
if (!(raw instanceof List)) {
return false;
Expand All @@ -82,6 +96,37 @@ private boolean shouldDeferVmAssociatedDeletion(CascadeAction action) {
return false;
}

private boolean hasCreatedVmInDeletionContext(CascadeAction action) {
Object raw = action.getParentIssuerContext();
if (!(raw instanceof List)) {
return false;
}
for (Object o : (List<?>) raw) {
if (!(o instanceof VmDeletionStruct)) {
continue;
}
VmInstanceInventory inv = ((VmDeletionStruct) o).getInventory();
if (inv != null && VmInstanceState.Created.toString().equals(inv.getState())) {
return true;
}
}
return false;
}

private String formatCreatedVmUuidsFromContext(CascadeAction action) {
Object raw = action.getParentIssuerContext();
if (!(raw instanceof List)) {
return "[]";
}
return ((List<?>) raw).stream()
.filter(VmDeletionStruct.class::isInstance)
.map(VmDeletionStruct.class::cast)
.map(VmDeletionStruct::getInventory)
.filter(inv -> inv != null && VmInstanceState.Created.toString().equals(inv.getState()))
.map(VmInstanceInventory::getUuid)
.collect(Collectors.joining(", "));
}

private List<VmHostFileVO> voFromAction(CascadeAction action) {
if (VmInstanceVO.class.getSimpleName().equals(action.getParentIssuer())) {
List<VmDeletionStruct> vmDeletionStructs = action.getParentIssuerContext();
Expand Down Expand Up @@ -111,11 +156,22 @@ private void handleDeletion(CascadeAction action, Completion completion) {
return;
}

Set<String> createdVmUuids = findVmUuidsInCreatedState(
voList.stream().map(VmHostFileVO::getVmInstanceUuid).collect(Collectors.toSet()));
if (!createdVmUuids.isEmpty()) {
logger.info(String.format(
"VmHostFileCascadeExtension: deleting VmHostFile row(s) for Created VM(s) %s with forceDelete=true "
+ "(same as expunge path; avoids leftovers when VM row is hard-deleted)",
String.join(", ", createdVmUuids)));
}

new While<>(voList).each((vo, whileCompletion) -> {
VmHostFileDeletionMsg msg = new VmHostFileDeletionMsg();
msg.setUuid(vo.getUuid());
msg.setForceDelete(action.isActionCode(CascadeConstant.DELETION_FORCE_DELETE_CODE)
|| CascadeConstant.VM_INSTANCE_EXPUNGE_CODE.equals(action.getActionCode()));
boolean force = action.isActionCode(CascadeConstant.DELETION_FORCE_DELETE_CODE)
|| CascadeConstant.VM_INSTANCE_EXPUNGE_CODE.equals(action.getActionCode())
|| createdVmUuids.contains(vo.getVmInstanceUuid());
msg.setForceDelete(force);
bus.makeLocalServiceId(msg, VmInstanceConstant.SECURE_BOOT_SERVICE_ID);
bus.send(msg, new CloudBusCallBack(whileCompletion) {
@Override
Expand Down Expand Up @@ -143,6 +199,17 @@ public void done(ErrorCodeList errorCodeList) {
});
}

private Set<String> findVmUuidsInCreatedState(Set<String> vmUuids) {
if (vmUuids.isEmpty()) {
return new HashSet<>();
}
return new HashSet<>(Q.New(VmInstanceVO.class)
.in(VmInstanceVO_.uuid, vmUuids)
.eq(VmInstanceVO_.state, VmInstanceState.Created)
.select(VmInstanceVO_.uuid)
.listValues());
}

private void handleDeletionCleanup(CascadeAction action, Completion completion) {
completion.success();
}
Expand Down