Skip to content

check state instead of power state on change offering for ROOT volume #13271

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
DaanHoogland wants to merge 2 commits into
base: 4.22
Choose a base branch
from
+233 −15
Open

check state instead of power state on change offering for ROOT volume #13271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63010c9
check state instead of power state on change offering for ROOT volume
May 28, 2026
69def68
tests
May 28, 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
16 changes: 10 additions & 6 deletions server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2316,9 +2316,11 @@ private VolumeVO resizeVolumeInternal(VolumeVO volume, DiskOfferingVO newDiskOff
HypervisorType hypervisorType = _volsDao.getHypervisorType(volume.getId());

if (userVm != null) {
if (volume.getVolumeType().equals(Volume.Type.ROOT) && userVm.getPowerState() != VirtualMachine.PowerState.PowerOff && hypervisorType == HypervisorType.VMware) {
logger.error(" For ROOT volume resize VM should be in Power Off state.");
throw new InvalidParameterValueException("VM current state is : " + userVm.getPowerState() + ". But VM should be in " + VirtualMachine.PowerState.PowerOff + " state.");
if (Volume.Type.ROOT.equals(volume.getVolumeType())
&& !State.Stopped.equals(userVm.getState())
&& HypervisorType.VMware.equals(hypervisorType)) {
logger.error("For ROOT volume resize VM should be in Stopped state.");
throw new InvalidParameterValueException("The current VM state is '" + userVm.getState() + "'. But the VM should be in " + State.Stopped + " state.");
}
Comment on lines +2319 to 2324
// serialize VM operation
AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
Expand Down Expand Up @@ -2396,9 +2398,11 @@ private void validateVolumeReadyStateAndHypervisorChecks(VolumeVO volume, long c

UserVmVO userVm = _userVmDao.findById(volume.getInstanceId());
if (userVm != null) {
if (volume.getVolumeType().equals(Volume.Type.ROOT) && userVm.getPowerState() != VirtualMachine.PowerState.PowerOff && hypervisorType == HypervisorType.VMware) {
logger.error(" For ROOT volume resize VM should be in Power Off state.");
throw new InvalidParameterValueException("VM current state is : " + userVm.getPowerState() + ". But VM should be in " + VirtualMachine.PowerState.PowerOff + " state.");
if (Volume.Type.ROOT.equals(volume.getVolumeType())
&& !State.Stopped.equals(userVm.getState())
&& hypervisorType == HypervisorType.VMware) {
logger.error("For ROOT volume resize VM should be in Stopped state.");
throw new InvalidParameterValueException("The current VM state is '" + userVm.getState() + "'. But VM should be in " + State.Stopped + " state.");
}
}
}
Expand Down
232 changes: 223 additions & 9 deletions server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,15 @@
import static org.mockito.Mockito.when;

import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.UUID;
import java.util.concurrent.ExecutionException;

import com.cloud.event.EventTypes;
import com.cloud.event.UsageEventUtils;
import com.cloud.host.HostVO;
import com.cloud.resourcelimit.CheckedReservation;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.vm.snapshot.VMSnapshot;
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd;
Expand Down Expand Up @@ -107,17 +100,23 @@
import com.cloud.dc.dao.ClusterDao;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.dc.dao.HostPodDao;
import com.cloud.event.EventTypes;
import com.cloud.event.UsageEventUtils;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.PermissionDeniedException;
import com.cloud.exception.ResourceAllocationException;
import com.cloud.host.HostVO;
import com.cloud.host.dao.HostDao;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.org.Grouping;
import com.cloud.projects.Project;
import com.cloud.projects.ProjectManager;
import com.cloud.resourcelimit.CheckedReservation;
import com.cloud.serializer.GsonHelper;
import com.cloud.server.ManagementService;
import com.cloud.server.TaggedResourceService;
import com.cloud.service.ServiceOfferingVO;
import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.storage.Storage.ProvisioningType;
import com.cloud.storage.Volume.Type;
import com.cloud.storage.dao.DiskOfferingDao;
Expand Down Expand Up @@ -145,8 +144,11 @@
import com.cloud.vm.VirtualMachineManager;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.VMInstanceDao;
import com.cloud.vm.snapshot.VMSnapshot;
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
import com.cloud.vm.snapshot.VMSnapshotVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;

@RunWith(MockitoJUnitRunner.class)
public class VolumeApiServiceImplTest {
Expand Down Expand Up @@ -2320,4 +2322,216 @@ private List<VMSnapshotVO> generateVmSnapshotVoList(VMSnapshot.Type t1, VMSnapsh
Mockito.doReturn(1L).when(mock2).getId();
return List.of(mock1, mock2);
}

// =====================================================================
// VMware ROOT-volume resize / offering-change: VM power_state-lag tests
//
// Both private guards that protect VMware ROOT-volume resize operations
// are covered here:
// • validateVolumeReadyStateAndHypervisorChecks (called by changeDiskOfferingForVolumeInternal)
// • resizeVolumeInternal (called by both resize and change-offering flows)
//
// The "power_state lag" scenario: when a VMware VM is stopped via CloudStack
// the VirtualMachine.State transitions to Stopped immediately, but the
// VMware-side VirtualMachine.PowerState (polled from ESX) may still read
// PoweredOn for some seconds. The production code must use only the
// authoritative CloudStack State field and must NOT additionally gate on
// PowerState.
// =====================================================================

/**
* Positive – validateVolumeReadyStateAndHypervisorChecks:
* The guard must allow a VMware ROOT-volume resize when the CloudStack VM
* state is {@code Stopped}, regardless of the VMware power_state value.
* getPowerState() is intentionally left un-stubbed so that any invocation
* of that method would cause a Mockito strict-stubbing error and surface a
* regression.
*/
@Test
public void testValidateVolumeReadyStateVMware_VMStopped_PowerStateLag_ShouldPass()
throws NoSuchMethodException, InvocationTargetException, IllegalAccessException {

long volumeId = 200L;
long vmId = 201L;

VolumeVO volume = Mockito.mock(VolumeVO.class);
when(volume.getId()).thenReturn(volumeId);
when(volume.getInstanceId()).thenReturn(vmId);
when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT);
when(volume.getState()).thenReturn(Volume.State.Ready);

// snapshotDaoMock returns an empty list by default (Mockito default behaviour)
when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware);

UserVmVO stoppedVm = Mockito.mock(UserVmVO.class);
// Authoritative cloud state: Stopped.
// getPowerState() is NOT stubbed – power_state lag scenario.
when(stoppedVm.getState()).thenReturn(State.Stopped);
when(userVmDaoMock.findById(vmId)).thenReturn(stoppedVm);

long currentSizeBytes = 10L << 30; // 10 GiB
Long newSizeBytes = 20L << 30; // 20 GiB (grow; VMware prohibits shrink)

Method method = VolumeApiServiceImpl.class.getDeclaredMethod(
"validateVolumeReadyStateAndHypervisorChecks",
VolumeVO.class, long.class, Long.class);
method.setAccessible(true);

// Must complete without throwing any exception
method.invoke(volumeApiServiceImpl, volume, currentSizeBytes, newSizeBytes);
}

/**
* Negative – validateVolumeReadyStateAndHypervisorChecks:
* The guard must reject a VMware ROOT-volume resize when the CloudStack VM
* state is {@code Running}.
*/
@Test
public void testValidateVolumeReadyStateVMware_VMRunning_ShouldThrowInvalidParameterValueException()
throws NoSuchMethodException, IllegalAccessException {

long volumeId = 200L;
long vmId = 201L;

VolumeVO volume = Mockito.mock(VolumeVO.class);
when(volume.getId()).thenReturn(volumeId);
when(volume.getInstanceId()).thenReturn(vmId);
when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT);
when(volume.getState()).thenReturn(Volume.State.Ready);

when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware);

UserVmVO runningVm = Mockito.mock(UserVmVO.class);
when(runningVm.getState()).thenReturn(State.Running);
when(userVmDaoMock.findById(vmId)).thenReturn(runningVm);

long currentSizeBytes = 10L << 30;
Long newSizeBytes = 20L << 30;

Method method = VolumeApiServiceImpl.class.getDeclaredMethod(
"validateVolumeReadyStateAndHypervisorChecks",
VolumeVO.class, long.class, Long.class);
method.setAccessible(true);

try {
method.invoke(volumeApiServiceImpl, volume, currentSizeBytes, newSizeBytes);
Assert.fail("Expected InvalidParameterValueException for VMware ROOT-volume resize "
+ "when VM state is Running");
} catch (InvocationTargetException e) {
Assert.assertNotNull("InvocationTargetException must carry a cause", e.getCause());
Assert.assertTrue(
"Cause must be InvalidParameterValueException, was: " + e.getCause().getClass(),
e.getCause() instanceof InvalidParameterValueException);
Assert.assertTrue(
"Exception message must reference Stopped-state requirement, was: " + e.getCause().getMessage(),
e.getCause().getMessage() != null
&& e.getCause().getMessage().contains("VM should be in"));
}
}

/**
* Positive – resizeVolumeInternal:
* The VMware stopped-state guard inside {@code resizeVolumeInternal} must NOT
* fire when the CloudStack VM state is {@code Stopped}, even when the VMware
* power_state has not yet transitioned to PowerOff.
* Any exception originating from deeper plumbing (job queue, storage layer)
* is acceptable; only the state-guard exception is a failure.
*/
@Test
public void testResizeVolumeInternal_VMware_VMStopped_PowerStateLag_ShouldNotThrowStateGuardError()
throws NoSuchMethodException, IllegalAccessException {

long volumeId = 300L;
long vmId = 301L;

VolumeVO volume = Mockito.mock(VolumeVO.class);
when(volume.getId()).thenReturn(volumeId);
when(volume.getInstanceId()).thenReturn(vmId);
when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT);

UserVmVO stoppedVm = Mockito.mock(UserVmVO.class);
when(stoppedVm.getState()).thenReturn(State.Stopped); // authoritative cloud state
// getPowerState() deliberately NOT stubbed – power_state lag scenario
when(userVmDaoMock.findById(vmId)).thenReturn(stoppedVm);

when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware);

