Skip to content

Commit a9ef05a

Browse files
weizhouapacheclaude
andcommitted
volume: enforce storage pool disable threshold when creating a volume on an explicit pool
createVolume with an admin-specified storageid bypassed the normal allocator path and thus skipped the pool's disable-threshold and allocated-capacity checks (checkUsagedSpace / checkPoolforSpace). createVolumeOnStoragePool now calls StorageManager.storagePoolHasEnoughSpace before creating the volume, matching the check already done for volume migration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent a3b4b59 commit a9ef05a

2 files changed

Lines changed: 103 additions & 0 deletions

File tree

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,14 @@ private VolumeVO createVolumeOnStoragePool(Long volumeId, Long storageId) throws
11271127
throw new InvalidParameterValueException(String.format("Disk offering: %s is not compatible with the storage pool", diskOffering.getUuid()));
11281128
}
11291129

1130+
HypervisorType hypervisorType = _volsDao.getHypervisorType(volume.getId());
1131+
DiskProfile diskProfile = new DiskProfile(volume, diskOffering, hypervisorType);
1132+
Pair<Volume, DiskProfile> volumeDiskProfilePair = new Pair<>(volume, diskProfile);
1133+
if (!storageMgr.storagePoolHasEnoughSpace(Collections.singletonList(volumeDiskProfilePair), storagePool)) {
1134+
throw new InvalidParameterValueException(String.format("Cannot create volume %s on storage pool %s as the pool does not have enough space " +
1135+
"or has crossed the disable threshold.", volume.getUuid(), storagePool.getName()));
1136+
}
1137+
11301138
DataStore dataStore = dataStoreMgr.getDataStore(storageId, DataStoreRole.Primary);
11311139
VolumeInfo volumeInfo = volFactory.getVolume(volumeId, dataStore);
11321140
AsyncCallFuture<VolumeApiResult> createVolumeFuture = volService.createVolumeAsync(volumeInfo, dataStore);

