From 03ca32900e1b408dc8742469f3990b3d8ae7d464 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 16 Jan 2026 15:39:36 +0530 Subject: [PATCH 1/6] NPE fix while deleting storage pool when pool has detached volumes --- .../src/main/java/com/cloud/storage/StorageManagerImpl.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 8b5e0b24f484..4511c862d145 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -1564,8 +1564,10 @@ protected String getStoragePoolNonDestroyedVolumesLog(long storagePoolId) { sb.append("["); for (VolumeVO vol : nonDestroyedVols) { - volInstance = _vmInstanceDao.findById(vol.getInstanceId()); - logMessageInfo.add(String.format("Volume [%s] (attached to VM [%s])", vol.getUuid(), volInstance.getUuid())); + if (vol.getInstanceId() != null) { + volInstance = _vmInstanceDao.findById(vol.getInstanceId()); + logMessageInfo.add(String.format("Volume [%s] (attached to VM [%s])", vol.getUuid(), volInstance.getUuid())); + } } sb.append(String.join(", ", logMessageInfo)); sb.append("]"); From 225341c116e83d6b98cf8f007471c49b5fe4838f Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 16 Jan 2026 16:18:48 +0530 Subject: [PATCH 2/6] review --- .../src/main/java/com/cloud/storage/StorageManagerImpl.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 4511c862d145..9d472a8936f2 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -1566,7 +1566,9 @@ protected String getStoragePoolNonDestroyedVolumesLog(long storagePoolId) { for (VolumeVO vol : nonDestroyedVols) { if (vol.getInstanceId() != null) { volInstance = _vmInstanceDao.findById(vol.getInstanceId()); - logMessageInfo.add(String.format("Volume [%s] (attached to VM [%s])", vol.getUuid(), volInstance.getUuid())); + if (volInstance != null) { + logMessageInfo.add(String.format("Volume [%s] (attached to VM [%s])", vol.getUuid(), volInstance.getUuid())); + } } } sb.append(String.join(", ", logMessageInfo)); From 7b88ab6e41c9b979b732a831224d544b1aee2598 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 16 Jan 2026 17:26:17 +0530 Subject: [PATCH 3/6] unit tests --- .../cloud/storage/StorageManagerImplTest.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index 4a28e044d9c5..52b91259474e 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -548,6 +548,37 @@ public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumesReturnLog Assert.assertEquals(expected, log); } + @Test + public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumesSkipLogForOneVolume() { + Mockito.doReturn(1L).when(storagePoolVOMock).getId(); + Mockito.doReturn(null).when(volume1VOMock).getInstanceId(); + Mockito.doReturn(1L).when(volume2VOMock).getInstanceId(); + Mockito.doReturn("ffb46333-e983-4c21-b5f0-51c5877a3805").when(volume2VOMock).getUuid(); + Mockito.doReturn("58760044-928f-4c4e-9fef-d0e48423595e").when(vmInstanceVOMock).getUuid(); + + Mockito.when(_volumeDao.findByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); + Mockito.doReturn(vmInstanceVOMock).when(vmInstanceDao).findById(Mockito.anyLong()); + + String log = storageManagerImpl.getStoragePoolNonDestroyedVolumesLog(storagePoolVOMock.getId()); + String expected = String.format("[Volume [%s] (attached to VM [%s])]", volume2VOMock.getUuid(), vmInstanceVOMock.getUuid()); + + Assert.assertEquals(expected, log); + } + + @Test + public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumesNoLog() { + Mockito.doReturn(1L).when(storagePoolVOMock).getId(); + Mockito.doReturn(1L).when(volume1VOMock).getInstanceId(); + Mockito.doReturn(1L).when(volume2VOMock).getInstanceId(); + + Mockito.when(_volumeDao.findByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); + Mockito.doReturn(null).when(vmInstanceDao).findById(Mockito.anyLong()); + + String log = storageManagerImpl.getStoragePoolNonDestroyedVolumesLog(storagePoolVOMock.getId()); + + Assert.assertEquals("[]", log); + } + private ChangeStoragePoolScopeCmd mockChangeStoragePooolScopeCmd(String newScope) { ChangeStoragePoolScopeCmd cmd = new ChangeStoragePoolScopeCmd(); ReflectionTestUtils.setField(cmd, "id", 1L); From 0a1a90c42b5cf4fadca2b8c6dab2a1bc796e4394 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Mon, 19 Jan 2026 18:36:52 +0530 Subject: [PATCH 4/6] Added log for volumes not attached to any VMs --- .../com/cloud/storage/StorageManagerImpl.java | 2 ++ .../cloud/storage/StorageManagerImplTest.java | 25 ++++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 9d472a8936f2..b54891cd77d4 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -1569,6 +1569,8 @@ protected String getStoragePoolNonDestroyedVolumesLog(long storagePoolId) { if (volInstance != null) { logMessageInfo.add(String.format("Volume [%s] (attached to VM [%s])", vol.getUuid(), volInstance.getUuid())); } + } else { + logMessageInfo.add(String.format("Volume [%s] (not attached to any VM)", vol.getUuid())); } } sb.append(String.join(", ", logMessageInfo)); diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index 52b91259474e..518f863bf1dc 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -531,7 +531,7 @@ public void testEnableDefaultDatastoreDownloadRedirectionForExistingInstallation } @Test - public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumesReturnLog() { + public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_VMAttachedLogs() { Mockito.doReturn(1L).when(storagePoolVOMock).getId(); Mockito.doReturn(1L).when(volume1VOMock).getInstanceId(); Mockito.doReturn("786633d1-a942-4374-9d56-322dd4b0d202").when(volume1VOMock).getUuid(); @@ -549,9 +549,10 @@ public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumesReturnLog } @Test - public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumesSkipLogForOneVolume() { + public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_VMLogForOneVolume() { Mockito.doReturn(1L).when(storagePoolVOMock).getId(); Mockito.doReturn(null).when(volume1VOMock).getInstanceId(); + Mockito.doReturn("786633d1-a942-4374-9d56-322dd4b0d202").when(volume1VOMock).getUuid(); Mockito.doReturn(1L).when(volume2VOMock).getInstanceId(); Mockito.doReturn("ffb46333-e983-4c21-b5f0-51c5877a3805").when(volume2VOMock).getUuid(); Mockito.doReturn("58760044-928f-4c4e-9fef-d0e48423595e").when(vmInstanceVOMock).getUuid(); @@ -560,13 +561,29 @@ public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumesSkipLogFo Mockito.doReturn(vmInstanceVOMock).when(vmInstanceDao).findById(Mockito.anyLong()); String log = storageManagerImpl.getStoragePoolNonDestroyedVolumesLog(storagePoolVOMock.getId()); - String expected = String.format("[Volume [%s] (attached to VM [%s])]", volume2VOMock.getUuid(), vmInstanceVOMock.getUuid()); + String expected = String.format("[Volume [%s] (not attached to any VM), Volume [%s] (attached to VM [%s])]", volume1VOMock.getUuid(), volume2VOMock.getUuid(), vmInstanceVOMock.getUuid()); + + Assert.assertEquals(expected, log); + } + + @Test + public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_NotAttachedLogs() { + Mockito.doReturn(1L).when(storagePoolVOMock).getId(); + Mockito.doReturn(null).when(volume1VOMock).getInstanceId(); + Mockito.doReturn("786633d1-a942-4374-9d56-322dd4b0d202").when(volume1VOMock).getUuid(); + Mockito.doReturn(null).when(volume2VOMock).getInstanceId(); + Mockito.doReturn("ffb46333-e983-4c21-b5f0-51c5877a3805").when(volume2VOMock).getUuid(); + + Mockito.when(_volumeDao.findByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); + + String log = storageManagerImpl.getStoragePoolNonDestroyedVolumesLog(storagePoolVOMock.getId()); + String expected = String.format("[Volume [%s] (not attached to any VM), Volume [%s] (not attached to any VM)]", volume1VOMock.getUuid(), volume2VOMock.getUuid()); Assert.assertEquals(expected, log); } @Test - public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumesNoLog() { + public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_NoLog() { Mockito.doReturn(1L).when(storagePoolVOMock).getId(); Mockito.doReturn(1L).when(volume1VOMock).getInstanceId(); Mockito.doReturn(1L).when(volume2VOMock).getInstanceId(); From 151444c7cf1ba307974b3927e523956d63e210da Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 20 Jan 2026 20:10:04 +0530 Subject: [PATCH 5/6] update filter, log and test --- .../main/java/com/cloud/storage/StorageManagerImpl.java | 4 ++-- .../java/com/cloud/storage/StorageManagerImplTest.java | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 8c84f80e1479..452a03a02fa7 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -1558,7 +1558,7 @@ private boolean deleteDataStoreInternal(StoragePoolVO sPool, boolean forced) { protected String getStoragePoolNonDestroyedVolumesLog(long storagePoolId) { StringBuilder sb = new StringBuilder(); - List nonDestroyedVols = volumeDao.findByPoolId(storagePoolId, null); + List nonDestroyedVols = volumeDao.findByPoolId(storagePoolId, null).stream().filter(vol -> vol.getState() != Volume.State.Destroy).collect(Collectors.toList()); VMInstanceVO volInstance; List logMessageInfo = new ArrayList<>(); @@ -1569,7 +1569,7 @@ protected String getStoragePoolNonDestroyedVolumesLog(long storagePoolId) { if (volInstance != null) { logMessageInfo.add(String.format("Volume [%s] (attached to VM [%s])", vol.getUuid(), volInstance.getUuid())); } else { - logMessageInfo.add(String.format("Volume [%s]", vol.getUuid())); + logMessageInfo.add(String.format("Volume [%s] (attached VM with ID [%d] doesn't exists)", vol.getUuid(), vol.getInstanceId())); } } else { logMessageInfo.add(String.format("Volume [%s] (not attached to any VM)", vol.getUuid())); diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index 518f863bf1dc..9fd890da21d6 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -583,17 +583,21 @@ public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_NotAttac } @Test - public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_NoLog() { + public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_VMNotExistsLog() { Mockito.doReturn(1L).when(storagePoolVOMock).getId(); Mockito.doReturn(1L).when(volume1VOMock).getInstanceId(); + Mockito.doReturn("786633d1-a942-4374-9d56-322dd4b0d202").when(volume1VOMock).getUuid(); Mockito.doReturn(1L).when(volume2VOMock).getInstanceId(); + Mockito.doReturn("ffb46333-e983-4c21-b5f0-51c5877a3805").when(volume2VOMock).getUuid(); Mockito.when(_volumeDao.findByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); Mockito.doReturn(null).when(vmInstanceDao).findById(Mockito.anyLong()); String log = storageManagerImpl.getStoragePoolNonDestroyedVolumesLog(storagePoolVOMock.getId()); + String expected = String.format("[Volume [%s] (attached VM with ID [%d] doesn't exists), Volume [%s] (attached VM with ID [%d] doesn't exists)]", + volume1VOMock.getUuid(), volume1VOMock.getInstanceId(), volume2VOMock.getUuid(), volume2VOMock.getInstanceId()); - Assert.assertEquals("[]", log); + Assert.assertEquals(expected, log); } private ChangeStoragePoolScopeCmd mockChangeStoragePooolScopeCmd(String newScope) { From 3f6f11a9b081d9ee7285ec7a70eed078a7d4c88b Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 21 Jan 2026 14:43:20 +0530 Subject: [PATCH 6/6] updated volume dao method names returning non destroyed volumes --- .../main/java/com/cloud/storage/dao/VolumeDao.java | 6 +++--- .../java/com/cloud/storage/dao/VolumeDaoImpl.java | 6 +++--- .../storage/datastore/PrimaryDataStoreImpl.java | 2 +- .../driver/DateraPrimaryDataStoreDriver.java | 2 +- .../datastore/provider/DateraHostListener.java | 4 ++-- .../driver/SolidFirePrimaryDataStoreDriver.java | 2 +- .../datastore/provider/SolidFireHostListener.java | 4 ++-- .../driver/StorPoolPrimaryDataStoreDriver.java | 2 +- .../java/com/cloud/resource/ResourceManagerImpl.java | 4 ++-- .../main/java/com/cloud/server/StatsCollector.java | 2 +- .../java/com/cloud/storage/StorageManagerImpl.java | 4 ++-- .../com/cloud/storage/StoragePoolAutomationImpl.java | 2 +- .../main/java/com/cloud/vm/UserVmManagerImpl.java | 2 +- .../com/cloud/resource/ResourceManagerImplTest.java | 12 ++++++------ .../com/cloud/storage/StorageManagerImplTest.java | 8 ++++---- 15 files changed, 31 insertions(+), 31 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java index 4936af3caab5..83f027195185 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java @@ -48,7 +48,7 @@ public interface VolumeDao extends GenericDao, StateDao findIncludingRemovedByInstanceAndType(long id, Volume.Type vType); - List findByInstanceIdAndPoolId(long instanceId, long poolId); + List findNonDestroyedVolumesByInstanceIdAndPoolId(long instanceId, long poolId); List findByInstanceIdDestroyed(long vmId); @@ -70,11 +70,11 @@ public interface VolumeDao extends GenericDao, StateDao findCreatedByInstance(long id); - List findByPoolId(long poolId); + List findNonDestroyedVolumesByPoolId(long poolId); VolumeVO findByPoolIdName(long poolId, String name); - List findByPoolId(long poolId, Volume.Type volumeType); + List findNonDestroyedVolumesByPoolId(long poolId, Volume.Type volumeType); List findByPoolIdAndState(long poolid, Volume.State state); diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java index 5ef64b046646..a72b4a258457 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java @@ -135,7 +135,7 @@ public List findByInstanceAndDeviceId(long instanceId, long deviceId) } @Override - public List findByPoolId(long poolId) { + public List findNonDestroyedVolumesByPoolId(long poolId) { SearchCriteria sc = AllFieldsSearch.create(); sc.setParameters("poolId", poolId); sc.setParameters("notDestroyed", Volume.State.Destroy, Volume.State.Expunged); @@ -144,7 +144,7 @@ public List findByPoolId(long poolId) { } @Override - public List findByInstanceIdAndPoolId(long instanceId, long poolId) { + public List findNonDestroyedVolumesByInstanceIdAndPoolId(long instanceId, long poolId) { SearchCriteria sc = AllFieldsSearch.create(); sc.setParameters("instanceId", instanceId); sc.setParameters("poolId", poolId); @@ -161,7 +161,7 @@ public VolumeVO findByPoolIdName(long poolId, String name) { } @Override - public List findByPoolId(long poolId, Volume.Type volumeType) { + public List findNonDestroyedVolumesByPoolId(long poolId, Volume.Type volumeType) { SearchCriteria sc = AllFieldsSearch.create(); sc.setParameters("poolId", poolId); sc.setParameters("notDestroyed", Volume.State.Destroy, Volume.State.Expunged); diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java index 6a10c26cc0bc..d864bf8cd8c7 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java @@ -126,7 +126,7 @@ public VolumeInfo getVolume(long id) { @Override public List getVolumes() { - List volumes = volumeDao.findByPoolId(getId()); + List volumes = volumeDao.findNonDestroyedVolumesByPoolId(getId()); List volumeInfos = new ArrayList(); for (VolumeVO volume : volumes) { volumeInfos.add(VolumeObject.getVolumeObject(this, volume)); diff --git a/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java index dcf84525748f..62393610499a 100644 --- a/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java @@ -563,7 +563,7 @@ public long getUsedBytes(StoragePool storagePool) { private long getUsedBytes(StoragePool storagePool, long volumeIdToIgnore) { long usedSpaceBytes = 0; - List lstVolumes = _volumeDao.findByPoolId(storagePool.getId(), null); + List lstVolumes = _volumeDao.findNonDestroyedVolumesByPoolId(storagePool.getId(), null); if (lstVolumes != null) { for (VolumeVO volume : lstVolumes) { diff --git a/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java index a0dc23da4861..08bc89737f26 100644 --- a/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java +++ b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/provider/DateraHostListener.java @@ -247,7 +247,7 @@ private List getStoragePaths(long clusterId, long storagePoolId) { List storagePaths = new ArrayList<>(); // If you do not pass in null for the second parameter, you only get back applicable ROOT disks. - List volumes = _volumeDao.findByPoolId(storagePoolId, null); + List volumes = _volumeDao.findNonDestroyedVolumesByPoolId(storagePoolId, null); if (volumes != null) { for (VolumeVO volume : volumes) { @@ -317,7 +317,7 @@ private List> getTargets(long clusterId, long storagePoolId) StoragePoolVO storagePool = _storagePoolDao.findById(storagePoolId); // If you do not pass in null for the second parameter, you only get back applicable ROOT disks. - List volumes = _volumeDao.findByPoolId(storagePoolId, null); + List volumes = _volumeDao.findNonDestroyedVolumesByPoolId(storagePoolId, null); if (volumes != null) { for (VolumeVO volume : volumes) { diff --git a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java index 6cc76d99d9ee..1e927e20168e 100644 --- a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java +++ b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java @@ -433,7 +433,7 @@ private long getUsedBytes(StoragePool storagePool, long volumeIdToIgnore) { public long getUsedIops(StoragePool storagePool) { long usedIops = 0; - List volumes = volumeDao.findByPoolId(storagePool.getId(), null); + List volumes = volumeDao.findNonDestroyedVolumesByPoolId(storagePool.getId(), null); if (volumes != null) { for (VolumeVO volume : volumes) { diff --git a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/provider/SolidFireHostListener.java b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/provider/SolidFireHostListener.java index 052191128f1c..c961c9267395 100644 --- a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/provider/SolidFireHostListener.java +++ b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/provider/SolidFireHostListener.java @@ -199,7 +199,7 @@ private List getStoragePaths(long clusterId, long storagePoolId) { List storagePaths = new ArrayList<>(); // If you do not pass in null for the second parameter, you only get back applicable ROOT disks. - List volumes = volumeDao.findByPoolId(storagePoolId, null); + List volumes = volumeDao.findNonDestroyedVolumesByPoolId(storagePoolId, null); if (volumes != null) { for (VolumeVO volume : volumes) { @@ -230,7 +230,7 @@ private List> getTargets(long clusterId, long storagePoolId) StoragePoolVO storagePool = storagePoolDao.findById(storagePoolId); // If you do not pass in null for the second parameter, you only get back applicable ROOT disks. - List volumes = volumeDao.findByPoolId(storagePoolId, null); + List volumes = volumeDao.findNonDestroyedVolumesByPoolId(storagePoolId, null); if (volumes != null) { for (VolumeVO volume : volumes) { diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java index 6ca67cb59235..619beee3ec6a 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java @@ -1276,7 +1276,7 @@ public Pair getVolumeStats(StoragePool storagePool, String volumeId) return volumeStats; } } else { - List volumes = volumeDao.findByPoolId(storagePool.getId()); + List volumes = volumeDao.findNonDestroyedVolumesByPoolId(storagePool.getId()); for (VolumeVO volume : volumes) { if (volume.getPath() != null && volume.getPath().equals(volumeId)) { long size = volume.getSize(); diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index e62e89eb0efc..a77ecfcb7fe1 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1026,8 +1026,8 @@ private void addVolumesToList(List volumes, List volumesToAd } protected void destroyLocalStoragePoolVolumes(long poolId) { - List rootDisks = volumeDao.findByPoolId(poolId); - List dataVolumes = volumeDao.findByPoolId(poolId, Volume.Type.DATADISK); + List rootDisks = volumeDao.findNonDestroyedVolumesByPoolId(poolId); + List dataVolumes = volumeDao.findNonDestroyedVolumesByPoolId(poolId, Volume.Type.DATADISK); List volumes = new ArrayList<>(); addVolumesToList(volumes, rootDisks); diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 7e83d452bb9f..1e0138f7cf90 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1646,7 +1646,7 @@ protected void runInContext() { List pools = _storagePoolDao.listAll(); for (StoragePoolVO pool : pools) { - List volumes = _volsDao.findByPoolId(pool.getId(), null); + List volumes = _volsDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); for (VolumeVO volume : volumes) { if (!List.of(ImageFormat.QCOW2, ImageFormat.VHD, ImageFormat.OVA, ImageFormat.RAW).contains(volume.getFormat()) && !List.of(Storage.StoragePoolType.PowerFlex, Storage.StoragePoolType.FiberChannel).contains(pool.getPoolType())) { diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 452a03a02fa7..13b7fbb00c20 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -1558,7 +1558,7 @@ private boolean deleteDataStoreInternal(StoragePoolVO sPool, boolean forced) { protected String getStoragePoolNonDestroyedVolumesLog(long storagePoolId) { StringBuilder sb = new StringBuilder(); - List nonDestroyedVols = volumeDao.findByPoolId(storagePoolId, null).stream().filter(vol -> vol.getState() != Volume.State.Destroy).collect(Collectors.toList()); + List nonDestroyedVols = volumeDao.findNonDestroyedVolumesByPoolId(storagePoolId, null); VMInstanceVO volInstance; List logMessageInfo = new ArrayList<>(); @@ -2644,7 +2644,7 @@ private void handleRemoveChildStoragePoolFromDatastoreCluster(Set childD for (String childDatastoreUUID : childDatastoreUUIDs) { StoragePoolVO dataStoreVO = _storagePoolDao.findPoolByUUID(childDatastoreUUID); - List allVolumes = volumeDao.findByPoolId(dataStoreVO.getId()); + List allVolumes = volumeDao.findNonDestroyedVolumesByPoolId(dataStoreVO.getId()); allVolumes.removeIf(volumeVO -> volumeVO.getInstanceId() == null); allVolumes.removeIf(volumeVO -> volumeVO.getState() != Volume.State.Ready); for (VolumeVO volume : allVolumes) { diff --git a/server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java b/server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java index 612582640f42..667af5a876f1 100644 --- a/server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java +++ b/server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java @@ -91,7 +91,7 @@ public boolean maintain(DataStore store) { boolean restart = !CollectionUtils.isEmpty(upPools); // 2. Get a list of all the ROOT volumes within this storage pool - List allVolumes = volumeDao.findByPoolId(pool.getId()); + List allVolumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId()); // 3. Enqueue to the work queue enqueueMigrationsForVolumes(allVolumes, pool); // 4. Process the queue diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index b00358caaa93..3e045f5a9057 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -2261,7 +2261,7 @@ public HashMap getVolumeStatistics(long clusterId, Str private List getVolumesByHost(HostVO host, StoragePool pool){ List vmsPerHost = _vmInstanceDao.listByHostId(host.getId()); return vmsPerHost.stream() - .flatMap(vm -> _volsDao.findByInstanceIdAndPoolId(vm.getId(),pool.getId()).stream().map(vol -> + .flatMap(vm -> _volsDao.findNonDestroyedVolumesByInstanceIdAndPoolId(vm.getId(),pool.getId()).stream().map(vol -> vol.getState() == Volume.State.Ready ? (vol.getFormat() == ImageFormat.OVA ? vol.getChainInfo() : vol.getPath()) : null).filter(Objects::nonNull)) .collect(Collectors.toList()); } diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index 414d41145f7b..5b7353bded6c 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -198,8 +198,8 @@ public void setup() throws Exception { rootDisks = Arrays.asList(rootDisk1, rootDisk2); dataDisks = Collections.singletonList(dataDisk); - when(volumeDao.findByPoolId(poolId)).thenReturn(rootDisks); - when(volumeDao.findByPoolId(poolId, Volume.Type.DATADISK)).thenReturn(dataDisks); + when(volumeDao.findNonDestroyedVolumesByPoolId(poolId)).thenReturn(rootDisks); + when(volumeDao.findNonDestroyedVolumesByPoolId(poolId, Volume.Type.DATADISK)).thenReturn(dataDisks); } @After @@ -564,22 +564,22 @@ public void testDestroyLocalStoragePoolVolumesBothRootDisksAndDataDisks() { @Test public void testDestroyLocalStoragePoolVolumesOnlyRootDisks() { - when(volumeDao.findByPoolId(poolId, Volume.Type.DATADISK)).thenReturn(null); + when(volumeDao.findNonDestroyedVolumesByPoolId(poolId, Volume.Type.DATADISK)).thenReturn(null); resourceManager.destroyLocalStoragePoolVolumes(poolId); verify(volumeDao, times(rootDisks.size())).updateAndRemoveVolume(any(VolumeVO.class)); } @Test public void testDestroyLocalStoragePoolVolumesOnlyDataDisks() { - when(volumeDao.findByPoolId(poolId)).thenReturn(null); + when(volumeDao.findNonDestroyedVolumesByPoolId(poolId)).thenReturn(null); resourceManager.destroyLocalStoragePoolVolumes(poolId); verify(volumeDao, times(dataDisks.size())).updateAndRemoveVolume(any(VolumeVO.class)); } @Test public void testDestroyLocalStoragePoolVolumesNoDisks() { - when(volumeDao.findByPoolId(poolId)).thenReturn(null); - when(volumeDao.findByPoolId(poolId, Volume.Type.DATADISK)).thenReturn(null); + when(volumeDao.findNonDestroyedVolumesByPoolId(poolId)).thenReturn(null); + when(volumeDao.findNonDestroyedVolumesByPoolId(poolId, Volume.Type.DATADISK)).thenReturn(null); resourceManager.destroyLocalStoragePoolVolumes(poolId); verify(volumeDao, never()).updateAndRemoveVolume(any(VolumeVO.class)); } diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index 9fd890da21d6..5f02c89339a0 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -539,7 +539,7 @@ public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_VMAttach Mockito.doReturn("ffb46333-e983-4c21-b5f0-51c5877a3805").when(volume2VOMock).getUuid(); Mockito.doReturn("58760044-928f-4c4e-9fef-d0e48423595e").when(vmInstanceVOMock).getUuid(); - Mockito.when(_volumeDao.findByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); + Mockito.when(_volumeDao.findNonDestroyedVolumesByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); Mockito.doReturn(vmInstanceVOMock).when(vmInstanceDao).findById(Mockito.anyLong()); String log = storageManagerImpl.getStoragePoolNonDestroyedVolumesLog(storagePoolVOMock.getId()); @@ -557,7 +557,7 @@ public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_VMLogFor Mockito.doReturn("ffb46333-e983-4c21-b5f0-51c5877a3805").when(volume2VOMock).getUuid(); Mockito.doReturn("58760044-928f-4c4e-9fef-d0e48423595e").when(vmInstanceVOMock).getUuid(); - Mockito.when(_volumeDao.findByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); + Mockito.when(_volumeDao.findNonDestroyedVolumesByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); Mockito.doReturn(vmInstanceVOMock).when(vmInstanceDao).findById(Mockito.anyLong()); String log = storageManagerImpl.getStoragePoolNonDestroyedVolumesLog(storagePoolVOMock.getId()); @@ -574,7 +574,7 @@ public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_NotAttac Mockito.doReturn(null).when(volume2VOMock).getInstanceId(); Mockito.doReturn("ffb46333-e983-4c21-b5f0-51c5877a3805").when(volume2VOMock).getUuid(); - Mockito.when(_volumeDao.findByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); + Mockito.when(_volumeDao.findNonDestroyedVolumesByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); String log = storageManagerImpl.getStoragePoolNonDestroyedVolumesLog(storagePoolVOMock.getId()); String expected = String.format("[Volume [%s] (not attached to any VM), Volume [%s] (not attached to any VM)]", volume1VOMock.getUuid(), volume2VOMock.getUuid()); @@ -590,7 +590,7 @@ public void getStoragePoolNonDestroyedVolumesLogTestNonDestroyedVolumes_VMNotExi Mockito.doReturn(1L).when(volume2VOMock).getInstanceId(); Mockito.doReturn("ffb46333-e983-4c21-b5f0-51c5877a3805").when(volume2VOMock).getUuid(); - Mockito.when(_volumeDao.findByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); + Mockito.when(_volumeDao.findNonDestroyedVolumesByPoolId(storagePoolVOMock.getId(), null)).thenReturn(List.of(volume1VOMock, volume2VOMock)); Mockito.doReturn(null).when(vmInstanceDao).findById(Mockito.anyLong()); String log = storageManagerImpl.getStoragePoolNonDestroyedVolumesLog(storagePoolVOMock.getId());