// resizeVolumeInternal(VolumeVO, DiskOfferingVO, Long, Long, Long, Long, Integer, boolean)
Method method = VolumeApiServiceImpl.class.getDeclaredMethod(
"resizeVolumeInternal",
VolumeVO.class, DiskOfferingVO.class,
Long.class, Long.class, Long.class, Long.class, Integer.class, boolean.class);
method.setAccessible(true);

try {
method.invoke(volumeApiServiceImpl,
volume,
/* newDiskOffering */ (DiskOfferingVO) null,
/* currentSize */ 0L,
/* newSize */ 1L,
/* newMinIops */ (Long) null,
/* newMaxIops */ (Long) null,
/* snapshotReserve */ (Integer) null,
/* shrinkOk */ false);
} catch (InvocationTargetException e) {
// If the state guard triggered it is a test failure; all other deeper
// exceptions (NullPointerException from job-queue plumbing, etc.) are
// acceptable because they are outside the scope of this test.
if (e.getCause() instanceof InvalidParameterValueException
&& e.getCause().getMessage() != null
&& e.getCause().getMessage().contains("VM should be in")) {
Assert.fail("VMware ROOT-volume resize must be allowed when CloudStack VM state is "
+ "Stopped, even under a power_state lag. Unexpected exception: "
+ e.getCause().getMessage());
}
}
}

