Skip to content

Drain per-host reservation when a VM starts on a different host #13363

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
Kukunin wants to merge 5 commits into
base: main
Choose a base branch
from
+122 −50
Open

Drain per-host reservation when a VM starts on a different host #13363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6f09730
Release stale per-host reservation when VM starts on a different host
Kukunin Jun 6, 2026
98ec46c
Guard null host in releaseVmCapacity and reuse resolved lastHost
Kukunin Jun 16, 2026
0aef8c8
Merge branch 'main' into fix-stale-host-reservation-on-cross-host-start
Pearl1594 Sep 10, 2026
65ddaf7
Merge branch 'main' into fix-stale-host-reservation-on-cross-host-start
Pearl1594 Sep 10, 2026
cce9cd4
Drain the last-host reservation only when a VM starts from Stopped
Kukunin Sep 21, 2026
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
4 changes: 2 additions & 2 deletions engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ public interface CapacityManager {
"'<vcpu />' element of domain XMLs. If it is set to a value less than or equal to '0', then the host's CPU cores capacity will be considered.",
true, ConfigKey.Scope.Cluster);

public boolean releaseVmCapacity(VirtualMachine vm, boolean moveFromReserved, boolean moveToReservered, Long hostId);
public boolean releaseVmCapacity(VirtualMachine vm, boolean moveFromReserved, boolean moveToReserved, Long hostId);

void allocateVmCapacity(VirtualMachine vm, boolean fromLastHost);
void allocateVmCapacity(VirtualMachine vm);

