Skip to content

kvm: fix direct-download live storage migration #13084

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

Open
Kunalbehbud wants to merge 3 commits into
base: 4.22
Choose a base branch
from
+565 −35
Open

kvm: fix direct-download live storage migration #13084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b6de36
kvm: fix direct-download live storage migration
Apr 29, 2026
20ee0aa
kvm: refine direct-download migration checks
Apr 29, 2026
3d103be
Drop the unused shouldForceFullCloneMigration overload
Kunalbehbud Jul 24, 2026
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
119 changes: 95 additions & 24 deletions ...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 @@ -1964,7 +1964,7 @@ protected MigrationOptions createLinkedCloneMigrationOptions(VolumeInfo srcVolum
Storage.StoragePoolType srcPoolType = srcPool.getPoolType();
Long srcPoolClusterId = srcPool.getClusterId();
VMTemplateStoragePoolVO ref = templatePoolDao.findByPoolTemplate(destVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId(), null);
boolean updateBackingFileReference = ref == null;
boolean updateBackingFileReference = ref == null || !StringUtils.equals(ref.getInstallPath(), srcVolumeBackingFile);
String backingFile = !updateBackingFileReference ? ref.getInstallPath() : srcVolumeBackingFile;
ScopeType scopeType = srcVolumeInfo.getDataStore().getScope().getScopeType();
return new MigrationOptions(srcPoolUuid, srcPoolType, backingFile, updateBackingFileReference, scopeType, srcPoolClusterId);
Expand Down Expand Up @@ -2009,6 +2009,45 @@ protected void setVolumeMigrationOptions(VolumeInfo srcVolumeInfo, VolumeInfo de
destVolumeInfo.setMigrationOptions(migrationOptions);
}

/**
* KVM/libvirt selects linked-clone or full-clone storage migration for the whole VM migration request.
* If any disk is backed by a direct-download template, force the request to full clone so libvirt does
* not use incremental shared-backing semantics for a disk whose backing chain is not guaranteed on the destination.
*/
protected boolean shouldForceFullCloneMigration(Map<VolumeInfo, DataStore> volumeDataStoreMap, Host destHost, Map<Long, StoragePoolVO> storagePoolsById) {
for (Map.Entry<VolumeInfo, DataStore> entry : volumeDataStoreMap.entrySet()) {
VolumeInfo srcVolumeInfo = entry.getKey();
DataStore destDataStore = entry.getValue();
StoragePoolVO sourceStoragePool = getStoragePool(storagePoolsById, srcVolumeInfo.getPoolId());
StoragePoolVO destStoragePool = getStoragePool(storagePoolsById, destDataStore.getId());

if (shouldSkipVolumeMigration(sourceStoragePool, destHost, destStoragePool)) {
continue;
}

if (srcVolumeInfo.isDirectDownload()) {
return true;
}
}
return false;
}

private StoragePoolVO getStoragePool(Map<Long, StoragePoolVO> storagePoolsById, long storagePoolId) {
StoragePoolVO storagePool = storagePoolsById.get(storagePoolId);
if (storagePool == null) {
storagePool = _storagePoolDao.findById(storagePoolId);
if (storagePool != null) {
storagePoolsById.put(storagePoolId, storagePool);
}
}
return storagePool;
}

protected boolean shouldSkipVolumeMigration(StoragePoolVO sourceStoragePool, Host destHost, StoragePoolVO destStoragePool) {
return (sourceStoragePool.getId() == destStoragePool.getId() && sourceStoragePool.getPoolType() == Storage.StoragePoolType.PowerFlex) ||
!shouldMigrateVolume(sourceStoragePool, destHost, destStoragePool);
}

/**
* For each disk to migrate:
* <ul>
Expand Down Expand Up @@ -2036,6 +2075,11 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
List<MigrateDiskInfo> migrateDiskInfoList = new ArrayList<MigrateDiskInfo>();

Map<String, MigrateCommand.MigrateDiskInfo> migrateStorage = new HashMap<>();
Map<Long, StoragePoolVO> storagePoolsById = new HashMap<>();
boolean forceFullCloneMigration = shouldForceFullCloneMigration(volumeDataStoreMap, destHost, storagePoolsById);
if (forceFullCloneMigration) {
logger.info("Using full clone live storage migration for VM [{}] because one or more migrated volumes are backed by direct-download templates.", vmTO);
}

boolean managedStorageDestination = false;
boolean migrateNonSharedInc = false;
Expand All @@ -2044,19 +2088,15 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
DataStore destDataStore = entry.getValue();

VolumeVO srcVolume = _volumeDao.findById(srcVolumeInfo.getId());
StoragePoolVO destStoragePool = _storagePoolDao.findById(destDataStore.getId());
StoragePoolVO sourceStoragePool = _storagePoolDao.findById(srcVolumeInfo.getPoolId());
StoragePoolVO destStoragePool = getStoragePool(storagePoolsById, destDataStore.getId());
StoragePoolVO sourceStoragePool = getStoragePool(storagePoolsById, srcVolumeInfo.getPoolId());

// do not initiate migration for the same PowerFlex/ScaleIO pool
if (sourceStoragePool.getId() == destStoragePool.getId() && sourceStoragePool.getPoolType() == Storage.StoragePoolType.PowerFlex) {
if (shouldSkipVolumeMigration(sourceStoragePool, destHost, destStoragePool)) {
continue;
}

if (!shouldMigrateVolume(sourceStoragePool, destHost, destStoragePool)) {
continue;
}

MigrationOptions.Type migrationType = decideMigrationTypeAndCopyTemplateIfNeeded(destHost, vmInstance, srcVolumeInfo, sourceStoragePool, destStoragePool, destDataStore);
MigrationOptions.Type migrationType = decideMigrationTypeAndCopyTemplateIfNeeded(destHost, vmInstance, srcVolumeInfo, sourceStoragePool, destStoragePool,
destDataStore, forceFullCloneMigration);
migrateNonSharedInc = migrateNonSharedInc || MigrationOptions.Type.LinkedClone.equals(migrationType);

VolumeVO destVolume = duplicateVolumeOnAnotherStorage(srcVolume, destStoragePool);
Expand All @@ -2068,6 +2108,7 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
destVolumeInfo.processEvent(Event.MigrationCopySucceeded);
// move the volume from Ready to Migrating
destVolumeInfo.processEvent(Event.MigrationRequested);
srcVolumeInfoToDestVolumeInfo.put(srcVolumeInfo, destVolumeInfo);

setVolumeMigrationOptions(srcVolumeInfo, destVolumeInfo, vmTO, srcHost, destStoragePool, migrationType);

Expand Down Expand Up @@ -2112,8 +2153,6 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
prepareDiskWithSecretConsumerDetail(vmTO, srcVolumeInfo, destVolumeInfo.getPath());

migrateStorage.put(srcVolumeInfo.getPath(), migrateDiskInfo);

srcVolumeInfoToDestVolumeInfo.put(srcVolumeInfo, destVolumeInfo);
}

PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(vmTO);
Expand Down Expand Up @@ -2211,7 +2250,13 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
}
}

private MigrationOptions.Type decideMigrationTypeAndCopyTemplateIfNeeded(Host destHost, VMInstanceVO vmInstance, VolumeInfo srcVolumeInfo, StoragePoolVO sourceStoragePool, StoragePoolVO destStoragePool, DataStore destDataStore) {
protected MigrationOptions.Type decideMigrationTypeAndCopyTemplateIfNeeded(Host destHost, VMInstanceVO vmInstance, VolumeInfo srcVolumeInfo, StoragePoolVO sourceStoragePool,
StoragePoolVO destStoragePool, DataStore destDataStore, boolean forceFullCloneMigration) {
if (forceFullCloneMigration) {
logger.debug("Skipping linked clone migration for volume [{}] because the migration request includes a direct-download backed volume.", srcVolumeInfo.getId());
return MigrationOptions.Type.FullClone;
}

VMTemplateVO vmTemplate = _vmTemplateDao.findById(vmInstance.getTemplateId());
String srcVolumeBackingFile = getVolumeBackingFile(srcVolumeInfo);
if (StringUtils.isNotBlank(srcVolumeBackingFile) && supportStoragePoolType(destStoragePool.getPoolType(), StoragePoolType.Filesystem) &&
Expand Down Expand Up @@ -2449,7 +2494,11 @@ private VolumeVO duplicateVolumeOnAnotherStorage(Volume volume, StoragePoolVO st
protected String connectHostToVolume(Host host, long storagePoolId, String iqn) {
ModifyTargetsCommand modifyTargetsCommand = getModifyTargetsCommand(storagePoolId, iqn, true);

return sendModifyTargetsCommand(modifyTargetsCommand, host.getId()).get(0);
List<String> connectedPaths = sendModifyTargetsCommand(modifyTargetsCommand, host.getId());
if (CollectionUtils.isEmpty(connectedPaths)) {
throw new CloudRuntimeException(String.format("Unable to modify targets on the following host: %s because no connected path was returned for target [%s].", host.getId(), iqn));
}
return connectedPaths.get(0);
}

private void disconnectHostFromVolume(Host host, long storagePoolId, String iqn) {
Expand Down Expand Up @@ -2484,14 +2533,25 @@ private ModifyTargetsCommand getModifyTargetsCommand(long storagePoolId, String
}

private List<String> sendModifyTargetsCommand(ModifyTargetsCommand cmd, long hostId) {
ModifyTargetsAnswer modifyTargetsAnswer = (ModifyTargetsAnswer)agentManager.easySend(hostId, cmd);
Answer answer = agentManager.easySend(hostId, cmd);

if (modifyTargetsAnswer == null) {
if (answer == null) {
throw new CloudRuntimeException("Unable to get an answer to the modify targets command");
}

if (!(answer instanceof ModifyTargetsAnswer)) {
String details = StringUtils.defaultIfBlank(answer.getDetails(),
String.format("Unexpected answer type returned: %s", answer.getClass().getName()));
throw new CloudRuntimeException(String.format("Unable to modify targets on the following host: %s due to [%s]", hostId, details));
}

ModifyTargetsAnswer modifyTargetsAnswer = (ModifyTargetsAnswer)answer;

if (!modifyTargetsAnswer.getResult()) {
String msg = "Unable to modify targets on the following host: " + hostId;
if (StringUtils.isNotBlank(modifyTargetsAnswer.getDetails())) {
msg = String.format("%s due to [%s]", msg, modifyTargetsAnswer.getDetails());
}

throw new CloudRuntimeException(msg);
}
Expand All @@ -2504,14 +2564,25 @@ private List<String> sendModifyTargetsCommand(ModifyTargetsCommand cmd, long hos
*/
protected void updateCopiedTemplateReference(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo) {
VMTemplateStoragePoolVO ref = templatePoolDao.findByPoolTemplate(srcVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId(), null);
VMTemplateStoragePoolVO newRef = new VMTemplateStoragePoolVO(destVolumeInfo.getPoolId(), ref.getTemplateId(), null);
newRef.setDownloadPercent(100);
newRef.setDownloadState(VMTemplateStorageResourceAssoc.Status.DOWNLOADED);
newRef.setState(ObjectInDataStoreStateMachine.State.Ready);
newRef.setTemplateSize(ref.getTemplateSize());
newRef.setLocalDownloadPath(ref.getLocalDownloadPath());
newRef.setInstallPath(ref.getInstallPath());
templatePoolDao.persist(newRef);
if (ref == null) {
throw new CloudRuntimeException(String.format("Unable to update copied template reference because source template reference was not found for pool [%s] and template [%s].",
srcVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId()));
}
VMTemplateStoragePoolVO destRef = templatePoolDao.findByPoolTemplate(destVolumeInfo.getPoolId(), ref.getTemplateId(), null);
if (destRef == null) {
destRef = new VMTemplateStoragePoolVO(destVolumeInfo.getPoolId(), ref.getTemplateId(), null);
}
destRef.setDownloadPercent(100);
destRef.setDownloadState(VMTemplateStorageResourceAssoc.Status.DOWNLOADED);
destRef.setState(ObjectInDataStoreStateMachine.State.Ready);
destRef.setTemplateSize(ref.getTemplateSize());
destRef.setLocalDownloadPath(ref.getLocalDownloadPath());
destRef.setInstallPath(ref.getInstallPath());
if (destRef.getId() == 0) {
templatePoolDao.persist(destRef);
} else {
templatePoolDao.update(destRef.getId(), destRef);
}
}

/**
Expand Down
23 changes: 23 additions & 0 deletions ...t/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,29 @@ public void migrateTemplateToTargetStorageIfNeededTestTemplateNotOnTargetHost()
configureAndTestcopyTemplateToTargetStorageIfNeeded(null, StoragePoolType.Filesystem, 1);
}

@Test
public void copyTemplateToTargetStorageIfNeededTestDirectDownloadTemplateAlreadyStaged() {
DataStore destDataStore = Mockito.mock(DataStore.class);
Host destHost = Mockito.mock(Host.class);
VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class);
StoragePool srcStoragePool = Mockito.mock(StoragePool.class);
StoragePool destStoragePool = Mockito.mock(StoragePool.class);
TemplateInfo directDownloadTemplateInfo = Mockito.mock(TemplateInfo.class);

Mockito.when(destDataStore.getId()).thenReturn(11L);
Mockito.when(destHost.getId()).thenReturn(12L);
Mockito.when(srcVolumeInfo.getTemplateId()).thenReturn(13L);
Mockito.when(srcVolumeInfo.getVolumeType()).thenReturn(Volume.Type.ROOT);
Mockito.when(templateDataFactory.getReadyBypassedTemplateOnPrimaryStore(13L, 11L, 12L)).thenReturn(directDownloadTemplateInfo);

kvmNonManagedStorageDataMotionStrategy.copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, srcStoragePool, destDataStore, destStoragePool, destHost);

Mockito.verify(templateDataFactory).getReadyBypassedTemplateOnPrimaryStore(13L, 11L, 12L);
Mockito.verify(vmTemplatePoolDao, Mockito.never()).findByPoolTemplate(Mockito.anyLong(), Mockito.anyLong(), nullable(String.class));
Mockito.verify(dataStoreManagerImpl, Mockito.never()).getRandomImageStore(Mockito.anyLong());
Mockito.verify(kvmNonManagedStorageDataMotionStrategy, Mockito.never()).sendCopyCommand(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
}

@Test
public void migrateTemplateToTargetStorageIfNeededTestNonDesiredStoragePoolType() throws AgentUnavailableException, OperationTimedoutException {
StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
Expand Down
Loading
Loading