Skip to content

Commit f3340e9

Browse files
nvazqueznvazquez
committed
Refactor to avoid duplicating the next ID for VM sequence
1 parent 58f9b5e commit f3340e9

File tree

4 files changed

+59
-51
lines changed

4 files changed

+59
-51
lines changed

‎api/src/main/java/com/cloud/vm/UserVmService.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,31 @@ UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableException
503503

504504
void collectVmNetworkStatistics (UserVm userVm);
505505

506-
UserVm importVM(final DataCenter zone, final Host host, final VirtualMachineTemplate template, final String instanceName, final String displayName, final Account owner, final String userData, final Account caller, final Boolean isDisplayVm, final String keyboard,
506+
/**
507+
* Import VM into CloudStack
508+
* @param zone importing zone
509+
* @param host importing host
510+
* @param template template for the imported VM
511+
* @param instanceNameInternal set to null to CloudStack to autogenerate from the next available VM ID on database
512+
* @param displayName display name for the imported VM
513+
* @param owner owner of the imported VM
514+
* @param userData user data for the imported VM
515+
* @param caller caller account
516+
* @param isDisplayVm true to display the imported VM
517+
* @param keyboard keyboard distribution for the imported VM
518+
* @param accountId account ID
519+
* @param userId user ID
520+
* @param serviceOffering service offering for the imported VM
521+
* @param sshPublicKey ssh key for the imported VM
522+
* @param hostName the name for the imported VM
523+
* @param hypervisorType hypervisor type for the imported VM
524+
* @param customParameters details for the imported VM
525+
* @param powerState power state of the imported VM
526+
* @param networkNicMap network to nic mapping
527+
* @return the imported VM
528+
* @throws InsufficientCapacityException in case of errors
529+
*/
530+
UserVm importVM(final DataCenter zone, final Host host, final VirtualMachineTemplate template, final String instanceNameInternal, final String displayName, final Account owner, final String userData, final Account caller, final Boolean isDisplayVm, final String keyboard,
507531
final long accountId, final long userId, final ServiceOffering serviceOffering, final String sshPublicKey,
508532
final String hostName, final HypervisorType hypervisorType, final Map<String, String> customParameters,
509533
final VirtualMachine.PowerState powerState, final LinkedHashMap<String, List<NicProfile>> networkNicMap) throws InsufficientCapacityException;

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8989,8 +8989,16 @@ private void destroyVolumeInContext(UserVmVO vm, boolean expunge, VolumeVO volum
89898989
}
89908990
}
89918991

8992+
private String getInternalName(long accountId, long vmId) {
8993+
String instanceSuffix = _configDao.getValue(Config.InstanceName.key());
8994+
if (instanceSuffix == null) {
8995+
instanceSuffix = "DEFAULT";
8996+
}
8997+
return VirtualMachineName.getVmName(vmId, accountId, instanceSuffix);
8998+
}
8999+
89929000
@Override
8993-
public UserVm importVM(final DataCenter zone, final Host host, final VirtualMachineTemplate template, final String instanceName, final String displayName,
9001+
public UserVm importVM(final DataCenter zone, final Host host, final VirtualMachineTemplate template, final String instanceNameInternal, final String displayName,
89949002
final Account owner, final String userData, final Account caller, final Boolean isDisplayVm, final String keyboard,
89959003
final long accountId, final long userId, final ServiceOffering serviceOffering, final String sshPublicKeys,
89969004
final String hostName, final HypervisorType hypervisorType, final Map<String, String> customParameters,
@@ -9004,6 +9012,9 @@ public UserVm importVM(final DataCenter zone, final Host host, final VirtualMach
90049012
}
90059013

90069014
final long id = _vmDao.getNextInSequence(Long.class, "id");
9015+
String instanceName = StringUtils.isBlank(instanceNameInternal) ?
9016+
getInternalName(owner.getAccountId(), id) :
9017+
instanceNameInternal;
90079018

90089019
if (hostName != null) {
90099020
// Check is hostName is RFC compliant

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

Lines changed: 22 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@
133133
import com.cloud.vm.VMInstanceVO;
134134
import com.cloud.vm.VirtualMachine;
135135
import com.cloud.vm.VirtualMachineManager;
136-
import com.cloud.vm.VirtualMachineName;
137136
import com.cloud.vm.VirtualMachineProfile;
138137
import com.cloud.vm.VirtualMachineProfileImpl;
139138
import com.cloud.vm.VmDetailConstants;
@@ -1111,13 +1110,13 @@ protected void checkUnmanagedDiskLimits(Account account, UnmanagedInstanceTO.Dis
11111110
}
11121111
}
11131112

1114-
private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedInstance, final String instanceName, final DataCenter zone, final Cluster cluster, final HostVO host,
1113+
private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedInstance, final String instanceNameInternal, final DataCenter zone, final Cluster cluster, final HostVO host,
11151114
final VirtualMachineTemplate template, final String displayName, final String hostName, final Account caller, final Account owner, final Long userId,
11161115
final ServiceOfferingVO serviceOffering, final Map<String, Long> dataDiskOfferingMap,
11171116
final Map<String, Long> nicNetworkMap, final Map<String, Network.IpAddresses> callerNicIpAddressMap,
11181117
final Map<String, String> details, final boolean migrateAllowed, final boolean forced, final boolean isImportUnmanagedFromSameHypervisor) {
11191118
logger.debug(LogUtils.logGsonWithoutException("Trying to import VM [%s] with name [%s], in zone [%s], cluster [%s], and host [%s], using template [%s], service offering [%s], disks map [%s], NICs map [%s] and details [%s].",
1120-
unmanagedInstance, instanceName, zone, cluster, host, template, serviceOffering, dataDiskOfferingMap, nicNetworkMap, details));
1119+
unmanagedInstance, displayName, zone, cluster, host, template, serviceOffering, dataDiskOfferingMap, nicNetworkMap, details));
11211120
UserVm userVm = null;
11221121
ServiceOfferingVO validatedServiceOffering = null;
11231122
try {
@@ -1129,8 +1128,8 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI
11291128
}
11301129

