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 2 commits into
base: 4.22
Choose a base branch
from
Open

kvm: fix direct-download live storage migration #13084

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
4b6de36
kvm: fix direct-download live storage migration
Apr 29, 2026
20ee0aa
kvm: refine direct-download migration checks
Apr 29, 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
123 changes: 99 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,49 @@ 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) {
return shouldForceFullCloneMigration(volumeDataStoreMap, destHost, new HashMap<>());
}

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());

Comment on lines +2017 to +2027
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldForceFullCloneMigration performs two _storagePoolDao.findById(...) lookups per volume, and copyAsync immediately repeats those same lookups in the main loop. This doubles DAO calls for every live migration request. Consider computing the “force full clone” decision inside the existing copyAsync loop (before decideMigrationType...) or refactoring to reuse the already-fetched sourceStoragePool/destStoragePool objects to avoid redundant database access.

Copilot uses AI. Check for mistakes.
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 +2079,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 +2092,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());

// do not initiate migration for the same PowerFlex/ScaleIO pool
if (sourceStoragePool.getId() == destStoragePool.getId() && sourceStoragePool.getPoolType() == Storage.StoragePoolType.PowerFlex) {
continue;
}
StoragePoolVO destStoragePool = getStoragePool(storagePoolsById, destDataStore.getId());
StoragePoolVO sourceStoragePool = getStoragePool(storagePoolsById, srcVolumeInfo.getPoolId());

if (!shouldMigrateVolume(sourceStoragePool, destHost, destStoragePool)) {
if (shouldSkipVolumeMigration(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 +2112,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 +2157,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 +2254,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 +2498,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 +2537,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 +2568,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