Skip to content

CLVM: Fix volume mapping and disk path matching for storage migration #13468

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
Pearl1594 wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

CLVM: Fix volume mapping and disk path matching for storage migration #13468

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
a7b9a41
CLVM: Fix volume mapping and disk path matching for storage migration
Jun 22, 2026
559f415
fix test
Jun 22, 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
3 changes: 2 additions & 1 deletion 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 @@ -543,7 +543,7 @@

@Override
@DB
public void allocate(final String vmInstanceName, final VirtualMachineTemplate template, final ServiceOffering serviceOffering,

Check warning on line 546 in engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 71 to 64, Complexity from 15 to 14, Nesting Level from 4 to 2, Number of Variables from 32 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ7xMgPMUQCvAIt9vHwF&open=AZ7xMgPMUQCvAIt9vHwF&pullRequest=13468
final DiskOfferingInfo rootDiskOfferingInfo, final List<DiskOfferingInfo> dataDiskOfferings, List<Long> dataDiskDeviceIds,
final LinkedHashMap<? extends Network, List<? extends NicProfile>> auxiliaryNetworks,final DeploymentPlan plan, final HypervisorType hyperType,
final Map<String, Map<Integer, String>> extraDhcpOptions, final Map<Long, DiskOffering> datadiskTemplateToDiskOfferingMap, Volume volume, Snapshot snapshot)
Expand Down Expand Up @@ -3543,7 +3543,8 @@
protected boolean shouldMapVolume(VirtualMachineProfile profile, StoragePoolVO currentPool) {
boolean isManaged = currentPool.isManaged();
boolean isNotKvm = HypervisorType.KVM != profile.getHypervisorType();
return isNotKvm || isManaged;
boolean isClvm = ClvmPoolManager.isClvmPoolType(currentPool.getPoolType());
return isNotKvm || isManaged || isClvm;
}

