bhouse-nexthop commented on code in PR #14233:
URL: https://github.com/apache/cloudstack/pull/14233#discussion_r4274254051
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -2514,6 +2569,16 @@ private void advanceStop(final VMInstanceVO vm, final
boolean cleanUpEvenIfUnabl
if (!cleanUpEvenIfUnableToStop) {
throw new CloudRuntimeException("We cannot stop " + vm + "
when it is in state " + vm.getState());
}
+ // cleanup() releases the resources whether or not the host
answers. For a destroy on a host that is not
+ // gone, go ahead only once the host has confirmed the instance is
stopped.
+ if (releaseOnlyIfHostIsGone && !isHostGone(vm.getHostId())) {
Review Comment:
Good catch, you're right. While Migrating, `hostId` is the destination and
`lastHostId` the source (`migrate()` sets `vm.setLastHostId(srcHostId)` then
moves to Migrating with `dstHostId`), and the check only looked at `hostId`. So
with the destination Down and the source Up, it released.
Fixed in 8e3c815411: the destroy stop now refuses for a Migrating instance
while the source host isn't gone:
```java
final Long migrationSourceHostId = releaseOnlyIfHostIsGone ?
migrationSourceThatMayBeRunningIt(vm) : null;
if (migrationSourceHostId != null) {
throw new CloudRuntimeException("Unable to stop " + vm + " in state " +
vm.getState()
+ ": the host it is migrating from may still be running it");
}
```
It only releases once both hosts are Down/Removed. An explicit `forced=true`
stop works as before. Tests cover source Up/Disconnected/Alert (refused), both
gone (released) and the explicit forced stop.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
