Skip to content

Migrate volume improvements, to bypass secondary storage when copy volume between pools is allowed directly #11625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
+128 −24
Draft

Migrate volume improvements, to bypass secondary storage when copy volume between pools is allowed directly #11625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e574259
Migrate volume improvements, to bypass secondary storage when copy vo…
sureshanaparti Sep 12, 2025
d646d67
Bypass secondary storage for copy volume between zone-wide pools and …
sureshanaparti Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ...n/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -182,10 +183,10 @@ List<DiskProfile> 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
Expand Down
6 changes: 6 additions & 0 deletions ...ne/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ClusterScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"));
}
}
10 changes: 9 additions & 1 deletion engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/HostScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
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 ScopeType type = ScopeType.HOST;
private Long hostId;
private Long clusterId;
private Long zoneId;
Expand All @@ -34,7 +36,7 @@ public HostScope(Long hostId, Long clusterId, Long zoneId) {

@Override
public ScopeType getScopeType() {
return ScopeType.HOST;
return this.type;
}

@Override
Expand All @@ -49,4 +51,10 @@ public Long getClusterId() {
public Long getZoneId() {
return zoneId;
}

@Override
public String toString() {
return String.format("HostScope %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(
this, "zoneId", "clusterId", "hostId"));
}
}
6 changes: 6 additions & 0 deletions engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/ZoneScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,4 +40,9 @@ public Long getScopeId() {
return this.zoneId;
}

@Override
public String toString() {
return String.format("ZoneScope %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(
this, "zoneId"));
}
}
1 change: 1 addition & 0 deletions engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
9 changes: 6 additions & 3 deletions ...stration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1422,7 +1422,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) {
Expand Down Expand Up @@ -2303,6 +2303,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);
Expand All @@ -2312,7 +2313,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();
}
Expand Down Expand Up @@ -2353,6 +2354,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);
Expand All @@ -2362,7 +2364,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) {
Expand Down Expand Up @@ -2396,6 +2398,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());
Expand Down
2 changes: 1 addition & 1 deletion ...tion/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
1 change: 1 addition & 0 deletions engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
62 changes: 57 additions & 5 deletions ...amotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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<StoragePoolType> SUPPORTED_POOL_TYPES_TO_BYPASS_SECONDARY_STORE = Arrays.asList(
StoragePoolType.NetworkFilesystem,
StoragePoolType.Filesystem
);

@Inject
EndPointSelector selector;
Expand Down Expand Up @@ -240,7 +247,6 @@ protected DataTO addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(DataTO
return dataTO;
}


protected Answer copyObject(DataObject srcData, DataObject destData) {
return copyObject(srcData, destData, null);
}
Expand Down Expand Up @@ -352,14 +358,12 @@ 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) {
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;
Expand Down Expand Up @@ -448,7 +452,54 @@ 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: {}, 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;
}

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;
}
}
}
}
}

return false;
}

protected Answer migrateVolumeToPool(DataObject srcData, DataObject destData) {
Expand Down Expand Up @@ -492,6 +543,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.
Expand Down
3 changes: 2 additions & 1 deletion ...n/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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());

Expand Down
1 change: 1 addition & 0 deletions ...hot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ protected void updateVolumePath(List<VolumeObjectTO> 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())) {
Expand Down
1 change: 1 addition & 0 deletions ...ge/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
8 changes: 8 additions & 0 deletions ...rage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions ...yperv/src/main/java/org/apache/cloudstack/storage/motion/HypervStorageMotionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ private void updateVolumePathsAfterMigration(Map<VolumeInfo, DataStore> 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.
Expand Down
7 changes: 5 additions & 2 deletions ...s/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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());
Expand Down
1 change: 1 addition & 0 deletions ...mware/src/main/java/org/apache/cloudstack/storage/motion/VmwareStorageMotionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ private void updateVolumesAfterMigration(Map<VolumeInfo, DataStore> 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;
Expand Down
1 change: 1 addition & 0 deletions ...main/java/org/apache/cloudstack/storage/datastore/driver/AdaptiveDataStoreDriverImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,7 @@ void persistVolumeData(StoragePoolVO storagePool, Map<String, String> 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);
Expand Down
Loading
Loading