From a4677f713e7f8f514aa600cd20de131060498e0a Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 9 Feb 2016 16:49:51 +0100 Subject: [PATCH] vmware: fix NPE case, disk controller improvements Signed-off-by: Rohit Yadav --- .../vmware/resource/VmwareResource.java | 31 ++++++++++++--- .../resource/VmwareStorageProcessor.java | 27 +++++-------- .../cloud/storage/VolumeApiServiceImpl.java | 1 + .../vmware/mo/GuestOsDescriptorType.java | 38 +++++++++++++++++++ .../vmware/mo/VirtualMachineMO.java | 7 +++- 5 files changed, 80 insertions(+), 24 deletions(-) create mode 100644 vmware-base/src/com/cloud/hypervisor/vmware/mo/GuestOsDescriptorType.java diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 008f3a39a64..e9fca8c7639 100755 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -33,7 +33,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Random; import java.util.Set; import java.util.TimeZone; @@ -228,6 +227,7 @@ import com.cloud.hypervisor.vmware.mo.DatacenterMO; import com.cloud.hypervisor.vmware.mo.DatastoreFile; import com.cloud.hypervisor.vmware.mo.DatastoreMO; import com.cloud.hypervisor.vmware.mo.DiskControllerType; +import com.cloud.hypervisor.vmware.mo.GuestOsDescriptorType; import com.cloud.hypervisor.vmware.mo.FeatureKeyConstants; import com.cloud.hypervisor.vmware.mo.HostMO; import com.cloud.hypervisor.vmware.mo.HostStorageSystemMO; @@ -1449,7 +1449,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa s_logger.error(msg); throw new Exception(msg); } - + String guestOsId = translateGuestOsIdentifierEx(vmSpec.getArch(), vmSpec.getOs(), vmSpec.getPlatformEmulator()); DiskTO[] disks = validateDisks(vmSpec.getDisks()); assert (disks.length > 0); NicTO[] nics = vmSpec.getNics(); @@ -1562,7 +1562,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa tearDownVm(vmMo); }else if (!hyperHost.createBlankVm(vmNameOnVcenter, vmInternalCSName, vmSpec.getCpus(), vmSpec.getMaxSpeed().intValue(), getReservedCpuMHZ(vmSpec), vmSpec.getLimitCpuUse(), (int)(vmSpec.getMaxRam() / (1024 * 1024)), getReservedMemoryMb(vmSpec), - translateGuestOsIdentifier(vmSpec.getArch(), vmSpec.getOs(), vmSpec.getPlatformEmulator()).value(), rootDiskDataStoreDetails.first(), false, controllerInfo, systemVm)) { + guestOsId, rootDiskDataStoreDetails.first(), false, controllerInfo, systemVm)) { throw new Exception("Failed to create VM. vmName: " + vmInternalCSName); } } @@ -1586,7 +1586,6 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa } VirtualMachineConfigSpec vmConfigSpec = new VirtualMachineConfigSpec(); - String guestOsId = translateGuestOsIdentifier(vmSpec.getArch(), vmSpec.getOs(), vmSpec.getPlatformEmulator()).value(); VmwareHelper.setBasicVmConfig(vmConfigSpec, vmSpec.getCpus(), vmSpec.getMaxSpeed(), getReservedCpuMHZ(vmSpec), (int)(vmSpec.getMaxRam() / (1024 * 1024)), getReservedMemoryMb(vmSpec), @@ -3357,6 +3356,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa s_logger.debug("Successfully consolidated disks of VM " + vmName + "."); } + VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder(); // Update and return volume path for every disk because that could have changed after migration for (Pair entry : volToFiler) { volume = entry.first(); @@ -3365,8 +3365,12 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa for (VirtualDisk disk : disks) { if (volumeDeviceKey.get(volumeId) == disk.getKey()) { VolumeObjectTO newVol = new VolumeObjectTO(); + String newPath = vmMo.getVmdkFileBaseName(disk); + String poolName = entry.second().getUuid().replace("-", ""); + VirtualMachineDiskInfo diskInfo = diskInfoBuilder.getDiskInfoByBackingFileBaseName(newPath, poolName); newVol.setId(volumeId); - newVol.setPath(vmMo.getVmdkFileBaseName(disk)); + newVol.setChainInfo(this._gson.toJson(diskInfo)); + newVol.setPath(newPath); volumeToList.add(newVol); break; } @@ -3498,7 +3502,11 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa } } - return new MigrateVolumeAnswer(cmd, true, null, volumePath); + VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder(); + String chainInfo = _gson.toJson(diskInfoBuilder.getDiskInfoByBackingFileBaseName(volumePath, poolTo.getUuid().replace("-", ""))); + MigrateVolumeAnswer answer = new MigrateVolumeAnswer(cmd, true, null, volumePath); + answer.setVolumeChainInfo(chainInfo); + return answer; } catch (Exception e) { String msg = "Catch Exception " + e.getClass().getName() + " due to " + e.toString(); s_logger.error(msg, e); @@ -4672,6 +4680,17 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa return VirtualMachineGuestOsIdentifier.OTHER_GUEST; } + private String translateGuestOsIdentifierEx(String cpuArchitecture, String guestOs, String cloudGuestOs) { + if (cloudGuestOs != null) { + GuestOsDescriptorType guestOsId = GuestOsDescriptorType.fromValue(cloudGuestOs); + if (guestOsId != null) { + s_logger.debug("Found guest OS mapping name for guest os: " + guestOs); + return guestOsId.toString(); + } + } + return translateGuestOsIdentifier(cpuArchitecture, guestOs, cloudGuestOs).value(); + } + private HashMap getHostVmStateReport() throws Exception { VmwareHypervisorHost hyperHost = getHyperHost(getServiceContext()); diff --git a/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java index 5c5f8423f88..5797c06ebf6 100644 --- a/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -1347,24 +1347,17 @@ public class VmwareStorageProcessor implements StorageProcessor { if (isAttach) { Map diskDetails = new HashMap(); - String dataDiskController = controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER); - String rootDiskController = controllerInfo.get(VmDetailConstants.ROOK_DISK_CONTROLLER); - DiskControllerType rootDiskControllerType = DiskControllerType.getType(rootDiskController); - - if (dataDiskController == null) { - dataDiskController = getLegacyVmDataDiskController(); - } else if ((rootDiskControllerType == DiskControllerType.lsilogic) || - (rootDiskControllerType == DiskControllerType.lsisas1068) || - (rootDiskControllerType == DiskControllerType.pvscsi) || - (rootDiskControllerType == DiskControllerType.buslogic)) { - //TODO: Support mix of SCSI controller types for single VM. If root disk is already over - //a SCSI controller then use the same for data volume as well. This limitation will go once mix - //of SCSI controller types for single VM. - dataDiskController = rootDiskController; - } else if (DiskControllerType.getType(dataDiskController) == DiskControllerType.osdefault) { - dataDiskController = vmMo.getRecommendedDiskController(null); + String diskController = null; + if (controllerInfo != null) { + diskController = controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER); } - String pciDevicePath = vmMo.attachDisk(new String[] {datastoreVolumePath}, morDs, dataDiskController); + if (diskController == null) { + diskController = getLegacyVmDataDiskController(); + } + if (DiskControllerType.getType(diskController) == DiskControllerType.osdefault) { + diskController = vmMo.getRecommendedDiskController(null); + } + String pciDevicePath = vmMo.attachDisk(new String[] {datastoreVolumePath}, morDs, diskController); diskDetails.put(ApiConstants.PCI_DEVICE_PATH, pciDevicePath); answer.setDiskDetails(diskDetails); } else { diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java index be143166dd6..68798f23d78 100644 --- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java @@ -2355,6 +2355,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic controllerInfo.put(VmDetailConstants.ROOK_DISK_CONTROLLER, vm.getDetail(VmDetailConstants.ROOK_DISK_CONTROLLER)); controllerInfo.put(VmDetailConstants.DATA_DISK_CONTROLLER, vm.getDetail(VmDetailConstants.DATA_DISK_CONTROLLER)); cmd.setControllerInfo(controllerInfo); + s_logger.debug("Attach volume id:" + volumeToAttach.getId() + " on VM id:" + vm.getId() + " has controller info:" + controllerInfo); try { answer = (AttachAnswer)_agentMgr.send(hostId, cmd); diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/GuestOsDescriptorType.java b/vmware-base/src/com/cloud/hypervisor/vmware/mo/GuestOsDescriptorType.java new file mode 100644 index 00000000000..364bd87b074 --- /dev/null +++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/GuestOsDescriptorType.java @@ -0,0 +1,38 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.vmware.mo; + +public enum GuestOsDescriptorType +{ + windows9Guest, + windows9_64Guest, + windows9Server64Guest, + rhel7Guest, + rhel7_64Guest; + + public static GuestOsDescriptorType fromValue(String value) { + if (value == null) { + return null; + } + GuestOsDescriptorType guestOsType = null; + try { + guestOsType = valueOf(value); + } catch (IllegalArgumentException e) { + } + return guestOsType; + } +} diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index ccbf04a500c..9fc89e0d4cc 100644 --- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -2127,8 +2127,12 @@ public class VirtualMachineMO extends BaseMO { List devices = (List)_context.getVimClient(). getDynamicProperty(_mor, "config.hardware.device"); + String deviceList = ""; if (devices != null && devices.size() > 0) { for (VirtualDevice device : devices) { + if (device != null) { + deviceList += String.format(" Device %s: info:%s controller key:%s;", device.getKey(), device.getDeviceInfo(), device.getControllerKey()); + } if ((DiskControllerType.getType(diskController) == DiskControllerType.lsilogic || DiskControllerType.getType(diskController) == DiskControllerType.scsi) && device instanceof VirtualLsiLogicController) { return ((VirtualLsiLogicController)device).getKey(); @@ -2146,7 +2150,8 @@ public class VirtualMachineMO extends BaseMO { } assert (false); - throw new Exception(diskController + " Controller Not Found"); + throw new Exception("Scsi disk controller of type " + diskController + " not found among configured devices:" + deviceList + + ". Unable to find and return scsi disk controller key."); } public int getScsiDiskControllerKeyNoException(String diskController) throws Exception {