From 1544fcf566c979c8589b61a805eb5094ab09e3b8 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 29 Sep 2026 19:36:48 +0530 Subject: [PATCH] Support migration between different PowerFlex clusters when connect on demand is enabled - Prepare the PowerFlex/ScaleIO pool on the host when needed during the migration - Register the PowerFlex/ScaleIO pool on agent if needed - Fix live migration between PowerFlex/ScaleIO pools --- .../com/cloud/storage/StorageManager.java | 2 + .../cloud/vm/VirtualMachineManagerImpl.java | 7 +- .../vm/VirtualMachineManagerImplTest.java | 16 +-- .../StorageSystemDataMotionStrategy.java | 5 +- .../storage/volume/VolumeServiceImpl.java | 37 ++++- .../driver/ScaleIOPrimaryDataStoreDriver.java | 22 +++ .../ScaleIOPrimaryDataStoreDriverTest.java | 16 +++ .../deploy/DeploymentPlanningManagerImpl.java | 30 +---- .../com/cloud/storage/StorageManagerImpl.java | 81 ++++++++++- .../cloud/storage/VolumeApiServiceImpl.java | 3 +- .../cloud/storage/StorageManagerImplTest.java | 127 ++++++++++++++++++ 11 files changed, 290 insertions(+), 56 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index 5c7348cbe6c3..0a9b45d77e88 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -327,6 +327,8 @@ static Boolean getFullCloneConfiguration(Long storeId) { boolean canHostAccessStoragePool(Host host, StoragePool pool); + boolean canHostAccessOrPrepareStoragePool(Host host, StoragePool pool); + boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool); boolean canDisconnectHostFromStoragePool(Host host, StoragePool pool); diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index c98391a654db..c794a310e5ce 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -260,7 +260,6 @@ import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSCategoryDao; import com.cloud.storage.dao.GuestOSDao; -import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.storage.dao.VolumeDao; @@ -372,8 +371,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Inject private NetworkDao _networkDao; @Inject - private StoragePoolHostDao _poolHostDao; - @Inject private VMSnapshotDao _vmSnapshotDao; @Inject private AffinityGroupVMMapDao _affinityGroupVMMapDao; @@ -3422,7 +3419,7 @@ protected Map buildMapUsingUserInformation(VirtualMachinePr StoragePoolVO currentPool = _storagePoolDao.findById(volume.getPoolId()); executeManagedStorageChecksWhenTargetStoragePoolProvided(currentPool, volume, targetPool); - if (targetHost != null && _poolHostDao.findByPoolHost(targetPool.getId(), targetHost.getId()) == null) { + if (targetHost != null && !storageMgr.canHostAccessOrPrepareStoragePool(targetHost, targetPool)) { throw new CloudRuntimeException( String.format("Cannot migrate the volume [%s] to the storage pool [%s] while migrating VM [%s] to target host [%s]. The host does not have access to the storage pool entered.", volume.getUuid(), targetPool.getUuid(), profile.getUuid(), targetHost.getUuid())); @@ -3503,7 +3500,7 @@ protected void executeManagedStorageChecksWhenTargetStoragePoolNotProvided(Host if (!currentPool.isManaged()) { return; } - if (targetHost != null && _poolHostDao.findByPoolHost(currentPool.getId(), targetHost.getId()) == null) { + if (targetHost != null && !storageMgr.canHostAccessOrPrepareStoragePool(targetHost, currentPool)) { throw new CloudRuntimeException(String.format("The target host does not have access to the volume's managed storage pool. [volumeId=%s, storageId=%s, targetHostId=%s].", volume.getUuid(), currentPool.getUuid(), targetHost.getUuid())); } diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index c9a404f9c89c..b4dc575c9085 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -143,13 +143,11 @@ import com.cloud.storage.Storage; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; -import com.cloud.storage.StoragePoolHostVO; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.VMTemplateZoneVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; -import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.storage.dao.VolumeDao; @@ -226,8 +224,6 @@ public class VirtualMachineManagerImplTest { private VolumeVO volumeVoMock; private long volumeMockId = 1111L; - @Mock - private StoragePoolHostDao storagePoolHostDaoMock; @Mock private StoragePoolAllocator storagePoolAllocatorMock; @@ -587,7 +583,7 @@ public void buildMapUsingUserInformationTestTargetHostDoesNotHaveAccessToPool() userDefinedVolumeToStoragePoolMap.put(volumeMockId, storagePoolVoMockId); Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(any(StoragePoolVO.class), any(VolumeVO.class), any(StoragePoolVO.class)); - Mockito.doReturn(null).when(storagePoolHostDaoMock).findByPoolHost(storagePoolVoMockId, hostMockId); + Mockito.doReturn(false).when(storageManager).canHostAccessOrPrepareStoragePool(hostMock, storagePoolVoMock); virtualMachineManagerImpl.buildMapUsingUserInformation(virtualMachineProfileMock, hostMock, userDefinedVolumeToStoragePoolMap); @@ -600,7 +596,7 @@ public void buildMapUsingUserInformationTestTargetHostHasAccessToPool() { Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(any(StoragePoolVO.class), any(VolumeVO.class), any(StoragePoolVO.class)); - Mockito.doReturn(Mockito.mock(StoragePoolHostVO.class)).when(storagePoolHostDaoMock).findByPoolHost(storagePoolVoMockId, hostMockId); + Mockito.doReturn(true).when(storageManager).canHostAccessOrPrepareStoragePool(hostMock, storagePoolVoMock); Map volumeToPoolObjectMap = virtualMachineManagerImpl.buildMapUsingUserInformation(virtualMachineProfileMock, hostMock, userDefinedVolumeToStoragePoolMap); @@ -635,24 +631,24 @@ public void executeManagedStorageChecksWhenTargetStoragePoolNotProvidedTestCurre virtualMachineManagerImpl.executeManagedStorageChecksWhenTargetStoragePoolNotProvided(hostMock, storagePoolVoMock, volumeVoMock); verify(storagePoolVoMock).isManaged(); - verify(storagePoolHostDaoMock, Mockito.times(0)).findByPoolHost(anyLong(), anyLong()); + Mockito.verify(storageManager, Mockito.times(0)).canHostAccessOrPrepareStoragePool(any(Host.class), any(StoragePool.class)); } @Test public void executeManagedStorageChecksWhenTargetStoragePoolNotProvidedTestCurrentStoragePoolManagedIsConnectedToHost() { Mockito.doReturn(true).when(storagePoolVoMock).isManaged(); - Mockito.doReturn(Mockito.mock(StoragePoolHostVO.class)).when(storagePoolHostDaoMock).findByPoolHost(storagePoolVoMockId, hostMockId); + Mockito.doReturn(true).when(storageManager).canHostAccessOrPrepareStoragePool(hostMock, storagePoolVoMock); virtualMachineManagerImpl.executeManagedStorageChecksWhenTargetStoragePoolNotProvided(hostMock, storagePoolVoMock, volumeVoMock); verify(storagePoolVoMock).isManaged(); - verify(storagePoolHostDaoMock, Mockito.times(1)).findByPoolHost(storagePoolVoMockId, hostMockId); + Mockito.verify(storageManager, Mockito.times(1)).canHostAccessOrPrepareStoragePool(hostMock, storagePoolVoMock); } @Test(expected = CloudRuntimeException.class) public void executeManagedStorageChecksWhenTargetStoragePoolNotProvidedTestCurrentStoragePoolManagedIsNotConnectedToHost() { Mockito.doReturn(true).when(storagePoolVoMock).isManaged(); - Mockito.doReturn(null).when(storagePoolHostDaoMock).findByPoolHost(storagePoolVoMockId, hostMockId); + Mockito.doReturn(false).when(storageManager).canHostAccessOrPrepareStoragePool(hostMock, storagePoolVoMock); virtualMachineManagerImpl.executeManagedStorageChecksWhenTargetStoragePoolNotProvided(hostMock, storagePoolVoMock, volumeVoMock); } diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index bcade3a371c4..df23e253d6c0 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java @@ -2565,8 +2565,9 @@ protected void verifyLiveMigrationForKVM(Map volumeDataSt throw new CloudRuntimeException("Destination storage pool with ID " + dataStore.getId() + " was not located."); } - if (srcStoragePoolVO.isManaged() && srcStoragePoolVO.getId() != destStoragePoolVO.getId()) { - throw new CloudRuntimeException("Migrating a volume online with KVM from managed storage is not currently supported."); + boolean isSrcAndDestPoolPowerFlexStorage = srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex) && destStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex); + if (srcStoragePoolVO.isManaged() && !isSrcAndDestPoolPowerFlexStorage && srcStoragePoolVO.getId() != destStoragePoolVO.getId()) { + throw new CloudRuntimeException("Migrating a volume online with KVM from managed storage (other than PowerFlex) is not currently supported."); } if (storageTypeConsistency == null) { diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 58807bdc6a65..aec71b6fbf4f 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -2060,6 +2060,10 @@ public CopyManagedVolumeContext(AsyncCompletionCallback callback, AsyncCallFu private AsyncCallFuture copyManagedVolume(VolumeInfo srcVolume, DataStore destStore) { AsyncCallFuture future = new AsyncCallFuture<>(); VolumeApiResult res = new VolumeApiResult(srcVolume); + Host hostWithPoolsAccess = null; + VolumeInfo destVolume = null; + boolean srcVolumeAccessGranted = false; + boolean destVolumeAccessGranted = false; try { if (!snapshotMgr.canOperateOnVolume(srcVolume)) { logger.debug("There are snapshots creating for this volume, can not move this volume"); @@ -2079,7 +2083,7 @@ private AsyncCallFuture copyManagedVolume(VolumeInfo srcVolume, poolIds.add(srcVolume.getPoolId()); poolIds.add(destStore.getId()); - Host hostWithPoolsAccess = _storageMgr.findUpAndEnabledHostWithAccessToStoragePools(poolIds); + hostWithPoolsAccess = _storageMgr.findUpAndEnabledHostWithAccessToStoragePools(poolIds); if (hostWithPoolsAccess == null) { logger.debug("No host(s) available with pool access, can not move this volume"); res.setResult("No host(s) available with pool access, can not move this volume"); @@ -2088,7 +2092,7 @@ private AsyncCallFuture copyManagedVolume(VolumeInfo srcVolume, } VolumeVO destVol = duplicateVolumeOnAnotherStorage(srcVolume, (StoragePool)destStore); - VolumeInfo destVolume = volFactory.getVolume(destVol.getId(), destStore); + destVolume = volFactory.getVolume(destVol.getId(), destStore); // Create a volume on managed storage. AsyncCallFuture createVolumeFuture = createVolumeAsync(destVolume, destStore); @@ -2117,6 +2121,7 @@ private AsyncCallFuture copyManagedVolume(VolumeInfo srcVolume, srcPrimaryDataStoreDetails.put(StorageManager.STORAGE_POOL_DISK_WAIT.toString(), String.valueOf(StorageManager.STORAGE_POOL_DISK_WAIT.valueIn(srcPrimaryDataStore.getId()))); srcPrimaryDataStore.setDetails(srcPrimaryDataStoreDetails); grantAccess(srcVolume, hostWithPoolsAccess, srcVolume.getDataStore()); + srcVolumeAccessGranted = true; } PrimaryDataStore destPrimaryDataStore = (PrimaryDataStore) destStore; @@ -2131,6 +2136,7 @@ private AsyncCallFuture copyManagedVolume(VolumeInfo srcVolume, destPrimaryDataStore.setDetails(destPrimaryDataStoreDetails); grantAccess(destVolume, hostWithPoolsAccess, destStore); + destVolumeAccessGranted = true; destVolume.processEvent(Event.CreateRequested); srcVolume.processEvent(Event.MigrationRequested); @@ -2141,10 +2147,11 @@ private AsyncCallFuture copyManagedVolume(VolumeInfo srcVolume, motionSrv.copyAsync(srcVolume, destVolume, hostWithPoolsAccess, caller); } catch (Exception e) { - logger.error("Copy to managed volume failed due to: " + e); - if(logger.isDebugEnabled()) { + logger.error("Copy to managed volume failed due to: {}", String.valueOf(e)); + if (logger.isDebugEnabled()) { logger.debug("Copy to managed volume failed.", e); } + revokeAccessOnFailedManagedVolumeCopy(srcVolume, destVolume, hostWithPoolsAccess, srcVolumeAccessGranted, destVolumeAccessGranted); res.setResult(e.toString()); future.complete(res); } @@ -2152,6 +2159,28 @@ private AsyncCallFuture copyManagedVolume(VolumeInfo srcVolume, return future; } + private void revokeAccessOnFailedManagedVolumeCopy(VolumeInfo srcVolume, VolumeInfo destVolume, Host host, boolean srcVolumeAccessGranted, boolean destVolumeAccessGranted) { + if (host == null) { + return; + } + + if (srcVolumeAccessGranted) { + try { + revokeAccess(srcVolume, host, srcVolume.getDataStore()); + } catch (Exception e) { + logger.warn("Failed to revoke access to volume {} on host {} after a failed managed volume copy", srcVolume, host, e); + } + } + + if (destVolumeAccessGranted) { + try { + revokeAccess(destVolume, host, destVolume.getDataStore()); + } catch (Exception e) { + logger.warn("Failed to revoke access to volume {} on host {} after a failed managed volume copy", destVolume, host, e); + } + } + } + protected Void copyManagedVolumeCallBack(AsyncCallbackDispatcher callback, CopyManagedVolumeContext context) { VolumeInfo srcVolume = context.srcVolume; VolumeInfo destVolume = context.destVolume; diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java index 2c6460422a49..06bab969497b 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java @@ -22,6 +22,7 @@ import javax.inject.Inject; +import com.cloud.hypervisor.Hypervisor; import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; @@ -147,6 +148,8 @@ public class ScaleIOPrimaryDataStoreDriver implements PrimaryDataStoreDriver { private VolumeService volumeService; @Inject private VolumeOrchestrationService volumeMgr; + @Inject + private StorageManager storageMgr; private ScaleIOSDCManager sdcManager; public ScaleIOPrimaryDataStoreDriver() { @@ -200,6 +203,7 @@ private boolean setVolumeLimitsFromDetails(VolumeVO volume, Host host, DataStore public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore) { try { sdcManager = ComponentContext.inject(sdcManager); + boolean hostConnectedToPool = storagePoolHostDao.findByPoolHost(dataStore.getId(), host.getId()) != null; final String sdcId = sdcManager.prepareSDC(host, dataStore); if (StringUtils.isBlank(sdcId)) { alertHostSdcDisconnection(host); @@ -209,6 +213,10 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore dataObject.getUuid(), host.getPrivateIpAddress())); } + if (!hostConnectedToPool) { + connectHostToStoragePool(host, dataStore); + } + if (DataObjectType.VOLUME.equals(dataObject.getType())) { final VolumeVO volume = volumeDao.findById(dataObject.getId()); logger.debug("Granting access for PowerFlex volume: {} at path {}", volume, volume.getPath()); @@ -231,6 +239,15 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore } } + private void connectHostToStoragePool(Host host, DataStore dataStore) { + try { + logger.debug("Connecting host {} to PowerFlex storage pool {}", host, dataStore); + storageMgr.connectHostToSharedPool(host, dataStore.getId()); + } catch (Exception e) { + throw new CloudRuntimeException(String.format("Failed to connect host %s to PowerFlex storage pool %s due to %s", host, dataStore, e.getMessage()), e); + } + } + private boolean grantAccess(DataObject dataObject, EndPoint ep, DataStore dataStore) { Host host = hostDao.findById(ep.getId()); return grantAccess(dataObject, host, dataStore); @@ -1522,6 +1539,11 @@ public boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool) { return false; } + if (!Hypervisor.HypervisorType.KVM.equals(host.getHypervisorType())) { + logger.debug("Host {} cannot prepare access to PowerFlex storage pool {}, unsupported hypervisor type: {}", host, pool, host.getHypervisorType()); + return false; + } + sdcManager = ComponentContext.inject(sdcManager); return sdcManager.areSDCConnectionsWithinLimit(pool.getId()); } diff --git a/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java b/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java index 610b595ee052..1ae1a617a600 100644 --- a/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java +++ b/plugins/storage/volume/scaleio/src/test/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriverTest.java @@ -30,7 +30,9 @@ import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.Storage; +import com.cloud.storage.StoragePool; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VolumeDao; @@ -597,4 +599,18 @@ public void testGetVolumeSizeRequiredOnPool() { 16L * (1024 * 1024 * 1024), true)); } + + @Test + public void testCanHostPrepareStoragePoolAccessWithNullArguments() { + Assert.assertFalse(scaleIOPrimaryDataStoreDriver.canHostPrepareStoragePoolAccess(null, Mockito.mock(StoragePool.class))); + Assert.assertFalse(scaleIOPrimaryDataStoreDriver.canHostPrepareStoragePoolAccess(Mockito.mock(Host.class), null)); + } + + @Test + public void testCanHostPrepareStoragePoolAccessWithUnsupportedHypervisorType() { + Host host = Mockito.mock(Host.class); + when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.VMware); + + Assert.assertFalse(scaleIOPrimaryDataStoreDriver.canHostPrepareStoragePoolAccess(host, Mockito.mock(StoragePool.class))); + } } diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java index f163a3d52a5a..7c012e600ea9 100644 --- a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -113,14 +113,12 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; -import com.cloud.storage.StoragePoolHostVO; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSCategoryDao; import com.cloud.storage.dao.GuestOSDao; -import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.template.VirtualMachineTemplate; @@ -227,8 +225,6 @@ public void setHostAllocators(List hostAllocators) { protected GuestOSCategoryDao _guestOSCategoryDao = null; @Inject protected DiskOfferingDao _diskOfferingDao; - @Inject - protected StoragePoolHostDao _poolHostDao; @Inject protected VolumeDao _volsDao; @@ -1667,31 +1663,7 @@ public boolean checkAffinity(Host potentialHost, List preferredHosts) { } protected boolean hostCanAccessSPool(Host host, StoragePool pool) { - if (!_storageMgr.checkIfHostAndStoragePoolHasCommonStorageAccessGroups(host, pool)) { - if (logger.isDebugEnabled()) { - logger.debug(String.format("StoragePool %s and host %s does not have matching storage access groups", pool, host)); - } - return false; - } - - boolean hostCanAccessSPool = false; - - StoragePoolHostVO hostPoolLinkage = _poolHostDao.findByPoolHost(pool.getId(), host.getId()); - if (hostPoolLinkage != null && _storageMgr.canHostAccessStoragePool(host, pool)) { - hostCanAccessSPool = true; - } - - logger.debug("Host: {}{} access pool: {}", host, hostCanAccessSPool ? " can" : " cannot", pool); - if (!hostCanAccessSPool) { - if (_storageMgr.canHostPrepareStoragePoolAccess(host, pool)) { - logger.debug("Host: {} can prepare access to pool: {}", host, pool); - hostCanAccessSPool = true; - } else { - logger.debug("Host: {} cannot prepare access to pool: {}", host, pool); - } - } - - return hostCanAccessSPool; + return _storageMgr.canHostAccessOrPrepareStoragePool(host, pool); } protected List findSuitableHosts(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo) { diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 0b2aca08e084..3ccc72ab41d8 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -3278,14 +3278,46 @@ public StoragePoolVO findLocalStorageOnHost(long hostId) { @Override public Host findUpAndEnabledHostWithAccessToStoragePools(List poolIds) { List hostIds = _storagePoolHostDao.findHostsConnectedToPools(poolIds); - if (hostIds.isEmpty()) { + if (CollectionUtils.isNotEmpty(hostIds)) { + Collections.shuffle(hostIds); + + for (Long hostId : hostIds) { + Host host = _hostDao.findById(hostId); + if (canHostAccessStoragePools(host, poolIds)) { + return host; + } + } + } + + return findUpAndEnabledHostAbleToPrepareAccessToStoragePools(poolIds); + } + + protected Host findUpAndEnabledHostAbleToPrepareAccessToStoragePools(List poolIds) { + if (CollectionUtils.isEmpty(poolIds)) { return null; } - Collections.shuffle(hostIds); - for (Long hostId : hostIds) { - Host host = _hostDao.findById(hostId); - if (canHostAccessStoragePools(host, poolIds)) { + Long zoneId = null; + for (Long poolId : poolIds) { + StoragePoolVO pool = _storagePoolDao.findById(poolId); + if (pool == null) { + return null; + } + if (zoneId == null) { + zoneId = pool.getDataCenterId(); + } else if (!zoneId.equals(pool.getDataCenterId())) { + return null; + } + } + + List hosts = _resourceMgr.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, zoneId); + if (CollectionUtils.isEmpty(hosts)) { + return null; + } + + Collections.shuffle(hosts); + for (HostVO host : hosts) { + if (canHostAccessOrPrepareStoragePools(host, poolIds)) { return host; } } @@ -3308,6 +3340,21 @@ private boolean canHostAccessStoragePools(Host host, List poolIds) { return true; } + private boolean canHostAccessOrPrepareStoragePools(Host host, List poolIds) { + if (CollectionUtils.isEmpty(poolIds)) { + return false; + } + + for (Long poolId : poolIds) { + StoragePool pool = _storagePoolDao.findById(poolId); + if (!canHostAccessOrPrepareStoragePool(host, pool)) { + return false; + } + } + + return true; + } + @Override @DB public List findStoragePoolsConnectedToHost(long hostId) { @@ -3341,6 +3388,30 @@ public boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool) { return storeDriver instanceof PrimaryDataStoreDriver && ((PrimaryDataStoreDriver)storeDriver).canHostPrepareStoragePoolAccess(host, pool); } + @Override + public boolean canHostAccessOrPrepareStoragePool(Host host, StoragePool pool) { + if (host == null || pool == null) { + return false; + } + + if (!checkIfHostAndStoragePoolHasCommonStorageAccessGroups(host, pool)) { + logger.debug("Storage pool {} and host {} do not have matching storage access groups", pool, host); + return false; + } + + if (_storagePoolHostDao.findByPoolHost(pool.getId(), host.getId()) != null && canHostAccessStoragePool(host, pool)) { + return true; + } + + if (canHostPrepareStoragePoolAccess(host, pool)) { + logger.debug("Host {} can prepare access to pool {}", host, pool); + return true; + } + + logger.debug("Host {} cannot access, nor prepare access to pool {}", host, pool); + return false; + } + @Override public boolean canDisconnectHostFromStoragePool(Host host, StoragePool pool) { if (pool == null || !pool.isManaged()) { diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 9b2f4f831d3e..169bac51408b 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -3550,7 +3550,8 @@ public Volume migrateVolume(MigrateVolumeCmd cmd) { throw new CloudRuntimeException(checkResult.second()); } - if (!liveMigrateVolume && vm != null) { + if (!liveMigrateVolume && vm != null + && storageMgr.findUpAndEnabledHostWithAccessToStoragePools(Arrays.asList(vol.getPoolId(), destPool.getId())) == null) { DataStore primaryStore = dataStoreMgr.getPrimaryDataStore(destPool.getId()); if (_epSelector.select(primaryStore) == null) { throw new CloudRuntimeException("Unable to find accessible host for volume migration"); diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index 8f88800d549f..8ee6b9cf6cf9 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -30,6 +30,7 @@ import com.cloud.host.dao.HostDao; import com.cloud.resource.ResourceManager; import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; +import com.cloud.storage.dao.StoragePoolHostDao; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.command.admin.storage.ChangeStoragePoolScopeCmd; import org.apache.cloudstack.api.command.admin.storage.ConfigureStorageAccessCmd; @@ -157,6 +158,9 @@ public class StorageManagerImplTest { @Mock private StoragePoolAndAccessGroupMapDao storagePoolAccessGroupMapDao; + @Mock + private StoragePoolHostDao storagePoolHostDao; + @Mock private ResourceManager resourceMgr; @@ -1716,4 +1720,127 @@ public void testDiscoverObjectStoreInitializationFailure() { storageManagerImpl.discoverObjectStore(name, url, size, providerName, details); } + + private Pair mockHostAndPool(long hostId, long poolId) { + HostVO host = Mockito.mock(HostVO.class); + Mockito.lenient().doReturn(hostId).when(host).getId(); + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.lenient().doReturn(poolId).when(pool).getId(); + return new Pair<>(host, pool); + } + + @Test + public void canHostAccessOrPrepareStoragePoolTestNullArguments() { + Assert.assertFalse(storageManagerImpl.canHostAccessOrPrepareStoragePool(null, Mockito.mock(StoragePoolVO.class))); + Assert.assertFalse(storageManagerImpl.canHostAccessOrPrepareStoragePool(Mockito.mock(HostVO.class), null)); + } + + @Test + public void canHostAccessOrPrepareStoragePoolTestNoCommonStorageAccessGroups() { + Pair hostAndPool = mockHostAndPool(1L, 2L); + HostVO host = hostAndPool.first(); + StoragePoolVO pool = hostAndPool.second(); + Mockito.doReturn(false).when(storageManagerImpl).checkIfHostAndStoragePoolHasCommonStorageAccessGroups(host, pool); + + Assert.assertFalse(storageManagerImpl.canHostAccessOrPrepareStoragePool(host, pool)); + Mockito.verify(storageManagerImpl, Mockito.never()).canHostPrepareStoragePoolAccess(host, pool); + } + + @Test + public void canHostAccessOrPrepareStoragePoolTestHostAlreadyConnectedToPool() { + Pair hostAndPool = mockHostAndPool(1L, 2L); + HostVO host = hostAndPool.first(); + StoragePoolVO pool = hostAndPool.second(); + Mockito.doReturn(true).when(storageManagerImpl).checkIfHostAndStoragePoolHasCommonStorageAccessGroups(host, pool); + Mockito.doReturn(Mockito.mock(StoragePoolHostVO.class)).when(storagePoolHostDao).findByPoolHost(2L, 1L); + Mockito.doReturn(true).when(storageManagerImpl).canHostAccessStoragePool(host, pool); + + Assert.assertTrue(storageManagerImpl.canHostAccessOrPrepareStoragePool(host, pool)); + Mockito.verify(storageManagerImpl, Mockito.never()).canHostPrepareStoragePoolAccess(host, pool); + } + + @Test + public void canHostAccessOrPrepareStoragePoolTestNoPoolHostRefButAccessCanBePrepared() { + Pair hostAndPool = mockHostAndPool(1L, 2L); + HostVO host = hostAndPool.first(); + StoragePoolVO pool = hostAndPool.second(); + Mockito.doReturn(true).when(storageManagerImpl).checkIfHostAndStoragePoolHasCommonStorageAccessGroups(host, pool); + Mockito.doReturn(null).when(storagePoolHostDao).findByPoolHost(2L, 1L); + Mockito.doReturn(true).when(storageManagerImpl).canHostPrepareStoragePoolAccess(host, pool); + + Assert.assertTrue(storageManagerImpl.canHostAccessOrPrepareStoragePool(host, pool)); + } + + @Test + public void canHostAccessOrPrepareStoragePoolTestNeitherAccessNorPreparePossible() { + Pair hostAndPool = mockHostAndPool(1L, 2L); + HostVO host = hostAndPool.first(); + StoragePoolVO pool = hostAndPool.second(); + Mockito.doReturn(true).when(storageManagerImpl).checkIfHostAndStoragePoolHasCommonStorageAccessGroups(host, pool); + Mockito.doReturn(null).when(storagePoolHostDao).findByPoolHost(2L, 1L); + Mockito.doReturn(false).when(storageManagerImpl).canHostPrepareStoragePoolAccess(host, pool); + + Assert.assertFalse(storageManagerImpl.canHostAccessOrPrepareStoragePool(host, pool)); + } + + @Test + public void findUpAndEnabledHostWithAccessToStoragePoolsTestReturnsConnectedHost() { + List poolIds = Arrays.asList(1L, 2L); + HostVO connectedHost = Mockito.mock(HostVO.class); + Mockito.doReturn(new ArrayList<>(Arrays.asList(10L))).when(storagePoolHostDao).findHostsConnectedToPools(poolIds); + Mockito.doReturn(connectedHost).when(hostDao).findById(10L); + Mockito.doReturn(Mockito.mock(StoragePoolVO.class)).when(storagePoolDao).findById(Mockito.anyLong()); + Mockito.doReturn(true).when(storageManagerImpl).canHostAccessStoragePool(Mockito.eq(connectedHost), Mockito.any()); + + Assert.assertEquals(connectedHost, storageManagerImpl.findUpAndEnabledHostWithAccessToStoragePools(poolIds)); + Mockito.verify(resourceMgr, Mockito.never()).listAllUpAndEnabledHostsInOneZoneByType(Mockito.any(), Mockito.anyLong()); + } + + @Test + public void findUpAndEnabledHostWithAccessToStoragePoolsTestFallsBackToHostThatCanPrepareAccess() { + List poolIds = Arrays.asList(1L, 2L); + StoragePoolVO srcPool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO destPool = Mockito.mock(StoragePoolVO.class); + Mockito.doReturn(1L).when(srcPool).getDataCenterId(); + Mockito.doReturn(1L).when(destPool).getDataCenterId(); + Mockito.doReturn(srcPool).when(storagePoolDao).findById(1L); + Mockito.doReturn(destPool).when(storagePoolDao).findById(2L); + Mockito.doReturn(new ArrayList()).when(storagePoolHostDao).findHostsConnectedToPools(poolIds); + + HostVO preparableHost = Mockito.mock(HostVO.class); + Mockito.doReturn(new ArrayList<>(Arrays.asList(preparableHost))).when(resourceMgr) + .listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, 1L); + Mockito.doReturn(true).when(storageManagerImpl).canHostAccessOrPrepareStoragePool(Mockito.eq(preparableHost), Mockito.any()); + + Assert.assertEquals(preparableHost, storageManagerImpl.findUpAndEnabledHostWithAccessToStoragePools(poolIds)); + } + + @Test + public void findUpAndEnabledHostWithAccessToStoragePoolsTestNoHostCanPrepareAccess() { + List poolIds = Arrays.asList(1L, 2L); + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.doReturn(1L).when(pool).getDataCenterId(); + Mockito.doReturn(pool).when(storagePoolDao).findById(Mockito.anyLong()); + Mockito.doReturn(new ArrayList()).when(storagePoolHostDao).findHostsConnectedToPools(poolIds); + + HostVO host = Mockito.mock(HostVO.class); + Mockito.doReturn(new ArrayList<>(Arrays.asList(host))).when(resourceMgr) + .listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, 1L); + Mockito.doReturn(false).when(storageManagerImpl).canHostAccessOrPrepareStoragePool(Mockito.eq(host), Mockito.any()); + + Assert.assertNull(storageManagerImpl.findUpAndEnabledHostWithAccessToStoragePools(poolIds)); + } + + @Test + public void findUpAndEnabledHostAbleToPrepareAccessToStoragePoolsTestPoolsInDifferentZones() { + StoragePoolVO srcPool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO destPool = Mockito.mock(StoragePoolVO.class); + Mockito.doReturn(1L).when(srcPool).getDataCenterId(); + Mockito.doReturn(2L).when(destPool).getDataCenterId(); + Mockito.doReturn(srcPool).when(storagePoolDao).findById(1L); + Mockito.doReturn(destPool).when(storagePoolDao).findById(2L); + + Assert.assertNull(storageManagerImpl.findUpAndEnabledHostAbleToPrepareAccessToStoragePools(Arrays.asList(1L, 2L))); + Mockito.verify(resourceMgr, Mockito.never()).listAllUpAndEnabledHostsInOneZoneByType(Mockito.any(), Mockito.anyLong()); + } }