From ef60abdc03214cda6491bca8e9ac49d9c90da047 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 12 Sep 2025 15:30:29 +0530 Subject: [PATCH 1/9] Migrate volume improvements, to bypass secondary storage when copy volume between pools is allowed directly --- .../service/VolumeOrchestrationService.java | 5 +- .../subsystem/api/storage/ClusterScope.java | 6 +++ .../subsystem/api/storage/HostScope.java | 7 +++ .../subsystem/api/storage/ZoneScope.java | 6 +++ .../cloud/vm/VirtualMachineManagerImpl.java | 1 + .../orchestration/VolumeOrchestrator.java | 9 ++-- .../orchestration/VolumeOrchestratorTest.java | 2 +- .../com/cloud/storage/dao/VolumeDaoImpl.java | 1 + .../motion/AncientDataMotionStrategy.java | 52 +++++++++++++++++-- .../StorageSystemDataMotionStrategy.java | 3 +- .../vmsnapshot/DefaultVMSnapshotStrategy.java | 1 + .../datastore/PrimaryDataStoreImpl.java | 1 + .../storage/volume/VolumeDataFactoryImpl.java | 8 +++ .../motion/HypervStorageMotionStrategy.java | 1 + .../kvm/storage/KVMStorageProcessor.java | 7 ++- .../motion/VmwareStorageMotionStrategy.java | 1 + .../driver/AdaptiveDataStoreDriverImpl.java | 1 + .../CloudStackPrimaryDataStoreDriverImpl.java | 1 + .../LinstorPrimaryDataStoreDriverImpl.java | 1 + .../motion/StorPoolDataMotionStrategy.java | 1 + .../com/cloud/storage/StorageManagerImpl.java | 1 + .../cloud/storage/VolumeApiServiceImpl.java | 3 ++ .../command/ReconcileCommandServiceImpl.java | 2 + .../VolumeImportUnmanageManagerImpl.java | 2 +- .../vm/UnmanagedVMsManagerImpl.java | 8 +-- .../VolumeImportUnmanageManagerImplTest.java | 2 +- .../ReflectionToStringBuilderUtilsTest.java | 2 +- 27 files changed, 114 insertions(+), 21 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java index ccb5bba1c0a4..168822c21ebc 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java @@ -23,6 +23,7 @@ import java.util.Set; import com.cloud.exception.ResourceAllocationException; +import com.cloud.storage.Storage; import com.cloud.utils.Pair; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -182,10 +183,10 @@ List allocateTemplatedVolumes(Type type, String name, DiskOffering */ DiskProfile importVolume(Type type, String name, DiskOffering offering, Long sizeInBytes, Long minIops, Long maxIops, Long zoneId, HypervisorType hypervisorType, VirtualMachine vm, VirtualMachineTemplate template, - Account owner, Long deviceId, Long poolId, String path, String chainInfo); + Account owner, Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo); DiskProfile updateImportedVolume(Type type, DiskOffering offering, VirtualMachine vm, VirtualMachineTemplate template, - Long deviceId, Long poolId, String path, String chainInfo, DiskProfile diskProfile); + Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile); /** * Unmanage VM volumes diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ClusterScope.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ClusterScope.java index b0ed7d7d52b5..68c7ed40d8f4 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ClusterScope.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ClusterScope.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.engine.subsystem.api.storage; import com.cloud.storage.ScopeType; +import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; public class ClusterScope extends AbstractScope { private ScopeType type = ScopeType.CLUSTER; @@ -51,4 +52,9 @@ public Long getZoneId() { return this.zoneId; } + @Override + public String toString() { + return String.format("ClusterScope %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields( + this, "zoneId", "clusterId", "podId")); + } } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/HostScope.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/HostScope.java index 6e0bc618bfe2..7eddce2e548b 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/HostScope.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/HostScope.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.engine.subsystem.api.storage; import com.cloud.storage.ScopeType; +import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; public class HostScope extends AbstractScope { private Long hostId; @@ -49,4 +50,10 @@ public Long getClusterId() { public Long getZoneId() { return zoneId; } + + @Override + public String toString() { + return String.format("HostScope %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields( + this, "zoneId", "clusterId", "hostId")); + } } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ZoneScope.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ZoneScope.java index a0d75b5cc7c9..fa704d05b1ae 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ZoneScope.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ZoneScope.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.engine.subsystem.api.storage; import com.cloud.storage.ScopeType; +import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; public class ZoneScope extends AbstractScope { private ScopeType type = ScopeType.ZONE; @@ -39,4 +40,9 @@ public Long getScopeId() { return this.zoneId; } + @Override + public String toString() { + return String.format("ZoneScope %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields( + this, "zoneId")); + } } 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 3a6e1b622774..3cbc597373bb 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2792,6 +2792,7 @@ private void markVolumesInPool(VMInstanceVO vm, Answer[] hypervisorMigrationResu } volume.setPath(result.getPath()); volume.setPoolId(pool.getId()); + volume.setPoolType(pool.getPoolType()); if (result.getChainInfo() != null) { volume.setChainInfo(result.getChainInfo()); } diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 7af9b6b84923..2b759235ac81 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -1423,7 +1423,7 @@ public Volume migrateVolume(Volume volume, StoragePool destPool) throws StorageU String volumeToString = getVolumeIdentificationInfos(volume); VolumeInfo vol = volFactory.getVolume(volume.getId()); - if (vol == null){ + if (vol == null) { throw new CloudRuntimeException(String.format("Volume migration failed because volume [%s] is null.", volumeToString)); } if (destPool == null) { @@ -2308,6 +2308,7 @@ public void updateVolumeDiskChain(long volumeId, String path, String chainInfo, StoragePoolVO pool = _storagePoolDao.findByUuid(updatedDataStoreUUID); if (pool != null) { vol.setPoolId(pool.getId()); + vol.setPoolType(pool.getPoolType()); } } _volsDao.update(volumeId, vol); @@ -2317,7 +2318,7 @@ public void updateVolumeDiskChain(long volumeId, String path, String chainInfo, @Override public DiskProfile importVolume(Type type, String name, DiskOffering offering, Long sizeInBytes, Long minIops, Long maxIops, Long zoneId, HypervisorType hypervisorType, VirtualMachine vm, VirtualMachineTemplate template, Account owner, - Long deviceId, Long poolId, String path, String chainInfo) { + Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo) { if (sizeInBytes == null) { sizeInBytes = offering.getDiskSize(); } @@ -2358,6 +2359,7 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L vol.setFormat(getSupportedImageFormatForCluster(hypervisorType)); vol.setPoolId(poolId); + vol.setPoolType(poolType); vol.setPath(path); vol.setChainInfo(chainInfo); vol.setState(Volume.State.Ready); @@ -2367,7 +2369,7 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L @Override public DiskProfile updateImportedVolume(Type type, DiskOffering offering, VirtualMachine vm, VirtualMachineTemplate template, - Long deviceId, Long poolId, String path, String chainInfo, DiskProfile diskProfile) { + Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile) { VolumeVO vol = _volsDao.findById(diskProfile.getVolumeId()); if (vm != null) { @@ -2401,6 +2403,7 @@ public DiskProfile updateImportedVolume(Type type, DiskOffering offering, Virtua vol.setFormat(getSupportedImageFormatForCluster(vm.getHypervisorType())); vol.setPoolId(poolId); + vol.setPoolType(poolType); vol.setPath(path); vol.setChainInfo(chainInfo); vol.setSize(diskProfile.getSize()); diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java index 817ff02ef745..b4a26c17e2e5 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java @@ -241,7 +241,7 @@ public void testImportVolume() { volumeOrchestrator.importVolume(volumeType, name, diskOffering, sizeInBytes, null, null, zoneId, hypervisorType, null, null, owner, - deviceId, poolId, path, chainInfo); + deviceId, poolId, Storage.StoragePoolType.NetworkFilesystem, path, chainInfo); VolumeVO volume = volumeVOMockedConstructionConstruction.constructed().get(0); Mockito.verify(volume, Mockito.never()).setInstanceId(Mockito.anyLong()); 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 f8b6bb3ed680..6814636bc66c 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 @@ -822,6 +822,7 @@ public void updateAndRemoveVolume(VolumeVO volume) { if (volume.getState() != Volume.State.Destroy) { volume.setState(Volume.State.Destroy); volume.setPoolId(null); + volume.setPoolType(null); volume.setInstanceId(null); update(volume.getId(), volume); remove(volume.getId()); diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index 0b0065361d07..9ad1101c1d2c 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -18,9 +18,11 @@ */ package org.apache.cloudstack.storage.motion; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import javax.inject.Inject; @@ -67,6 +69,7 @@ import com.cloud.host.Host; import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.DataStoreRole; +import com.cloud.storage.ScopeType; import com.cloud.storage.Snapshot.Type; import com.cloud.storage.SnapshotVO; import com.cloud.storage.StorageManager; @@ -85,6 +88,10 @@ public class AncientDataMotionStrategy implements DataMotionStrategy { protected Logger logger = LogManager.getLogger(getClass()); private static final String NO_REMOTE_ENDPOINT_SSVM = "No remote endpoint to send command, check if host or ssvm is down?"; private static final String NO_REMOTE_ENDPOINT_WITH_ENCRYPTION = "No remote endpoint to send command, unable to find a valid endpoint. Requires encryption support: %s"; + private static final List SUPPORTED_POOL_TYPES_TO_BYPASS_SECONDARY_STORE = Arrays.asList( + StoragePoolType.NetworkFilesystem, + StoragePoolType.Filesystem + ); @Inject EndPointSelector selector; @@ -240,7 +247,6 @@ protected DataTO addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(DataTO return dataTO; } - protected Answer copyObject(DataObject srcData, DataObject destData) { return copyObject(srcData, destData, null); } @@ -352,10 +358,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) Scope destScope = getZoneScope(destData.getDataStore().getScope()); DataStore cacheStore = cacheMgr.getCacheStorage(destScope); - boolean bypassSecondaryStorage = false; - if (srcData instanceof VolumeInfo && ((VolumeInfo)srcData).isDirectDownload()) { - bypassSecondaryStorage = true; - } + boolean bypassSecondaryStorage = canBypassSecondaryStorage(srcData, destData); boolean encryptionRequired = anyVolumeRequiresEncryption(srcData, destData); if (cacheStore == null) { @@ -448,7 +451,45 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) } return answer; } + } + + private boolean canBypassSecondaryStorage(DataObject srcData, DataObject destData) { + if (srcData instanceof VolumeInfo) { + if (((VolumeInfo)srcData).isDirectDownload()) { + return true; + } + + if (destData instanceof VolumeInfo) { + Scope srcDataStoreScope = srcData.getDataStore().getScope(); + Scope destDataStoreScope = destData.getDataStore().getScope(); + logger.info("srcDataStoreScope: {}, destDataStoreScope: {}", srcDataStoreScope, destDataStoreScope); + logger.info("srcData - pool type: {}, scope: {}; destData - pool type: {}, scope: {}", + ((VolumeInfo)srcData).getStoragePoolType(), srcDataStoreScope != null ? srcDataStoreScope.getScopeType() : null, ((VolumeInfo)destData).getStoragePoolType(), destDataStoreScope != null ? destDataStoreScope.getScopeType() : null); + + if (srcDataStoreScope != null && destDataStoreScope != null && + SUPPORTED_POOL_TYPES_TO_BYPASS_SECONDARY_STORE.contains(((VolumeInfo)srcData).getStoragePoolType()) && + SUPPORTED_POOL_TYPES_TO_BYPASS_SECONDARY_STORE.contains(((VolumeInfo)destData).getStoragePoolType())) { + + if (srcDataStoreScope.isSameScope(destDataStoreScope)) { + return true; + } + + if (srcDataStoreScope.getScopeType() == ScopeType.CLUSTER && + destDataStoreScope.getScopeType() == ScopeType.HOST && + (Objects.equals(((ClusterScope) srcDataStoreScope).getScopeId(), ((HostScope) destDataStoreScope).getClusterId()))) { + return true; + } + + if (srcDataStoreScope.getScopeType() == ScopeType.HOST && + destDataStoreScope.getScopeType() == ScopeType.CLUSTER && + (Objects.equals(((HostScope) srcDataStoreScope).getClusterId(), ((ClusterScope) destDataStoreScope).getScopeId()))) { + return true; + } + } + } + } + return false; } protected Answer migrateVolumeToPool(DataObject srcData, DataObject destData) { @@ -492,6 +533,7 @@ protected Answer migrateVolumeToPool(DataObject srcData, DataObject destData) { } volumeVo.setPodId(destPool.getPodId()); volumeVo.setPoolId(destPool.getId()); + volumeVo.setPoolType(destPool.getPoolType()); volumeVo.setLastPoolId(oldPoolId); // For SMB, pool credentials are also stored in the uri query string. We trim the query string // part here to make sure the credentials do not get stored in the db unencrypted. 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 0a211ab1934d..4f3a6b3b3608 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 @@ -796,6 +796,7 @@ private void handleSuccessfulVolumeMigration(VolumeInfo srcVolumeInfo, StoragePo volumeVO.setPodId(destPool.getPodId()); volumeVO.setPoolId(destPool.getId()); + volumeVO.setPoolType(destPool.getPoolType()); volumeVO.setLastPoolId(srcVolumeInfo.getPoolId()); _volumeDao.update(srcVolumeInfo.getId(), volumeVO); @@ -2398,13 +2399,13 @@ private VolumeVO duplicateVolumeOnAnotherStorage(Volume volume, StoragePoolVO st Long lastPoolId = volume.getPoolId(); VolumeVO newVol = new VolumeVO(volume); - newVol.setInstanceId(null); newVol.setChainInfo(null); newVol.setPath(null); newVol.setFolder(null); newVol.setPodId(storagePoolVO.getPodId()); newVol.setPoolId(storagePoolVO.getId()); + newVol.setPoolType(storagePoolVO.getPoolType()); newVol.setLastPoolId(lastPoolId); newVol.setLastId(volume.getId()); diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java index 2d9b1e67e097..9ccb84c3794a 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java @@ -345,6 +345,7 @@ protected void updateVolumePath(List volumeTOs) { StoragePool pool = primaryDataStoreDao.findPoolByUUID(volume.getDataStoreUuid()); if (pool != null && pool.getId() != volumeVO.getPoolId()) { volumeVO.setPoolId(pool.getId()); + volumeVO.setPoolType(pool.getPoolType()); } } if (StringUtils.isNotEmpty(volume.getPath())) { 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..d0184359c8b6 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 @@ -334,6 +334,7 @@ public DataObject create(DataObject obj, boolean createEntryInTempSpoolRef, Stri VolumeVO vol = volumeDao.findById(obj.getId()); if (vol != null) { vol.setPoolId(getId()); + vol.setPoolType(getPoolType()); volumeDao.update(vol.getId(), vol); } } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java index 53fa21f3a794..5c2a774f8a2b 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java @@ -30,6 +30,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao; import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO; @@ -46,6 +48,8 @@ public class VolumeDataFactoryImpl implements VolumeDataFactory { DataStoreManager storeMgr; @Inject VMTemplateDao templateDao; + @Inject + PrimaryDataStoreDao storagePoolDao; @Override public VolumeInfo getVolume(long volumeId, DataStore store) { @@ -92,6 +96,10 @@ public VolumeInfo getVolume(long volumeId) { vol = VolumeObject.getVolumeObject(store, volumeVO); } else { DataStore store = storeMgr.getDataStore(volumeVO.getPoolId(), DataStoreRole.Primary); + StoragePoolVO pool = storagePoolDao.findById(volumeVO.getPoolId()); + if (pool != null) { + volumeVO.setPoolType(pool.getPoolType()); + } vol = VolumeObject.getVolumeObject(store, volumeVO); } if (vol.getTemplateId() != null) { diff --git a/plugins/hypervisors/hyperv/src/main/java/org/apache/cloudstack/storage/motion/HypervStorageMotionStrategy.java b/plugins/hypervisors/hyperv/src/main/java/org/apache/cloudstack/storage/motion/HypervStorageMotionStrategy.java index 0e189d050006..55944a082427 100644 --- a/plugins/hypervisors/hyperv/src/main/java/org/apache/cloudstack/storage/motion/HypervStorageMotionStrategy.java +++ b/plugins/hypervisors/hyperv/src/main/java/org/apache/cloudstack/storage/motion/HypervStorageMotionStrategy.java @@ -155,6 +155,7 @@ private void updateVolumePathsAfterMigration(Map volumeTo volumeVO.setPath(volumeTo.getPath()); volumeVO.setPodId(pool.getPodId()); volumeVO.setPoolId(pool.getId()); + volumeVO.setPoolType(pool.getPoolType()); volumeVO.setLastPoolId(oldPoolId); // For SMB, pool credentials are also stored in the uri query string. We trim the query string // part here to make sure the credentials do not get stored in the db unencrypted. diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 6aa5b0c0931b..6c06d14db80f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -3037,7 +3037,7 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { destVolumeName = volumeName + "." + destFormat.getFileExtension(); // Update path in the command for reconciliation - if (destData.getPath() == null) { + if (StringUtils.isBlank(destVolumePath)) { ((VolumeObjectTO) destData).setPath(destVolumeName); } } @@ -3071,7 +3071,10 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { } final VolumeObjectTO newVol = new VolumeObjectTO(); - String path = destPrimaryStore.isManaged() ? destVolumeName : destVolumePath + File.separator + destVolumeName; + String path = destVolumeName; + if (!destPrimaryStore.isManaged() && StringUtils.isNotBlank(destVolumePath)) { + path = destVolumePath + File.separator + destVolumeName; + } newVol.setPath(path); newVol.setFormat(destFormat); newVol.setEncryptFormat(destVol.getEncryptFormat()); diff --git a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java index b0cacf60a176..d2d319ed9d03 100644 --- a/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java +++ b/plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java @@ -433,6 +433,7 @@ private void updateVolumesAfterMigration(Map volumeToPool volumeVO.setFolder(pool.getPath()); volumeVO.setPodId(pool.getPodId()); volumeVO.setPoolId(pool.getId()); + volumeVO.setPoolType(pool.getPoolType()); volDao.update(volume.getId(), volumeVO); updated = true; break; diff --git a/plugins/storage/volume/adaptive/src/main/java/org/apache/cloudstack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java b/plugins/storage/volume/adaptive/src/main/java/org/apache/cloudstack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java index 40d995263940..2ccd2bab6cdd 100644 --- a/plugins/storage/volume/adaptive/src/main/java/org/apache/cloudstack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java +++ b/plugins/storage/volume/adaptive/src/main/java/org/apache/cloudstack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java @@ -857,6 +857,7 @@ void persistVolumeData(StoragePoolVO storagePool, Map details, D volumeVO.setPath(finalPath); volumeVO.setFormat(ImageFormat.RAW); volumeVO.setPoolId(storagePool.getId()); + volumeVO.setPoolType(storagePool.getPoolType()); volumeVO.setExternalUuid(managedVolume.getExternalUuid()); volumeVO.setDisplay(true); volumeVO.setDisplayVolume(true); diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java index d2f0076f95f4..e407bc6c2f31 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java @@ -505,6 +505,7 @@ private void updateVolumePathDetails(VolumeObject vol, ResizeVolumeAnswer answer StoragePoolVO storagePoolVO = primaryStoreDao.findByUuid(datastoreUUID); if (storagePoolVO != null) { volumeVO.setPoolId(storagePoolVO.getId()); + volumeVO.setPoolType(storagePoolVO.getPoolType()); } else { logger.warn("Unable to find datastore {} while updating the new datastore of the volume {}", datastoreUUID, vol); } diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java index 306e92599366..d3b797e319ff 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java @@ -868,6 +868,7 @@ public void createAsync(DataStore dataStore, DataObject vol, AsyncCompletionCall devPath = createVolume(volumeInfo, storagePool); volume.setFolder("/dev/"); volume.setPoolId(storagePool.getId()); + volume.setPoolType(storagePool.getPoolType()); volume.setUuid(vol.getUuid()); volume.setPath(vol.getUuid()); diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/motion/StorPoolDataMotionStrategy.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/motion/StorPoolDataMotionStrategy.java index f260c5669869..5789ac50871b 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/motion/StorPoolDataMotionStrategy.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/motion/StorPoolDataMotionStrategy.java @@ -425,6 +425,7 @@ private VolumeVO duplicateVolumeOnAnotherStorage(Volume volume, StoragePoolVO st newVol.setFolder(null); newVol.setPodId(storagePoolVO.getPodId()); newVol.setPoolId(storagePoolVO.getId()); + newVol.setPoolType(storagePoolVO.getPoolType()); newVol.setLastPoolId(lastPoolId); return _volumeDao.persist(newVol); diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index cbeec1a60b36..037cbe170c84 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -3049,6 +3049,7 @@ private void handleRemoveChildStoragePoolFromDatastoreCluster(Set childD StoragePoolVO storagePoolVO = _storagePoolDao.findByUuid(datastoreName); if (storagePoolVO != null) { volumeVO.setPoolId(storagePoolVO.getId()); + volumeVO.setPoolType(storagePoolVO.getPoolType()); } else { logger.warn("Unable to find datastore {} while updating the new datastore of the volume {}", datastoreName, volumeVO); } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index f4536e8549fd..3f27ba0b718a 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2937,8 +2937,10 @@ public Volume updateVolume(long volumeId, String path, String state, Long storag List childDatastores = _storagePoolDao.listChildStoragePoolsInDatastoreCluster(storageId); Collections.shuffle(childDatastores); volume.setPoolId(childDatastores.get(0).getId()); + volume.setPoolType(childDatastores.get(0).getPoolType()); } else { volume.setPoolId(pool.getId()); + volume.setPoolType(pool.getPoolType()); } } @@ -3225,6 +3227,7 @@ private Volume orchestrateDetachVolumeFromVM(long vmId, long volumeId) { if (storagePoolVO != null) { VolumeVO volumeVO = _volsDao.findById(volumeId); volumeVO.setPoolId(storagePoolVO.getId()); + volumeVO.setPoolType(storagePoolVO.getPoolType()); _volsDao.update(volumeVO.getId(), volumeVO); } else { logger.warn("Unable to find datastore {} while updating the new datastore of the volume {}", datastoreName, volume); diff --git a/server/src/main/java/org/apache/cloudstack/command/ReconcileCommandServiceImpl.java b/server/src/main/java/org/apache/cloudstack/command/ReconcileCommandServiceImpl.java index d0dcc1f86de2..5edf05d1a5c0 100644 --- a/server/src/main/java/org/apache/cloudstack/command/ReconcileCommandServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/command/ReconcileCommandServiceImpl.java @@ -1064,6 +1064,7 @@ private void updateVolumeAndDestroyOldVolume(VolumeVO sourceVolume, DataTO srcDa logger.debug(String.format("Updating volume %s to %s state", sourceVolume, Volume.State.Ready)); sourceVolume.setState(Volume.State.Ready); sourceVolume.setPoolId(srcDataStore.getId()); // restore pool_id and update path + sourceVolume.setPoolType(srcDataStore.getPoolType()); sourceVolume.setPath(srcData.getPath()); sourceVolume.set_iScsiName(srcData.getPath()); sourceVolume.setUpdated(new Date()); @@ -1075,6 +1076,7 @@ private void updateVolumeAndDestroyOldVolume(VolumeVO sourceVolume, DataTO srcDa VolumeVO newVolume = (VolumeVO) newVol; newVolume.setInstanceId(null); newVolume.setPoolId(destDataStore.getId()); + newVolume.setPoolType(destDataStore.getPoolType()); newVolume.setState(Volume.State.Creating); newVolume.setPath(destData.getPath()); newVolume.set_iScsiName(destData.getPath()); diff --git a/server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java b/server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java index 9e1fc46e02eb..383644f9aa20 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java @@ -452,7 +452,7 @@ private VolumeVO importVolumeInternal(VolumeOnStorageTO volume, DiskOfferingVO d Account owner, StoragePoolVO pool, String volumeName) { DiskProfile diskProfile = volumeManager.importVolume(Volume.Type.DATADISK, volumeName, diskOffering, volume.getVirtualSize(), null, null, pool.getDataCenterId(), volume.getHypervisorType(), null, null, - owner, null, pool.getId(), volume.getPath(), null); + owner, null, pool.getId(), pool.getPoolType(), volume.getPath(), null); return volumeDao.findById(diskProfile.getVolumeId()); } diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index ad3d2a0f0ec2..f204f10515b4 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -821,7 +821,7 @@ private Pair importExternalDisk(UnmanagedInstanceTO.Di } diskProfile.setSize(copyRemoteVolumeAnswer.getSize()); DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId, - storagePool.getId(), copyRemoteVolumeAnswer.getFilename(), chainInfo, diskProfile); + storagePool.getId(), storagePool.getPoolType(), copyRemoteVolumeAnswer.getFilename(), chainInfo, diskProfile); return new Pair<>(profile, storagePool); } @@ -837,7 +837,7 @@ private Pair importKVMLocalDisk(VirtualMachine vm, Dis StoragePool storagePool = storagePools.get(0); DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId, - storagePool.getId(), diskPath, null, diskProfile); + storagePool.getId(), storagePool.getPoolType(), diskPath, null, diskProfile); return new Pair<>(profile, storagePool); } @@ -848,7 +848,7 @@ private Pair importKVMSharedDisk(VirtualMachine vm, Di StoragePool storagePool = primaryDataStoreDao.findById(poolId); DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId, - poolId, diskPath, null, diskProfile); + poolId, storagePool.getPoolType(), diskPath, null, diskProfile); return new Pair<>(profile, storagePool); } @@ -867,7 +867,7 @@ private Pair importDisk(UnmanagedInstanceTO.Disk disk, } StoragePool storagePool = getStoragePool(disk, zone, cluster, diskOffering); DiskProfile profile = volumeManager.importVolume(type, name, diskOffering, diskSize, - minIops, maxIops, vm.getDataCenterId(), vm.getHypervisorType(), vm, template, owner, deviceId, storagePool.getId(), path, chainInfo); + minIops, maxIops, vm.getDataCenterId(), vm.getHypervisorType(), vm, template, owner, deviceId, storagePool.getId(), storagePool.getPoolType(), path, chainInfo); return new Pair(profile, storagePool); } diff --git a/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java index 419acc0ca0b6..9ee34d32a177 100644 --- a/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java @@ -277,7 +277,7 @@ public void testImportVolumeAllGood() throws ResourceAllocationException { doNothing().when(volumeApiService).validateCustomDiskOfferingSizeRange(anyLong()); doReturn(true).when(volumeApiService).doesStoragePoolSupportDiskOffering(any(), any()); doReturn(diskProfile).when(volumeManager).importVolume(any(), anyString(), any(), eq(virtualSize), isNull(), isNull(), anyLong(), - any(), isNull(), isNull(), any(), isNull(), anyLong(), anyString(), isNull()); + any(), isNull(), isNull(), any(), isNull(), anyLong(), any(), anyString(), isNull()); when(diskProfile.getVolumeId()).thenReturn(volumeId); when(volumeDao.findById(volumeId)).thenReturn(volumeVO); diff --git a/utils/src/test/java/org/apache/cloudstack/utils/reflectiontostringbuilderutils/ReflectionToStringBuilderUtilsTest.java b/utils/src/test/java/org/apache/cloudstack/utils/reflectiontostringbuilderutils/ReflectionToStringBuilderUtilsTest.java index afd033c7b04b..9168ec0d2fad 100644 --- a/utils/src/test/java/org/apache/cloudstack/utils/reflectiontostringbuilderutils/ReflectionToStringBuilderUtilsTest.java +++ b/utils/src/test/java/org/apache/cloudstack/utils/reflectiontostringbuilderutils/ReflectionToStringBuilderUtilsTest.java @@ -53,7 +53,7 @@ public class ReflectionToStringBuilderUtilsTest extends TestCase { private static final String DEFAULT_MULTIPLE_VALUES_SEPARATOR = ","; @Before - public void setup(){ + public void setup() { classToReflect = String.class; classToReflectFieldsNamesList = ReflectionUtils.getAllFields(classToReflect).stream().map(objectField -> objectField.getName()).collect(Collectors.toList()); classToReflectRemovedField = classToReflectFieldsNamesList.remove(0); From faf94a651e31f338c68336bf39145397e0a5189e Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 12 Sep 2025 18:06:07 +0530 Subject: [PATCH 2/9] Bypass secondary storage for copy volume between zone-wide pools and - local storage on host in the same zone - cluser-wide pools in the same zone --- .../subsystem/api/storage/HostScope.java | 3 +- .../motion/AncientDataMotionStrategy.java | 78 +++++++++++++------ 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/HostScope.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/HostScope.java index 7eddce2e548b..7e6ffe7d4f74 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/HostScope.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/HostScope.java @@ -22,6 +22,7 @@ import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; public class HostScope extends AbstractScope { + private ScopeType type = ScopeType.HOST; private Long hostId; private Long clusterId; private Long zoneId; @@ -35,7 +36,7 @@ public HostScope(Long hostId, Long clusterId, Long zoneId) { @Override public ScopeType getScopeType() { - return ScopeType.HOST; + return this.type; } @Override diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index 9ad1101c1d2c..0e114b52aae6 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -363,6 +363,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) if (cacheStore == null) { if (bypassSecondaryStorage) { + logger.debug("Secondary storage is bypassed, copy volume between pools directly"); CopyCommand cmd = new CopyCommand(srcData.getTO(), destData.getTO(), _copyvolumewait, VirtualMachineManager.ExecuteInSequence.value()); EndPoint ep = selector.select(srcData, destData, encryptionRequired); Answer answer = null; @@ -391,8 +392,8 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) answer = copyObject(srcData, objOnImageStore); if (answer == null || !answer.getResult()) { - if (answer != null) { - if (logger.isDebugEnabled()) logger.debug("copy to image store failed: " + answer.getDetails()); + if (answer != null && logger.isDebugEnabled()) { + logger.debug("copy to image store failed: {}", answer.getDetails()); } objOnImageStore.processEvent(Event.OperationFailed); imageStore.delete(objOnImageStore); @@ -414,8 +415,8 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) } if (answer == null || !answer.getResult()) { - if (answer != null) { - if (logger.isDebugEnabled()) logger.debug("copy to primary store failed: " + answer.getDetails()); + if (answer != null && logger.isDebugEnabled()) { + logger.debug("copy to primary store failed: {}", answer.getDetails()); } objOnImageStore.processEvent(Event.OperationFailed); imageStore.delete(objOnImageStore); @@ -426,7 +427,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) objOnImageStore.processEvent(Event.OperationFailed); imageStore.delete(objOnImageStore); } - logger.error("Failed to perform operation: "+ e.getLocalizedMessage()); + logger.error("Failed to perform operation: {}", e.getLocalizedMessage()); throw e; } @@ -462,30 +463,63 @@ private boolean canBypassSecondaryStorage(DataObject srcData, DataObject destDat if (destData instanceof VolumeInfo) { Scope srcDataStoreScope = srcData.getDataStore().getScope(); Scope destDataStoreScope = destData.getDataStore().getScope(); - logger.info("srcDataStoreScope: {}, destDataStoreScope: {}", srcDataStoreScope, destDataStoreScope); - logger.info("srcData - pool type: {}, scope: {}; destData - pool type: {}, scope: {}", - ((VolumeInfo)srcData).getStoragePoolType(), srcDataStoreScope != null ? srcDataStoreScope.getScopeType() : null, ((VolumeInfo)destData).getStoragePoolType(), destDataStoreScope != null ? destDataStoreScope.getScopeType() : null); + logger.info("srcDataStoreScope: {}, srcData pool type: {}; destDataStoreScope: {}, destData pool type: {}", + srcDataStoreScope, ((VolumeInfo)srcData).getStoragePoolType(), destDataStoreScope, ((VolumeInfo)destData).getStoragePoolType()); if (srcDataStoreScope != null && destDataStoreScope != null && SUPPORTED_POOL_TYPES_TO_BYPASS_SECONDARY_STORE.contains(((VolumeInfo)srcData).getStoragePoolType()) && SUPPORTED_POOL_TYPES_TO_BYPASS_SECONDARY_STORE.contains(((VolumeInfo)destData).getStoragePoolType())) { - if (srcDataStoreScope.isSameScope(destDataStoreScope)) { - return true; - } + return canDirectlyCopyBetweenDataStoreScopes(srcDataStoreScope, destDataStoreScope); + } + } + } - if (srcDataStoreScope.getScopeType() == ScopeType.CLUSTER && - destDataStoreScope.getScopeType() == ScopeType.HOST && - (Objects.equals(((ClusterScope) srcDataStoreScope).getScopeId(), ((HostScope) destDataStoreScope).getClusterId()))) { - return true; - } + return false; + } - if (srcDataStoreScope.getScopeType() == ScopeType.HOST && - destDataStoreScope.getScopeType() == ScopeType.CLUSTER && - (Objects.equals(((HostScope) srcDataStoreScope).getClusterId(), ((ClusterScope) destDataStoreScope).getScopeId()))) { - return true; - } - } + private boolean canDirectlyCopyBetweenDataStoreScopes(Scope srcDataStoreScope, Scope destDataStoreScope) { + if (srcDataStoreScope == null || destDataStoreScope == null) { + return false; + } + + if (srcDataStoreScope.isSameScope(destDataStoreScope)) { + return true; + } + + if (srcDataStoreScope.getScopeType() == ScopeType.HOST) { + if (destDataStoreScope.getScopeType() == ScopeType.CLUSTER && + (Objects.equals(((HostScope) srcDataStoreScope).getClusterId(), ((ClusterScope) destDataStoreScope).getScopeId()))) { + return true; + } + if (destDataStoreScope.getScopeType() == ScopeType.ZONE && + (Objects.equals(((HostScope) srcDataStoreScope).getZoneId(), ((ZoneScope) destDataStoreScope).getScopeId()))) { + return true; + } + } + + if (destDataStoreScope.getScopeType() == ScopeType.HOST) { + if (srcDataStoreScope.getScopeType() == ScopeType.CLUSTER && + (Objects.equals(((ClusterScope) srcDataStoreScope).getScopeId(), ((HostScope) destDataStoreScope).getClusterId()))) { + return true; + } + if (srcDataStoreScope.getScopeType() == ScopeType.ZONE && + (Objects.equals(((ZoneScope) srcDataStoreScope).getScopeId(), ((HostScope) destDataStoreScope).getZoneId()))) { + return true; + } + } + + if (srcDataStoreScope.getScopeType() == ScopeType.CLUSTER) { + if (destDataStoreScope.getScopeType() == ScopeType.ZONE && + (Objects.equals(((ClusterScope) srcDataStoreScope).getZoneId(), ((ZoneScope) destDataStoreScope).getScopeId()))) { + return true; + } + } + + if (destDataStoreScope.getScopeType() == ScopeType.CLUSTER) { + if (srcDataStoreScope.getScopeType() == ScopeType.ZONE && + (Objects.equals(((ZoneScope) srcDataStoreScope).getScopeId(), ((ClusterScope) destDataStoreScope).getZoneId()))) { + return true; } } From 72617d16213371dfd026c69677b62287b9319830 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 16 Sep 2025 13:42:40 +0530 Subject: [PATCH 3/9] Bypass secondary storage for volumes on ceph/rdb pool when the scope permits --- .../cloudstack/storage/motion/AncientDataMotionStrategy.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index 0e114b52aae6..c494ca315944 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -90,7 +90,8 @@ public class AncientDataMotionStrategy implements DataMotionStrategy { private static final String NO_REMOTE_ENDPOINT_WITH_ENCRYPTION = "No remote endpoint to send command, unable to find a valid endpoint. Requires encryption support: %s"; private static final List SUPPORTED_POOL_TYPES_TO_BYPASS_SECONDARY_STORE = Arrays.asList( StoragePoolType.NetworkFilesystem, - StoragePoolType.Filesystem + StoragePoolType.Filesystem, + StoragePoolType.RBD ); @Inject From 0eeed5571a9b51214a879a4521c7f0496ec29ad1 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 16 Sep 2025 18:09:28 +0530 Subject: [PATCH 4/9] Fix dest disk format while migrating volume from ceph/rbd to nfs, and some code improvements --- .../kvm/storage/KVMStorageProcessor.java | 48 ++++++++++++------- .../kvm/storage/LibvirtStorageAdaptor.java | 4 +- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 6c06d14db80f..f8c5903b66f6 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -364,16 +364,7 @@ public Answer copyTemplateToPrimaryStorage(final CopyCommand cmd) { final TemplateObjectTO newTemplate = new TemplateObjectTO(); newTemplate.setPath(primaryVol.getName()); newTemplate.setSize(primaryVol.getSize()); - - if(List.of( - StoragePoolType.RBD, - StoragePoolType.PowerFlex, - StoragePoolType.Linstor, - StoragePoolType.FiberChannel).contains(primaryPool.getType())) { - newTemplate.setFormat(ImageFormat.RAW); - } else { - newTemplate.setFormat(ImageFormat.QCOW2); - } + newTemplate.setFormat(getFormat(primaryPool.getType())); data = newTemplate; } else if (destData.getObjectType() == DataObjectType.VOLUME) { final VolumeObjectTO volumeObjectTO = new VolumeObjectTO(); @@ -2990,7 +2981,7 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { final VolumeObjectTO srcVol = (VolumeObjectTO)srcData; final VolumeObjectTO destVol = (VolumeObjectTO)destData; final ImageFormat srcFormat = srcVol.getFormat(); - final ImageFormat destFormat = destVol.getFormat(); + ImageFormat destFormat = destVol.getFormat(); final DataStoreTO srcStore = srcData.getDataStore(); final DataStoreTO destStore = destData.getDataStore(); final PrimaryDataStoreTO srcPrimaryStore = (PrimaryDataStoreTO)srcStore; @@ -3025,14 +3016,17 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { volume.setFormat(PhysicalDiskFormat.valueOf(srcFormat.toString())); volume.setDispName(srcVol.getName()); volume.setVmName(srcVol.getVmName()); - - String destVolumeName = null; + KVMPhysicalDisk newVolume; + String destVolumeName; + destPool = storagePoolMgr.getStoragePool(destPrimaryStore.getPoolType(), destPrimaryStore.getUuid()); if (destPrimaryStore.isManaged()) { if (!storagePoolMgr.connectPhysicalDisk(destPrimaryStore.getPoolType(), destPrimaryStore.getUuid(), destVolumePath, destPrimaryStore.getDetails())) { logger.warn("Failed to connect dest volume {}, in storage pool {}", destVol, destPrimaryStore); } destVolumeName = derivePath(destPrimaryStore, destData, destPrimaryStore.getDetails()); } else { + PhysicalDiskFormat destPoolDefaultFormat = destPool.getDefaultFormat(); + destFormat = getFormat(destPoolDefaultFormat); final String volumeName = UUID.randomUUID().toString(); destVolumeName = volumeName + "." + destFormat.getFileExtension(); @@ -3042,16 +3036,15 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { } } - destPool = storagePoolMgr.getStoragePool(destPrimaryStore.getPoolType(), destPrimaryStore.getUuid()); try { Volume.Type volumeType = srcVol.getVolumeType(); resource.createOrUpdateLogFileForCommand(cmd, Command.State.PROCESSING_IN_BACKEND); if (srcVol.getPassphrase() != null && (Volume.Type.ROOT.equals(volumeType) || Volume.Type.DATADISK.equals(volumeType))) { volume.setQemuEncryptFormat(QemuObject.EncryptFormat.LUKS); - storagePoolMgr.copyPhysicalDisk(volume, destVolumeName, destPool, cmd.getWaitInMillSeconds(), srcVol.getPassphrase(), destVol.getPassphrase(), srcVol.getProvisioningType()); + newVolume = storagePoolMgr.copyPhysicalDisk(volume, destVolumeName, destPool, cmd.getWaitInMillSeconds(), srcVol.getPassphrase(), destVol.getPassphrase(), srcVol.getProvisioningType()); } else { - storagePoolMgr.copyPhysicalDisk(volume, destVolumeName, destPool, cmd.getWaitInMillSeconds()); + newVolume = storagePoolMgr.copyPhysicalDisk(volume, destVolumeName, destPool, cmd.getWaitInMillSeconds()); } resource.createOrUpdateLogFileForCommand(cmd, Command.State.COMPLETED); } catch (Exception e) { // Any exceptions while copying the disk, should send failed answer with the error message @@ -3076,7 +3069,8 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { path = destVolumePath + File.separator + destVolumeName; } newVol.setPath(path); - newVol.setFormat(destFormat); + ImageFormat newVolumeFormat = getFormat(newVolume.getFormat()); + newVol.setFormat(newVolumeFormat); newVol.setEncryptFormat(destVol.getEncryptFormat()); return new CopyCmdAnswer(newVol); } catch (final CloudRuntimeException e) { @@ -3088,6 +3082,26 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { } } + private Storage.ImageFormat getFormat(PhysicalDiskFormat format) { + if (format == null) { + return null; + } + + return ImageFormat.valueOf(format.toString().toUpperCase()); + } + + private Storage.ImageFormat getFormat(StoragePoolType poolType) { + if(List.of( + StoragePoolType.RBD, + StoragePoolType.PowerFlex, + StoragePoolType.Linstor, + StoragePoolType.FiberChannel).contains(poolType)) { + return ImageFormat.RAW; + } else { + return ImageFormat.QCOW2; + } + } + /** * True if location exists */ diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index d5119ea55b71..02e5c08fa333 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1116,7 +1116,7 @@ private KVMPhysicalDisk createPhysicalDiskByQemuImg(String name, KVMStoragePool // make room for encryption header on raw format, use LUKS if (format == PhysicalDiskFormat.RAW) { - destFile.setSize(destFile.getSize() - (16<<20)); + destFile.setSize(destFile.getSize() - (16 << 20)); destFile.setFormat(PhysicalDiskFormat.LUKS); } @@ -1593,7 +1593,7 @@ to support snapshots(backuped) as qcow2 files. */ String sourcePath = disk.getPath(); KVMPhysicalDisk newDisk; - logger.debug("copyPhysicalDisk: disk size:" + toHumanReadableSize(disk.getSize()) + ", virtualsize:" + toHumanReadableSize(disk.getVirtualSize())+" format:"+disk.getFormat()); + logger.debug("copyPhysicalDisk: disk size:{}, virtualsize:{} format:{}", toHumanReadableSize(disk.getSize()), toHumanReadableSize(disk.getVirtualSize()), disk.getFormat()); if (destPool.getType() != StoragePoolType.RBD) { if (disk.getFormat() == PhysicalDiskFormat.TAR) { newDisk = destPool.createPhysicalDisk(name, PhysicalDiskFormat.DIR, Storage.ProvisioningType.THIN, disk.getVirtualSize(), null); From b13478248cf9baeb2e836b9e6d52de51eb8773c5 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 17 Sep 2025 11:44:27 +0530 Subject: [PATCH 5/9] unit tests --- .../motion/AncientDataMotionStrategyTest.java | 200 ++++++++++++++++++ 1 file changed, 200 insertions(+) diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java index 67e3ea844d58..e167cc0a9653 100755 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java @@ -21,20 +21,34 @@ import static org.mockito.Mockito.when; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.never; import static org.mockito.Mockito.any; +import com.cloud.storage.Storage; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope; +import org.apache.cloudstack.engine.subsystem.api.storage.HostScope; +import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager; +import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.storage.image.store.TemplateObject; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.volume.VolumeObject; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; @@ -57,6 +71,10 @@ public class AncientDataMotionStrategyTest { StorageManager storageManager; @Mock StoragePool storagePool; + @Mock + StorageCacheManager cacheMgr; + @Mock + ConfigurationDao configDao; private static final long POOL_ID = 1l; private static final Boolean FULL_CLONE_FLAG = true; @@ -88,4 +106,186 @@ public void testAddFullCloneFlagOnNotVmwareDest(){ verify(dataStoreTO, never()).setFullCloneFlag(any(Boolean.class)); } + @Test + public void testCanBypassSecondaryStorageForDirectDownload() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + Mockito.doReturn(true).when(srcVolumeInfo).isDirectDownload(); + + VolumeObject destVolumeInfo = Mockito.spy(new VolumeObject()); + + Method method; + method = AncientDataMotionStrategy.class.getDeclaredMethod("canBypassSecondaryStorage", DataObject.class, DataObject.class); + method.setAccessible(true); + boolean canBypassSecondaryStorage = (boolean) method.invoke(strategy, srcVolumeInfo, destVolumeInfo); + Assert.assertTrue(canBypassSecondaryStorage); + } + + @Test + public void testCanBypassSecondaryStorageForUnsupportedDataObject() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + + TemplateObject destTemplateInfo = Mockito.spy(new TemplateObject()); + + Method method; + method = AncientDataMotionStrategy.class.getDeclaredMethod("canBypassSecondaryStorage", DataObject.class, DataObject.class); + method.setAccessible(true); + boolean canBypassSecondaryStorage = (boolean) method.invoke(strategy, srcVolumeInfo, destTemplateInfo); + Assert.assertFalse(canBypassSecondaryStorage); + } + + @Test + public void testCanBypassSecondaryStorageForUnsupportedSrcPoolType() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore srcDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ZoneScope(1L)).when(srcDataStore).getScope(); + Mockito.doReturn(srcDataStore).when(srcVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.PowerFlex).when(srcVolumeInfo).getStoragePoolType(); + + VolumeObject destVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore destDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ZoneScope(1L)).when(destDataStore).getScope(); + Mockito.doReturn(destDataStore).when(destVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(destVolumeInfo).getStoragePoolType(); + + Method method; + method = AncientDataMotionStrategy.class.getDeclaredMethod("canBypassSecondaryStorage", DataObject.class, DataObject.class); + method.setAccessible(true); + boolean canBypassSecondaryStorage = (boolean) method.invoke(strategy, srcVolumeInfo, destVolumeInfo); + Assert.assertFalse(canBypassSecondaryStorage); + } + + @Test + public void testCanBypassSecondaryStorageForUnsupportedDestPoolType() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore srcDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ZoneScope(1L)).when(srcDataStore).getScope(); + Mockito.doReturn(srcDataStore).when(srcVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(srcVolumeInfo).getStoragePoolType(); + + VolumeObject destVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore destDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ZoneScope(1L)).when(destDataStore).getScope(); + Mockito.doReturn(destDataStore).when(destVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.Iscsi).when(destVolumeInfo).getStoragePoolType(); + + Method method; + method = AncientDataMotionStrategy.class.getDeclaredMethod("canBypassSecondaryStorage", DataObject.class, DataObject.class); + method.setAccessible(true); + boolean canBypassSecondaryStorage = (boolean) method.invoke(strategy, srcVolumeInfo, destVolumeInfo); + Assert.assertFalse(canBypassSecondaryStorage); + } + + @Test + public void testCanBypassSecondaryStorageWithZoneWideNFSPoolsInSameZone() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore srcDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ZoneScope(1L)).when(srcDataStore).getScope(); + Mockito.doReturn(srcDataStore).when(srcVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(srcVolumeInfo).getStoragePoolType(); + + VolumeObject destVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore destDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ZoneScope(1L)).when(destDataStore).getScope(); + Mockito.doReturn(destDataStore).when(destVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(destVolumeInfo).getStoragePoolType(); + + Method method; + method = AncientDataMotionStrategy.class.getDeclaredMethod("canBypassSecondaryStorage", DataObject.class, DataObject.class); + method.setAccessible(true); + boolean canBypassSecondaryStorage = (boolean) method.invoke(strategy, srcVolumeInfo, destVolumeInfo); + Assert.assertTrue(canBypassSecondaryStorage); + } + + @Test + public void testCanBypassSecondaryStorageWithClusterWideNFSPoolsInSameCluster() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore srcDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ClusterScope(5L, 2L, 1L)).when(srcDataStore).getScope(); + Mockito.doReturn(srcDataStore).when(srcVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(srcVolumeInfo).getStoragePoolType(); + + VolumeObject destVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore destDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ClusterScope(5L, 2L, 1L)).when(destDataStore).getScope(); + Mockito.doReturn(destDataStore).when(destVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(destVolumeInfo).getStoragePoolType(); + + Method method; + method = AncientDataMotionStrategy.class.getDeclaredMethod("canBypassSecondaryStorage", DataObject.class, DataObject.class); + method.setAccessible(true); + boolean canBypassSecondaryStorage = (boolean) method.invoke(strategy, srcVolumeInfo, destVolumeInfo); + Assert.assertTrue(canBypassSecondaryStorage); + } + + @Test + public void testCanBypassSecondaryStorageWithLocalAndClusterWideNFSPoolsInSameCluster() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore srcDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new HostScope(1L, 1L, 1L)).when(srcDataStore).getScope(); + Mockito.doReturn(srcDataStore).when(srcVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.Filesystem).when(srcVolumeInfo).getStoragePoolType(); + + VolumeObject destVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore destDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ClusterScope(1L, 1L, 1L)).when(destDataStore).getScope(); + Mockito.doReturn(destDataStore).when(destVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(destVolumeInfo).getStoragePoolType(); + + Method method; + method = AncientDataMotionStrategy.class.getDeclaredMethod("canBypassSecondaryStorage", DataObject.class, DataObject.class); + method.setAccessible(true); + boolean canBypassSecondaryStorage = (boolean) method.invoke(strategy, srcVolumeInfo, destVolumeInfo); + Assert.assertTrue(canBypassSecondaryStorage); + + canBypassSecondaryStorage = (boolean) method.invoke(strategy, destVolumeInfo, srcVolumeInfo); + Assert.assertTrue(canBypassSecondaryStorage); + } + + @Test + public void testCanBypassSecondaryStorageWithLocalAndZoneWideNFSPoolsInSameZone() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore srcDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new HostScope(1L, 1L, 1L)).when(srcDataStore).getScope(); + Mockito.doReturn(srcDataStore).when(srcVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.Filesystem).when(srcVolumeInfo).getStoragePoolType(); + + VolumeObject destVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore destDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ZoneScope(1L)).when(destDataStore).getScope(); + Mockito.doReturn(destDataStore).when(destVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(destVolumeInfo).getStoragePoolType(); + + Method method; + method = AncientDataMotionStrategy.class.getDeclaredMethod("canBypassSecondaryStorage", DataObject.class, DataObject.class); + method.setAccessible(true); + boolean canBypassSecondaryStorage = (boolean) method.invoke(strategy, srcVolumeInfo, destVolumeInfo); + Assert.assertTrue(canBypassSecondaryStorage); + + canBypassSecondaryStorage = (boolean) method.invoke(strategy, destVolumeInfo, srcVolumeInfo); + Assert.assertTrue(canBypassSecondaryStorage); + } + + @Test + public void testCanBypassSecondaryStorageWithClusterWideNFSAndZoneWideNFSPoolsInSameZone() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { + VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore srcDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ClusterScope(5L, 2L, 1L)).when(srcDataStore).getScope(); + Mockito.doReturn(srcDataStore).when(srcVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(srcVolumeInfo).getStoragePoolType(); + + VolumeObject destVolumeInfo = Mockito.spy(new VolumeObject()); + DataStore destDataStore = Mockito.mock(DataStore.class); + Mockito.doReturn(new ZoneScope(1L)).when(destDataStore).getScope(); + Mockito.doReturn(destDataStore).when(destVolumeInfo).getDataStore(); + Mockito.doReturn(Storage.StoragePoolType.NetworkFilesystem).when(destVolumeInfo).getStoragePoolType(); + + Method method; + method = AncientDataMotionStrategy.class.getDeclaredMethod("canBypassSecondaryStorage", DataObject.class, DataObject.class); + method.setAccessible(true); + boolean canBypassSecondaryStorage = (boolean) method.invoke(strategy, srcVolumeInfo, destVolumeInfo); + Assert.assertTrue(canBypassSecondaryStorage); + + canBypassSecondaryStorage = (boolean) method.invoke(strategy, destVolumeInfo, srcVolumeInfo); + Assert.assertTrue(canBypassSecondaryStorage); + } } From 1a03c3f955d0d1212efe916f85f64a393e4d96e3 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 25 Sep 2025 14:10:11 +0530 Subject: [PATCH 6/9] Update suitable disk offering(s) for volume(s) after migrate VM with volumes when change in pool type (shared or local) Currently, Migrate VM with volume(s) bypasses the service and disk offerings of the volumes, as the target pools for migration are specified, which ignores the offerings. Offering change is required when pool type (shared or local) is changed, mainly - when volume on shared pool is migrated to local pool - when volume on local pool is migrated to shared pool --- .../java/com/cloud/offering/DiskOffering.java | 5 ++- .../com/cloud/storage/VolumeApiService.java | 2 ++ .../com/cloud/storage/DiskOfferingVO.java | 2 +- .../cloud/storage/dao/DiskOfferingDao.java | 2 ++ .../storage/dao/DiskOfferingDaoImpl.java | 16 ++++++++++ .../StorageSystemDataMotionStrategy.java | 31 ++++++++++++++++++- .../cloud/storage/VolumeApiServiceImpl.java | 6 ++-- 7 files changed, 57 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/com/cloud/offering/DiskOffering.java b/api/src/main/java/com/cloud/offering/DiskOffering.java index e1c41f77cbf5..2218b90fe9bd 100644 --- a/api/src/main/java/com/cloud/offering/DiskOffering.java +++ b/api/src/main/java/com/cloud/offering/DiskOffering.java @@ -69,6 +69,8 @@ public String toString() { boolean isCustomized(); + boolean isShared(); + void setDiskSize(long diskSize); long getDiskSize(); @@ -99,7 +101,6 @@ public String toString() { Long getBytesReadRateMaxLength(); - void setBytesWriteRate(Long bytesWriteRate); Long getBytesWriteRate(); @@ -112,7 +113,6 @@ public String toString() { Long getBytesWriteRateMaxLength(); - void setIopsReadRate(Long iopsReadRate); Long getIopsReadRate(); @@ -133,7 +133,6 @@ public String toString() { Long getIopsWriteRateMax(); - void setIopsWriteRateMaxLength(Long iopsWriteRateMaxLength); Long getIopsWriteRateMaxLength(); diff --git a/api/src/main/java/com/cloud/storage/VolumeApiService.java b/api/src/main/java/com/cloud/storage/VolumeApiService.java index 4140d51a800d..19c2ebe455a5 100644 --- a/api/src/main/java/com/cloud/storage/VolumeApiService.java +++ b/api/src/main/java/com/cloud/storage/VolumeApiService.java @@ -180,6 +180,8 @@ Volume updateVolume(long volumeId, String path, String state, Long storageId, */ boolean doesStoragePoolSupportDiskOfferingTags(StoragePool destPool, String diskOfferingTags); + boolean validateConditionsToReplaceDiskOfferingOfVolume(Volume volume, DiskOffering newDiskOffering, StoragePool destPool); + Volume destroyVolume(long volumeId, Account caller, boolean expunge, boolean forceExpunge); void destroyVolume(long volumeId); diff --git a/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java b/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java index 79f5bcb51578..7f6b6d8adf0e 100644 --- a/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java +++ b/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java @@ -577,11 +577,11 @@ public Integer getHypervisorSnapshotReserve() { @Override public void setEncrypt(boolean encrypt) { this.encrypt = encrypt; } + @Override public boolean isShared() { return !useLocalStorage; } - public boolean getDiskSizeStrictness() { return diskSizeStrictness; } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java index f726bca3c5d6..9beea0037444 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDao.java @@ -31,6 +31,8 @@ public interface DiskOfferingDao extends GenericDao { List listAllBySizeAndProvisioningType(long size, Storage.ProvisioningType provisioningType); List findCustomDiskOfferings(); + List listByStorageTag(String tag); + List listAllActiveAndNonComputeDiskOfferings(); } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java index 93e747662775..4ca3fe9f12ac 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/DiskOfferingDaoImpl.java @@ -26,6 +26,7 @@ import javax.inject.Inject; import javax.persistence.EntityExistsException; +import com.cloud.offering.DiskOffering; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.springframework.stereotype.Component; @@ -45,6 +46,8 @@ public class DiskOfferingDaoImpl extends GenericDaoBase im protected DiskOfferingDetailsDao detailsDao; protected final SearchBuilder UniqueNameSearch; + protected final SearchBuilder ActiveAndNonComputeSearch; + private final String SizeDiskOfferingSearch = "SELECT * FROM disk_offering WHERE " + "disk_size = ? AND provisioning_type = ? AND removed IS NULL"; @@ -56,6 +59,11 @@ protected DiskOfferingDaoImpl() { UniqueNameSearch.and("name", UniqueNameSearch.entity().getUniqueName(), SearchCriteria.Op.EQ); UniqueNameSearch.done(); + ActiveAndNonComputeSearch = createSearchBuilder(); + ActiveAndNonComputeSearch.and("state", ActiveAndNonComputeSearch.entity().getState(), SearchCriteria.Op.EQ); + ActiveAndNonComputeSearch.and("computeOnly", ActiveAndNonComputeSearch.entity().isComputeOnly(), SearchCriteria.Op.EQ); + ActiveAndNonComputeSearch.done(); + _computeOnlyAttr = _allAttributes.get("computeOnly"); } @@ -164,4 +172,12 @@ public List listByStorageTag(String tag) { sc.setParameters("tagEndLike", "%," + tag); return listBy(sc); } + + @Override + public List listAllActiveAndNonComputeDiskOfferings() { + SearchCriteria sc = ActiveAndNonComputeSearch.create(); + sc.setParameters("state", DiskOffering.State.Active); + sc.setParameters("computeOnly", false); + return listBy(sc); + } } 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 4f3a6b3b3608..fa8a7a79aa37 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 @@ -122,6 +122,7 @@ import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; +import com.cloud.storage.VolumeApiService; import com.cloud.storage.VolumeDetailVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; @@ -194,6 +195,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { @Inject private VolumeService _volumeService; @Inject + public VolumeApiService _volumeApiService; + @Inject private StorageCacheManager cacheMgr; @Inject private EndPointSelector selector; @@ -2349,11 +2352,22 @@ private void handlePostMigration(boolean success, Map sr volumeVO.setFormat(ImageFormat.QCOW2); volumeVO.setLastId(srcVolumeInfo.getId()); + if (Objects.equals(srcVolumeInfo.getDiskOfferingId(), destVolumeInfo.getDiskOfferingId())) { + StoragePoolVO srcPoolVO = _storagePoolDao.findById(srcVolumeInfo.getPoolId()); + StoragePoolVO destPoolVO = _storagePoolDao.findById(destVolumeInfo.getPoolId()); + if (srcPoolVO != null && destPoolVO != null && + ((srcPoolVO.isShared() && destPoolVO.isLocal()) || (srcPoolVO.isLocal() && destPoolVO.isShared()))) { + Long offeringId = getSuitableDiskOfferingForVolumeOnPool(volumeVO, destPoolVO); + if (offeringId != null) { + volumeVO.setDiskOfferingId(offeringId); + } + } + } + _volumeDao.update(volumeVO.getId(), volumeVO); _volumeService.copyPoliciesBetweenVolumesAndDestroySourceVolumeAfterMigration(Event.OperationSuccessed, null, srcVolumeInfo, destVolumeInfo, false); - // Update the volume ID for snapshots on secondary storage if (!_snapshotDao.listByVolumeId(srcVolumeInfo.getId()).isEmpty()) { _snapshotDao.updateVolumeIds(srcVolumeInfo.getId(), destVolumeInfo.getId()); @@ -2395,6 +2409,21 @@ private void handlePostMigration(boolean success, Map sr } } + private Long getSuitableDiskOfferingForVolumeOnPool(VolumeVO volume, StoragePoolVO pool) { + List diskOfferings = _diskOfferingDao.listAllActiveAndNonComputeDiskOfferings(); + for (DiskOfferingVO diskOffering : diskOfferings) { + try { + if (_volumeApiService.validateConditionsToReplaceDiskOfferingOfVolume(volume, diskOffering, pool)) { + logger.debug("Found suitable disk offering {} for the volume {}", diskOffering, volume); + return diskOffering.getId(); + } + } catch (Exception ignore) { + } + } + logger.warn("Unable to find suitable disk offering for the volume {}", volume); + return null; + } + private VolumeVO duplicateVolumeOnAnotherStorage(Volume volume, StoragePoolVO storagePoolVO) { Long lastPoolId = volume.getPoolId(); diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 3f27ba0b718a..8d6c1b0d59c8 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -3648,9 +3648,10 @@ private DiskOfferingVO retrieveAndValidateNewDiskOffering(MigrateVolumeCmd cmd) * * If all of the above validations pass, we check if the size of the new disk offering is different from the volume. If it is, we log a warning message. */ - protected void validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume, DiskOfferingVO newDiskOffering, StoragePool destPool) { + @Override + public boolean validateConditionsToReplaceDiskOfferingOfVolume(Volume volume, DiskOffering newDiskOffering, StoragePool destPool) { if (newDiskOffering == null) { - return; + return false; } if ((destPool.isShared() && newDiskOffering.isUseLocalStorage()) || destPool.isLocal() && newDiskOffering.isShared()) { throw new InvalidParameterValueException("You cannot move the volume to a shared storage and assign a disk offering for local storage and vice versa."); @@ -3678,6 +3679,7 @@ protected void validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume, volume, oldDiskOffering, newDiskOffering); } logger.info("Changing disk offering to [{}] while migrating volume [{}].", newDiskOffering, volume); + return true; } /** From 11c27765f76389e701aba77a16b4606d0de7362d Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 25 Sep 2025 15:43:26 +0530 Subject: [PATCH 7/9] Update with proper message while migrate volume when target pool and offering type mismatches (both are not shared/local) --- .../main/java/com/cloud/storage/VolumeApiServiceImpl.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 8d6c1b0d59c8..de6dd4ec67f7 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -3653,8 +3653,11 @@ public boolean validateConditionsToReplaceDiskOfferingOfVolume(Volume volume, Di if (newDiskOffering == null) { return false; } - if ((destPool.isShared() && newDiskOffering.isUseLocalStorage()) || destPool.isLocal() && newDiskOffering.isShared()) { - throw new InvalidParameterValueException("You cannot move the volume to a shared storage and assign a disk offering for local storage and vice versa."); + if (destPool.isShared() && newDiskOffering.isUseLocalStorage()) { + throw new InvalidParameterValueException("You cannot move the volume to shared storage, with the disk offering configured for local storage."); + } + if (destPool.isLocal() && newDiskOffering.isShared()) { + throw new InvalidParameterValueException("You cannot move the volume to local storage, with the disk offering configured for shared storage."); } if (!doesStoragePoolSupportDiskOffering(destPool, newDiskOffering)) { throw new InvalidParameterValueException(String.format("Migration failed: target pool [%s, tags:%s] has no matching tags for volume [%s, uuid:%s, tags:%s]", destPool.getName(), From 6ae97f601897287fadb14cbacd4d08ea058cd6e7 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 25 Sep 2025 17:39:54 +0530 Subject: [PATCH 8/9] Consider host scope first during endpoint selection while copying between primary storages --- .../storage/endpoint/DefaultEndPointSelector.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java index da6b1cfede38..061d18dc3769 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java @@ -232,7 +232,13 @@ protected EndPoint findEndPointForImageMove(DataStore srcStore, DataStore destSt // assumption, at least one of scope should be zone, find the least // scope - if (srcScope.getScopeType() != ScopeType.ZONE) { + if (srcScope.getScopeType() == ScopeType.HOST) { + selectedScope = srcScope; + poolId = srcStore.getId(); + } else if (destScope.getScopeType() == ScopeType.HOST) { + selectedScope = destScope; + poolId = destStore.getId(); + } else if (srcScope.getScopeType() != ScopeType.ZONE) { selectedScope = srcScope; poolId = srcStore.getId(); } else if (destScope.getScopeType() != ScopeType.ZONE) { From a0a30386f843659cea5d30c8fad9e7466cb777ad Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 7 Oct 2025 12:54:44 +0530 Subject: [PATCH 9/9] Update disk offering count (for listDiskOfferings api) while removing offerings with tags mismatch with storage tags --- server/src/main/java/com/cloud/api/query/QueryManagerImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index f51aca02af05..ef7d8505420b 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -3675,6 +3675,7 @@ private Pair, Integer> searchForDiskOfferingsInternal(L String[] offeringTagsArray = (offeringTags == null || offeringTags.isEmpty()) ? new String[0] : offeringTags.split(","); if (!CollectionUtils.isSubCollection(Arrays.asList(requiredTagsArray), Arrays.asList(offeringTagsArray))) { iteratorForTagsChecking.remove(); + count--; } } }