Skip to content

Commit b6cebe2

Browse files
sureshanapartisureshanaparti
authored
Fixed VMware import issue - check and update pools in the order of the disks (do not update by position) (#10409)
1 parent 212f2a3 commit b6cebe2

File tree

7 files changed

+72
-76
lines changed

7 files changed

+72
-76
lines changed

‎api/src/main/java/com/cloud/storage/VolumeApiService.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,13 +133,13 @@ Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account acc
133133
Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName) throws ResourceAllocationException;
134134

135135
/**
136-
* Checks if the target storage supports the disk offering.
136+
* Checks if the storage pool supports the disk offering tags.
137137
* This validation is consistent with the mechanism used to select a storage pool to deploy a volume when a virtual machine is deployed or when a data disk is allocated.
138138
*
139139
* The scenarios when this method returns true or false is presented in the following table.
140140
* <table border="1">
141141
* <tr>
142-
* <th>#</th><th>Disk offering tags</th><th>Storage tags</th><th>Does the storage support the disk offering?</th>
142+
* <th>#</th><th>Disk offering diskOfferingTags</th><th>Storage diskOfferingTags</th><th>Does the storage support the disk offering?</th>
143143
* </tr>
144144
* <body>
145145
* <tr>
@@ -163,7 +163,7 @@ Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account acc
163163
* </body>
164164
* </table>
165165
*/
166-
boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, String diskOfferingTags);
166+
boolean doesStoragePoolSupportDiskOfferingTags(StoragePool destPool, String diskOfferingTags);
167167

168168
Volume destroyVolume(long volumeId, Account caller, boolean expunge, boolean forceExpunge);
169169

‎server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3519,7 +3519,7 @@ protected void validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
35193519
if ((destPool.isShared() && newDiskOffering.isUseLocalStorage()) || destPool.isLocal() && newDiskOffering.isShared()) {
35203520
throw new InvalidParameterValueException("You cannot move the volume to a shared storage and assign a disk offering for local storage and vice versa.");
35213521
}
3522-
if (!doesTargetStorageSupportDiskOffering(destPool, newDiskOffering)) {
3522+
if (!doesStoragePoolSupportDiskOffering(destPool, newDiskOffering)) {
35233523
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(),
35243524
storagePoolTagsDao.getStoragePoolTags(destPool.getId()), volume.getName(), volume.getUuid(), newDiskOffering.getTags()));
35253525
}
@@ -3546,7 +3546,7 @@ protected void validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
35463546
}
35473547