11311130
String internalCSName = unmanagedInstance.getInternalCSName();
1132-
if (StringUtils.isEmpty(internalCSName)) {
1133-
internalCSName = instanceName;
1131+
if (StringUtils.isEmpty(instanceNameInternal)) {
1132+
internalCSName = instanceNameInternal;
11341133
}
11351134
Map<String, String> allDetails = new HashMap<>(details);
11361135
if (validatedServiceOffering.isDynamic()) {
@@ -1142,18 +1141,18 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI
11421141
}
11431142

11441143
if (!migrateAllowed && host != null && !hostSupportsServiceOfferingAndTemplate(host, validatedServiceOffering, template)) {
1145-
throw new InvalidParameterValueException(String.format("Service offering: %s or template: %s is not compatible with host: %s of unmanaged VM: %s", serviceOffering.getUuid(), template.getUuid(), host.getUuid(), instanceName));
1144+
throw new InvalidParameterValueException(String.format("Service offering: %s or template: %s is not compatible with host: %s of unmanaged VM: %s", serviceOffering.getUuid(), template.getUuid(), host.getUuid(), displayName));
11461145
}
11471146
// Check disks and supplied disk offerings
11481147
List<UnmanagedInstanceTO.Disk> unmanagedInstanceDisks = unmanagedInstance.getDisks();
11491148
if (CollectionUtils.isEmpty(unmanagedInstanceDisks)) {
1150-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("No attached disks found for the unmanaged VM: %s", instanceName));
1149+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("No attached disks found for the unmanaged VM: %s", displayName));
11511150
}
11521151
Pair<UnmanagedInstanceTO.Disk, List<UnmanagedInstanceTO.Disk>> rootAndDataDisksPair = getRootAndDataDisks(unmanagedInstanceDisks, dataDiskOfferingMap);
11531152
final UnmanagedInstanceTO.Disk rootDisk = rootAndDataDisksPair.first();
11541153
final List<UnmanagedInstanceTO.Disk> dataDisks = rootAndDataDisksPair.second();
11551154
if (rootDisk == null || StringUtils.isEmpty(rootDisk.getController())) {
1156-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("VM import failed. Unable to retrieve root disk details for VM: %s ", instanceName));
1155+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("VM import failed. Unable to retrieve root disk details for VM: %s ", displayName));
11571156
}
11581157
if (cluster.getHypervisorType() == Hypervisor.HypervisorType.KVM) {
11591158
Long rootDiskOfferingId = validatedServiceOffering.getDiskOfferingId();
@@ -1203,13 +1202,13 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI
12031202
validatedServiceOffering, null, hostName,
12041203
cluster.getHypervisorType(), allDetails, powerState, null);
12051204
} catch (InsufficientCapacityException ice) {
1206-
String errorMsg = String.format("Failed to import VM [%s] due to [%s].", instanceName, ice.getMessage());
1205+
String errorMsg = String.format("Failed to import VM [%s] due to [%s].", displayName, ice.getMessage());
12071206
logger.error(errorMsg, ice);
12081207
throw new ServerApiException(ApiErrorCode.INSUFFICIENT_CAPACITY_ERROR, errorMsg);
12091208
}
12101209