/**
* Negative – resizeVolumeInternal:
* The VMware stopped-state guard inside {@code resizeVolumeInternal} must
* reject the operation when the CloudStack VM state is {@code Running}.
*/
@Test
public void testResizeVolumeInternal_VMware_VMRunning_ShouldThrowStateGuardError()
throws NoSuchMethodException, IllegalAccessException {

long volumeId = 300L;
long vmId = 301L;

VolumeVO volume = Mockito.mock(VolumeVO.class);
when(volume.getId()).thenReturn(volumeId);
when(volume.getInstanceId()).thenReturn(vmId);
when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT);

UserVmVO runningVm = Mockito.mock(UserVmVO.class);
when(runningVm.getState()).thenReturn(State.Running);
when(userVmDaoMock.findById(vmId)).thenReturn(runningVm);

when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.VMware);

Method method = VolumeApiServiceImpl.class.getDeclaredMethod(
"resizeVolumeInternal",
VolumeVO.class, DiskOfferingVO.class,
Long.class, Long.class, Long.class, Long.class, Integer.class, boolean.class);
method.setAccessible(true);

try {
method.invoke(volumeApiServiceImpl,
volume,
(DiskOfferingVO) null,
0L, 1L, (Long) null, (Long) null, (Integer) null, false);
Assert.fail("Expected InvalidParameterValueException for VMware ROOT-volume resize "
+ "when VM state is Running");
} catch (InvocationTargetException e) {
Assert.assertNotNull("InvocationTargetException must carry a cause", e.getCause());
Assert.assertTrue(
"Cause must be InvalidParameterValueException, was: " + e.getCause().getClass(),
e.getCause() instanceof InvalidParameterValueException);
Assert.assertTrue(
"Exception message must reference Stopped-state requirement, was: " + e.getCause().getMessage(),
e.getCause().getMessage() != null
&& e.getCause().getMessage().contains("VM should be in"));
}
}
}
Loading