From b21891e30d25c09af284d9cb1492dbb2d87d9956 Mon Sep 17 00:00:00 2001 From: "Srivastava, Piyush" Date: Thu, 9 Jul 2026 22:11:27 +0530 Subject: [PATCH 1/4] feature/CSTACKEX-213: Added provider check for attach volume db refresh call --- .../main/java/com/cloud/storage/VolumeApiServiceImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index e24d862bb62f..2985deaa8fe3 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -63,6 +63,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.engine.subsystem.api.storage.HostScope; @@ -4851,7 +4852,9 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, L // Reload volume from DB after grantAccess — managed storage drivers (e.g. ONTAP) // may update the volume's path and iScsiName during grantAccess, so the local // volumeToAttach object can be stale. - volumeToAttach = _volsDao.findById(volumeToAttach.getId()); + if(DataStoreProvider.ONTAP_PLUGIN_NAME.equals(volumeToAttachStoragePool.getStorageProviderName())){ + volumeToAttach = _volsDao.findById(volumeToAttach.getId()); + } } if (sendCommand) { From e712667ead5c4facd2422b2a02baa1939eb839a0 Mon Sep 17 00:00:00 2001 From: "Srivastava, Piyush" Date: Fri, 10 Jul 2026 11:33:46 +0530 Subject: [PATCH 2/4] feature/CSTACKEX-213: passed hypervisor type to attachZone instead of explictily setting --- .../storage/lifecycle/OntapPrimaryDatastoreLifecycle.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java index 242b9d89a6de..c1cfb0a4c218 100755 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java @@ -344,10 +344,6 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper logger.error("attachZone : ONTAP primary storage is supported only for KVM hypervisor"); throw new CloudRuntimeException("ONTAP primary storage is supported only for KVM hypervisor"); } - storagePool.setHypervisor(hypervisorType); - storagePoolDao.update(storagePool.getId(),storagePool); - logger.debug("attachZone : Set Hypervisor type for storage pool {} to {}", storagePool.getName(), hypervisorType); - PrimaryDataStoreInfo primaryStore = (PrimaryDataStoreInfo)dataStore; List hostsToConnect = _resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(dataStore, scope.getScopeId(), hypervisorType); logger.debug(String.format("In createPool. Attaching the pool to each of the hosts in %s.", hostsToConnect)); @@ -374,7 +370,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper return false; } } - _dataStoreHelper.attachZone(dataStore); + _dataStoreHelper.attachZone(dataStore, hypervisorType); return true; } From 43bb7dbde86d4cdd75cd5b6d465181de89a167fc Mon Sep 17 00:00:00 2001 From: "Srivastava, Piyush" Date: Fri, 10 Jul 2026 14:03:17 +0530 Subject: [PATCH 3/4] feature/CSTACKEX-213: update UT for attachZone --- .../OntapPrimaryDatastoreLifecycleTest.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java index 3b576df97e37..751b864ecfcc 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycleTest.java @@ -612,7 +612,7 @@ public void testAttachZone_positive() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(mockHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -633,7 +633,7 @@ public void testAttachZone_positive() throws Exception { verify(storagePoolDetailsDao, times(1)).listDetailsKeyPairs(1L); verify(storageStrategy, times(1)).createAccessGroup(any(AccessGroup.class)); verify(_storageMgr, times(2)).connectHostToSharedPool(any(HostVO.class), eq(1L)); - verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -647,7 +647,7 @@ public void testAttachZone_withSingleHost() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(singleHost); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -662,7 +662,7 @@ public void testAttachZone_withSingleHost() throws Exception { // Verify assertTrue(result, "attachZone should return true with single host"); verify(_storageMgr, times(1)).connectHostToSharedPool(any(HostVO.class), eq(1L)); - verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -679,7 +679,7 @@ public void testAttachZone_withMultipleHosts() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(mockHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -694,7 +694,7 @@ public void testAttachZone_withMultipleHosts() throws Exception { // Verify assertTrue(result, "attachZone should return true with multiple hosts"); verify(_storageMgr, times(3)).connectHostToSharedPool(any(HostVO.class), eq(1L)); - verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -724,7 +724,7 @@ public void testAttachZone_hostConnectionFailure() throws Exception { verify(storageStrategy, times(1)).createAccessGroup(any(AccessGroup.class)); verify(_storageMgr, times(1)).connectHostToSharedPool(any(HostVO.class), eq(1L)); // _dataStoreHelper.attachZone should NOT be called due to early return - verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -737,7 +737,7 @@ public void testAttachZone_emptyHostList() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(emptyHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -751,7 +751,7 @@ public void testAttachZone_emptyHostList() throws Exception { // Verify assertTrue(result, "attachZone should return true even with no hosts"); verify(_storageMgr, times(0)).connectHostToSharedPool(any(HostVO.class), anyLong()); - verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -780,7 +780,7 @@ public void testAttachZone_secondHostConnectionFails() throws Exception { // Verify assertFalse(result, "attachZone should return false when any host connection fails"); verify(_storageMgr, times(2)).connectHostToSharedPool(any(HostVO.class), eq(1L)); - verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class)); + verify(_dataStoreHelper, times(0)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } @@ -791,7 +791,7 @@ public void testAttachZone_createAccessGroupCalled() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(mockHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -838,7 +838,7 @@ public void testAttachZone_kvmHypervisorSetsAndUpdatesPool() throws Exception { when(_resourceMgr.getEligibleUpAndEnabledHostsInZoneForStorageConnection(any(), eq(1L), eq(Hypervisor.HypervisorType.KVM))) .thenReturn(mockHosts); when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); - when(_dataStoreHelper.attachZone(any(DataStore.class))).thenReturn(dataStore); + when(_dataStoreHelper.attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM))).thenReturn(dataStore); try (MockedStatic utilityMock = Mockito.mockStatic(OntapStorageUtils.class)) { utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) @@ -850,7 +850,7 @@ public void testAttachZone_kvmHypervisorSetsAndUpdatesPool() throws Exception { dataStore, zoneScope, Hypervisor.HypervisorType.KVM); assertTrue(result, "attachZone should succeed for KVM hypervisor"); - verify(storagePoolDao, times(1)).update(eq(1L), any(StoragePoolVO.class)); + verify(_dataStoreHelper, times(1)).attachZone(any(DataStore.class), eq(Hypervisor.HypervisorType.KVM)); } } From c364f3e45546190f362d36329cc876f1cd3e181d Mon Sep 17 00:00:00 2001 From: "Srivastava, Piyush" Date: Fri, 10 Jul 2026 16:44:59 +0530 Subject: [PATCH 4/4] feature/CSTACKEX-213: select the equivalent size aggregate also --- .../org/apache/cloudstack/storage/service/StorageStrategy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index d42effec083c..d349305e5e43 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -266,7 +266,7 @@ public Volume createStorageVolume(String volumeName, Long size) { final long availableBytes = aggrResp.getAvailableBlockStorageSpace().longValue(); logger.debug("Aggregate " + aggr.getName() + " available bytes=" + availableBytes + ", requested=" + size); - if (availableBytes <= size) { + if (availableBytes < size) { logger.warn("Aggregate " + aggr.getName() + " does not have sufficient available space. Required=" + size + " bytes, available=" + availableBytes + " bytes. Skipping this aggregate."); continue;