12111210
if (userVm == null) {
1212-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import vm name: %s", instanceName));
1211+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import vm name: %s", displayName));
12131212
}
12141213
List<Pair<DiskProfile, StoragePool>> diskProfileStoragePoolList = new ArrayList<>();
12151214
try {
@@ -1239,9 +1238,9 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI
12391238
deviceId++;
12401239
}
12411240
} catch (Exception e) {
1242-
logger.error(String.format("Failed to import volumes while importing vm: %s", instanceName), e);
1241+
logger.error(String.format("Failed to import volumes while importing vm: %s", displayName), e);
12431242
cleanupFailedImportVM(userVm);
1244-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import volumes while importing vm: %s. %s", instanceName, StringUtils.defaultString(e.getMessage())));
1243+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import volumes while importing vm: %s. %s", displayName, StringUtils.defaultString(e.getMessage())));
12451244
}
12461245
try {
12471246
int nicIndex = 0;
@@ -1252,9 +1251,9 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI
12521251
nicIndex++;
12531252
}
12541253
} catch (Exception e) {
1255-
logger.error(String.format("Failed to import NICs while importing vm: %s", instanceName), e);
1254+
logger.error(String.format("Failed to import NICs while importing vm: %s", displayName), e);
12561255
cleanupFailedImportVM(userVm);
1257-
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import NICs while importing vm: %s. %s", instanceName, StringUtils.defaultString(e.getMessage())));
1256+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import NICs while importing vm: %s. %s", displayName, StringUtils.defaultString(e.getMessage())));
12581257
}
12591258
if (migrateAllowed) {
12601259
userVm = migrateImportedVM(host, template, validatedServiceOffering, userVm, owner, diskProfileStoragePoolList);
@@ -1668,8 +1667,7 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
16681667
checkConversionSupportOnHost(convertHost, sourceVMName, true);
16691668
}
16701669

1671-
String instanceName = getGeneratedInstanceName(owner);
1672-
checkNetworkingBeforeConvertingVmwareInstance(zone, owner, instanceName, hostName, sourceVMwareInstance, nicNetworkMap, nicIpAddressMap, forced);
1670+
checkNetworkingBeforeConvertingVmwareInstance(zone, owner, displayName, hostName, sourceVMwareInstance, nicNetworkMap, nicIpAddressMap, forced);
16731671
UnmanagedInstanceTO convertedInstance;
16741672
if (cmd.getForceMsToImportVmFiles() || !conversionSupportAnswer.isOvfExportSupported()) {
16751673
// Uses MS for OVF export to temporary conversion location
@@ -1690,14 +1688,14 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
16901688
}
16911689

16921690
sanitizeConvertedInstance(convertedInstance, sourceVMwareInstance);
1693-
UserVm userVm = importVirtualMachineInternal(convertedInstance, instanceName, zone, destinationCluster, null,
1691+
UserVm userVm = importVirtualMachineInternal(convertedInstance, null, zone, destinationCluster, null,
16941692
template, displayName, hostName, caller, owner, userId,
16951693
serviceOffering, dataDiskOfferingMap,
16961694
nicNetworkMap, nicIpAddressMap,
16971695
details, false, forced, false);
16981696
long timeElapsedInSecs = (System.currentTimeMillis() - importStartTime) / 1000;
16991697
logger.debug(String.format("VMware VM %s imported successfully to CloudStack instance %s (%s), Time taken: %d secs, OVF files imported from %s, Source VMware VM details - OS: %s, PowerState: %s, Disks: %s, NICs: %s",
1700-
sourceVMName, instanceName, displayName, timeElapsedInSecs, (ovfTemplateOnConvertLocation != null)? "MS" : "KVM Host", sourceVMwareInstance.getOperatingSystem(), sourceVMwareInstance.getPowerState(), sourceVMwareInstance.getDisks(), sourceVMwareInstance.getNics()));
1698+
sourceVMName, displayName, displayName, timeElapsedInSecs, (ovfTemplateOnConvertLocation != null)? "MS" : "KVM Host", sourceVMwareInstance.getOperatingSystem(), sourceVMwareInstance.getPowerState(), sourceVMwareInstance.getDisks(), sourceVMwareInstance.getNics()));
17011699
return userVm;
17021700
} catch (CloudRuntimeException e) {
17031701
logger.error(String.format("Error importing VM: %s", e.getMessage()), e);
@@ -1714,7 +1712,7 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
17141712
}
17151713
}
17161714