/**
* @param hostId Id of the host to check capacity
Expand Down
2 changes: 1 addition & 1 deletion engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5338,7 +5338,7 @@ private VMInstanceVO orchestrateReConfigureVm(String vmUuid, ServiceOffering old
}

if (reconfiguringOnExistingHost) {
_capacityMgr.allocateVmCapacity(vm, false);
_capacityMgr.allocateVmCapacity(vm);
}

} catch (final OperationTimedoutException e) {
Expand Down
69 changes: 22 additions & 47 deletions server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,19 +167,16 @@ public boolean start() {

@DB
@Override
public boolean releaseVmCapacity(VirtualMachine vm, final boolean moveFromReserved, final boolean moveToReservered, final Long hostId) {
public boolean releaseVmCapacity(VirtualMachine vm, final boolean moveFromReserved, final boolean moveToReserved, final Long hostId) {
if (hostId == null) {
return true;
}
HostVO host = _hostDao.findById(hostId);
if (HypervisorType.External.equals(host.getHypervisorType())) {
return true;
}
return releaseVmCapacity(vm, moveFromReserved, moveToReservered, host);
return releaseVmCapacity(vm, moveFromReserved, moveToReserved, host);
Comment on lines 174 to +175
}

@DB
public boolean releaseVmCapacity(VirtualMachine vm, final boolean moveFromReserved, final boolean moveToReservered, final Host host) {
public boolean releaseVmCapacity(VirtualMachine vm, final boolean moveFromReserved, final boolean moveToReserved, final Host host) {
if (host == null) {
return true;
}
Expand Down Expand Up @@ -241,7 +238,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
capacityCpuCore.setUsedCapacity(usedCpuCore - vmCPUCore);
}

if (moveToReservered) {
if (moveToReserved) {
if (reservedCpu + vmCPU <= totalCpu) {
capacityCpu.setReservedCapacity(reservedCpu + vmCPU);
}
Expand All @@ -264,11 +261,11 @@ public void doInTransactionWithoutResult(TransactionStatus status) {

logger.debug("release cpu from host: {}, old used: {}, " +
"reserved: {}, actual total: {}, total with overprovisioning: {}; " +
"new used: {},reserved:{}; movedfromreserved: {},moveToReservered: {}", host, usedCpu, reservedCpu, actualTotalCpu, totalCpu, capacityCpu.getUsedCapacity(), capacityCpu.getReservedCapacity(), moveFromReserved, moveToReservered);
"new used: {},reserved:{}; movedfromreserved: {},moveToReserved: {}", host, usedCpu, reservedCpu, actualTotalCpu, totalCpu, capacityCpu.getUsedCapacity(), capacityCpu.getReservedCapacity(), moveFromReserved, moveToReserved);

logger.debug("release mem from host: {}, old used: {}, " +
"reserved: {}, total: {}; new used: {}, reserved: {}; " +
"movedfromreserved: {}, moveToReservered: {}", host, toHumanReadableSize(usedMem), toHumanReadableSize(reservedMem), toHumanReadableSize(totalMem), toHumanReadableSize(capacityMemory.getUsedCapacity()), toHumanReadableSize(capacityMemory.getReservedCapacity()), moveFromReserved, moveToReservered);
"movedfromreserved: {}, moveToReserved: {}", host, toHumanReadableSize(usedMem), toHumanReadableSize(reservedMem), toHumanReadableSize(totalMem), toHumanReadableSize(capacityMemory.getUsedCapacity()), toHumanReadableSize(capacityMemory.getReservedCapacity()), moveFromReserved, moveToReserved);

_capacityDao.update(capacityCpu.getId(), capacityCpu);
_capacityDao.update(capacityMemory.getId(), capacityMemory);
Expand All @@ -285,7 +282,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) {

@DB
@Override
public void allocateVmCapacity(VirtualMachine vm, final boolean fromLastHost) {
public void allocateVmCapacity(VirtualMachine vm) {

final long hostId = vm.getHostId();
final HostVO host = _hostDao.findById(hostId);
Expand Down Expand Up @@ -328,7 +325,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
long usedCpuCore = capacityCpuCore.getUsedCapacity();
long reservedCpu = capacityCpu.getReservedCapacity();
long reservedMem = capacityMem.getReservedCapacity();
long reservedCpuCore = capacityCpuCore.getReservedCapacity();
long actualTotalCpu = capacityCpu.getTotalCapacity();
long actualTotalMem = capacityMem.getTotalCapacity();
long totalCpu = (long)(actualTotalCpu * cpuOvercommitRatio);
Expand All @@ -349,41 +345,25 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
capacityMem.setUsedCapacity(usedMem + ram);
capacityCpuCore.setUsedCapacity(usedCpuCore + cpucore);

if (fromLastHost) {
/* alloc from reserved */
if (logger.isDebugEnabled()) {
logger.debug("We are allocating VM to the last host again, so adjusting the reserved capacity if it is not less than required");
logger.debug("Reserved CPU: " + reservedCpu + " , Requested CPU: " + cpu);
logger.debug("Reserved RAM: " + toHumanReadableSize(reservedMem) + " , Requested RAM: " + toHumanReadableSize(ram));
}
if (reservedCpu >= cpu && reservedMem >= ram) {
capacityCpu.setReservedCapacity(reservedCpu - cpu);
capacityMem.setReservedCapacity(reservedMem - ram);
capacityCpuCore.setReservedCapacity(reservedCpuCore - cpucore);
}
} else {
/* alloc from free resource */
if (!((reservedCpu + usedCpu + cpu <= totalCpu) && (reservedMem + usedMem + ram <= totalMem))) {
if (logger.isDebugEnabled()) {
logger.debug("Host doesn't seem to have enough free capacity, but increasing the used capacity anyways, " +
"since the VM is already starting on this host ");
}
}
/* alloc from free resource */
if (!((reservedCpu + usedCpu + cpu <= totalCpu) && (reservedMem + usedMem + ram <= totalMem))) {
logger.debug("Host doesn't seem to have enough free capacity, but increasing the used capacity anyways, " +
"since the VM is already starting on this host");
}

logger.debug(String.format("CPU STATS after allocation: for host: %s, " +
"old used: %d, old reserved: %d, actual total: %d, " +
"total with overprovisioning: %d; new used: %d, reserved: %d; " +
"requested cpu: %d, alloc_from_last: %s",
logger.debug("CPU STATS after allocation: for host: {}, " +
"old used: {}, old reserved: {}, actual total: {}, " +
"total with overprovisioning: {}; new used: {}, reserved: {}; " +
"requested cpu: {}",
host, usedCpu, reservedCpu, actualTotalCpu, totalCpu,
capacityCpu.getUsedCapacity(), capacityCpu.getReservedCapacity(), cpu, fromLastHost));
capacityCpu.getUsedCapacity(), capacityCpu.getReservedCapacity(), cpu);

logger.debug("RAM STATS after allocation: for host: {}, " +
"old used: {}, old reserved: {}, total: {}; new used: {}, reserved: {}; " +
"requested mem: {}, alloc_from_last: {}",
"requested mem: {}",
host, toHumanReadableSize(usedMem), toHumanReadableSize(reservedMem),
toHumanReadableSize(totalMem), toHumanReadableSize(capacityMem.getUsedCapacity()),
toHumanReadableSize(capacityMem.getReservedCapacity()), toHumanReadableSize(ram), fromLastHost);
toHumanReadableSize(capacityMem.getReservedCapacity()), toHumanReadableSize(ram));

