From e4972c998177057dd30d3ba1868b6e6603592112 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 1 Feb 2021 14:08:57 +0530 Subject: [PATCH 1/7] doc: fix typo in install notes (#4633) Signed-off-by: Abhishek Kumar --- INSTALL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/INSTALL.md b/INSTALL.md index 6840626156f..90a51968f24 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -130,7 +130,7 @@ To create rpms, install the following extra packages: # yum -y install rpm-build # yum -y install ws-commons-util - # yum -y instal gcc + # yum -y install gcc # yum -y install glibc-devel # yum -y install MySQL-python From 890e84777cc0b99d5dd3fe577e28aeb19907d363 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 1 Feb 2021 09:43:50 +0100 Subject: [PATCH 2/7] server: throw exception when update vm nic on L2 network (#4625) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit without this change ``` root@mgt01:~# cmk update vmnicip nicid=afab73cb-f4f4-490f-a524-365edef432b7 { "accountid": "a27bffc1-48ee-11eb-8680-069fc4003392", "cmd": "org.apache.cloudstack.api.command.user.vm.UpdateVmNicIpCmd", "completed": "2020-12-28T12:55:27+0000", "created": "2020-12-28T12:55:27+0000", "jobid": "88af30e0-214b-4379-9c39-49648f998ff6", "jobprocstatus": 0, "jobresult": { "errorcode": 530, "errortext": "Failed to update ip address on vm NIC. Refer to server logs for details." }, "jobresultcode": 530, "jobresulttype": "object", "jobstatus": 2, "userid": "a27f45f5-48ee-11eb-8680-069fc4003392" } 🙈 Error: async API failed for job 88af30e0-214b-4379-9c39-49648f998ff6 ``` with this change ``` root@mgt01:~# cmk update vmnicip nicid=22d79189-6754-4dfe-a8c4-0ba21be2ada5 { "accountid": "044bb5a9-32fd-11eb-b251-06b6e80033a6", "cmd": "org.apache.cloudstack.api.command.user.vm.UpdateVmNicIpCmd", "completed": "2020-12-28T12:55:15+0000", "created": "2020-12-28T12:55:15+0000", "jobid": "f4ccb545-0e47-4e9c-b562-b0e6cf52ddd7", "jobprocstatus": 0, "jobresult": { "errorcode": 431, "errortext": "UpdateVmNicIpCmd is not supported in L2 network" }, "jobresultcode": 530, "jobresulttype": "object", "jobstatus": 2, "userid": "044cc6e3-32fd-11eb-b251-06b6e80033a6" } 🙈 Error: async API failed for job f4ccb545-0e47-4e9c-b562-b0e6cf52ddd7 ``` --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index a084d63eb39..45782d9d4cc 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -1652,8 +1652,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return null; } } else { - s_logger.error("UpdateVmNicIpCmd is not supported in this network..."); - return null; + throw new InvalidParameterValueException("UpdateVmNicIpCmd is not supported in L2 network"); } s_logger.debug("Updating IPv4 address of NIC " + nicVO + " to " + ipaddr + "/" + nicVO.getIPv4Netmask() + " with gateway " + nicVO.getIPv4Gateway()); From 1913c6854e357842db79ac74a62c2effc4f0f9f4 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 1 Feb 2021 09:44:20 +0100 Subject: [PATCH 3/7] server: keep networks order and ips while move a vm with multiple networks (#4602) This PR fixes an issue when move a vm from an account to another account. Steps to reproduce the issue (1) create a vm with multiple shared networks (in advanced zone, or advanced zone with security groups) (2) create another account (in same domain who can also access the shared networks) (3) move vm to new account, with a list of networkid expected result: the vm has nics on the networks in same order as specified in API request, and nics have the same ips as before actual result: network order is not same as specified, ips are changed. --- .../service/NetworkOrchestrationService.java | 2 +- .../orchestration/NetworkOrchestrator.java | 6 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 96 ++++++++++++------- .../com/cloud/vpc/MockNetworkManagerImpl.java | 4 +- 4 files changed, 69 insertions(+), 39 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java index 1e8135460ab..183606d5cc6 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java @@ -129,7 +129,7 @@ public interface NetworkOrchestrationService { void cleanupNics(VirtualMachineProfile vm); - void expungeNics(VirtualMachineProfile vm); + void removeNics(VirtualMachineProfile vm); List getNicProfiles(VirtualMachine vm); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 5cf195ea0b9..363d38eebd0 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -2168,10 +2168,10 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } @Override - public void expungeNics(final VirtualMachineProfile vm) { - final List nics = _nicDao.listByVmIdIncludingRemoved(vm.getId()); + public void removeNics(final VirtualMachineProfile vm) { + final List nics = _nicDao.listByVmId(vm.getId()); for (final NicVO nic : nics) { - _nicDao.expunge(nic.getId()); + _nicDao.remove(nic.getId()); } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 45782d9d4cc..adf00ba1935 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -24,8 +24,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Date; import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -6221,7 +6221,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir _securityGroupMgr.removeInstanceFromGroups(cmd.getVmId()); // cleanup the network for the oldOwner _networkMgr.cleanupNics(vmOldProfile); - _networkMgr.expungeNics(vmOldProfile); + _networkMgr.removeNics(vmOldProfile); // security groups will be recreated for the new account, when the // VM is started List networkList = new ArrayList(); @@ -6283,34 +6283,25 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir s_logger.debug("AssignVM: Basic zone, adding security groups no " + securityGroupIdList.size() + " to " + vm.getInstanceName()); } else { + Set applicableNetworks = new LinkedHashSet<>(); + Map requestedIPv4ForNics = new HashMap<>(); + Map requestedIPv6ForNics = new HashMap<>(); if (zone.isSecurityGroupEnabled()) { // advanced zone with security groups // cleanup the old security groups _securityGroupMgr.removeInstanceFromGroups(cmd.getVmId()); - - Set applicableNetworks = new HashSet(); - String requestedIPv4ForDefaultNic = null; - String requestedIPv6ForDefaultNic = null; // if networkIdList is null and the first network of vm is shared network, then keep it if possible if (networkIdList == null || networkIdList.isEmpty()) { NicVO defaultNicOld = _nicDao.findDefaultNicForVM(vm.getId()); if (defaultNicOld != null) { NetworkVO defaultNetworkOld = _networkDao.findById(defaultNicOld.getNetworkId()); - if (defaultNetworkOld != null && defaultNetworkOld.getGuestType() == Network.GuestType.Shared && defaultNetworkOld.getAclType() == ACLType.Domain) { - try { - _networkModel.checkNetworkPermissions(newAccount, defaultNetworkOld); - applicableNetworks.add(defaultNetworkOld); - requestedIPv4ForDefaultNic = defaultNicOld.getIPv4Address(); - requestedIPv6ForDefaultNic = defaultNicOld.getIPv6Address(); - s_logger.debug("AssignVM: use old shared network " + defaultNetworkOld.getName() + " with old ip " + requestedIPv4ForDefaultNic + " on default nic of vm:" + vm.getInstanceName()); - } catch (PermissionDeniedException e) { - s_logger.debug("AssignVM: the shared network on old default nic can not be applied to new account"); - } + if (canAccountUseNetwork(newAccount, defaultNetworkOld)) { + applicableNetworks.add(defaultNetworkOld); + requestedIPv4ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv4Address()); + requestedIPv6ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv6Address()); + s_logger.debug("AssignVM: use old shared network " + defaultNetworkOld.getName() + " with old ip " + defaultNicOld.getIPv4Address() + " on default nic of vm:" + vm.getInstanceName()); } } } - // cleanup the network for the oldOwner - _networkMgr.cleanupNics(vmOldProfile); - _networkMgr.expungeNics(vmOldProfile); if (networkIdList != null && !networkIdList.isEmpty()) { // add any additional networks @@ -6333,10 +6324,24 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir ex.addProxyObject(network.getUuid(), "networkId"); throw ex; } + + if (network.getGuestType() == Network.GuestType.Shared && network.getAclType() == ACLType.Domain) { + NicVO nicOld = _nicDao.findByNtwkIdAndInstanceId(network.getId(), vm.getId()); + if (nicOld != null) { + requestedIPv4ForNics.put(network.getId(), nicOld.getIPv4Address()); + requestedIPv6ForNics.put(network.getId(), nicOld.getIPv6Address()); + s_logger.debug("AssignVM: use old shared network " + network.getName() + " with old ip " + nicOld.getIPv4Address() + " on nic of vm:" + vm.getInstanceName()); + } + } + s_logger.debug("AssignVM: Added network " + network.getName() + " to vm " + vm.getId()); applicableNetworks.add(network); } } + // cleanup the network for the oldOwner + _networkMgr.cleanupNics(vmOldProfile); + _networkMgr.removeNics(vmOldProfile); + // add the new nics LinkedHashMap> networks = new LinkedHashMap>(); int toggle = 0; @@ -6345,11 +6350,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir NicProfile defaultNic = new NicProfile(); if (toggle == 0) { defaultNic.setDefaultNic(true); - defaultNic.setRequestedIPv4(requestedIPv4ForDefaultNic); - defaultNic.setRequestedIPv6(requestedIPv6ForDefaultNic); defaultNetwork = appNet; toggle++; } + + defaultNic.setRequestedIPv4(requestedIPv4ForNics.get(appNet.getId())); + defaultNic.setRequestedIPv6(requestedIPv6ForNics.get(appNet.getId())); networks.put(appNet, new ArrayList(Arrays.asList(defaultNic))); } @@ -6412,27 +6418,20 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (securityGroupIdList != null && !securityGroupIdList.isEmpty()) { throw new InvalidParameterValueException("Can't move vm with security groups; security group feature is not enabled in this zone"); } - Set applicableNetworks = new HashSet(); // if networkIdList is null and the first network of vm is shared network, then keep it if possible if (networkIdList == null || networkIdList.isEmpty()) { NicVO defaultNicOld = _nicDao.findDefaultNicForVM(vm.getId()); if (defaultNicOld != null) { NetworkVO defaultNetworkOld = _networkDao.findById(defaultNicOld.getNetworkId()); - if (defaultNetworkOld != null && defaultNetworkOld.getGuestType() == Network.GuestType.Shared && defaultNetworkOld.getAclType() == ACLType.Domain) { - try { - _networkModel.checkNetworkPermissions(newAccount, defaultNetworkOld); - applicableNetworks.add(defaultNetworkOld); - } catch (PermissionDeniedException e) { - s_logger.debug("AssignVM: the shared network on old default nic can not be applied to new account"); - } + if (canAccountUseNetwork(newAccount, defaultNetworkOld)) { + applicableNetworks.add(defaultNetworkOld); + requestedIPv4ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv4Address()); + requestedIPv6ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv6Address()); + s_logger.debug("AssignVM: use old shared network " + defaultNetworkOld.getName() + " with old ip " + defaultNicOld.getIPv4Address() + " on default nic of vm:" + vm.getInstanceName()); } } } - // cleanup the network for the oldOwner - _networkMgr.cleanupNics(vmOldProfile); - _networkMgr.expungeNics(vmOldProfile); - if (networkIdList != null && !networkIdList.isEmpty()) { // add any additional networks for (Long networkId : networkIdList) { @@ -6452,6 +6451,16 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir ex.addProxyObject(network.getUuid(), "networkId"); throw ex; } + + if (network.getGuestType() == Network.GuestType.Shared && network.getAclType() == ACLType.Domain) { + NicVO nicOld = _nicDao.findByNtwkIdAndInstanceId(network.getId(), vm.getId()); + if (nicOld != null) { + requestedIPv4ForNics.put(network.getId(), nicOld.getIPv4Address()); + requestedIPv6ForNics.put(network.getId(), nicOld.getIPv6Address()); + s_logger.debug("AssignVM: use old shared network " + network.getName() + " with old ip " + nicOld.getIPv4Address() + " on nic of vm:" + vm.getInstanceName()); + } + } + s_logger.debug("AssignVM: Added network " + network.getName() + " to vm " + vm.getId()); applicableNetworks.add(network); } } else if (applicableNetworks.isEmpty()) { @@ -6515,6 +6524,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir applicableNetworks.add(defaultNetwork); } + // cleanup the network for the oldOwner + _networkMgr.cleanupNics(vmOldProfile); + _networkMgr.removeNics(vmOldProfile); + // add the new nics LinkedHashMap> networks = new LinkedHashMap>(); int toggle = 0; @@ -6524,6 +6537,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir defaultNic.setDefaultNic(true); toggle++; } + defaultNic.setRequestedIPv4(requestedIPv4ForNics.get(appNet.getId())); + defaultNic.setRequestedIPv6(requestedIPv6ForNics.get(appNet.getId())); networks.put(appNet, new ArrayList(Arrays.asList(defaultNic))); } VirtualMachine vmi = _itMgr.findById(vm.getId()); @@ -6536,6 +6551,21 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return vm; } + private boolean canAccountUseNetwork(Account newAccount, Network network) { + if (network != null && network.getAclType() == ACLType.Domain + && (network.getGuestType() == Network.GuestType.Shared + || network.getGuestType() == Network.GuestType.L2)) { + try { + _networkModel.checkNetworkPermissions(newAccount, network); + return true; + } catch (PermissionDeniedException e) { + s_logger.debug(String.format("AssignVM: %s network %s can not be used by new account %s", network.getGuestType(), network.getName(), newAccount.getAccountName())); + return false; + } + } + return false; + } + @Override public UserVm restoreVM(RestoreVMCmd cmd) throws InsufficientCapacityException, ResourceUnavailableException { // Input validation diff --git a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java index c962d90d2dd..0cc10ea7bb5 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -580,10 +580,10 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches } /* (non-Javadoc) - * @see com.cloud.network.NetworkManager#expungeNics(com.cloud.vm.VirtualMachineProfile) + * @see com.cloud.network.NetworkManager#removeNics(com.cloud.vm.VirtualMachineProfile) */ @Override - public void expungeNics(VirtualMachineProfile vm) { + public void removeNics(VirtualMachineProfile vm) { // TODO Auto-generated method stub } From a44fb11a02f561923f969dc810ae775dc75d850d Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 1 Feb 2021 09:44:48 +0100 Subject: [PATCH 4/7] server: add possibility to scale vm to current customer offerings (#4622) We can use cloudmonkey to scale a vm with dynamic offering, to same offering but with different cpunumber or memory. Enable it on UI to improve user experience. --- .../src/main/java/com/cloud/api/query/QueryManagerImpl.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 18ea640bf14..c5a1df005b9 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2820,7 +2820,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q _accountMgr.checkAccess(caller, null, true, vmInstance); currentVmOffering = _srvOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId()); - sc.addAnd("id", SearchCriteria.Op.NEQ, currentVmOffering.getId()); + if (! currentVmOffering.isDynamic()) { + sc.addAnd("id", SearchCriteria.Op.NEQ, currentVmOffering.getId()); + } // 1. Only return offerings with the same storage type sc.addAnd("useLocalStorage", SearchCriteria.Op.EQ, currentVmOffering.isUseLocalStorage()); From 313ae1f44978462052726de4a3dbc7479a7381f2 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 1 Feb 2021 09:45:47 +0100 Subject: [PATCH 5/7] server: fix wrong error message when create isolated network without SourceNat (#4624) This PR fixes wrong message when create isolated network without SourceNat. --- .../engine/orchestration/NetworkOrchestrator.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 363d38eebd0..560ca50ec8c 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -2412,8 +2412,11 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra && (ntwkOff.getGuestType() == GuestType.Shared || (ntwkOff.getGuestType() == GuestType.Isolated && !_networkModel.areServicesSupportedByNetworkOffering(ntwkOff.getId(), Service.SourceNat))); if (cidr == null && ip6Cidr == null && cidrRequired) { - throw new InvalidParameterValueException("StartIp/endIp/gateway/netmask are required when create network of" + " type " + Network.GuestType.Shared - + " and network of type " + GuestType.Isolated + " with service " + Service.SourceNat.getName() + " disabled"); + if (ntwkOff.getGuestType() == GuestType.Shared) { + throw new InvalidParameterValueException("StartIp/endIp/gateway/netmask are required when create network of" + " type " + Network.GuestType.Shared); + } else { + throw new InvalidParameterValueException("gateway/netmask are required when create network of" + " type " + GuestType.Isolated + " with service " + Service.SourceNat.getName() + " disabled"); + } } checkL2OfferingServices(ntwkOff); From e9dda98a87dc6c305f7b1dac35f3fe2106609dc8 Mon Sep 17 00:00:00 2001 From: jairov4 <1904410+jairov4@users.noreply.github.com> Date: Mon, 1 Feb 2021 03:52:29 -0500 Subject: [PATCH 6/7] kvm: Use Q35 chipset for UEFI x86_64 (#4576) Fix #4245 This PR uses Q35 chipset for UEFI in x86_64. Currently this mistakenly only enabled for secure boot --- .../cloud/hypervisor/kvm/resource/LibvirtComputingResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index efda1a20ed7..8fe84498094 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -2247,8 +2247,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) { guest.setBootType(GuestDef.BootType.UEFI); guest.setBootMode(GuestDef.BootMode.LEGACY); + guest.setMachineType("q35"); if (StringUtils.isNotBlank(customParams.get(GuestDef.BootType.UEFI.toString())) && "secure".equalsIgnoreCase(customParams.get(GuestDef.BootType.UEFI.toString()))) { - guest.setMachineType("q35"); guest.setBootMode(GuestDef.BootMode.SECURE); // setting to secure mode } } From 9b45ec275a371d8fb22d7814a5410202ec70179b Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 1 Feb 2021 15:25:09 +0530 Subject: [PATCH 7/7] server: select root disk based on user input during vm import (#4591) Signed-off-by: Abhishek Kumar --- .../cloudstack/vm/VmImportManagerImpl.java | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/vm/VmImportManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/VmImportManagerImpl.java index 1856a125661..a3ee04e0885 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/VmImportManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/VmImportManagerImpl.java @@ -500,6 +500,35 @@ public class VmImportManagerImpl implements VmImportService { return storagePool; } + private Pair> getRootAndDataDisks(List disks, final Map dataDiskOfferingMap) { + UnmanagedInstanceTO.Disk rootDisk = null; + List dataDisks = new ArrayList<>(); + if (disks.size() == 1) { + rootDisk = disks.get(0); + return new Pair<>(rootDisk, dataDisks); + } + Set callerDiskIds = dataDiskOfferingMap.keySet(); + if (callerDiskIds.size() != disks.size() - 1) { + String msg = String.format("VM has total %d disks for which %d disk offering mappings provided. %d disks need a disk offering for import", disks.size(), callerDiskIds.size(), disks.size()-1); + LOGGER.error(String.format("%s. %s parameter can be used to provide disk offerings for the disks", msg, ApiConstants.DATADISK_OFFERING_LIST)); + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, msg); + } + List diskIdsWithoutOffering = new ArrayList<>(); + for (UnmanagedInstanceTO.Disk disk : disks) { + String diskId = disk.getDiskId(); + if (!callerDiskIds.contains(diskId)) { + diskIdsWithoutOffering.add(diskId); + rootDisk = disk; + } else { + dataDisks.add(disk); + } + } + if (diskIdsWithoutOffering.size() > 1) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("VM has total %d disks, disk offering mapping not provided for %d disks. Disk IDs that may need a disk offering - %s", disks.size(), diskIdsWithoutOffering.size()-1, String.join(", ", diskIdsWithoutOffering))); + } + return new Pair<>(rootDisk, dataDisks); + } + private void checkUnmanagedDiskAndOfferingForImport(UnmanagedInstanceTO.Disk disk, DiskOffering diskOffering, ServiceOffering serviceOffering, final Account owner, final DataCenter zone, final Cluster cluster, final boolean migrateAllowed) throws ServerApiException, PermissionDeniedException, ResourceAllocationException { if (serviceOffering == null && diskOffering == null) { @@ -910,17 +939,16 @@ public class VmImportManagerImpl implements VmImportService { if (CollectionUtils.isEmpty(unmanagedInstanceDisks)) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("No attached disks found for the unmanaged VM: %s", instanceName)); } - final UnmanagedInstanceTO.Disk rootDisk = unmanagedInstance.getDisks().get(0); + Pair> rootAndDataDisksPair = getRootAndDataDisks(unmanagedInstanceDisks, dataDiskOfferingMap); + final UnmanagedInstanceTO.Disk rootDisk = rootAndDataDisksPair.first(); + final List dataDisks = rootAndDataDisksPair.second(); if (rootDisk == null || Strings.isNullOrEmpty(rootDisk.getController())) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("VM import failed. Unable to retrieve root disk details for VM: %s ", instanceName)); } allDetails.put(VmDetailConstants.ROOT_DISK_CONTROLLER, rootDisk.getController()); - List dataDisks = new ArrayList<>(); try { checkUnmanagedDiskAndOfferingForImport(rootDisk, null, validatedServiceOffering, owner, zone, cluster, migrateAllowed); - if (unmanagedInstanceDisks.size() > 1) { // Data disk(s) present - dataDisks.addAll(unmanagedInstanceDisks); - dataDisks.remove(0); + if (CollectionUtils.isNotEmpty(dataDisks)) { // Data disk(s) present checkUnmanagedDiskAndOfferingForImport(dataDisks, dataDiskOfferingMap, owner, zone, cluster, migrateAllowed); allDetails.put(VmDetailConstants.DATA_DISK_CONTROLLER, dataDisks.get(0).getController()); }