1717-
private void checkNetworkingBeforeConvertingVmwareInstance(DataCenter zone, Account owner, String instanceName,
1715+
private void checkNetworkingBeforeConvertingVmwareInstance(DataCenter zone, Account owner, String displayName,
17181716
String hostName, UnmanagedInstanceTO sourceVMwareInstance,
17191717
Map<String, Long> nicNetworkMap,
17201718
Map<String, Network.IpAddresses> nicIpAddressMap,
@@ -1742,9 +1740,9 @@ private void checkNetworkingBeforeConvertingVmwareInstance(DataCenter zone, Acco
17421740
}
17431741
boolean autoImport = ipAddresses != null && ipAddresses.getIp4Address() != null && ipAddresses.getIp4Address().equalsIgnoreCase("auto");
17441742
checkUnmanagedNicAndNetworkMacAddressForImport(network, nic, forced);
1745-
checkUnmanagedNicAndNetworkForImport(instanceName, nic, network, zone, owner, autoImport, Hypervisor.HypervisorType.KVM);
1746-
checkUnmanagedNicAndNetworkHostnameForImport(instanceName, nic, network, hostName);
1747-
checkUnmanagedNicIpAndNetworkForImport(instanceName, nic, network, ipAddresses);
1743+
checkUnmanagedNicAndNetworkForImport(displayName, nic, network, zone, owner, autoImport, Hypervisor.HypervisorType.KVM);
1744+
checkUnmanagedNicAndNetworkHostnameForImport(displayName, nic, network, hostName);
1745+
checkUnmanagedNicIpAndNetworkForImport(displayName, nic, network, ipAddresses);
17481746
}
17491747
}
17501748

@@ -1758,15 +1756,6 @@ private void checkUnmanagedNicAndNetworkMacAddressForImport(NetworkVO network, U
17581756
}
17591757
}
17601758

1761-
private String getGeneratedInstanceName(Account owner) {
1762-
long id = vmDao.getNextInSequence(Long.class, "id");
1763-
String instanceSuffix = configurationDao.getValue(Config.InstanceName.key());
1764-
if (instanceSuffix == null) {
1765-
instanceSuffix = "DEFAULT";
1766-
}
1767-
return VirtualMachineName.getVmName(id, owner.getId(), instanceSuffix);
1768-
}
1769-
17701759
private void sanitizeConvertedInstance(UnmanagedInstanceTO convertedInstance, UnmanagedInstanceTO sourceVMwareInstance) {
17711760
convertedInstance.setCpuCores(sourceVMwareInstance.getCpuCores());
17721761
convertedInstance.setCpuSpeed(sourceVMwareInstance.getCpuSpeed());
@@ -2512,10 +2501,8 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag
25122501
}
25132502
VirtualMachine.PowerState powerState = VirtualMachine.PowerState.PowerOff;
25142503

2515-
String internalName = getInternalName(owner.getAccountId());
2516-
25172504
try {
2518-
userVm = userVmManager.importVM(zone, null, template, internalName, displayName, owner,
2505+
userVm = userVmManager.importVM(zone, null, template, null, displayName, owner,
25192506
null, caller, true, null, owner.getAccountId(), userId,
25202507
serviceOffering, null, hostName,
25212508
Hypervisor.HypervisorType.KVM, allDetails, powerState, null);
@@ -2654,10 +2641,8 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
26542641
profiles.add(nicProfile);
26552642
networkNicMap.put(network.getUuid(), profiles);
26562643

2657-
String internalName = getInternalName(owner.getAccountId());
2658-
26592644
try {
2660-
userVm = userVmManager.importVM(zone, null, template, internalName, displayName, owner,
2645+
userVm = userVmManager.importVM(zone, null, template, null, displayName, owner,
26612646
null, caller, true, null, owner.getAccountId(), userId,
26622647
serviceOffering, null, hostName,
26632648
Hypervisor.HypervisorType.KVM, allDetails, powerState, networkNicMap);
@@ -2868,15 +2853,6 @@ private HashMap<String, UnmanagedInstanceTO> getRemoteVmsOnKVMHost(long zoneId,
28682853
return getRemoteVmsAnswer.getUnmanagedInstances();
28692854
}
28702855

2871-
private String getInternalName(long accounId) {
2872-
String instanceSuffix = configurationDao.getValue(Config.InstanceName.key());
2873-
if (instanceSuffix == null) {
2874-
instanceSuffix = "DEFAULT";
2875-
}
2876-
long vmId = userVmDao.getNextInSequence(Long.class, "id");
2877-
return VirtualMachineName.getVmName(vmId, accounId, instanceSuffix);
2878-
}
2879-
28802856
@Override
28812857
public String getConfigComponentName() {
28822858
return UnmanagedVMsManagerImpl.class.getSimpleName();

0 commit comments

Comments
 (0)