Skip to content

VMware import issue fix - check and update pools in the order of disks #10409

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

Merged
DaanHoogland merged 1 commit into from
Feb 18, 2025
Merged

VMware import issue fix - check and update pools in the order of disks #10409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions api/src/main/java/com/cloud/storage/VolumeApiService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,13 @@ Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account acc
Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName) throws ResourceAllocationException;

/**
* Checks if the target storage supports the disk offering.
* Checks if the storage pool supports the disk offering tags.
* 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.
*
* The scenarios when this method returns true or false is presented in the following table.
* <table border="1">
* <tr>
* <th>#</th><th>Disk offering tags</th><th>Storage tags</th><th>Does the storage support the disk offering?</th>
* <th>#</th><th>Disk offering diskOfferingTags</th><th>Storage diskOfferingTags</th><th>Does the storage support the disk offering?</th>
* </tr>
* <body>
* <tr>
Expand All @@ -163,7 +163,7 @@ Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account acc
* </body>
* </table>
*/
boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, String diskOfferingTags);
boolean doesStoragePoolSupportDiskOfferingTags(StoragePool destPool, String diskOfferingTags);

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

Expand Down
20 changes: 10 additions & 10 deletions server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3519,7 +3519,7 @@ protected void validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
if ((destPool.isShared() && newDiskOffering.isUseLocalStorage()) || destPool.isLocal() && newDiskOffering.isShared()) {
throw new InvalidParameterValueException("You cannot move the volume to a shared storage and assign a disk offering for local storage and vice versa.");
}
if (!doesTargetStorageSupportDiskOffering(destPool, newDiskOffering)) {
if (!doesStoragePoolSupportDiskOffering(destPool, newDiskOffering)) {
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(),
storagePoolTagsDao.getStoragePoolTags(destPool.getId()), volume.getName(), volume.getUuid(), newDiskOffering.getTags()));
}
Expand All @@ -3546,7 +3546,7 @@ protected void validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
}

/**
* Checks if the target storage supports the new disk offering.
* Checks if the storage pool supports the new disk offering.
* 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.
*
* The scenarios when this method returns true or false is presented in the following table.
Expand Down Expand Up @@ -3577,9 +3577,9 @@ protected void validateConditionsToReplaceDiskOfferingOfVolume(VolumeVO volume,
* </body>
* </table>
*/
protected boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, DiskOfferingVO diskOffering) {
String targetStoreTags = diskOffering.getTags();
return doesTargetStorageSupportDiskOffering(destPool, targetStoreTags);
protected boolean doesStoragePoolSupportDiskOffering(StoragePool destPool, DiskOfferingVO diskOffering) {
String offeringTags = diskOffering.getTags();
return doesStoragePoolSupportDiskOfferingTags(destPool, offeringTags);
}

public static boolean doesNewDiskOfferingHasTagsAsOldDiskOffering(DiskOfferingVO oldDO, DiskOfferingVO newDO) {
Expand All @@ -3595,18 +3595,18 @@ public static boolean doesNewDiskOfferingHasTagsAsOldDiskOffering(DiskOfferingVO
}

@Override
public boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, String diskOfferingTags) {
public boolean doesStoragePoolSupportDiskOfferingTags(StoragePool destPool, String diskOfferingTags) {
Pair<List<String>, Boolean> storagePoolTags = getStoragePoolTags(destPool);
if ((storagePoolTags == null || !storagePoolTags.second()) && org.apache.commons.lang.StringUtils.isBlank(diskOfferingTags)) {
if (storagePoolTags == null) {
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()));
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()));
} else {
s_logger.debug("Destination storage pool has tags [%s], and the disk offering has no tags. Therefore, they are compatible.");
s_logger.debug("Storage pool has tags [%s], and the disk offering has no tags. Therefore, they are compatible.");
}
return true;
}
if (storagePoolTags == null || CollectionUtils.isEmpty(storagePoolTags.first())) {
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(),
s_logger.debug(String.format("Storage pool [%s] has no tags, while disk offering has tags [%s]. Therefore, they are not compatible", destPool.getUuid(),
diskOfferingTags));
return false;
}
Expand All @@ -3619,7 +3619,7 @@ public boolean doesTargetStorageSupportDiskOffering(StoragePool destPool, String
} else {
result = CollectionUtils.isSubCollection(Arrays.asList(newDiskOfferingTagsAsStringArray), storageTagsList);
}
s_logger.debug(String.format("Destination storage pool [%s] accepts tags [%s]? %s", destPool.getUuid(), diskOfferingTags, result));
s_logger.debug(String.format("Storage pool [%s] accepts tags [%s]? %s", destPool.getUuid(), diskOfferingTags, result));
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion ...r/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public VolumeResponse importVolume(ImportVolumeCmd cmd) {
if (diskOffering.isCustomized()) {
volumeApiService.validateCustomDiskOfferingSizeRange(volume.getVirtualSize() / ByteScaleUtils.GiB);
}
if (!volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOffering.getTags())) {
if (!volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOffering.getTags())) {
logFailureAndThrowException(String.format("Disk offering: %s storage tags are not compatible with selected storage pool: %s", diskOffering.getUuid(), pool.getUuid()));
}

