You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2015/12/12 14:39:25 UTC

[2/4] git commit: updated refs/heads/4.6 to 8c78f89

CLOUDSTACK-4787 Allow selection of scsi controller type in vSphere

    commit #7
    So far only 1 controller (scsi or ide) is supported in Cloudstack for ide or
    scsi, this is existing limitation. Added support for 2nd IDE controller. Support adding IDE
    virtual disk to VM. Also added check if VM is running as IDE virtual disk cannot be attached
    to VM if VM is runnning.If user detaches a virtual disk on lower unit number of controller,
    then subsequent attach operation should find free unit number on the controller and attach
    the virtual disk there.

    commit #6
    Let the controllers of existing VMs continue without flip, current busInfo retrieved from
    chain_info field of volume record from database would be preferred over
    controller settings from all configuration settings.

    commit #5
    Editing global configuration param vmware.root.disk.controller osdefault value results
    in loss of previous root disk controller type. Hence root disk's controller type for legacy
    VMs is unknow post that modificaiton by user. If VM is stop/start then we could get this
    infromation from bus info of existing volume. But if user resets VM and then try to start VM.
    The existing bus info would be lost. Hence existing disk info is not available to depend on.
    Using lsilogic or generic scsi controller for ROOT disk of legacy VMs if reset.

    commit #4
    Avoid adding additional (>1) scsi controllers to system vms. While attaching volume to legacy VM
    don't use osdefault optoin which applicable only for VM created with the option enabled, use
    legacy data disk controller type (lsilogic)

    commit #3
    If root disk's controller type is scsi and data disk controller type condenses
    to any of scsi sub-types then data disk controller type would fall back to root disk controller itself. This
    ensures data volumes would be accessible in all cases as controller of root volume would be reliable
    and it means VM has the supported controller. It also avoids mix of scsi controller sub-types in a user instance.
    Also translating disk controller type scsi to lsilogic.

    commit #2
    Support auto detection of recommended virtual disk controller type for specific guest OS.

    commit #1
    Support granual controller types. Add support for controller types in template registration as well.

    Fix white spaces.
    Removed stale HEAD merge lines
    Removed tail of merge lines
    Fixed VmwareResource, removing storage commands that moved to VmwareStorageProcessor.
    removed stale code of controller that is present in processor
    Fixed check style errors.
    Fixed injection.
    Tested with Linux and windows templates. Unable to run iso based tests due to few bugs in register iso area.

    Signed-off-by: Sateesh Chodapuneedi <sa...@apache.org>

(cherry picked from commit a4cc987a6f66f20c434942956fffe5951df09e43)
Signed-off-by: Rohit Yadav <ro...@shapeblue.com>


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/6a9956e0
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/6a9956e0
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/6a9956e0

Branch: refs/heads/4.6
Commit: 6a9956e0f50ec873c61e57397f0baf3729574a72
Parents: ad2bcc0
Author: Sateesh Chodapuneedi <sa...@apache.org>
Authored: Wed Jan 14 08:13:10 2015 +0530
Committer: Rohit Yadav <ro...@shapeblue.com>
Committed: Fri Nov 27 18:39:41 2015 +0530

----------------------------------------------------------------------
 api/src/com/cloud/vm/VmDetailConstants.java     |   1 +
 .../storage/command/AttachCommand.java          |  19 +-
 .../com/cloud/hypervisor/guru/VMwareGuru.java   |   7 +
 .../vmware/manager/VmwareManager.java           |   1 +
 .../vmware/manager/VmwareManagerImpl.java       |   8 +
 .../vmware/resource/VmwareResource.java         | 205 +++++++-
 .../resource/VmwareStorageProcessor.java        |  52 +-
 .../com/cloud/storage/VolumeApiServiceImpl.java |   6 +
 server/src/com/cloud/vm/UserVmManagerImpl.java  |   9 +
 .../cloud/hypervisor/vmware/mo/ClusterMO.java   |  35 +-
 .../vmware/mo/DiskControllerType.java           |  31 +-
 .../com/cloud/hypervisor/vmware/mo/HostMO.java  |  17 +-
 .../vmware/mo/HypervisorHostHelper.java         | 112 +++-
 .../vmware/mo/ScsiDiskControllerType.java       |  24 +
 .../hypervisor/vmware/mo/VirtualMachineMO.java  | 514 +++++++++++++++++--
 .../hypervisor/vmware/mo/VmdkAdapterType.java   |  48 ++
 .../vmware/mo/VmdkFileDescriptor.java           |  70 +++
 .../vmware/mo/VmwareHypervisorHost.java         |   5 +-
 .../hypervisor/vmware/util/VmwareHelper.java    |  24 +
 19 files changed, 1123 insertions(+), 65 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/api/src/com/cloud/vm/VmDetailConstants.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/vm/VmDetailConstants.java b/api/src/com/cloud/vm/VmDetailConstants.java
