rafaelweingartner commented on a change in pull request #3389: when destroy the
vms, delete the tags from virtual router
URL: https://github.com/apache/cloudstack/pull/3389#discussion_r291819297
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -4601,17 +4627,23 @@ public UserVm destroyVm(long vmId, boolean expunge)
throws ResourceUnavailableEx
}
boolean status;
+ boolean resultRemovedTags;
State vmState = vm.getState();
try {
VirtualMachineEntity vmEntity =
_orchSrvc.getVirtualMachine(vm.getUuid());
status = vmEntity.destroy(Long.toString(userId), expunge);
+ resultRemovedTags = removeTagsFromVm(vmId);
} catch (CloudException e) {
CloudRuntimeException ex = new CloudRuntimeException("Unable to
destroy with specified vmId", e);
ex.addProxyObject(vm.getUuid(), "vmId");
throw ex;
}
+ if (!resultRemovedTags) {
Review comment:
I do agree with the error message if there is a problem when deleting them.
However, the problem here is the design used to "identify" when there is an
error. If there are no tags, you return "false" in that delete tags method.
Personally, we are using Java; and therefore, we should avoid these designs
(commonly used in C) where we return true/false to indicate if the method was
executed properly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