Expand Down
92 changes: 44 additions & 48 deletions server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ private boolean storagePoolSupportsDiskOffering(StoragePool pool, DiskOffering d
if (diskOffering == null) {
return false;
}
return volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOffering.getTags());
return volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOffering.getTags());
}

private ServiceOfferingVO getUnmanagedInstanceServiceOffering(final UnmanagedInstanceTO instance, ServiceOfferingVO serviceOffering, final Account owner, final DataCenter zone, final Map<String, String> details, Hypervisor.HypervisorType hypervisorType)
Expand Down Expand Up @@ -547,7 +547,7 @@ private StoragePool getStoragePool(final UnmanagedInstanceTO.Disk disk, final Da
for (StoragePool pool : pools) {
if (pool.getDataCenterId() == zone.getId() &&
(pool.getClusterId() == null || pool.getClusterId().equals(cluster.getId())) &&
volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOfferingTags)) {
volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOfferingTags)) {
storagePool = pool;
break;
}
Expand All @@ -560,7 +560,7 @@ private StoragePool getStoragePool(final UnmanagedInstanceTO.Disk disk, final Da
for (StoragePool pool : pools) {
String searchPoolParam = StringUtils.isNotBlank(dsPath) ? dsPath : dsName;
if (StringUtils.contains(pool.getPath(), searchPoolParam) &&
volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOfferingTags)) {
volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, diskOfferingTags)) {
storagePool = pool;
break;
}
Expand Down Expand Up @@ -1732,9 +1732,9 @@ private void sanitizeConvertedInstance(UnmanagedInstanceTO convertedInstance, Un
convertedInstance.setPowerState(UnmanagedInstanceTO.PowerState.PowerOff);
List<UnmanagedInstanceTO.Disk> convertedInstanceDisks = convertedInstance.getDisks();
List<UnmanagedInstanceTO.Disk> sourceVMwareInstanceDisks = sourceVMwareInstance.getDisks();
for (UnmanagedInstanceTO.Disk sourceVMwareInstanceDisk : sourceVMwareInstanceDisks) {
UnmanagedInstanceTO.Disk convertedDisk = convertedInstanceDisks.get(sourceVMwareInstanceDisk.getPosition());
convertedDisk.setDiskId(sourceVMwareInstanceDisk.getDiskId());
for (int i = 0; i < convertedInstanceDisks.size(); i++) {
UnmanagedInstanceTO.Disk disk = convertedInstanceDisks.get(i);
disk.setDiskId(sourceVMwareInstanceDisks.get(i).getDiskId());
}
List<UnmanagedInstanceTO.Nic> convertedInstanceNics = convertedInstance.getNics();
List<UnmanagedInstanceTO.Nic> sourceVMwareInstanceNics = sourceVMwareInstance.getNics();
Expand Down Expand Up @@ -2018,79 +2018,75 @@ private List<StoragePoolVO> findInstanceConversionStoragePoolsInCluster(
List<StoragePoolVO> pools = new ArrayList<>();
pools.addAll(primaryDataStoreDao.findClusterWideStoragePoolsByHypervisorAndPoolType(destinationCluster.getId(), Hypervisor.HypervisorType.KVM, Storage.StoragePoolType.NetworkFilesystem));
pools.addAll(primaryDataStoreDao.findZoneWideStoragePoolsByHypervisorAndPoolType(destinationCluster.getDataCenterId(), Hypervisor.HypervisorType.KVM, Storage.StoragePoolType.NetworkFilesystem));
List<String> diskOfferingTags = new ArrayList<>();
if (pools.isEmpty()) {
String msg = String.format("Cannot find suitable storage pools in the cluster %s for the conversion", destinationCluster.getName());
LOGGER.error(msg);
throw new CloudRuntimeException(msg);
}

if (serviceOffering.getDiskOfferingId() != null) {
DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
if (diskOffering == null) {
String msg = String.format("Cannot find disk offering with ID %s that belongs to the service offering %s", serviceOffering.getDiskOfferingId(), serviceOffering.getName());
LOGGER.error(msg);
throw new CloudRuntimeException(msg);
}
if (getStoragePoolWithTags(pools, diskOffering.getTags()) == null) {
String msg = String.format("Cannot find suitable storage pool for disk offering %s that belongs to the service offering %s", diskOffering.getName(), serviceOffering.getName());
LOGGER.error(msg);
throw new CloudRuntimeException(msg);
}
}
for (Long diskOfferingId : dataDiskOfferingMap.values()) {
DiskOfferingVO diskOffering = diskOfferingDao.findById(diskOfferingId);
if (diskOffering == null) {
String msg = String.format("Cannot find disk offering with ID %s", diskOfferingId);
LOGGER.error(msg);
throw new CloudRuntimeException(msg);
}
diskOfferingTags.add(diskOffering.getTags());
}
if (serviceOffering.getDiskOfferingId() != null) {
DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
if (diskOffering != null) {
diskOfferingTags.add(diskOffering.getTags());
if (getStoragePoolWithTags(pools, diskOffering.getTags()) == null) {
String msg = String.format("Cannot find suitable storage pool for disk offering %s", diskOffering.getName());
LOGGER.error(msg);
throw new CloudRuntimeException(msg);
}
}

pools = getPoolsWithMatchingTags(pools, diskOfferingTags);
if (pools.isEmpty()) {
String msg = String.format("Cannot find suitable storage pools in cluster %s for the conversion", destinationCluster.getName());
LOGGER.error(msg);
throw new CloudRuntimeException(msg);
}
return pools;
}

private List<StoragePoolVO> getPoolsWithMatchingTags(List<StoragePoolVO> pools, List<String> diskOfferingTags) {
if (diskOfferingTags.isEmpty()) {
return pools;
private StoragePoolVO getStoragePoolWithTags(List<StoragePoolVO> pools, String tags) {
if (StringUtils.isEmpty(tags)) {
return pools.get(0);
}
List<StoragePoolVO> poolsSupportingTags = new ArrayList<>(pools);
for (String tags : diskOfferingTags) {
boolean tagsMatched = false;
for (StoragePoolVO pool : pools) {
if (volumeApiService.doesTargetStorageSupportDiskOffering(pool, tags)) {
poolsSupportingTags.add(pool);
tagsMatched = true;
}
}
if (!tagsMatched) {
String msg = String.format("Cannot find suitable storage pools for the conversion with disk offering tags %s", tags);
LOGGER.error(msg);
throw new CloudRuntimeException(msg);
for (StoragePoolVO pool : pools) {
if (volumeApiService.doesStoragePoolSupportDiskOfferingTags(pool, tags)) {
return pool;
}
}
return poolsSupportingTags;
return null;
}

private List<String> selectInstanceConversionStoragePools(
List<StoragePoolVO> pools, List<UnmanagedInstanceTO.Disk> disks,
ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap
) {
List<String> storagePools = new ArrayList<>(disks.size());
for (int i = 0; i < disks.size(); i++) {
storagePools.add(null);
}
Set<String> dataDiskIds = dataDiskOfferingMap.keySet();
for (UnmanagedInstanceTO.Disk disk : disks) {
Long diskOfferingId = dataDiskOfferingMap.get(disk.getDiskId());
if (diskOfferingId == null && !dataDiskIds.contains(disk.getDiskId())) {
Long diskOfferingId = null;
if (dataDiskIds.contains(disk.getDiskId())) {
diskOfferingId = dataDiskOfferingMap.get(disk.getDiskId());
} else {
diskOfferingId = serviceOffering.getDiskOfferingId();
}

//TODO: Choose pools by capacity
if (diskOfferingId == null) {
storagePools.set(disk.getPosition(), pools.get(0).getUuid());
storagePools.add(pools.get(0).getUuid());
} else {
DiskOfferingVO diskOffering = diskOfferingDao.findById(diskOfferingId);
for (StoragePoolVO pool : pools) {
if (volumeApiService.doesTargetStorageSupportDiskOffering(pool, diskOffering.getTags())) {
storagePools.set(disk.getPosition(), pool.getUuid());
break;
}
}
StoragePoolVO pool = getStoragePoolWithTags(pools, diskOffering.getTags());
storagePools.add(pool.getUuid());
}
}
return storagePools;
Expand Down
Loading