index bd6f0e2..d06ad67 100644
--- a/api/src/com/cloud/vm/VmDetailConstants.java
+++ b/api/src/com/cloud/vm/VmDetailConstants.java
@@ -22,4 +22,5 @@ public interface VmDetailConstants {
     public static final String ROOK_DISK_CONTROLLER = "rootDiskController";
     public static final String NESTED_VIRTUALIZATION_FLAG = "nestedVirtualizationFlag";
     public static final String HYPERVISOR_TOOLS_VERSION = "hypervisortoolsversion";
+    public static final String DATA_DISK_CONTROLLER = "dataDiskController";
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/core/src/org/apache/cloudstack/storage/command/AttachCommand.java
----------------------------------------------------------------------
diff --git a/core/src/org/apache/cloudstack/storage/command/AttachCommand.java b/core/src/org/apache/cloudstack/storage/command/AttachCommand.java
index 34aaac9..1f11352 100644
--- a/core/src/org/apache/cloudstack/storage/command/AttachCommand.java
+++ b/core/src/org/apache/cloudstack/storage/command/AttachCommand.java
@@ -19,18 +19,35 @@
 
 package org.apache.cloudstack.storage.command;
 
+import java.util.Map;
+
+import com.cloud.agent.api.Command;
 import com.cloud.agent.api.to.DiskTO;
 
 public final class AttachCommand extends StorageSubSystemCommand {
     private DiskTO disk;
     private String vmName;
-    private boolean inSeq;
+    private boolean inSeq = false;
+    private Map<String, String> controllerInfo;
 
     public AttachCommand(final DiskTO disk, final String vmName) {
         super();
         this.disk = disk;
         this.vmName = vmName;
     }
+    public AttachCommand(DiskTO disk, String vmName, Map<String, String> controllerInfo) {
+        super();
+        this.disk = disk;
+        this.vmName = vmName;
+        this.controllerInfo = controllerInfo;
+    }
+
+    public Map<String, String> getControllerInfo() {
+        return controllerInfo;
+    }
+    public void setControllerInfo(Map<String, String> controllerInfo) {
+        this.controllerInfo = controllerInfo;
+    }
 
     @Override
     public boolean executeInSequence() {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java
index 62d82c2..7254eb1 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java
@@ -68,6 +68,7 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.hypervisor.HypervisorGuru;
 import com.cloud.hypervisor.HypervisorGuruBase;
 import com.cloud.hypervisor.vmware.manager.VmwareManager;
+import com.cloud.hypervisor.vmware.mo.DiskControllerType;
 import com.cloud.hypervisor.vmware.mo.VirtualEthernetCardType;
 import com.cloud.network.Network.Provider;
 import com.cloud.network.Network.Service;
@@ -207,6 +208,12 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co
                 details.put(VmDetailConstants.ROOK_DISK_CONTROLLER, _vmwareMgr.getRootDiskController());
             }
         }
+        String diskController = details.get(VmDetailConstants.DATA_DISK_CONTROLLER);
+        if (userVm) {
+            if (diskController == null) {
+                details.put(VmDetailConstants.DATA_DISK_CONTROLLER, DiskControllerType.lsilogic.toString());
+            }
+        }
 
         List<NicProfile> nicProfiles = vm.getNics();
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java
index 35e275e..72ee218 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java
@@ -78,5 +78,6 @@ public interface VmwareManager {
 
     boolean isLegacyZone(long dcId);
 
+    public String getDataDiskController();
     boolean hasNexusVSM(Long clusterId);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java
index eb0ec55..1b934b7 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java
@@ -188,6 +188,8 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
 
     private String _rootDiskController = DiskControllerType.ide.toString();
 
+    private String _dataDiskController = DiskControllerType.osdefault.toString();
+
     private final Map<String, String> _storageMounts = new HashMap<String, String>();
 
     private final Random _rand = new Random(System.currentTimeMillis());
@@ -486,6 +488,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
         params.put("service.console.name", _serviceConsoleName);
         params.put("management.portgroup.name", _managemetPortGroupName);
         params.put("vmware.root.disk.controller", _rootDiskController);
+        params.put("vmware.data.disk.controller", _dataDiskController);
         params.put("vmware.recycle.hung.wokervm", _recycleHungWorker);
         params.put("ports.per.dvportgroup", _portsPerDvPortGroup);
     }
@@ -939,6 +942,11 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw
     }
 
     @Override
+    public String getDataDiskController() {
+        return _dataDiskController;
+    }
+
+    @Override
     public int getVcenterSessionTimeout() {
         return _vCenterSessionTimeout;
     }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.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 da97cdd..e85191c 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
@@ -80,6 +80,7 @@ import com.vmware.vim25.VirtualDeviceBackingInfo;
 import com.vmware.vim25.VirtualDeviceConfigSpec;
 import com.vmware.vim25.VirtualDeviceConfigSpecOperation;
 import com.vmware.vim25.VirtualDisk;
+import com.vmware.vim25.VirtualDiskFlatVer2BackingInfo;
 import com.vmware.vim25.VirtualEthernetCard;
 import com.vmware.vim25.VirtualEthernetCardDistributedVirtualPortBackingInfo;
 import com.vmware.vim25.VirtualEthernetCardNetworkBackingInfo;
@@ -594,7 +595,18 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                     s_logger.trace("resize volume done (failed)");
                 throw new Exception("No such disk device: " + path);
             }
+            // IDE virtual disk cannot be re-sized if VM is running
+            if (vdisk.second() != null && vdisk.second().contains("ide")) {
+                throw new Exception("Re-sizing a virtual disk over IDE controller is not supported in VMware hypervisor. " +
+                            "Please re-try when virtual disk is attached to a VM using SCSI controller.");
+            }
+
             VirtualDisk disk = vdisk.first();
+            String vmdkAbsFile = getAbsoluteVmdkFile(disk);
+            if (vmdkAbsFile != null && !vmdkAbsFile.isEmpty()) {
+                vmMo.updateAdapterTypeIfRequired(vmdkAbsFile);
+            }
+
             disk.setCapacityInKB(newSize);
 
             VirtualMachineConfigSpec vmConfigSpec = new VirtualMachineConfigSpec();
@@ -1325,6 +1337,63 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
         return new ScaleVmAnswer(cmd, true, null);
     }
 