server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.junit.Assert.fail;
2121
import static org.mockito.ArgumentMatchers.any;
2222
import static org.mockito.ArgumentMatchers.anyBoolean;
23+
import static org.mockito.ArgumentMatchers.anyList;
2324
import static org.mockito.ArgumentMatchers.anyLong;
2425
import static org.mockito.ArgumentMatchers.anyString;
2526
import static org.mockito.ArgumentMatchers.eq;
@@ -2896,4 +2897,98 @@ public void testResizeVolumeInternal_VMware_VMRunning_ShouldThrowStateGuardError
28962897
e.getMessage() != null && e.getMessage().contains("VM should be in"));
28972898
}
28982899
}
2900+
2901+
/**
2902+
* createVolumeOnStoragePool must reject the request when the target pool has crossed its
2903+
* storage capacity disable threshold, instead of silently creating the volume there.
2904+
*/
2905+
@Test
2906+
public void testCreateVolumeOnStoragePool_DisableThresholdCrossed_ShouldThrow()
2907+
throws ExecutionException, InterruptedException {
2908+
long volumeId = 400L;
2909+
long storageId = 401L;
2910+
long diskOfferingId = 402L;
2911+
long dataCenterId = 1L;
2912+
2913+
VolumeVO volume = Mockito.mock(VolumeVO.class);
2914+
when(volume.getId()).thenReturn(volumeId);
2915+
when(volume.getDataCenterId()).thenReturn(dataCenterId);
2916+
when(volume.getDiskOfferingId()).thenReturn(diskOfferingId);
2917+
when(volume.getUuid()).thenReturn("volume-uuid");
2918+
when(volumeDaoMock.findById(volumeId)).thenReturn(volume);
2919+
when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.KVM);
2920+
2921+
PrimaryDataStore storagePool = Mockito.mock(PrimaryDataStore.class);
2922+
when(storagePool.getStatus()).thenReturn(StoragePoolStatus.Up);
2923+
when(storagePool.getDataCenterId()).thenReturn(dataCenterId);
2924+
when(storagePool.getName()).thenReturn("pool-crossing-threshold");
2925+
when(dataStoreMgr.getDataStore(storageId, DataStoreRole.Primary)).thenReturn(storagePool);
2926+
2927+
DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class);
2928+
when(_diskOfferingDao.findById(diskOfferingId)).thenReturn(diskOffering);
2929+
2930+
Mockito.doReturn(true).when(volumeApiServiceImpl).doesStoragePoolSupportDiskOffering(storagePool, diskOffering);
2931+
2932+
// Simulate the pool having crossed its storage.capacity/allocated disable threshold.
2933+
when(storageMgr.storagePoolHasEnoughSpace(anyList(), eq(storagePool))).thenReturn(false);
2934+
2935+
try {
2936+
invokePrivateMethod("createVolumeOnStoragePool", new Class[]{Long.class, Long.class}, volumeId, storageId);
2937+
Assert.fail("Expected an InvalidParameterValueException because the pool has crossed its disable threshold");
2938+
} catch (RuntimeException e) {
2939+
// invokePrivateMethod wraps the reflectively-thrown exception as:
2940+
// RuntimeException -> InvocationTargetException -> actual exception
2941+
Throwable cause = e.getCause();
2942+
if (cause instanceof InvocationTargetException) {
2943+
cause = cause.getCause();
2944+
}
2945+
Assert.assertTrue("Expected InvalidParameterValueException, was: " + cause,
2946+
cause instanceof InvalidParameterValueException);
2947+
Assert.assertTrue("Exception message must reference the disable threshold, was: " + cause.getMessage(),
2948+
cause.getMessage() != null && cause.getMessage().contains("disable threshold"));
2949+
}
2950+
2951+
Mockito.verify(volumeServiceMock, Mockito.never()).createVolumeAsync(any(), any());
2952+
}
2953+
2954+
/**
2955+
* createVolumeOnStoragePool must proceed with volume creation when the target pool has
2956+
* enough space and has not crossed its disable threshold.
2957+
*/
2958+
@Test
2959+
public void testCreateVolumeOnStoragePool_EnoughSpace_ShouldCreateVolume()
2960+
throws ExecutionException, InterruptedException {
2961+
long volumeId = 410L;
2962+
long storageId = 411L;
2963+
long diskOfferingId = 412L;
2964+
long dataCenterId = 1L;
2965+
2966+
VolumeVO volume = Mockito.mock(VolumeVO.class);
2967+
when(volume.getId()).thenReturn(volumeId);
2968+
when(volume.getDataCenterId()).thenReturn(dataCenterId);
2969+
when(volume.getDiskOfferingId()).thenReturn(diskOfferingId);
2970+
when(volumeDaoMock.findById(volumeId)).thenReturn(volume);
2971+
when(volumeDaoMock.getHypervisorType(volumeId)).thenReturn(HypervisorType.KVM);
2972+
2973+
PrimaryDataStore storagePool = Mockito.mock(PrimaryDataStore.class);
2974+
when(storagePool.getStatus()).thenReturn(StoragePoolStatus.Up);
2975+
when(storagePool.getDataCenterId()).thenReturn(dataCenterId);
2976+
when(dataStoreMgr.getDataStore(storageId, DataStoreRole.Primary)).thenReturn(storagePool);
2977+
2978+
DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class);
2979+
when(_diskOfferingDao.findById(diskOfferingId)).thenReturn(diskOffering);
2980+
2981+
Mockito.doReturn(true).when(volumeApiServiceImpl).doesStoragePoolSupportDiskOffering(storagePool, diskOffering);
2982+
when(storageMgr.storagePoolHasEnoughSpace(anyList(), eq(storagePool))).thenReturn(true);
2983+
2984+
when(volumeDataFactoryMock.getVolume(volumeId, storagePool)).thenReturn(volumeInfoMock);
2985+
when(volumeInfoMock.getId()).thenReturn(volumeId);
2986+
when(volumeServiceMock.createVolumeAsync(volumeInfoMock, storagePool)).thenReturn(asyncCallFutureVolumeapiResultMock);
2987+
2988+
VolumeVO result = invokePrivateMethod("createVolumeOnStoragePool",
2989+
new Class[]{Long.class, Long.class}, volumeId, storageId);
2990+
2991+
Assert.assertEquals(volume, result);
2992+
Mockito.verify(volumeServiceMock).createVolumeAsync(volumeInfoMock, storagePool);
2993+
}
28992994
}

0 commit comments

Comments
 (0)