35483548
/**
3549-
* Checks if the target storage supports the new disk offering.
3549+
* Checks if the storage pool supports the new disk offering.
35503550
* This validation is consistent with the mechanism used to select a storage pool to deploy a volume when a virtual machine is deployed or when a new data disk is allocated.
35513551
*
35523552
* The scenarios when this method returns true or false is presented in the following table.
@@ -3577,9 +3577,9 @@ protected void validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
35773577
* </body>
35783578
* </table>
35793579
*/
3580-
protected boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, DiskOfferingVO diskOffering) {
3581-
String targetStoreTags = diskOffering.getTags();
3582-
return doesTargetStorageSupportDiskOffering(destPool, targetStoreTags);
3580+
protected boolean doesStoragePoolSupportDiskOffering(StoragePool destPool, DiskOfferingVO diskOffering) {
3581+
String offeringTags = diskOffering.getTags();
3582+
return doesStoragePoolSupportDiskOfferingTags(destPool, offeringTags);
35833583
}
35843584

35853585
public static boolean doesNewDiskOfferingHasTagsAsOldDiskOffering(DiskOfferingVO oldDO, DiskOfferingVO newDO) {
@@ -3595,18 +3595,18 @@ public static boolean doesNewDiskOfferingHasTagsAsOldDiskOffering(DiskOfferingVO
35953595
}
35963596

35973597
@Override
3598-
public boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, String diskOfferingTags) {
3598+
public boolean doesStoragePoolSupportDiskOfferingTags(StoragePool destPool, String diskOfferingTags) {
35993599
Pair<List<String>, Boolean> storagePoolTags = getStoragePoolTags(destPool);
36003600
if ((storagePoolTags == null || !storagePoolTags.second()) && org.apache.commons.lang.StringUtils.isBlank(diskOfferingTags)) {
36013601
if (storagePoolTags == null) {
3602-
s_logger.debug(String.format("Destination storage pool [%s] does not have any tags, and so does the disk offering. Therefore, they are compatible", destPool.getUuid()));
3602+
s_logger.debug(String.format("Storage pool [%s] does not have any tags, and so does the disk offering. Therefore, they are compatible", destPool.getUuid()));
36033603
} else {
3604-
s_logger.debug("Destination storage pool has tags [%s], and the disk offering has no tags. Therefore, they are compatible.");
3604+
s_logger.debug("Storage pool has tags [%s], and the disk offering has no tags. Therefore, they are compatible.");
36053605
}
36063606
return true;
36073607
}
36083608
if (storagePoolTags == null || CollectionUtils.isEmpty(storagePoolTags.first())) {
3609-
s_logger.debug(String.format("Destination storage pool [%s] has no tags, while disk offering has tags [%s]. Therefore, they are not compatible", destPool.getUuid(),
3609+
s_logger.debug(String.format("Storage pool [%s] has no tags, while disk offering has tags [%s]. Therefore, they are not compatible", destPool.getUuid(),
36103610
diskOfferingTags));
36113611
return false;
36123612
}
@@ -3619,7 +3619,7 @@ public boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, String
36193619
} else {
36203620
result = CollectionUtils.isSubCollection(Arrays.asList(newDiskOfferingTagsAsStringArray), storageTagsList);
36213621
}
3622-
s_logger.debug(String.format("Destination storage pool [%s] accepts tags [%s]? %s", destPool.getUuid(), diskOfferingTags, result));
3622+
s_logger.debug(String.format("Storage pool [%s] accepts tags [%s]? %s", destPool.getUuid(), diskOfferingTags, result));
36233623
return result;
36243624
}
36253625

‎server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ public VolumeResponse importVolume(ImportVolumeCmd cmd) {
205205
if (diskOffering.isCustomized()) {
206206
volumeApiService.validateCustomDiskOfferingSizeRange(volume.getVirtualSize() / ByteScaleUtils.GiB);
207207
}
208-
if (!volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOffering.getTags())) {
208+
if (!volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOffering.getTags())) {
209209
logFailureAndThrowException(String.format("Disk offering: %s storage tags are not compatible with selected storage pool: %s", diskOffering.getUuid(), pool.getUuid()));
210210
}
211211

‎server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

Lines changed: 44 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ private boolean storagePoolSupportsDiskOffering(StoragePool pool, DiskOffering d
456456
if (diskOffering == null) {
457457
return false;
458458
}
459-
return volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOffering.getTags());
459+
return volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOffering.getTags());
460460
}
461461

462462
private ServiceOfferingVO getUnmanagedInstanceServiceOffering(final UnmanagedInstanceTO instance, ServiceOfferingVO serviceOffering, final Account owner, final DataCenter zone, final Map<String, String> details, Hypervisor.HypervisorType hypervisorType)
@@ -547,7 +547,7 @@ private StoragePool getStoragePool(final UnmanagedInstanceTO.Disk disk, final Da
547547
for (StoragePool pool : pools) {
548548
if (pool.getDataCenterId() == zone.getId() &&
549549
(pool.getClusterId() == null || pool.getClusterId().equals(cluster.getId())) &&
550-
volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOfferingTags)) {
550+
volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOfferingTags)) {
551551
storagePool = pool;
552552
break;
553553
}
@@ -560,7 +560,7 @@ private StoragePool getStoragePool(final UnmanagedInstanceTO.Disk disk, final Da
560560
for (StoragePool pool : pools) {
561561
String searchPoolParam = StringUtils.isNotBlank(dsPath) ? dsPath : dsName;
562562
if (StringUtils.contains(pool.getPath(), searchPoolParam) &&
563-
volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOfferingTags)) {
563+
volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOfferingTags)) {
564564
storagePool = pool;
565565
break;
566566
}
@@ -1732,9 +1732,9 @@ private void sanitizeConvertedInstance(UnmanagedInstanceTO convertedInstance, Un
17321732
convertedInstance.setPowerState(UnmanagedInstanceTO.PowerState.PowerOff);
17331733
List<UnmanagedInstanceTO.Disk> convertedInstanceDisks = convertedInstance.getDisks();
17341734
List<UnmanagedInstanceTO.Disk> sourceVMwareInstanceDisks = sourceVMwareInstance.getDisks();
1735-
for (UnmanagedInstanceTO.Disk sourceVMwareInstanceDisk : sourceVMwareInstanceDisks) {
1736-
UnmanagedInstanceTO.Disk convertedDisk = convertedInstanceDisks.get(sourceVMwareInstanceDisk.getPosition());
1737-
convertedDisk.setDiskId(sourceVMwareInstanceDisk.getDiskId());
1735+
for (int i = 0; i < convertedInstanceDisks.size(); i++) {
1736+
UnmanagedInstanceTO.Disk disk = convertedInstanceDisks.get(i);
1737+
disk.setDiskId(sourceVMwareInstanceDisks.get(i).getDiskId());
17381738
}
17391739
List<UnmanagedInstanceTO.Nic> convertedInstanceNics = convertedInstance.getNics();
17401740
List<UnmanagedInstanceTO.Nic> sourceVMwareInstanceNics = sourceVMwareInstance.getNics();
@@ -2018,79 +2018,75 @@ private List<StoragePoolVO> findInstanceConversionStoragePoolsInCluster(
20182018
List<StoragePoolVO> pools = new ArrayList<>();
20192019
pools.addAll(primaryDataStoreDao.findClusterWideStoragePoolsByHypervisorAndPoolType(destinationCluster.getId(), Hypervisor.HypervisorType.KVM, Storage.StoragePoolType.NetworkFilesystem));
20202020
pools.addAll(primaryDataStoreDao.findZoneWideStoragePoolsByHypervisorAndPoolType(destinationCluster.getDataCenterId(), Hypervisor.HypervisorType.KVM, Storage.StoragePoolType.NetworkFilesystem));
2021-
List<String> diskOfferingTags = new ArrayList<>();
2021+
if (pools.isEmpty()) {
2022+
String msg = String.format("Cannot find suitable storage pools in the cluster %s for the conversion", destinationCluster.getName());
2023+
LOGGER.error(msg);
2024+
throw new CloudRuntimeException(msg);
2025+
}
2026+
2027+
if (serviceOffering.getDiskOfferingId() != null) {
2028+
DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
2029+
if (diskOffering == null) {
2030+
String msg = String.format("Cannot find disk offering with ID %s that belongs to the service offering %s", serviceOffering.getDiskOfferingId(), serviceOffering.getName());
2031+
LOGGER.error(msg);
2032+
throw new CloudRuntimeException(msg);
2033+
}
2034+
if (getStoragePoolWithTags(pools, diskOffering.getTags()) == null) {
2035+
String msg = String.format("Cannot find suitable storage pool for disk offering %s that belongs to the service offering %s", diskOffering.getName(), serviceOffering.getName());
2036+
LOGGER.error(msg);
2037+
throw new CloudRuntimeException(msg);
2038+
}
2039+
}
20222040
for (Long diskOfferingId : dataDiskOfferingMap.values()) {
20232041
DiskOfferingVO diskOffering = diskOfferingDao.findById(diskOfferingId);
20242042
if (diskOffering == null) {
20252043
String msg = String.format("Cannot find disk offering with ID %s", diskOfferingId);
20262044
LOGGER.error(msg);
20272045
throw new CloudRuntimeException(msg);
20282046
}
2029-
diskOfferingTags.add(diskOffering.getTags());
2030-
}
2031-
if (serviceOffering.getDiskOfferingId() != null) {
2032-
DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
2033-
if (diskOffering != null) {
2034-
diskOfferingTags.add(diskOffering.getTags());
2047+
if (getStoragePoolWithTags(pools, diskOffering.getTags()) == null) {
2048+
String msg = String.format("Cannot find suitable storage pool for disk offering %s", diskOffering.getName());
2049+
LOGGER.error(msg);
2050+
throw new CloudRuntimeException(msg);
20352051
}
20362052
}
20372053

2038-
pools = getPoolsWithMatchingTags(pools, diskOfferingTags);
2039-
if (pools.isEmpty()) {
2040-
String msg = String.format("Cannot find suitable storage pools in cluster %s for the conversion", destinationCluster.getName());
2041-
LOGGER.error(msg);
2042-
throw new CloudRuntimeException(msg);
2043-
}
20442054
return pools;
20452055
}
20462056

2047-
private List<StoragePoolVO> getPoolsWithMatchingTags(List<StoragePoolVO> pools, List<String> diskOfferingTags) {
2048-
if (diskOfferingTags.isEmpty()) {
2049-
return pools;
2057+
private StoragePoolVO getStoragePoolWithTags(List<StoragePoolVO> pools, String tags) {
2058+
if (StringUtils.isEmpty(tags)) {
2059+
return pools.get(0);
20502060
}
2051-
List<StoragePoolVO> poolsSupportingTags = new ArrayList<>(pools);
2052-
for (String tags : diskOfferingTags) {
2053-
boolean tagsMatched = false;
2054-
for (StoragePoolVO pool : pools) {
2055-
if (volumeApiService.doesTargetStorageSupportDiskOffering(pool, tags)) {
2056-
poolsSupportingTags.add(pool);
2057-
tagsMatched = true;
2058-
}
2059-
}
2060-
if (!tagsMatched) {
2061-
String msg = String.format("Cannot find suitable storage pools for the conversion with disk offering tags %s", tags);
2062-
LOGGER.error(msg);
2063-
throw new CloudRuntimeException(msg);
2061+
for (StoragePoolVO pool : pools) {
2062+
if (volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, tags)) {
2063+
return pool;
20642064
}
20652065
}
2066-
return poolsSupportingTags;
2066+
return null;
20672067
}
20682068

20692069
private List<String> selectInstanceConversionStoragePools(
20702070
List<StoragePoolVO> pools, List<UnmanagedInstanceTO.Disk> disks,
20712071
ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap
20722072
) {
20732073
List<String> storagePools = new ArrayList<>(disks.size());
2074-
for (int i = 0; i < disks.size(); i++) {
2075-
storagePools.add(null);
2076-
}
20772074
Set<String> dataDiskIds = dataDiskOfferingMap.keySet();
20782075
for (UnmanagedInstanceTO.Disk disk : disks) {
2079-
Long diskOfferingId = dataDiskOfferingMap.get(disk.getDiskId());
2080-
if (diskOfferingId == null && !dataDiskIds.contains(disk.getDiskId())) {
2076+
Long diskOfferingId = null;
2077+
if (dataDiskIds.contains(disk.getDiskId())) {
2078+
diskOfferingId = dataDiskOfferingMap.get(disk.getDiskId());
2079+
} else {
20812080
diskOfferingId = serviceOffering.getDiskOfferingId();
20822081
}
2082+
20832083
//TODO: Choose pools by capacity
20842084
if (diskOfferingId == null) {
2085-
storagePools.set(disk.getPosition(), pools.get(0).getUuid());
2085+
storagePools.add(pools.get(0).getUuid());
20862086
} else {
20872087
DiskOfferingVO diskOffering = diskOfferingDao.findById(diskOfferingId);
2088-
for (StoragePoolVO pool : pools) {
2089-
if (volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOffering.getTags())) {
2090-
storagePools.set(disk.getPosition(), pool.getUuid());
2091-
break;
2092-
}
2093-
}
2088+
StoragePoolVO pool = getStoragePoolWithTags(pools, diskOffering.getTags());
2089+
storagePools.add(pool.getUuid());
20942090
}
20952091
}
20962092
return storagePools;

0 commit comments

Comments
 (0)