+    protected void ensureDiskControllers(VirtualMachineMO vmMo, Pair<String, String> controllerInfo) throws Exception {
+        if (vmMo == null) {
+            return;
+        }
+
+        String msg;
+        String rootDiskController = controllerInfo.first();
+        String dataDiskController = controllerInfo.second();
+        String scsiDiskController;
+        String recommendedDiskController = null;
+
+        if (VmwareHelper.isControllerOsRecommended(dataDiskController) || VmwareHelper.isControllerOsRecommended(rootDiskController)) {
+            recommendedDiskController = vmMo.getRecommendedDiskController(null);
+        }
+        scsiDiskController = HypervisorHostHelper.getScsiController(new Pair<String, String>(rootDiskController, dataDiskController), recommendedDiskController);
+        if (scsiDiskController == null) {
+            return;
+        }
+
+        vmMo.getScsiDeviceControllerKeyNoException();
+        // This VM needs SCSI controllers.
+        // Get count of existing scsi controllers. Helps not to attempt to create more than the maximum allowed 4
+        // Get maximum among the bus numbers in use by scsi controllers. Safe to pick maximum, because we always go sequential allocating bus numbers.
+        Ternary<Integer, Integer, DiskControllerType> scsiControllerInfo = vmMo.getScsiControllerInfo();
+        int requiredNumScsiControllers = VmwareHelper.MAX_SCSI_CONTROLLER_COUNT - scsiControllerInfo.first();
+        int availableBusNum = scsiControllerInfo.second() + 1; // method returned current max. bus number
+
+        if (requiredNumScsiControllers == 0) {
+            return;
+        }
+        if (scsiControllerInfo.first() > 0) {
+            // For VMs which already have a SCSI controller, do NOT attempt to add any more SCSI controllers & return the sub type.
+            // For Legacy VMs would have only 1 LsiLogic Parallel SCSI controller, and doesn't require more.
+            // For VMs created post device ordering support, 4 SCSI subtype controllers are ensured during deployment itself. No need to add more.
+            // For fresh VM deployment only, all required controllers should be ensured.
+            return;
+        }
+        ensureScsiDiskControllers(vmMo, scsiDiskController, requiredNumScsiControllers, availableBusNum);
+    }
+
+    private void ensureScsiDiskControllers(VirtualMachineMO vmMo, String scsiDiskController, int requiredNumScsiControllers, int availableBusNum) throws Exception {
+        // Pick the sub type of scsi
+        if (DiskControllerType.getType(scsiDiskController) == DiskControllerType.pvscsi) {
+            if (!vmMo.isPvScsiSupported()) {
+                String msg = "This VM doesn't support Vmware Paravirtual SCSI controller for virtual disks, because the virtual hardware version is less than 7.";
+                throw new Exception(msg);
+            }
+            vmMo.ensurePvScsiDeviceController(requiredNumScsiControllers, availableBusNum);
+        } else if (DiskControllerType.getType(scsiDiskController) == DiskControllerType.lsisas1068) {
+            vmMo.ensureLsiLogicSasDeviceControllers(requiredNumScsiControllers, availableBusNum);
+        } else if (DiskControllerType.getType(scsiDiskController) == DiskControllerType.buslogic) {
+            vmMo.ensureBusLogicDeviceControllers(requiredNumScsiControllers, availableBusNum);
+        } else if (DiskControllerType.getType(scsiDiskController) == DiskControllerType.lsilogic) {
+            vmMo.ensureScsiDeviceControllers(requiredNumScsiControllers, availableBusNum);
+        }
+    }
+
     protected StartAnswer execute(StartCommand cmd) {
         if (s_logger.isInfoEnabled()) {
             s_logger.info("Executing resource StartCommand: " + _gson.toJson(cmd));
@@ -1340,7 +1409,29 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
         Pair<String, String> names = composeVmNames(vmSpec);
         String vmInternalCSName = names.first();
         String vmNameOnVcenter = names.second();
+        String dataDiskController = vmSpec.getDetails().get(VmDetailConstants.DATA_DISK_CONTROLLER);
+        String rootDiskController = vmSpec.getDetails().get(VmDetailConstants.ROOK_DISK_CONTROLLER);
+
+        // If root disk controller is scsi, then data disk controller would also be scsi instead of using 'osdefault'
+        // This helps avoid mix of different scsi subtype controllers in instance.
+        if (DiskControllerType.lsilogic == DiskControllerType.getType(rootDiskController)) {
+            dataDiskController = DiskControllerType.scsi.toString();
+        }
 
+        // Validate the controller types
+        dataDiskController = DiskControllerType.getType(dataDiskController).toString();
+        rootDiskController = DiskControllerType.getType(rootDiskController).toString();
+
+        if (DiskControllerType.getType(rootDiskController) == DiskControllerType.none) {
+            throw new CloudRuntimeException("Invalid root disk controller detected : " + rootDiskController);
+        }
+        if (DiskControllerType.getType(dataDiskController) == DiskControllerType.none) {
+            throw new CloudRuntimeException("Invalid data disk controller detected : " + dataDiskController);
+        }
+
+        Pair<String, String> controllerInfo = new Pair<String, String>(rootDiskController, dataDiskController);
+
+        Boolean systemVm = vmSpec.getType().isUsedBySystem();
         // Thus, vmInternalCSName always holds i-x-y, the cloudstack generated internal VM name.
         VmwareContext context = getServiceContext();
         DatacenterMO dcMo = null;
@@ -1379,6 +1470,9 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
 
             VirtualMachineDiskInfoBuilder diskInfoBuilder = null;
             VirtualMachineMO vmMo = hyperHost.findVmOnHyperHost(vmInternalCSName);
+            DiskControllerType systemVmScsiControllerType = DiskControllerType.lsilogic;
+            int firstScsiControllerBusNum = 0;
+            int numScsiControllerForSystemVm = 1;
             boolean hasSnapshot = false;
             if (vmMo != null) {
                 s_logger.info("VM " + vmInternalCSName + " already exists, tear down devices for reconfiguration");
@@ -1392,7 +1486,11 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                     vmMo.tearDownDevices(new Class<?>[] {VirtualDisk.class, VirtualEthernetCard.class});
                 else
                     vmMo.tearDownDevices(new Class<?>[] {VirtualEthernetCard.class});
-                vmMo.ensureScsiDeviceController();
+                if (systemVm) {
+                    ensureScsiDiskControllers(vmMo, systemVmScsiControllerType.toString(), numScsiControllerForSystemVm, firstScsiControllerBusNum);
+                } else {
+                    ensureDiskControllers(vmMo, controllerInfo);
+                }
             } else {
                 ManagedObjectReference morDc = hyperHost.getHyperHostDatacenter();
                 assert (morDc != null);
@@ -1414,7 +1512,13 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                         vmMo.tearDownDevices(new Class<?>[] {VirtualDisk.class, VirtualEthernetCard.class});
                     else
                         vmMo.tearDownDevices(new Class<?>[] {VirtualEthernetCard.class});
-                    vmMo.ensureScsiDeviceController();
+
+                    if (systemVm) {
+                        // System volumes doesn't require more than 1 SCSI controller as there is no requirement for data volumes.
+                        ensureScsiDiskControllers(vmMo, systemVmScsiControllerType.toString(), numScsiControllerForSystemVm, firstScsiControllerBusNum);
+                    } else {
+                        ensureDiskControllers(vmMo, controllerInfo);
+                    }
                 } else {
                     // If a VM with the same name is found in a different cluster in the DC, unregister the old VM and configure a new VM (cold-migration).
                     VirtualMachineMO existingVmInDc = dcMo.findVm(vmInternalCSName);
@@ -1458,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)) {
+                            translateGuestOsIdentifier(vmSpec.getArch(), vmSpec.getOs(), vmSpec.getPlatformEmulator()).value(), rootDiskDataStoreDetails.first(), false, controllerInfo, systemVm)) {
                         throw new Exception("Failed to create VM. vmName: " + vmInternalCSName);
                     }
                 }
@@ -1520,7 +1624,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
             int scsiUnitNumber = 0;
             int nicUnitNumber = 0;
             int ideControllerKey = vmMo.getIDEDeviceControllerKey();
-            int scsiControllerKey = vmMo.getScsiDeviceControllerKey();
+            int scsiControllerKey = vmMo.getGenericScsiDeviceControllerKeyNoException();
             int controllerKey;
 
             //
@@ -1619,7 +1723,31 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
 
                 VirtualMachineDiskInfo matchingExistingDisk = getMatchingExistingDisk(diskInfoBuilder, vol, hyperHost, context);
                 controllerKey = getDiskController(matchingExistingDisk, vol, vmSpec, ideControllerKey, scsiControllerKey);
+                String diskController = getDiskController(vmMo, matchingExistingDisk, vol, new Pair<String, String>(rootDiskController, dataDiskController));
 