long cluster_id = host.getClusterId();
ClusterDetailsVO cluster_detail_cpu = _clusterDetailsDao.findDetail(cluster_id, VmDetailConstants.CPU_OVER_COMMIT_RATIO);
Expand Down Expand Up @@ -959,8 +939,8 @@ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> t
Host lastHost = _hostDao.findById(vm.getLastHostId());
Host oldHost = _hostDao.findById(oldHostId);
Host newHost = _hostDao.findById(vm.getHostId());
logger.debug(String.format("%s state transited from [%s] to [%s] with event [%s]. VM's original host: %s, new host: %s, host before state transition: %s", vm, oldState,
newState, event, lastHost, newHost, oldHost));
logger.debug("{} state transited from [{}] to [{}] with event [{}]. VM's original host: {}, new host: {}, host before state transition: {}",
vm, oldState, newState, event, lastHost, newHost, oldHost);

if (oldState == State.Starting) {
if (newState != State.Running) {
Expand Down Expand Up @@ -992,20 +972,15 @@ public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> t
releaseVmCapacity(vm, false, false, oldHost);
}
} else if (oldState == State.Stopped) {
if (event == Event.DestroyRequested || event == Event.ExpungeOperation) {
if (event == Event.DestroyRequested || event == Event.ExpungeOperation || event == Event.StartRequested) {
releaseVmCapacity(vm, true, false, lastHost);
Comment on lines +975 to 976
} else if (event == Event.AgentReportMigrated) {
releaseVmCapacity(vm, false, false, oldHost);
}
}

if ((newState == State.Starting || newState == State.Migrating || event == Event.AgentReportMigrated) && vm.getHostId() != null) {
boolean fromLastHost = false;
if (vm.getHostId().equals(vm.getLastHostId())) {
logger.debug("VM starting again on the last host it was stopped on");
fromLastHost = true;
}
allocateVmCapacity(vm, fromLastHost);
allocateVmCapacity(vm);
Comment on lines 982 to +983

@Pearl1594 Pearl1594 Sep 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe this comment is valid and needs attention. reservedCapacity on a host is only ever populated when a VM is stopped there. A running VM's footprint is never moved into that bucket. So the release should be gated on oldState == State.Stopped rather than on newState: that still covers Stopped -> Starting (restart) and Stopped -> Migrating (storage migration
of a stopped VM), both of which genuinely have reserved capacity to release on lastHost, while skipping it for live migration (Running -> Migrating) and AgentReportMigrated, where the source host's footprint is in used, not reserved;releasing there was decrementing unrelated reserved capacity that happened to exist on that host, since it was never this VM's to begin with.

  if ((newState == State.Starting || newState == State.Migrating || event == Event.AgentReportMigrated) && vm.getHostId() != null) {
    if (oldState == State.Stopped) {
      releaseVmCapacity(vm, true, false, lastHost);
    }
    allocateVmCapacity(vm);
  }

}
Comment on lines 982 to 984
Comment thread
Copilot marked this conversation as resolved.