/**
Expand Down
21 changes: 21 additions & 0 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 @@ -408,6 +408,9 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData)
answer = new Answer(cmd, false, errMsg);
} else {
answer = ep.sendMessage(cmd);
if (answer != null && answer.getResult()) {
setClvmLockHostIdIfApplicable(destData, ep);
}
}
return answer;
}
Expand Down Expand Up @@ -463,6 +466,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData)
imageStore.delete(objOnImageStore);
return answer;
}
setClvmLockHostIdIfApplicable(destData, ep);
} catch (Exception e) {
if (imageStore.exists(objOnImageStore)) {
objOnImageStore.processEvent(Event.OperationFailed);
Expand All @@ -486,6 +490,9 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData)
answer = new Answer(cmd, false, errMsg);
} else {
answer = ep.sendMessage(cmd);
if (answer != null && answer.getResult()) {
setClvmLockHostIdIfApplicable(destData, ep);
}
}
// delete volume on cache store
if (cacheData != null) {
Expand All @@ -495,6 +502,17 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData)
}
}

private void setClvmLockHostIdIfApplicable(DataObject destData, EndPoint ep) {
if (ep == null || !(destData instanceof VolumeInfo)) {
return;
}
VolumeInfo destVolume = (VolumeInfo) destData;
if (ClvmPoolManager.isClvmPoolType(destVolume.getStoragePoolType())) {
clvmPoolManager.setClvmLockHostId(destVolume.getId(), ep.getId());
logger.debug("Set CLVM lock host {} for migrated volume {}", ep.getId(), destVolume.getUuid());
}
}

private boolean canBypassSecondaryStorage(DataObject srcData, DataObject destData) {
if (srcData instanceof VolumeInfo) {
if (((VolumeInfo)srcData).isDirectDownload()) {
Expand Down Expand Up @@ -613,6 +631,9 @@ protected Answer migrateVolumeToPool(DataObject srcData, DataObject destData) {
if (destPool.getPoolType() == StoragePoolType.CLVM) {
volumeVo.setFormat(ImageFormat.RAW);
}
if (ClvmPoolManager.isClvmPoolType(destPool.getPoolType())) {
clvmPoolManager.setClvmLockHostId(volume.getId(), ep.getId());
}
// 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.
String folder = destPool.getPath();
Expand Down
6 changes: 6 additions & 0 deletions .../storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3096,6 +3096,12 @@ public boolean isLockTransferRequired(VolumeInfo volumeToAttach, StoragePoolType
}

if (volumePoolId == null || !volumePoolId.equals(vmPoolId)) {
Long volumeLockHostId = findVolumeLockHost(volumeToAttach);
if (volumeLockHostId != null && vmHostId != null && !volumeLockHostId.equals(vmHostId)) {
logger.info("CLVM cross-pool lock transfer required: Volume {} on pool {} lock is on host {} but VM is on host {}",
volumeToAttach.getUuid(), volumePoolId, volumeLockHostId, vmHostId);
return true;
}
return false;
}

Expand Down
39 changes: 37 additions & 2 deletions .../volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeServiceImplClvmTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,49 @@ public void testIsLockTransferRequired_NonCLVMPool() {
}

@Test
public void testIsLockTransferRequired_DifferentPools() {
public void testIsLockTransferRequired_DifferentPools_LockOnDifferentHost() {
when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(HOST_ID_2);

assertTrue(volumeService.isLockTransferRequired(
volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM,
POOL_ID_1, POOL_ID_2, HOST_ID_1));
}

@Test
public void testIsLockTransferRequired_DifferentPools_LockOnSameHost() {
when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(HOST_ID_1);

assertFalse(volumeService.isLockTransferRequired(
volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM,
POOL_ID_1, POOL_ID_2, HOST_ID_1));
}

@Test
public void testIsLockTransferRequired_DifferentPools_NoLockHost() {
when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(null);

assertFalse(volumeService.isLockTransferRequired(
volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM,
POOL_ID_1, POOL_ID_2, HOST_ID_1));
}

@Test
public void testIsLockTransferRequired_NullPoolIds() {
public void testIsLockTransferRequired_NullPoolIds_LockOnDifferentHost() {
when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(HOST_ID_2);

assertTrue(volumeService.isLockTransferRequired(
volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM,
null, POOL_ID_1, HOST_ID_1));

assertTrue(volumeService.isLockTransferRequired(
volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM,
POOL_ID_1, null, HOST_ID_1));
}

@Test
public void testIsLockTransferRequired_NullPoolIds_NoLockHost() {
when(volumeService.findVolumeLockHost(volumeInfoMock)).thenReturn(null);

assertFalse(volumeService.isLockTransferRequired(
volumeInfoMock, StoragePoolType.CLVM, StoragePoolType.CLVM,
null, POOL_ID_1, HOST_ID_1));
Expand Down
2 changes: 1 addition & 1 deletion ...rvisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7022,7 +7022,7 @@ private static boolean isClvmVolume(DiskDef disk, VirtualMachineTO vmSpec) {
continue;
}
VolumeObjectTO volumeTO = (VolumeObjectTO) diskTO.getData();
if (!diskPath.equals(volumeTO.getPath()) && !diskPath.equals(diskTO.getPath())) {
if (!diskPath.substring(diskPath.lastIndexOf(File.separator) + 1).equals(volumeTO.getPath())) {
continue;
}
Comment on lines 7024 to 7027
DataStoreTO dataStore = volumeTO.getDataStore();
Expand Down
12 changes: 3 additions & 9 deletions .../org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,16 +437,10 @@ public void revertSnapshot(SnapshotInfo snapshot, SnapshotInfo snapshotOnPrimary
try {
EndPoint ep = null;
VolumeInfo volumeInfo = volFactory.getVolume(snapshot.getVolumeId(), DataStoreRole.Primary);

StoragePoolVO storagePool = primaryStoreDao.findById(volumeInfo.getPoolId());
if (storagePool != null && storagePool.getPoolType() == StoragePoolType.CLVM) {
ep = epSelector.select(volumeInfo);
if (snapshotOnPrimaryStore != null) {
ep = epSelector.select(snapshotOnPrimaryStore);
} else {
if (snapshotOnPrimaryStore != null) {
ep = epSelector.select(snapshotOnPrimaryStore);
} else {
ep = epSelector.select(volumeInfo);
}
ep = epSelector.select(volumeInfo);
}

if ( ep == null ){
Expand Down
Loading