+                if (DiskControllerType.getType(diskController) == DiskControllerType.osdefault) {
+                    diskController = vmMo.getRecommendedDiskController(null);
+                }
+                if (DiskControllerType.getType(diskController) == DiskControllerType.ide) {
+                    controllerKey = vmMo.getIDEControllerKey(ideUnitNumber);
+                    if (vol.getType() == Volume.Type.DATADISK) {
+                        // Could be result of flip due to user configured setting or "osdefault" for data disks
+                        // Ensure maximum of 2 data volumes over IDE controller, 3 includeing root volume
+                        if (vmMo.getNumberOfVirtualDisks() > 3) {
+                            throw new CloudRuntimeException("Found more than 3 virtual disks attached to this VM [" + vmMo.getVmName() + "]. Unable to implement the disks over "
+                                    + diskController + " controller, as maximum number of devices supported over IDE controller is 4 includeing CDROM device.");
+                        }
+                    }
+                } else {
+                    controllerKey = vmMo.getScsiDiskControllerKeyNoException(diskController);
+                    if (controllerKey == -1) {
+                        // This may happen for ROOT legacy VMs which doesn't have recommended disk controller when global configuration parameter 'vmware.root.disk.controller' is set to "osdefault"
+                        // Retrieve existing controller and use.
+                        Ternary<Integer, Integer, DiskControllerType> vmScsiControllerInfo = vmMo.getScsiControllerInfo();
+                        DiskControllerType existingControllerType = vmScsiControllerInfo.third();
+                        controllerKey = vmMo.getScsiDiskControllerKeyNoException(existingControllerType.toString());
+                    }
+                }
                 if (!hasSnapshot) {
                     deviceConfigSpecArray[i] = new VirtualDeviceConfigSpec();
 
@@ -1648,7 +1776,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                     VirtualDevice device = VmwareHelper.prepareDiskDevice(vmMo, null, controllerKey,
                             diskChain,
                             volumeDsDetails.first(),
-                            (controllerKey == ideControllerKey) ? ideUnitNumber++ : scsiUnitNumber++, i + 1);
+                            (controllerKey == vmMo.getIDEControllerKey(ideUnitNumber)) ? ((ideUnitNumber++) % VmwareHelper.MAX_IDE_CONTROLLER_COUNT) : scsiUnitNumber++, i + 1);
 
                     deviceConfigSpecArray[i].setDevice(device);
                     deviceConfigSpecArray[i].setOperation(VirtualDeviceConfigSpecOperation.ADD);
@@ -1660,7 +1788,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                 } else {
                     if (controllerKey == scsiControllerKey && VmwareHelper.isReservedScsiDeviceNumber(scsiUnitNumber))
                         scsiUnitNumber++;
-                    if (controllerKey == ideControllerKey)
+                    if (controllerKey == vmMo.getIDEControllerKey(ideUnitNumber))
                         ideUnitNumber++;
                     else
                         scsiUnitNumber++;
@@ -2187,6 +2315,36 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
         return controllerKey;
     }
 
+    private String getDiskController(VirtualMachineMO vmMo, VirtualMachineDiskInfo matchingExistingDisk, DiskTO vol, Pair<String, String> controllerInfo) throws Exception {
+        int controllerKey;
+        DiskControllerType controllerType = DiskControllerType.none;
+        if (matchingExistingDisk != null) {
+            String currentBusName = matchingExistingDisk.getDiskDeviceBusName();
+            if (currentBusName != null) {
+                s_logger.info("Chose disk controller based on existing information: " + currentBusName);
+                if (currentBusName.startsWith("ide")) {
+                    controllerType = DiskControllerType.ide;
+                } else if (currentBusName.startsWith("scsi")) {
+                    controllerType = DiskControllerType.scsi;
+                }
+            }
+            if (controllerType == DiskControllerType.scsi || controllerType == DiskControllerType.none) {
+                Ternary<Integer, Integer, DiskControllerType> vmScsiControllerInfo = vmMo.getScsiControllerInfo();
+                controllerType = vmScsiControllerInfo.third();
+            }
+            return controllerType.toString();
+        }
+
+        if (vol.getType() == Volume.Type.ROOT) {
+            s_logger.info("Chose disk controller for vol " + vol.getType() + " -> " + controllerInfo.first()
+                    + ", based on root disk controller settings at global configuration setting.");
+            return controllerInfo.first();
+        } else {
+            s_logger.info("Chose disk controller for vol " + vol.getType() + " -> " + controllerInfo.second()
+                    + ", based on default data disk controller setting i.e. Operating system recommended."); // Need to bring in global configuration setting & template level setting.
+            return controllerInfo.second();
+        }
+    }
     private void postDiskConfigBeforeStart(VirtualMachineMO vmMo, VirtualMachineTO vmSpec, DiskTO[] sortedDisks, int ideControllerKey,
             int scsiControllerKey, Map<String, String> iqnToPath, VmwareHypervisorHost hyperHost, VmwareContext context) throws Exception {
         VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder();
@@ -3127,8 +3285,13 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                     relocateSpec.setDatastore(morTgtDatastore);
                 }
                 diskLocator = new VirtualMachineRelocateSpecDiskLocator();
-                diskLocator.setDatastore(morTgtDatastore);
-                int diskId = getVirtualDiskInfo(vmMo, volume.getPath() + ".vmdk");
+                diskLocator.setDatastore(morDsAtSource);
+                Pair<VirtualDisk, String> diskInfo = getVirtualDiskInfo(vmMo, volume.getPath() + ".vmdk");
+                String vmdkAbsFile = getAbsoluteVmdkFile(diskInfo.first());
+                if (vmdkAbsFile != null && !vmdkAbsFile.isEmpty()) {
+                    vmMo.updateAdapterTypeIfRequired(vmdkAbsFile);
+                }
+                int diskId = diskInfo.first().getKey();
                 diskLocator.setDiskId(diskId);
 
                 diskLocators.add(diskLocator);
@@ -3295,7 +3458,13 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
 
             DatastoreMO targetDsMo = new DatastoreMO(srcHyperHost.getContext(), morDs);
             String fullVolumePath = VmwareStorageLayoutHelper.getVmwareDatastorePathFromVmdkFileName(targetDsMo, vmName, volumePath + ".vmdk");
-            int diskId = getVirtualDiskInfo(vmMo, volumePath + ".vmdk");
+            Pair<VirtualDisk, String> diskInfo = getVirtualDiskInfo(vmMo, volumePath + ".vmdk");
+            String vmdkAbsFile = getAbsoluteVmdkFile(diskInfo.first());
+            if (vmdkAbsFile != null && !vmdkAbsFile.isEmpty()) {
+                vmMo.updateAdapterTypeIfRequired(vmdkAbsFile);
+            }
+            int diskId = diskInfo.first().getKey();
+
             diskLocator = new VirtualMachineRelocateSpecDiskLocator();
             diskLocator.setDatastore(morDs);
             diskLocator.setDiskId(diskId);
@@ -3354,12 +3523,12 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
         }
     }
 
-    private int getVirtualDiskInfo(VirtualMachineMO vmMo, String srcDiskName) throws Exception {
+    private Pair<VirtualDisk, String> getVirtualDiskInfo(VirtualMachineMO vmMo, String srcDiskName) throws Exception {
         Pair<VirtualDisk, String> deviceInfo = vmMo.getDiskDevice(srcDiskName);
         if (deviceInfo == null) {
             throw new Exception("No such disk device: " + srcDiskName);
         }
-        return deviceInfo.first().getKey();
+        return deviceInfo;
     }
 
     private VmwareHypervisorHost getTargetHyperHost(DatacenterMO dcMo, String destIp) throws Exception {
@@ -4962,8 +5131,10 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
             value = (String)params.get("vmware.root.disk.controller");
             if (value != null && value.equalsIgnoreCase("scsi"))
                 _rootDiskController = DiskControllerType.scsi;
-            else
+            else if (value != null && value.equalsIgnoreCase("ide"))
                 _rootDiskController = DiskControllerType.ide;
+            else
+                _rootDiskController = DiskControllerType.osdefault;
 
             Integer intObj = (Integer)params.get("ports.per.dvportgroup");
             if (intObj != null)
@@ -5187,4 +5358,14 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
             return new Answer(cmd, false, msg);
         }
     }
