vishesh92 commented on code in PR #10409:
URL: https://github.com/apache/cloudstack/pull/10409#discussion_r1959143292
##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -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());
Review Comment:
Do we need a null check here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