if (newState == State.Stopped && event != Event.RestoringFailed && event != Event.RestoringSuccess && vm.getType() == VirtualMachine.Type.User) {
Expand Down
97 changes: 97 additions & 0 deletions server/src/test/java/com/cloud/capacity/CapacityManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

Expand All @@ -43,10 +46,16 @@

import com.cloud.dc.ClusterDetailsDao;
import com.cloud.host.Host;
import com.cloud.host.HostVO;
import com.cloud.host.dao.HostDao;
import com.cloud.offering.ServiceOffering;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.utils.Pair;
import com.cloud.utils.fsm.StateMachine2;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachine.Event;
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.VmDetailConstants;

@RunWith(MockitoJUnitRunner.class)
Expand All @@ -55,6 +64,8 @@ public class CapacityManagerImplTest {
ClusterDetailsDao clusterDetailsDao;
@Mock
ServiceOfferingDao serviceOfferingDao;
@Mock
HostDao hostDao;

@Spy
@InjectMocks
Expand Down Expand Up @@ -160,6 +171,92 @@ public void testCheckIfHostHasNoCpuCapabilityButHasCapacity() {
eq(false), eq(cpuOvercommit), eq(memoryOvercommit), eq(false));
}

@Test
public void testPostStateTransitionReleasesStaleReservationWhenStartingOnDifferentHost() {
final Long oldHostId = 1L;
final Long newHostId = 2L;
HostVO oldHost = mock(HostVO.class);
when(hostDao.findById(oldHostId)).thenReturn(oldHost);

VirtualMachine vm = mock(VirtualMachine.class);
when(vm.getHostId()).thenReturn(newHostId);
when(vm.getLastHostId()).thenReturn(oldHostId);

doNothing().when(capacityManager).allocateVmCapacity(any(VirtualMachine.class));
doReturn(true).when(capacityManager).releaseVmCapacity(
any(VirtualMachine.class), anyBoolean(), anyBoolean(), any(Host.class));

StateMachine2.Transition<State, Event> transition = new StateMachine2.Transition<>(
State.Stopped, Event.StartRequested, State.Starting, Collections.emptyList());
Pair<Long, Long> opaque = new Pair<>(null, 0L);

capacityManager.postStateTransitionEvent(transition, vm, true, opaque);

verify(capacityManager).releaseVmCapacity(vm, true, false, oldHost);
verify(capacityManager).allocateVmCapacity(vm);
}

@Test
public void testPostStateTransitionReleasesReservationWhenStartingOnSameHost() {
final Long hostId = 1L;
HostVO h = mock(HostVO.class);
when(hostDao.findById(hostId)).thenReturn(h);

VirtualMachine vm = mock(VirtualMachine.class);
when(vm.getHostId()).thenReturn(hostId);
when(vm.getLastHostId()).thenReturn(hostId);

doNothing().when(capacityManager).allocateVmCapacity(any(VirtualMachine.class));
doReturn(true).when(capacityManager).releaseVmCapacity(
any(VirtualMachine.class), anyBoolean(), anyBoolean(), any(Host.class));

StateMachine2.Transition<State, Event> transition = new StateMachine2.Transition<>(
State.Stopped, Event.StartRequested, State.Starting, Collections.emptyList());
Pair<Long, Long> opaque = new Pair<>(null, 0L);

capacityManager.postStateTransitionEvent(transition, vm, true, opaque);

verify(capacityManager).releaseVmCapacity(vm, true, false, h);
verify(capacityManager).allocateVmCapacity(vm);
}

@Test
public void testPostStateTransitionKeepsReservedCapacityWhenMigratingFromRunning() {
assertNoReservedCapacityDrainedOnTransition(State.Running, Event.MigrationRequested, State.Migrating);
}

@Test
public void testPostStateTransitionKeepsReservedCapacityWhenRetryingStart() {
assertNoReservedCapacityDrainedOnTransition(State.Starting, Event.OperationRetry, State.Starting);
}

/**
* Only a VM leaving {@link State#Stopped} owns a reservation on its last host. On every other transition
* reaching the allocation block, the last host's reserved pool belongs to other, genuinely stopped VMs.
*/
private void assertNoReservedCapacityDrainedOnTransition(State from, Event event, State to) {
final Long srcHostId = 1L;
final Long dstHostId = 2L;
HostVO srcHost = mock(HostVO.class);
when(hostDao.findById(srcHostId)).thenReturn(srcHost);

VirtualMachine vm = mock(VirtualMachine.class);
when(vm.getHostId()).thenReturn(dstHostId);
when(vm.getLastHostId()).thenReturn(srcHostId);

doNothing().when(capacityManager).allocateVmCapacity(any(VirtualMachine.class));
doReturn(true).when(capacityManager).releaseVmCapacity(
any(VirtualMachine.class), anyBoolean(), anyBoolean(), any(Host.class));

StateMachine2.Transition<State, Event> transition = new StateMachine2.Transition<>(
from, event, to, Collections.emptyList());

capacityManager.postStateTransitionEvent(transition, vm, true, new Pair<>(srcHostId, dstHostId));

verify(capacityManager, never()).releaseVmCapacity(any(VirtualMachine.class), eq(true), anyBoolean(), any(Host.class));
verify(capacityManager).allocateVmCapacity(vm);
}

@Test
public void testCheckIfHostHasCpuCapabilityButNoCapacity() {
Float cpuOvercommit = 2.0f;
Expand Down