+
+    private String getAbsoluteVmdkFile(VirtualDisk disk) {
+        String vmdkAbsFile = null;
+        VirtualDeviceBackingInfo backingInfo = disk.getBacking();
+        if (backingInfo instanceof VirtualDiskFlatVer2BackingInfo) {
+            VirtualDiskFlatVer2BackingInfo diskBackingInfo = (VirtualDiskFlatVer2BackingInfo)backingInfo;
+            vmdkAbsFile = diskBackingInfo.getFileName();
+        }
+        return vmdkAbsFile;
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
----------------------------------------------------------------------
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 cc70715..fa2f369 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
@@ -75,12 +75,14 @@ import com.cloud.agent.api.to.DataTO;
 import com.cloud.agent.api.to.DiskTO;
 import com.cloud.agent.api.to.NfsTO;
 import com.cloud.hypervisor.vmware.manager.VmwareHostService;
+import com.cloud.hypervisor.vmware.manager.VmwareManager;
 import com.cloud.hypervisor.vmware.manager.VmwareStorageMount;
 import com.cloud.hypervisor.vmware.mo.ClusterMO;
 import com.cloud.hypervisor.vmware.mo.CustomFieldConstants;
 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.HostDatastoreSystemMO;
 import com.cloud.hypervisor.vmware.mo.HostMO;
 import com.cloud.hypervisor.vmware.mo.HostStorageSystemMO;
@@ -104,6 +106,7 @@ import com.cloud.utils.Ternary;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 import com.cloud.vm.VirtualMachine.PowerState;
+import com.cloud.vm.VmDetailConstants;
 
 public class VmwareStorageProcessor implements StorageProcessor {
     private static final Logger s_logger = Logger.getLogger(VmwareStorageProcessor.class);
@@ -1292,10 +1295,10 @@ public class VmwareStorageProcessor implements StorageProcessor {
         String storageHost = details.get(DiskTO.STORAGE_HOST);
         int storagePort = Integer.parseInt(details.get(DiskTO.STORAGE_PORT));
 
-        return this.attachVolume(cmd, cmd.getDisk(), true, isManaged, cmd.getVmName(), iScsiName, storageHost, storagePort);
+        return this.attachVolume(cmd, cmd.getDisk(), true, isManaged, cmd.getVmName(), iScsiName, storageHost, storagePort, cmd.getControllerInfo());
     }
 
-    private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean isManaged, String vmName, String iScsiName, String storageHost, int storagePort) {
+    private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean isManaged, String vmName, String iScsiName, String storageHost, int storagePort, Map<String, String> controllerInfo) {
         VolumeObjectTO volumeTO = (VolumeObjectTO)disk.getData();
         DataStoreTO primaryStore = volumeTO.getDataStore();
         try {
@@ -1360,7 +1363,24 @@ public class VmwareStorageProcessor implements StorageProcessor {
             AttachAnswer answer = new AttachAnswer(disk);
 
             if (isAttach) {
-                vmMo.attachDisk(new String[] { datastoreVolumePath }, morDs);
+                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);
+                }
+                vmMo.attachDisk(new String[] {datastoreVolumePath}, morDs, dataDiskController);
             } else {
                 vmMo.removeAllSnapshots();
                 vmMo.detachDisk(datastoreVolumePath, false);
@@ -1504,7 +1524,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
 
     @Override
     public Answer dettachVolume(DettachCommand cmd) {
-        return this.attachVolume(cmd, cmd.getDisk(), false, cmd.isManaged(), cmd.getVmName(), cmd.get_iScsiName(), cmd.getStorageHost(), cmd.getStoragePort());
+        return this.attachVolume(cmd, cmd.getDisk(), false, cmd.isManaged(), cmd.getVmName(), cmd.get_iScsiName(), cmd.getStorageHost(), cmd.getStoragePort(), null);
     }
 
     @Override
@@ -2277,4 +2297,28 @@ public class VmwareStorageProcessor implements StorageProcessor {
         templateUuid = templateUuid.replaceAll("-", "");
         return templateUuid;
     }
+
+    private String getControllerFromConfigurationSetting() throws Exception {
+        String diskController = null;
+        VmwareContext context = null;
+        try {
+            context = hostService.getServiceContext(null);
+            VmwareManager mgr = context.getStockObject(VmwareManager.CONTEXT_STOCK_NAME);
+            diskController = mgr.getDataDiskController();
+        } catch (Throwable e) {
+            if (e instanceof RemoteException) {
+                s_logger.warn("Encounter remote exception to vCenter, invalidate VMware session context");
+                hostService.invalidateServiceContext(context);
+            }
+
+            String details = "Failed to connect to vCenter due to " + VmwareHelper.getExceptionMessage(e);
+            s_logger.error(details, e);
+        }
+
+        return diskController;
+    }
+
+    private String getLegacyVmDataDiskController() throws Exception {
+        return DiskControllerType.lsilogic.toString();
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/server/src/com/cloud/storage/VolumeApiServiceImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
index 6fe0a14..a492670 100644
--- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
@@ -150,6 +150,7 @@ import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.VmDetailConstants;
 import com.cloud.vm.VmWork;
 import com.cloud.vm.VmWorkAttachVolume;
 import com.cloud.vm.VmWorkConstants;
@@ -2441,6 +2442,11 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                 details.put(DiskTO.CHAP_TARGET_USERNAME, chapInfo.getTargetUsername());
                 details.put(DiskTO.CHAP_TARGET_SECRET, chapInfo.getTargetSecret());
             }
+            _userVmDao.loadDetails(vm);
+            Map<String, String> controllerInfo = new HashMap<String, String>();
+            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);
 
             try {
                 answer = (AttachAnswer)_agentMgr.send(hostId, cmd);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/server/src/com/cloud/vm/UserVmManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java
index 32b0d3d..3f068af 100644
--- a/server/src/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/com/cloud/vm/UserVmManagerImpl.java
@@ -3371,8 +3371,17 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
                     if (guestOS.getDisplayName().toLowerCase().contains("apple mac os")) {
                         vm.setDetail("smc.present", "TRUE");
                         vm.setDetail(VmDetailConstants.ROOK_DISK_CONTROLLER, "scsi");
+                        vm.setDetail(VmDetailConstants.DATA_DISK_CONTROLLER, "scsi");
                         vm.setDetail("firmware", "efi");
                         s_logger.info("guestOS is OSX : overwrite root disk controller to scsi, use smc and efi");
+                    } else {
+                        String controllerSetting = _configDao.getValue("vmware.root.disk.controller");
+                        vm.setDetail(VmDetailConstants.ROOK_DISK_CONTROLLER, controllerSetting);
+                        if (controllerSetting.equalsIgnoreCase("scsi")) {
+                            vm.setDetail(VmDetailConstants.DATA_DISK_CONTROLLER, "scsi");
+                        } else {
+                            vm.setDetail(VmDetailConstants.DATA_DISK_CONTROLLER, "osdefault");
+                        }
                     }
                 }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java
----------------------------------------------------------------------
diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java b/vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java
index 1b72b73..b5f0500 100644
--- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java
+++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/ClusterMO.java
@@ -34,6 +34,7 @@ import com.vmware.vim25.ComputeResourceSummary;
 import com.vmware.vim25.CustomFieldStringValue;
 import com.vmware.vim25.DatastoreInfo;
 import com.vmware.vim25.DynamicProperty;
+import com.vmware.vim25.GuestOsDescriptor;
 import com.vmware.vim25.HostHardwareSummary;
 import com.vmware.vim25.HostIpRouteEntry;
 import com.vmware.vim25.HostRuntimeInfo;
@@ -46,9 +47,11 @@ import com.vmware.vim25.OptionValue;
 import com.vmware.vim25.PropertyFilterSpec;
 import com.vmware.vim25.PropertySpec;
 import com.vmware.vim25.TraversalSpec;
+import com.vmware.vim25.VirtualMachineConfigOption;
 import com.vmware.vim25.VirtualMachineConfigSpec;
 
 import com.cloud.hypervisor.vmware.util.VmwareContext;
+import com.cloud.hypervisor.vmware.util.VmwareHelper;
 import com.cloud.utils.Pair;
 import com.cloud.utils.exception.CloudRuntimeException;
 
@@ -271,7 +274,7 @@ public class ClusterMO extends BaseMO implements VmwareHypervisorHost {
 
     @Override
     public boolean createBlankVm(String vmName, String vmInternalCSName, int cpuCount, int cpuSpeedMHz, int cpuReservedMHz, boolean limitCpuUse, int memoryMB,
-            int memoryReserveMB, String guestOsIdentifier, ManagedObjectReference morDs, boolean snapshotDirToParent) throws Exception {
+            int memoryReserveMB, String guestOsIdentifier, ManagedObjectReference morDs, boolean snapshotDirToParent, Pair<String, String> controllerInfo, Boolean systemVm) throws Exception {
 
         if (s_logger.isTraceEnabled())
             s_logger.trace("vCenter API trace - createBlankVm(). target MOR: " + _mor.getValue() + ", vmName: " + vmName + ", cpuCount: " + cpuCount + ", cpuSpeedMhz: " +
@@ -280,7 +283,7 @@ public class ClusterMO extends BaseMO implements VmwareHypervisorHost {
 
         boolean result =
                 HypervisorHostHelper.createBlankVm(this, vmName, vmInternalCSName, cpuCount, cpuSpeedMHz, cpuReservedMHz, limitCpuUse, memoryMB, memoryReserveMB,
-                        guestOsIdentifier, morDs, snapshotDirToParent);
+                        guestOsIdentifier, morDs, snapshotDirToParent, controllerInfo, systemVm);
 
         if (s_logger.isTraceEnabled())
             s_logger.trace("vCenter API trace - createBlankVm() done");
@@ -582,4 +585,32 @@ public class ClusterMO extends BaseMO implements VmwareHypervisorHost {
         // LicenseAssignmentManager deals with only host/vcenter licenses only. Has nothing todo with cluster
         throw new CloudRuntimeException("Unable to get LicenseAssignmentManager at cluster level");
     }
+    private ManagedObjectReference getEnvironmentBrowser() throws Exception {
+        if (_environmentBrowser == null) {
+            _environmentBrowser = _context.getVimClient().getMoRefProp(_mor, "environmentBrowser");
+        }
+        return _environmentBrowser;
+    }
+    @Override
+    public String getRecommendedDiskController(String guestOsId) throws Exception {
+        VirtualMachineConfigOption vmConfigOption = _context.getService().queryConfigOption(getEnvironmentBrowser(), null, null);
+        GuestOsDescriptor guestOsDescriptor = null;
+        String diskController = null;
+        List<GuestOsDescriptor> guestDescriptors = vmConfigOption.getGuestOSDescriptor();
+        for (GuestOsDescriptor descriptor : guestDescriptors) {
+            if (guestOsId != null && guestOsId.equalsIgnoreCase(descriptor.getId())) {
+                guestOsDescriptor = descriptor;
+                break;
+            }
+        }
+        if (guestOsDescriptor != null) {
+            diskController = VmwareHelper.getRecommendedDiskControllerFromDescriptor(guestOsDescriptor);
+            s_logger.debug("Retrieved recommended disk controller for guest OS : " + guestOsId + " in cluster " + getHyperHostName() + " : " + diskController);
+            return diskController;
+        } else {
+            String msg = "Unable to retrieve recommended disk controller for guest OS : " + guestOsId + " in cluster " + getHyperHostName();
+            s_logger.error(msg);
+            throw new CloudRuntimeException(msg);
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/vmware-base/src/com/cloud/hypervisor/vmware/mo/DiskControllerType.java
----------------------------------------------------------------------
diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/DiskControllerType.java b/vmware-base/src/com/cloud/hypervisor/vmware/mo/DiskControllerType.java
index def584d..3e6b8bb 100644
--- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/DiskControllerType.java
+++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/DiskControllerType.java
@@ -17,5 +17,34 @@
 package com.cloud.hypervisor.vmware.mo;
 
 public enum DiskControllerType {
-    ide, scsi
+    ide,
+    scsi,
+    osdefault,
+    lsilogic,
+    lsisas1068,
+    buslogic,
+    pvscsi,
+    none;
+    public static DiskControllerType getType(String diskController) {
+        if (diskController == null || diskController.equalsIgnoreCase("osdefault")) {
+            return DiskControllerType.osdefault;
+        } else if (diskController.equalsIgnoreCase("vim.vm.device.VirtualLsiLogicSASController") || diskController.equalsIgnoreCase("VirtualLsiLogicSASController")
+                || diskController.equalsIgnoreCase(ScsiDiskControllerType.LSILOGIC_SAS)) {
+            return DiskControllerType.lsisas1068;
+        } else if (diskController.equalsIgnoreCase("vim.vm.device.VirtualLsiLogicController") || diskController.equalsIgnoreCase("VirtualLsiLogicController")
+                || diskController.equalsIgnoreCase(ScsiDiskControllerType.LSILOGIC_PARALLEL) || diskController.equalsIgnoreCase("scsi")) {
+            return DiskControllerType.lsilogic;
+        } else if (diskController.equalsIgnoreCase("vim.vm.device.VirtualIDEController") || diskController.equalsIgnoreCase("VirtualIDEController")
+                || diskController.equalsIgnoreCase("ide")) {
+            return DiskControllerType.ide;
+        } else if (diskController.equalsIgnoreCase("vim.vm.device.ParaVirtualSCSIController") || diskController.equalsIgnoreCase("ParaVirtualSCSIController")
+                || diskController.equalsIgnoreCase(ScsiDiskControllerType.VMWARE_PARAVIRTUAL)) {
+            return DiskControllerType.pvscsi;
+        } else if (diskController.equalsIgnoreCase("vim.vm.device.VirtualBusLogicController") || diskController.equalsIgnoreCase("VirtualBusLogicController")
+                || diskController.equalsIgnoreCase(ScsiDiskControllerType.BUSLOGIC)) {
+            return DiskControllerType.buslogic;
+        } else {
+            return DiskControllerType.none;
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java
----------------------------------------------------------------------
diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java b/vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java
index 4513b52..d12585e 100644
--- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java
+++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java
@@ -734,16 +734,17 @@ public class HostMO extends BaseMO implements VmwareHypervisorHost {
 
     @Override
     public boolean createBlankVm(String vmName, String vmInternalCSName, int cpuCount, int cpuSpeedMHz, int cpuReservedMHz, boolean limitCpuUse, int memoryMB,
-            int memoryReserveMB, String guestOsIdentifier, ManagedObjectReference morDs, boolean snapshotDirToParent) throws Exception {
+            int memoryReserveMB, String guestOsIdentifier, ManagedObjectReference morDs, boolean snapshotDirToParent, Pair<String, String> controllerInfo, Boolean systemVm) throws Exception {
 
         if (s_logger.isTraceEnabled())
             s_logger.trace("vCenter API trace - createBlankVm(). target MOR: " + _mor.getValue() + ", vmName: " + vmName + ", cpuCount: " + cpuCount + ", cpuSpeedMhz: " +
                     cpuSpeedMHz + ", cpuReservedMHz: " + cpuReservedMHz + ", limitCpu: " + limitCpuUse + ", memoryMB: " + memoryMB + ", guestOS: " + guestOsIdentifier +
-                    ", datastore: " + morDs.getValue() + ", snapshotDirToParent: " + snapshotDirToParent);
+                    ", datastore: " + morDs.getValue() + ", snapshotDirToParent: " + snapshotDirToParent +
+                    ", controllerInfo:[" + controllerInfo.first() + "," + controllerInfo.second() + "], systemvm: " + systemVm);
 
         boolean result =
                 HypervisorHostHelper.createBlankVm(this, vmName, vmInternalCSName, cpuCount, cpuSpeedMHz, cpuReservedMHz, limitCpuUse, memoryMB, memoryReserveMB,
-                        guestOsIdentifier, morDs, snapshotDirToParent);
+                        guestOsIdentifier, morDs, snapshotDirToParent, controllerInfo, systemVm);
 
         if (s_logger.isTraceEnabled())
             s_logger.trace("vCenter API trace - createBlankVm() done");
@@ -1052,6 +1053,16 @@ public class HostMO extends BaseMO implements VmwareHypervisorHost {
         }
     }
 
+    @Override
+    public String getRecommendedDiskController(String guestOsId) throws Exception {
+        ManagedObjectReference morParent = getParentMor();
+        if (morParent.getType().equals("ClusterComputeResource")) {
+            ClusterMO clusterMo = new ClusterMO(_context, morParent);
+            return clusterMo.getRecommendedDiskController(guestOsId);
+        }
+        return null;
+    }
+
     public String getHostManagementIp(String managementPortGroup) throws Exception {
         HostNetworkInfo netInfo = getHostNetworkInfo();
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
----------------------------------------------------------------------
diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java b/vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
index 44c6a85..67b4c5e 100644
--- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
+++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
@@ -68,18 +68,24 @@ import com.vmware.vim25.ObjectContent;
 import com.vmware.vim25.OvfCreateImportSpecParams;
 import com.vmware.vim25.OvfCreateImportSpecResult;
 import com.vmware.vim25.OvfFileItem;
+import com.vmware.vim25.ParaVirtualSCSIController;
 import com.vmware.vim25.VMwareDVSConfigSpec;
 import com.vmware.vim25.VMwareDVSPortSetting;
 import com.vmware.vim25.VMwareDVSPortgroupPolicy;
 import com.vmware.vim25.VMwareDVSPvlanConfigSpec;
 import com.vmware.vim25.VMwareDVSPvlanMapEntry;
+import com.vmware.vim25.VirtualBusLogicController;
+import com.vmware.vim25.VirtualController;
 import com.vmware.vim25.VirtualDeviceConfigSpec;
 import com.vmware.vim25.VirtualDeviceConfigSpecOperation;
+import com.vmware.vim25.VirtualIDEController;
 import com.vmware.vim25.VirtualLsiLogicController;
+import com.vmware.vim25.VirtualLsiLogicSASController;
 import com.vmware.vim25.VirtualMachineConfigSpec;
 import com.vmware.vim25.VirtualMachineFileInfo;
 import com.vmware.vim25.VirtualMachineGuestOsIdentifier;
 import com.vmware.vim25.VirtualMachineVideoCard;
+import com.vmware.vim25.VirtualSCSIController;
 import com.vmware.vim25.VirtualSCSISharing;
 import com.vmware.vim25.VmwareDistributedVirtualSwitchPvlanSpec;
 import com.vmware.vim25.VmwareDistributedVirtualSwitchVlanIdSpec;
@@ -1233,11 +1239,13 @@ public class HypervisorHostHelper {
     }
 
     public static boolean createBlankVm(VmwareHypervisorHost host, String vmName, String vmInternalCSName, int cpuCount, int cpuSpeedMHz, int cpuReservedMHz,
-            boolean limitCpuUse, int memoryMB, int memoryReserveMB, String guestOsIdentifier, ManagedObjectReference morDs, boolean snapshotDirToParent) throws Exception {
+            boolean limitCpuUse, int memoryMB, int memoryReserveMB, String guestOsIdentifier, ManagedObjectReference morDs, boolean snapshotDirToParent,
+            Pair<String, String> controllerInfo, Boolean systemVm) throws Exception {
 
         if (s_logger.isInfoEnabled())
             s_logger.info("Create blank VM. cpuCount: " + cpuCount + ", cpuSpeed(MHz): " + cpuSpeedMHz + ", mem(Mb): " + memoryMB);
 
+        VirtualDeviceConfigSpec controllerSpec = null;
         // VM config basics
         VirtualMachineConfigSpec vmConfig = new VirtualMachineConfigSpec();
         vmConfig.setName(vmName);
@@ -1246,14 +1254,33 @@ public class HypervisorHostHelper {
 
         VmwareHelper.setBasicVmConfig(vmConfig, cpuCount, cpuSpeedMHz, cpuReservedMHz, memoryMB, memoryReserveMB, guestOsIdentifier, limitCpuUse);
 
-        // Scsi controller
-        VirtualLsiLogicController scsiController = new VirtualLsiLogicController();
-        scsiController.setSharedBus(VirtualSCSISharing.NO_SHARING);
-        scsiController.setBusNumber(0);
-        scsiController.setKey(1);
-        VirtualDeviceConfigSpec scsiControllerSpec = new VirtualDeviceConfigSpec();
-        scsiControllerSpec.setDevice(scsiController);
-        scsiControllerSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD);
+        String recommendedController = host.getRecommendedDiskController(guestOsIdentifier);
+        String newRootDiskController = controllerInfo.first();
+        String newDataDiskController = controllerInfo.second();
+        if (DiskControllerType.getType(controllerInfo.first()) == DiskControllerType.osdefault) {
+            newRootDiskController = recommendedController;
+        }
+        if (DiskControllerType.getType(controllerInfo.second()) == DiskControllerType.osdefault) {
+            newDataDiskController = recommendedController;
+        }
+
+        Pair<String, String> updatedControllerInfo = new Pair<String, String>(newRootDiskController, newDataDiskController);
+        String scsiDiskController = HypervisorHostHelper.getScsiController(updatedControllerInfo, recommendedController);
+        // If there is requirement for a SCSI controller, ensure to create those.
+        if (scsiDiskController != null) {
+        int busNum = 0;
+            int maxControllerCount = VmwareHelper.MAX_SCSI_CONTROLLER_COUNT;
+            if (systemVm) {
+                maxControllerCount = 1;
+            }
+            while (busNum < maxControllerCount) {
+            VirtualDeviceConfigSpec scsiControllerSpec = new VirtualDeviceConfigSpec();
+                scsiControllerSpec = getControllerSpec(DiskControllerType.getType(scsiDiskController).toString(), busNum);
+
+            vmConfig.getDeviceChange().add(scsiControllerSpec);
+            busNum++;
+            }
+        }
 
         VirtualMachineFileInfo fileInfo = new VirtualMachineFileInfo();
         DatastoreMO dsMo = new DatastoreMO(host.getContext(), morDs);
@@ -1268,7 +1295,6 @@ public class HypervisorHostHelper {
         videoDeviceSpec.setDevice(videoCard);
         videoDeviceSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD);
 
-        vmConfig.getDeviceChange().add(scsiControllerSpec);
         vmConfig.getDeviceChange().add(videoDeviceSpec);
         if (host.createVm(vmConfig)) {
             // Here, when attempting to find the VM, we need to use the name
@@ -1298,6 +1324,34 @@ public class HypervisorHostHelper {
         return false;
     }
 
+    private static VirtualDeviceConfigSpec getControllerSpec(String diskController, int busNum) {
+        VirtualDeviceConfigSpec controllerSpec = new VirtualDeviceConfigSpec();
+        VirtualController controller = null;
+
+        if (diskController.equalsIgnoreCase(DiskControllerType.ide.toString())) {
+           controller = new VirtualIDEController();
+        } else if (DiskControllerType.pvscsi == DiskControllerType.getType(diskController)) {
+            controller = new ParaVirtualSCSIController();
+        } else if (DiskControllerType.lsisas1068 == DiskControllerType.getType(diskController)) {
+            controller = new VirtualLsiLogicSASController();
+        } else if (DiskControllerType.buslogic == DiskControllerType.getType(diskController)) {
+            controller = new VirtualBusLogicController();
+        } else if (DiskControllerType.lsilogic == DiskControllerType.getType(diskController)) {
+            controller = new VirtualLsiLogicController();
+        }
+
+        if (!diskController.equalsIgnoreCase(DiskControllerType.ide.toString())) {
+            ((VirtualSCSIController)controller).setSharedBus(VirtualSCSISharing.NO_SHARING);
+        }
+
+        controller.setBusNumber(busNum);
+        controller.setKey(busNum - VmwareHelper.MAX_SCSI_CONTROLLER_COUNT);
+
+        controllerSpec.setDevice(controller);
+        controllerSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD);
+
+        return controllerSpec;
+    }
     public static VirtualMachineMO createWorkerVM(VmwareHypervisorHost hyperHost, DatastoreMO dsMo, String vmName) throws Exception {
 
         // Allow worker VM to float within cluster so that we will have better chance to
@@ -1507,4 +1561,42 @@ public class HypervisorHostHelper {
             }
         }
     }
+
+    public static String getScsiController(Pair<String, String> controllerInfo, String recommendedController) {
+        String rootDiskController = controllerInfo.first();
+        String dataDiskController = controllerInfo.second();
+
+        // If "osdefault" is specified as controller type, then translate to actual recommended controller.
+        if (VmwareHelper.isControllerOsRecommended(rootDiskController)) {
+            rootDiskController = recommendedController;
+        }
+        if (VmwareHelper.isControllerOsRecommended(dataDiskController)) {
+            dataDiskController = recommendedController;
+        }
+
+        String scsiDiskController = null; //If any of the controller provided is SCSI then return it's sub-type.
+        if (isIdeController(rootDiskController) && isIdeController(dataDiskController)) {
+            //Default controllers would exist
+            return null;
+        } else if (isIdeController(rootDiskController) || isIdeController(dataDiskController)) {
+            // Only one of the controller types is IDE. Pick the other controller type to create controller.
+            if (isIdeController(rootDiskController)) {
+                scsiDiskController = dataDiskController;
+            } else {
+                scsiDiskController = rootDiskController;
+            }
+        } else if (DiskControllerType.getType(rootDiskController) != DiskControllerType.getType(dataDiskController)) {
+            // Both ROOT and DATA controllers are SCSI controllers but different sub-types, then prefer ROOT controller
+            scsiDiskController = rootDiskController;
+        } else {
+            // Both are SCSI controllers.
+            scsiDiskController = rootDiskController;
+        }
+        return scsiDiskController;
+    }
+
+    public static boolean isIdeController(String controller) {
+        return DiskControllerType.getType(controller) == DiskControllerType.ide;
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6a9956e0/vmware-base/src/com/cloud/hypervisor/vmware/mo/ScsiDiskControllerType.java
----------------------------------------------------------------------
diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/ScsiDiskControllerType.java b/vmware-base/src/com/cloud/hypervisor/vmware/mo/ScsiDiskControllerType.java
new file mode 100644
index 0000000..8b9a949
--- /dev/null
+++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/ScsiDiskControllerType.java
@@ -0,0 +1,24 @@
+//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 interface ScsiDiskControllerType {
+    public final static String LSILOGIC_PARALLEL = "lsilogic";
+    public final static String LSILOGIC_SAS = "lsisas1068";
+    public final static String BUSLOGIC = "buslogic";
+    public final static String VMWARE_PARAVIRTUAL = "pvscsi";
+}