From de3bca85c8a5a43d506a1f137dccef7638e11416 Mon Sep 17 00:00:00 2001 From: Brad House Date: Mon, 9 Nov 2026 19:13:26 +0000 Subject: [PATCH 1/2] Refuse to resize the volume of a running system VM A size-only resizeVolume of a running system VM's volume, such as a virtual router's ROOT, was accepted. orchestrateResizeVolume only looks the VM up as a user VM, so for a system VM the resize was sent with no instance name and no host. The storage and the database got the new size, but the hypervisor was never told and the running guest kept the old size until its next start. The XenServer and KVM host checks in the same method were skipped for the same reason. The resize validation now refuses to change the size of a volume attached to a system VM that is not Stopped. A stopped system VM's volume can still be resized. A ROOT resize through a new disk offering was already refused unless the VM was stopped. Fixes #14645 Signed-off-by: Brad House --- .../cloud/storage/VolumeApiServiceImpl.java | 17 +++++ .../storage/VolumeApiServiceImplTest.java | 73 +++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 9b2f4f831d3e..ce1313107cbc 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1481,6 +1481,22 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep } } + /** + * The resize orchestration (orchestrateResizeVolume) only tells the hypervisor about a running user VM. For a + * system VM the volume is resized on the storage, but the running guest is not told and keeps seeing the old size + * until it next starts. So a system VM's volume can only be resized while the system VM is stopped. + */ + protected void validateVolumeIsNotOnRunningSystemVm(VolumeVO volume) { + if (volume.getInstanceId() == null) { + return; + } + VMInstanceVO vm = _vmInstanceDao.findById(volume.getInstanceId()); + if (vm != null && vm.getType().isUsedBySystem() && !State.Stopped.equals(vm.getState())) { + throw new InvalidParameterValueException(String.format("Cannot resize volume [%s] of system VM %s while it is in state %s. " + + "Stop the system VM before resizing its volume.", volume.getName(), vm.getInstanceName(), vm.getState())); + } + } + protected void validateNoVmSnapshots(VolumeVO volume) { if (volume.getInstanceId() != null) { if (vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(volume.getInstanceId())) { @@ -2496,6 +2512,7 @@ private void validateVolumeResizeWithSize(VolumeVO volume, long currentSize, Lon // if the caller is looking to change the size of the volume if (newSize != null && currentSize != newSize) { validateNoVmSnapshots(volume); + validateVolumeIsNotOnRunningSystemVm(volume); if (!validateVolumeSizeInBytes(newSize)) { throw new InvalidParameterValueException("Requested size out of range"); diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index b806eb4afe5c..5c863efbff48 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -139,6 +139,7 @@ import com.cloud.vm.DiskProfile; import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; +import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.VirtualMachineManager; @@ -2267,6 +2268,78 @@ public void validateNoVmSnapshotsTestVmHasVmSnapshotsExceptKvmDiskOnlySnapshotsI volumeApiServiceImpl.validateNoVmSnapshots(volumeVoMock); } + private VMInstanceVO mockVmInstance(VirtualMachine.Type type, State state) { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getType()).thenReturn(type); + Mockito.lenient().when(vm.getState()).thenReturn(state); + Mockito.when(volumeVoMock.getInstanceId()).thenReturn(vmInstanceMockId); + Mockito.when(_vmInstanceDao.findById(vmInstanceMockId)).thenReturn(vm); + return vm; + } + + @Test + public void validateVolumeIsNotOnRunningSystemVmTestDetachedVolume() { + Mockito.when(volumeVoMock.getInstanceId()).thenReturn(null); + + volumeApiServiceImpl.validateVolumeIsNotOnRunningSystemVm(volumeVoMock); + + Mockito.verify(_vmInstanceDao, Mockito.never()).findById(Mockito.anyLong()); + } + + @Test + public void validateVolumeIsNotOnRunningSystemVmTestRunningUserVm() { + mockVmInstance(VirtualMachine.Type.User, State.Running); + + volumeApiServiceImpl.validateVolumeIsNotOnRunningSystemVm(volumeVoMock); + } + + @Test + public void validateVolumeIsNotOnRunningSystemVmTestStoppedRouter() { + mockVmInstance(VirtualMachine.Type.DomainRouter, State.Stopped); + + volumeApiServiceImpl.validateVolumeIsNotOnRunningSystemVm(volumeVoMock); + } + + @Test + public void validateVolumeIsNotOnRunningSystemVmTestNotStoppedSystemVms() { + for (VirtualMachine.Type type : List.of(VirtualMachine.Type.DomainRouter, VirtualMachine.Type.ConsoleProxy, + VirtualMachine.Type.SecondaryStorageVm, VirtualMachine.Type.InternalLoadBalancerVm)) { + for (State state : List.of(State.Running, State.Starting, State.Stopping, State.Migrating)) { + mockVmInstance(type, state); + try { + volumeApiServiceImpl.validateVolumeIsNotOnRunningSystemVm(volumeVoMock); + Assert.fail("Resizing the volume of a " + type + " in state " + state + " must be refused"); + } catch (InvalidParameterValueException e) { + Assert.assertTrue(e.getMessage(), e.getMessage().contains("Stop the system VM")); + } + } + } + } + + @Test + public void resizeVolumeSizeOnlyOnRunningRouterIsRefused() throws ResourceAllocationException { + ResizeVolumeCmd cmd = new ResizeVolumeCmd(); + ReflectionTestUtils.setField(cmd, "id", volumeMockId); + ReflectionTestUtils.setField(cmd, "size", 1000L); // GiB, a grow of the 425 GiB volumeVoMock + + Mockito.when(volumeDaoMock.findById(volumeMockId)).thenReturn(volumeVoMock); + Mockito.when(volumeDaoMock.getHypervisorType(volumeMockId)).thenReturn(HypervisorType.KVM); + Mockito.when(volumeVoMock.getState()).thenReturn(Volume.State.Ready); + Mockito.when(volumeVoMock.getDiskOfferingId()).thenReturn(diskOfferingMockId); + Mockito.when(_diskOfferingDao.findById(diskOfferingMockId)).thenReturn(diskOfferingMock); + Mockito.doReturn(false).when(volumeApiServiceImpl).isNotPossibleToResize(volumeVoMock, diskOfferingMock); + Mockito.when(volumeDataFactoryMock.getVolume(volumeMockId)).thenReturn(volumeInfoMock); + mockVmInstance(VirtualMachine.Type.DomainRouter, State.Running); + + try { + volumeApiServiceImpl.resizeVolume(cmd); + Assert.fail("A size-only resize of a running router's volume must be refused"); + } catch (InvalidParameterValueException e) { + Assert.assertTrue(e.getMessage(), e.getMessage().contains("Stop the system VM")); + } + Mockito.verify(volumeServiceMock, Mockito.never()).resize(any(VolumeInfo.class)); + } + @Test public void vmHasVmSnapshotsExceptKvmDiskOnlySnapshotsTestMultipleDiskAndMemory () { List snapList = generateVmSnapshotVoList(VMSnapshot.Type.DiskAndMemory, VMSnapshot.Type.DiskAndMemory); From f4a2abf4b1a077e10e8be426c9689ccf84ba0966 Mon Sep 17 00:00:00 2001 From: Brad House Date: Tue, 10 Nov 2026 18:24:50 +0000 Subject: [PATCH 2/2] Rename validateVolumeIsNotOnRunningSystemVm to validateSystemVmIsStoppedForResize The check requires the system VM to be Stopped, not merely not Running, so name it after what it enforces. Signed-off-by: Brad House --- .../com/cloud/storage/VolumeApiServiceImpl.java | 4 ++-- .../cloud/storage/VolumeApiServiceImplTest.java | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index ce1313107cbc..384f9e6fe269 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1486,7 +1486,7 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep * system VM the volume is resized on the storage, but the running guest is not told and keeps seeing the old size * until it next starts. So a system VM's volume can only be resized while the system VM is stopped. */ - protected void validateVolumeIsNotOnRunningSystemVm(VolumeVO volume) { + protected void validateSystemVmIsStoppedForResize(VolumeVO volume) { if (volume.getInstanceId() == null) { return; } @@ -2512,7 +2512,7 @@ private void validateVolumeResizeWithSize(VolumeVO volume, long currentSize, Lon // if the caller is looking to change the size of the volume if (newSize != null && currentSize != newSize) { validateNoVmSnapshots(volume); - validateVolumeIsNotOnRunningSystemVm(volume); + validateSystemVmIsStoppedForResize(volume); if (!validateVolumeSizeInBytes(newSize)) { throw new InvalidParameterValueException("Requested size out of range"); diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 5c863efbff48..30810f68e238 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -2278,36 +2278,36 @@ private VMInstanceVO mockVmInstance(VirtualMachine.Type type, State state) { } @Test - public void validateVolumeIsNotOnRunningSystemVmTestDetachedVolume() { + public void validateSystemVmIsStoppedForResizeTestDetachedVolume() { Mockito.when(volumeVoMock.getInstanceId()).thenReturn(null); - volumeApiServiceImpl.validateVolumeIsNotOnRunningSystemVm(volumeVoMock); + volumeApiServiceImpl.validateSystemVmIsStoppedForResize(volumeVoMock); Mockito.verify(_vmInstanceDao, Mockito.never()).findById(Mockito.anyLong()); } @Test - public void validateVolumeIsNotOnRunningSystemVmTestRunningUserVm() { + public void validateSystemVmIsStoppedForResizeTestRunningUserVm() { mockVmInstance(VirtualMachine.Type.User, State.Running); - volumeApiServiceImpl.validateVolumeIsNotOnRunningSystemVm(volumeVoMock); + volumeApiServiceImpl.validateSystemVmIsStoppedForResize(volumeVoMock); } @Test - public void validateVolumeIsNotOnRunningSystemVmTestStoppedRouter() { + public void validateSystemVmIsStoppedForResizeTestStoppedRouter() { mockVmInstance(VirtualMachine.Type.DomainRouter, State.Stopped); - volumeApiServiceImpl.validateVolumeIsNotOnRunningSystemVm(volumeVoMock); + volumeApiServiceImpl.validateSystemVmIsStoppedForResize(volumeVoMock); } @Test - public void validateVolumeIsNotOnRunningSystemVmTestNotStoppedSystemVms() { + public void validateSystemVmIsStoppedForResizeTestNotStoppedSystemVms() { for (VirtualMachine.Type type : List.of(VirtualMachine.Type.DomainRouter, VirtualMachine.Type.ConsoleProxy, VirtualMachine.Type.SecondaryStorageVm, VirtualMachine.Type.InternalLoadBalancerVm)) { for (State state : List.of(State.Running, State.Starting, State.Stopping, State.Migrating)) { mockVmInstance(type, state); try { - volumeApiServiceImpl.validateVolumeIsNotOnRunningSystemVm(volumeVoMock); + volumeApiServiceImpl.validateSystemVmIsStoppedForResize(volumeVoMock); Assert.fail("Resizing the volume of a " + type + " in state " + state + " must be refused"); } catch (InvalidParameterValueException e) { Assert.assertTrue(e.getMessage(), e.getMessage().contains("Stop the system VM"));