[
https://issues.apache.org/jira/browse/CLOUDSTACK-9280?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15582348#comment-15582348
]
ASF GitHub Bot commented on CLOUDSTACK-9280:
--------------------------------------------
Github user GabrielBrascher commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1559#discussion_r83595364
--- Diff:
engine/storage/src/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java
---
@@ -345,15 +354,34 @@ public EndPoint select(DataObject object,
StorageAction action) {
}
} else if (action == StorageAction.DELETEVOLUME) {
VolumeInfo volume = (VolumeInfo)object;
+ VirtualMachine vm = volume.getAttachedVM();
+
if (volume.getHypervisorType() ==
Hypervisor.HypervisorType.VMware) {
- VirtualMachine vm = volume.getAttachedVM();
if (vm != null) {
Long hostId = vm.getHostId() != null ? vm.getHostId()
: vm.getLastHostId();
if (hostId != null) {
return getEndPointFromHostId(hostId);
}
}
}
+
+ //Handle case where the volume is a volume of an expunging
system VM and there are
+ //no other system VMs existing in the zone.
+ if (vm != null) {
+ VirtualMachine.Type type =
volume.getAttachedVM().getType();
+ if ((type == VirtualMachine.Type.SecondaryStorageVm ||
type == VirtualMachine.Type.ConsoleProxy) &&
+ (vm.getState() == State.Expunging || vm.getState()
== State.Destroyed)) {
+
+ List<SecondaryStorageVmVO> ssvms =
ssvmDao.listByZoneId(Role.templateProcessor, volume.getDataCenterId());
+ if (CollectionUtils.isEmpty(ssvms)) {
+
+ s_logger.info("Volume " + volume.getName() + " is
attached to a " + vm.getState() + " " + type + " and zone " +
+ volume.getDataCenterId() + " has
no SSVMs.");
+ s_logger.info("Volume " + volume.getName() + "
will be handled by dummy endpoint.");
+ return DummyEndpoint.getEndpoint();
+ }
+ }
+ }
--- End diff --
@ProjectMoon, it might be interesting to extract the code (lines 370 - 384)
for a new method [e.g. `getDummyEndpoint(VolumeInfo volume, VirtualMachine
vm)`].
With that, the `select(DataObject object, StorageAction action)` code gets
a bit cleaner and the test case is "isolated" (test for `select(DataObject
object, StorageAction action)` would just require a verify for the
`getDummyEndpoint(VolumeInfo volume, VirtualMachine vm)`, which has its own
unit test for its inner logic).
Commented lines (368 and 369) could become Javadoc for the new method.
Thanks.
> System VM volumes cannot be deleted when there are no system VMs
> ----------------------------------------------------------------
>
> Key: CLOUDSTACK-9280
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9280
> Project: CloudStack
> Issue Type: Bug
> Security Level: Public(Anyone can view this level - this is the
> default.)
> Components: Management Server
> Affects Versions: 4.6.0, 4.7.0
> Reporter: Jeff Hair
>
> Scenario: When deleting a zone, everything under it must be removed. This
> results in the system VMs being destroyed as there are no more hosts running.
> The storage cleanup thread properly detects that there are volumes to be
> deleted, but it cannot delete them because the endpoint selection fails with
> "No remote endpoint to send DeleteCommand, check if host or ssvm is down?"
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
