Skip to content

Commit 9da01e3

Browse files
winterhazelwinterhazel
committed
Fix NPE on external/unmanaged instance import using custom offerings
1 parent c19630f commit 9da01e3

File tree

1 file changed

+99
-26
lines changed

1 file changed

+99
-26
lines changed

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

Lines changed: 99 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,10 +1352,7 @@ private UserVmResponse baseImportInstance(ImportUnmanagedInstanceCmd cmd) {
13521352
List<String> managedVms = new ArrayList<>(additionalNameFilters);
13531353
managedVms.addAll(getHostsManagedVms(hosts));
13541354

1355-
List<String> resourceLimitHostTags = resourceLimitService.getResourceLimitHostTags(serviceOffering, template);
1356-
try (CheckedReservation vmReservation = new CheckedReservation(owner, Resource.ResourceType.user_vm, resourceLimitHostTags, 1L, reservationDao, resourceLimitService);
1357-
CheckedReservation cpuReservation = new CheckedReservation(owner, Resource.ResourceType.cpu, resourceLimitHostTags, Long.valueOf(serviceOffering.getCpu()), reservationDao, resourceLimitService);
1358-
CheckedReservation memReservation = new CheckedReservation(owner, Resource.ResourceType.memory, resourceLimitHostTags, Long.valueOf(serviceOffering.getRamSize()), reservationDao, resourceLimitService)) {
1355+
try {
13591356

13601357
ActionEventUtils.onStartedActionEvent(userId, owner.getId(), EventTypes.EVENT_VM_IMPORT,
13611358
cmd.getEventDescription(), null, null, true, 0);
@@ -1497,7 +1494,7 @@ private UserVm importUnmanagedInstanceFromHypervisor(DataCenter zone, Cluster cl
14971494
String hostName, Account caller, Account owner, long userId,
14981495
ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap,
14991496
Map<String, Long> nicNetworkMap, Map<String, Network.IpAddresses> nicIpAddressMap,
1500-
Map<String, String> details, Boolean migrateAllowed, List<String> managedVms, boolean forced) {
1497+
Map<String, String> details, Boolean migrateAllowed, List<String> managedVms, boolean forced) throws ResourceAllocationException {
15011498
UserVm userVm = null;
15021499
for (HostVO host : hosts) {
15031500
HashMap<String, UnmanagedInstanceTO> unmanagedInstances = getUnmanagedInstancesForHost(host, instanceName, managedVms);
@@ -1541,11 +1538,18 @@ private UserVm importUnmanagedInstanceFromHypervisor(DataCenter zone, Cluster cl
15411538

15421539
template.setGuestOSId(guestOSHypervisor.getGuestOsId());
15431540
}
1544-
userVm = importVirtualMachineInternal(unmanagedInstance, instanceName, zone, cluster, host,
1545-
template, displayName, hostName, CallContext.current().getCallingAccount(), owner, userId,
1546-
serviceOffering, dataDiskOfferingMap,
1547-
nicNetworkMap, nicIpAddressMap,
1548-
details, migrateAllowed, forced, true);
1541+
1542+
List<Reserver> reservations = new ArrayList<>();
1543+
try {
1544+
checkVmResourceLimitsForUnmanagedInstanceImport(owner, unmanagedInstance, serviceOffering, template, reservations);
1545+
userVm = importVirtualMachineInternal(unmanagedInstance, instanceName, zone, cluster, host,
1546+
template, displayName, hostName, CallContext.current().getCallingAccount(), owner, userId,
1547+
serviceOffering, dataDiskOfferingMap,
1548+
nicNetworkMap, nicIpAddressMap,
1549+
details, migrateAllowed, forced, true);
1550+
} finally {
1551+
ReservationHelper.closeAll(reservations);
1552+
}
15491553
break;
15501554
}
15511555
if (userVm != null) {
@@ -1555,6 +1559,36 @@ private UserVm importUnmanagedInstanceFromHypervisor(DataCenter zone, Cluster cl
15551559
return userVm;
15561560
}
15571561

1562+
private void checkVmResourceLimitsForUnmanagedInstanceImport(Account owner, UnmanagedInstanceTO unmanagedInstance, ServiceOfferingVO serviceOffering, VMTemplateVO template, List<Reserver> reservations) throws ResourceAllocationException {
1563+
// When importing a unmanaged instance, the amount of CPUs and memory is obtained from the hypervisor unless powered off
1564+
// and not using a dynamic offering, unlike the external VM import that always obtains it from the compute offering
1565+
Integer cpu = serviceOffering.getCpu();
1566+
Integer memory = serviceOffering.getRamSize();
1567+
1568+
if (serviceOffering.isDynamic() || !unmanagedInstance.getPowerState().equals(UnmanagedInstanceTO.PowerState.PowerOff)) {
1569+
cpu = unmanagedInstance.getCpuCores();
1570+
memory = unmanagedInstance.getMemory();
1571+
}
1572+
1573+
if (cpu == null || cpu == 0) {
1574+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("CPU cores [%s] is not valid for importing VM [%s].", cpu, unmanagedInstance.getName()));
1575+
}
1576+
if (memory == null || memory == 0) {
1577+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Memory [%s] is not valid for importing VM [%s].", memory, unmanagedInstance.getName()));
1578+
}
1579+
1580+
List<String> resourceLimitHostTags = resourceLimitService.getResourceLimitHostTags(serviceOffering, template);
1581+
1582+
CheckedReservation vmReservation = new CheckedReservation(owner, Resource.ResourceType.user_vm, resourceLimitHostTags, 1L, reservationDao, resourceLimitService);
1583+
reservations.add(vmReservation);
1584+
1585+
CheckedReservation cpuReservation = new CheckedReservation(owner, Resource.ResourceType.cpu, resourceLimitHostTags, cpu.longValue(), reservationDao, resourceLimitService);
1586+
reservations.add(cpuReservation);
1587+
1588+
CheckedReservation memReservation = new CheckedReservation(owner, Resource.ResourceType.memory, resourceLimitHostTags, memory.longValue(), reservationDao, resourceLimitService);
1589+
reservations.add(memReservation);
1590+
}
1591+
15581592
private Pair<UnmanagedInstanceTO, Boolean> getSourceVmwareUnmanagedInstance(String vcenter, String datacenterName, String username,
15591593
String password, String clusterName, String sourceHostName,
15601594
String sourceVM) {
@@ -1582,7 +1616,7 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
15821616
Account caller, Account owner, long userId,
15831617
ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap,
15841618
Map<String, Long> nicNetworkMap, Map<String, Network.IpAddresses> nicIpAddressMap,
1585-
Map<String, String> details, ImportVmCmd cmd, boolean forced) {
1619+
Map<String, String> details, ImportVmCmd cmd, boolean forced) throws ResourceAllocationException {
15861620
Long existingVcenterId = cmd.getExistingVcenterId();
15871621
String vcenter = cmd.getVcenter();
15881622
String datacenterName = cmd.getDatacenterName();
@@ -1620,6 +1654,8 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
16201654
UnmanagedInstanceTO sourceVMwareInstance = null;
16211655
DataStoreTO temporaryConvertLocation = null;
16221656
String ovfTemplateOnConvertLocation = null;
1657+
List<Reserver> reservations = new ArrayList<>();
1658+
16231659
try {
16241660
HostVO convertHost = selectKVMHostForConversionInCluster(destinationCluster, convertInstanceHostId);
16251661
HostVO importHost = selectKVMHostForImportingInCluster(destinationCluster, importInstanceHostId);
@@ -1634,6 +1670,10 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
16341670
Pair<UnmanagedInstanceTO, Boolean> sourceInstanceDetails = getSourceVmwareUnmanagedInstance(vcenter, datacenterName, username, password, clusterName, sourceHostName, sourceVMName);
16351671
sourceVMwareInstance = sourceInstanceDetails.first();
16361672
isClonedInstance = sourceInstanceDetails.second();
1673+
1674+
// Ensure that the configured resource limits will not be exceeded before beggining the conversion process
1675+
checkVmResourceLimitsForUnmanagedInstanceImport(owner, sourceVMwareInstance, serviceOffering, template, reservations);
1676+
16371677
boolean isWindowsVm = sourceVMwareInstance.getOperatingSystem().toLowerCase().contains("windows");
16381678
if (isWindowsVm) {
16391679
checkConversionSupportOnHost(convertHost, sourceVMName, true);
@@ -1681,6 +1721,7 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
16811721
if (temporaryConvertLocation != null && StringUtils.isNotBlank(ovfTemplateOnConvertLocation)) {
16821722
removeTemplate(temporaryConvertLocation, ovfTemplateOnConvertLocation);
16831723
}
1724+
ReservationHelper.closeAll(reservations);
16841725
}
16851726
}
16861727

@@ -2400,10 +2441,7 @@ private UserVmResponse importKvmInstance(ImportVmCmd cmd) {
24002441

24012442
UserVm userVm = null;
24022443

2403-
List<String> resourceLimitHostTags = resourceLimitService.getResourceLimitHostTags(serviceOffering, template);
2404-
try (CheckedReservation vmReservation = new CheckedReservation(owner, Resource.ResourceType.user_vm, resourceLimitHostTags, 1L, reservationDao, resourceLimitService);
2405-
CheckedReservation cpuReservation = new CheckedReservation(owner, Resource.ResourceType.cpu, resourceLimitHostTags, Long.valueOf(serviceOffering.getCpu()), reservationDao, resourceLimitService);
2406-
CheckedReservation memReservation = new CheckedReservation(owner, Resource.ResourceType.memory, resourceLimitHostTags, Long.valueOf(serviceOffering.getRamSize()), reservationDao, resourceLimitService)) {
2444+
try {
24072445

24082446
if (ImportSource.EXTERNAL == importSource) {
24092447
String username = cmd.getUsername();
@@ -2466,6 +2504,7 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag
24662504

24672505
List<Reserver> reservations = new ArrayList<>();
24682506
try {
2507+
checkVmResourceLimitsForExternalKvmVmImport(owner, serviceOffering, (VMTemplateVO) template, details, reservations);
24692508
checkVolumeResourceLimitsForExternalKvmVmImport(owner, rootDisk, dataDisks, diskOffering, dataDiskOfferingMap, reservations);
24702509

24712510
// Check NICs and supplied networks
@@ -2630,24 +2669,20 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
26302669
profiles.add(nicProfile);
26312670
networkNicMap.put(network.getUuid(), profiles);
26322671

2672+
List<Reserver> reservations = new ArrayList<>();
26332673
try {
2674+
checkVmResourceLimitsForExternalKvmVmImport(owner, serviceOffering, (VMTemplateVO) template, details, reservations);
26342675
userVm = userVmManager.importVM(zone, null, template, null, displayName, owner,
26352676
null, caller, true, null, owner.getAccountId(), userId,
26362677
serviceOffering, null, hostName,
26372678
Hypervisor.HypervisorType.KVM, allDetails, powerState, networkNicMap);
2638-
} catch (InsufficientCapacityException ice) {
2639-
logger.error(String.format("Failed to import vm name: %s", instanceName), ice);
2640-
throw new ServerApiException(ApiErrorCode.INSUFFICIENT_CAPACITY_ERROR, ice.getMessage());
2641-
}
2642-
if (userVm == null) {
2643-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import vm name: %s", instanceName));
2644-
}
26452679

2646-
DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
2680+
if (userVm == null) {
2681+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import vm name: %s", instanceName));
2682+
}
26472683

2648-
List<Reserver> reservations = new ArrayList<>();
2649-
List<String> resourceLimitStorageTags = resourceLimitService.getResourceLimitStorageTagsForResourceCountOperation(true, diskOffering);
2650-
try {
2684+
DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
2685+
List<String> resourceLimitStorageTags = resourceLimitService.getResourceLimitStorageTagsForResourceCountOperation(true, diskOffering);
26512686
CheckedReservation volumeReservation = new CheckedReservation(owner, Resource.ResourceType.volume, resourceLimitStorageTags,
26522687
CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? 1L : 0L, reservationDao, resourceLimitService);
26532688
reservations.add(volumeReservation);
@@ -2720,6 +2755,9 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
27202755
publishVMUsageUpdateResourceCount(userVm, dummyOffering, template);
27212756
return userVm;
27222757

2758+
} catch (InsufficientCapacityException ice) { // This will be thrown by com.cloud.vm.UserVmService.importVM
2759+
logger.error(String.format("Failed to import vm name: %s", instanceName), ice);
2760+
throw new ServerApiException(ApiErrorCode.INSUFFICIENT_CAPACITY_ERROR, ice.getMessage());
27232761
} catch (ResourceAllocationException e) {
27242762
cleanupFailedImportVM(userVm);
27252763
throw e;
@@ -2728,6 +2766,41 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
27282766
}
27292767
}
27302768

2769+
private void checkVmResourceLimitsForExternalKvmVmImport(Account owner, ServiceOfferingVO serviceOffering, VMTemplateVO template, Map<String, String> details, List<Reserver> reservations) throws ResourceAllocationException {
2770+
// When importing an external VM, the amount of CPUs and memory is always obtained from the compute offering,
2771+
// unlike the unmanaged instance import that obtains it from the hypervisor unless the VM is powered off and the offering is fixed
2772+
Integer cpu = serviceOffering.getCpu();
2773+
Integer memory = serviceOffering.getRamSize();
2774+
2775+
if (serviceOffering.isDynamic()) {
2776+
cpu = getDetailAsInteger(VmDetailConstants.CPU_NUMBER, details);
2777+
memory = getDetailAsInteger(VmDetailConstants.MEMORY, details);
2778+
}
2779+
2780+
List<String> resourceLimitHostTags = resourceLimitService.getResourceLimitHostTags(serviceOffering, template);
2781+
2782+
CheckedReservation vmReservation = new CheckedReservation(owner, Resource.ResourceType.user_vm, resourceLimitHostTags, 1L, reservationDao, resourceLimitService);
2783+
reservations.add(vmReservation);
2784+
2785+
CheckedReservation cpuReservation = new CheckedReservation(owner, Resource.ResourceType.cpu, resourceLimitHostTags, cpu.longValue(), reservationDao, resourceLimitService);
2786+
reservations.add(cpuReservation);
2787+
2788+
CheckedReservation memReservation = new CheckedReservation(owner, Resource.ResourceType.memory, resourceLimitHostTags, memory.longValue(), reservationDao, resourceLimitService);
2789+
reservations.add(memReservation);
2790+
}
2791+
2792+
private Integer getDetailAsInteger(String key, Map<String, String> details) {
2793+
String detail = details.get(key);
2794+
if (detail == null) {
2795+
throw new InvalidParameterValueException(String.format("Detail '%s' must be provided.", key));
2796+
}
2797+
try {
2798+
return Integer.valueOf(detail);
2799+
} catch (NumberFormatException e) {
2800+
throw new InvalidParameterValueException(String.format("Please provide a valid integer value for detail '%s'.", key));
2801+
}
2802+
}
2803+
27312804
private void checkVolume(Map<VolumeOnStorageTO.Detail, String> volumeDetails) {
27322805
if (MapUtils.isEmpty(volumeDetails)) {
27332806
return;

0 commit comments

Comments
 (0)