cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sw...@apache.org
Subject [3/8] git commit: updated refs/heads/4.7 to 987e800
Date Mon, 02 May 2016 21:01:56 GMT
vmware: improve support for disks

- Improve disk chain usage while attaching, migrating disks
- Gets root disk controller based diskDeviceBusName from volume's chain info
- Refactor and move VirtualMachineDiskInfo to cloud-utils
- Allows mixing of scsi controller types
- Fixes a NPE case with map passed as null, for example in case of detach volume
  command
- Use a osdefault translator that allow use of recent os types added (enums of
  which) are not available in the sdk

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>


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

Branch: refs/heads/4.7
Commit: 8e4644e413777d0a58edd5405a928df8256fdbd9
Parents: 6f703c4
Author: Rohit Yadav <rohit.yadav@shapeblue.com>
Authored: Fri Feb 5 19:09:03 2016 +0100
Committer: Rohit Yadav <rohit.yadav@shapeblue.com>
Committed: Mon Apr 18 22:44:18 2016 +0530

----------------------------------------------------------------------
 api/src/com/cloud/vm/VmDetailConstants.java     |  2 +-
 .../com/cloud/hypervisor/guru/VMwareGuru.java   |  4 +-
 .../vmware/resource/VmwareResource.java         | 17 +++---
 .../resource/VmwareStorageProcessor.java        | 28 ++++------
 pom.xml                                         |  2 +-
 .../com/cloud/storage/VolumeApiServiceImpl.java | 36 ++++++++++++-
 server/src/com/cloud/vm/UserVmManager.java      |  2 +
 server/src/com/cloud/vm/UserVmManagerImpl.java  | 18 +++++--
 .../cloud/storage/VolumeApiServiceImplTest.java | 25 +++++++++
 server/test/com/cloud/vm/UserVmManagerTest.java | 21 ++++++++
 .../utils/volume/VirtualMachineDiskInfo.java    | 48 +++++++++++++++++
 .../volume/VirtualMachineDiskInfoTest.java      | 55 ++++++++++++++++++++
 .../vmware/mo/VirtualMachineDiskInfo.java       | 42 ---------------
 .../mo/VirtualMachineDiskInfoBuilder.java       |  2 +
 .../hypervisor/vmware/mo/VirtualMachineMO.java  |  2 +-
 15 files changed, 226 insertions(+), 78 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/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 d06ad67..d34afc1 100644
--- a/api/src/com/cloud/vm/VmDetailConstants.java
+++ b/api/src/com/cloud/vm/VmDetailConstants.java
@@ -19,7 +19,7 @@ package com.cloud.vm;
 public interface VmDetailConstants {
     public static final String KEYBOARD = "keyboard";
     public static final String NIC_ADAPTER = "nicAdapter";
-    public static final String ROOK_DISK_CONTROLLER = "rootDiskController";
+    public static final String ROOT_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/8e4644e4/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 fd96bc8..986000a 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/guru/VMwareGuru.java
@@ -200,10 +200,10 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru,
Co
             }
         }
 
-        String diskDeviceType = details.get(VmDetailConstants.ROOK_DISK_CONTROLLER);
+        String diskDeviceType = details.get(VmDetailConstants.ROOT_DISK_CONTROLLER);
         if (userVm) {
             if (diskDeviceType == null) {
-                details.put(VmDetailConstants.ROOK_DISK_CONTROLLER, _vmwareMgr.getRootDiskController());
+                details.put(VmDetailConstants.ROOT_DISK_CONTROLLER, _vmwareMgr.getRootDiskController());
             }
         }
         String diskController = details.get(VmDetailConstants.DATA_DISK_CONTROLLER);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/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 fdbc244..dd419f2 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
@@ -236,7 +236,7 @@ import com.cloud.hypervisor.vmware.mo.HypervisorHostHelper;
 import com.cloud.hypervisor.vmware.mo.NetworkDetails;
 import com.cloud.hypervisor.vmware.mo.TaskMO;
 import com.cloud.hypervisor.vmware.mo.VirtualEthernetCardType;
-import com.cloud.hypervisor.vmware.mo.VirtualMachineDiskInfo;
+import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
 import com.cloud.hypervisor.vmware.mo.VirtualMachineDiskInfoBuilder;
 import com.cloud.hypervisor.vmware.mo.VirtualMachineMO;
 import com.cloud.hypervisor.vmware.mo.VirtualSwitchType;
@@ -1412,7 +1412,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource,
Vmwa
         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);
+        String rootDiskController = vmSpec.getDetails().get(VmDetailConstants.ROOT_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.
@@ -1451,7 +1451,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource,
Vmwa
                 s_logger.error(msg);
                 throw new Exception(msg);
             }
-
+            String guestOsId = translateGuestOsIdentifier(vmSpec.getArch(), vmSpec.getOs(),
vmSpec.getPlatformEmulator()).value();
             DiskTO[] disks = validateDisks(vmSpec.getDisks());
             assert (disks.length > 0);
             NicTO[] nics = vmSpec.getNics();
@@ -1564,7 +1564,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);
                     }
                 }
@@ -1588,7 +1588,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),
@@ -2322,14 +2321,14 @@ public class VmwareResource implements StoragePoolResource, ServerResource,
Vmwa
 
         if (vol.getType() == Volume.Type.ROOT) {
             Map<String, String> vmDetails = vmSpec.getDetails();
-            if (vmDetails != null && vmDetails.get(VmDetailConstants.ROOK_DISK_CONTROLLER)
!= null) {
-                if (vmDetails.get(VmDetailConstants.ROOK_DISK_CONTROLLER).equalsIgnoreCase("scsi"))
{
+            if (vmDetails != null && vmDetails.get(VmDetailConstants.ROOT_DISK_CONTROLLER)
!= null) {
+                if (vmDetails.get(VmDetailConstants.ROOT_DISK_CONTROLLER).equalsIgnoreCase("scsi"))
{
                     s_logger.info("Chose disk controller for vol " + vol.getType() + " ->
scsi, based on root disk controller settings: " +
-                            vmDetails.get(VmDetailConstants.ROOK_DISK_CONTROLLER));
+                            vmDetails.get(VmDetailConstants.ROOT_DISK_CONTROLLER));
                     controllerKey = scsiControllerKey;
                 } else {
                     s_logger.info("Chose disk controller for vol " + vol.getType() + " ->
ide, based on root disk controller settings: " +
-                            vmDetails.get(VmDetailConstants.ROOK_DISK_CONTROLLER));
+                            vmDetails.get(VmDetailConstants.ROOT_DISK_CONTROLLER));
                     controllerKey = ideControllerKey;
                 }
             } else {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/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 fa2f369..567f857 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
@@ -33,6 +33,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 
+import com.google.common.base.Strings;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 
@@ -88,7 +89,7 @@ import com.cloud.hypervisor.vmware.mo.HostMO;
 import com.cloud.hypervisor.vmware.mo.HostStorageSystemMO;
 import com.cloud.hypervisor.vmware.mo.HypervisorHostHelper;
 import com.cloud.hypervisor.vmware.mo.NetworkDetails;
-import com.cloud.hypervisor.vmware.mo.VirtualMachineDiskInfo;
+import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
 import com.cloud.hypervisor.vmware.mo.VirtualMachineMO;
 import com.cloud.hypervisor.vmware.mo.VmwareHypervisorHost;
 import com.cloud.hypervisor.vmware.resource.VmwareResource;
@@ -1363,24 +1364,15 @@ public class VmwareStorageProcessor implements StorageProcessor {
             AttachAnswer answer = new AttachAnswer(disk);
 
             if (isAttach) {
-                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 = getLegacyVmDataDiskController();
+                if (controllerInfo != null &&
+                        !Strings.isNullOrEmpty(controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER)))
{
+                    diskController = controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER);
                 }
-                vmMo.attachDisk(new String[] {datastoreVolumePath}, morDs, dataDiskController);
+                if (DiskControllerType.getType(diskController) == DiskControllerType.osdefault)
{
+                    diskController = vmMo.getRecommendedDiskController(null);
+                }
+                vmMo.attachDisk(new String[] {datastoreVolumePath}, morDs, diskController);
             } else {
                 vmMo.removeAllSnapshots();
                 vmMo.detachDisk(datastoreVolumePath, false);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 3530569..af9eb76 100644
--- a/pom.xml
+++ b/pom.xml
@@ -92,7 +92,7 @@
     <cs.servlet.version>2.5</cs.servlet.version>
     <cs.jstl.version>1.2</cs.jstl.version>
     <cs.selenium.server.version>1.0-20081010.060147</cs.selenium.server.version>
-    <cs.vmware.api.version>5.5</cs.vmware.api.version>
+    <cs.vmware.api.version>6.0</cs.vmware.api.version>
     <org.springframework.version>3.2.12.RELEASE</org.springframework.version>
     <cs.mockito.version>1.9.5</cs.mockito.version>
     <cs.powermock.version>1.5.3</cs.powermock.version>

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/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 5a53f9c..fdd237c 100644
--- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
@@ -30,8 +30,10 @@ import javax.inject.Inject;
 
 import com.cloud.utils.EncryptionUtil;
 import com.cloud.utils.db.TransactionCallbackWithException;
+import com.google.common.base.Strings;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
+import com.google.gson.JsonParseException;
 
 import org.apache.cloudstack.api.command.user.volume.GetUploadParamsForVolumeCmd;
 import org.apache.cloudstack.api.response.GetUploadParamsResponse;
@@ -39,6 +41,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
 import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
 import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
 import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
+import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
 import org.apache.log4j.Logger;
 import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
@@ -112,12 +115,14 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.hypervisor.HypervisorCapabilitiesVO;
 import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
 import com.cloud.org.Grouping;
+import com.cloud.serializer.GsonHelper;
 import com.cloud.service.dao.ServiceOfferingDetailsDao;
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.storage.dao.SnapshotDao;
 import com.cloud.storage.dao.VMTemplateDao;
 import com.cloud.storage.dao.VolumeDao;
+import com.cloud.storage.dao.VolumeDetailsDao;
 import com.cloud.storage.snapshot.SnapshotApiService;
 import com.cloud.storage.snapshot.SnapshotManager;
 import com.cloud.template.TemplateManager;
@@ -146,6 +151,7 @@ import com.cloud.utils.db.UUIDManager;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.fsm.NoTransitionException;
 import com.cloud.utils.fsm.StateMachine2;
+import com.cloud.vm.UserVmManager;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
@@ -191,6 +197,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     @Inject
     VolumeDao _volsDao;
     @Inject
+    VolumeDetailsDao _volDetailDao;
+    @Inject
     HostDao _hostDao;
     @Inject
     SnapshotDao _snapshotDao;
@@ -240,6 +248,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
     VmWorkJobDao _workJobDao;
     @Inject
     ClusterDetailsDao _clusterDetailsDao;
+    @Inject
+    UserVmManager _userVmMgr;
+    protected Gson _gson;
 
     private List<StoragePoolAllocator> _storagePoolAllocators;
 
@@ -253,6 +264,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
 
     protected VolumeApiServiceImpl() {
         _volStateMachine = Volume.State.getStateMachine();
+        _gson = GsonHelper.getGsonLogger();
     }
 
     /*
@@ -1835,6 +1847,26 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         }
     }
 
+    public void updateMissingRootDiskController(final VMInstanceVO vm, final String rootVolChainInfo)
{
+        if (vm == null || !VirtualMachine.Type.User.equals(vm.getType()) || Strings.isNullOrEmpty(rootVolChainInfo))
{
+            return;
+        }
+        String rootDiskController = null;
+        try {
+            final VirtualMachineDiskInfo infoInChain = _gson.fromJson(rootVolChainInfo, VirtualMachineDiskInfo.class);
+            if (infoInChain != null) {
+                rootDiskController = infoInChain.getControllerFromDeviceBusName();
+            }
+            final UserVmVO userVmVo = _userVmDao.findById(vm.getId());
+            if ((rootDiskController != null) && (!rootDiskController.isEmpty()))
{
+                _userVmDao.loadDetails(userVmVo);
+                _userVmMgr.persistDeviceBusInfo(userVmVo, rootDiskController);
+            }
+        } catch (JsonParseException e) {
+            s_logger.debug("Error parsing chain info json: " + e.getMessage());
+        }
+    }
+
     @DB
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_VOLUME_MIGRATE, eventDescription = "migrating
volume", async = true)
@@ -1924,6 +1956,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
                                 throw new InvalidParameterValueException("Cannot migrate
ROOT volume of a stopped VM to a storage pool in a different VMware datacenter");
                             }
                         }
+                        updateMissingRootDiskController(vm, vol.getChainInfo());
                     }
                 }
             }
@@ -2472,9 +2505,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             }
             _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.ROOT_DISK_CONTROLLER, vm.getDetail(VmDetailConstants.ROOT_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);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/server/src/com/cloud/vm/UserVmManager.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/UserVmManager.java b/server/src/com/cloud/vm/UserVmManager.java
index 5d9a661..fe0e98c 100644
--- a/server/src/com/cloud/vm/UserVmManager.java
+++ b/server/src/com/cloud/vm/UserVmManager.java
@@ -114,4 +114,6 @@ public interface UserVmManager extends UserVmService {
     public void removeCustomOfferingDetails(long vmId);
 
     void generateUsageEvent(VirtualMachine vm, boolean isDisplay, String eventType);
+
+    void persistDeviceBusInfo(UserVmVO paramUserVmVO, String paramString);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/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 c586e14..832a948 100644
--- a/server/src/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/com/cloud/vm/UserVmManagerImpl.java
@@ -89,6 +89,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
 import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
@@ -3509,15 +3510,15 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager,
Vir
                 if (hypervisorType.equals(HypervisorType.VMware)) {
                     if (guestOS.getDisplayName().toLowerCase().contains("apple mac os"))
{
                         vm.setDetail("smc.present", "TRUE");
-                        vm.setDetail(VmDetailConstants.ROOK_DISK_CONTROLLER, "scsi");
+                        vm.setDetail(VmDetailConstants.ROOT_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");
                         // Don't override if VM already has root/data disk controller detail
-                        if (vm.getDetail(VmDetailConstants.ROOK_DISK_CONTROLLER) == null)
{
-                            vm.setDetail(VmDetailConstants.ROOK_DISK_CONTROLLER, controllerSetting);
+                        if (vm.getDetail(VmDetailConstants.ROOT_DISK_CONTROLLER) == null)
{
+                            vm.setDetail(VmDetailConstants.ROOT_DISK_CONTROLLER, controllerSetting);
                         }
                         if (vm.getDetail(VmDetailConstants.DATA_DISK_CONTROLLER) == null)
{
                             if (controllerSetting.equalsIgnoreCase("scsi")) {
@@ -5455,6 +5456,17 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager,
Vir
         }
     }
 
+    public void persistDeviceBusInfo(UserVmVO vm, String rootDiskController) {
+        String existingVmRootDiskController = vm.getDetail(VmDetailConstants.ROOT_DISK_CONTROLLER);
+        if (StringUtils.isEmpty(existingVmRootDiskController) && !StringUtils.isEmpty(rootDiskController))
{
+            vm.setDetail(VmDetailConstants.ROOT_DISK_CONTROLLER, rootDiskController);
+            _vmDao.saveDetails(vm);
+            if (s_logger.isDebugEnabled()) {
+                s_logger.debug("Persisted device bus information rootDiskController=" + rootDiskController
+ " for vm: " + vm.getDisplayName());
+            }
+        }
+    }
+
     @Override
     public String getConfigComponentName() {
         return UserVmManager.class.getSimpleName();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/server/test/com/cloud/storage/VolumeApiServiceImplTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/storage/VolumeApiServiceImplTest.java b/server/test/com/cloud/storage/VolumeApiServiceImplTest.java
index 4b502a4..71f6ded 100644
--- a/server/test/com/cloud/storage/VolumeApiServiceImplTest.java
+++ b/server/test/com/cloud/storage/VolumeApiServiceImplTest.java
@@ -18,7 +18,11 @@ package com.cloud.storage;
 
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.lang.reflect.Field;
@@ -28,7 +32,10 @@ import java.util.UUID;
 
 import javax.inject.Inject;
 
+import com.cloud.serializer.GsonHelper;
 import com.cloud.user.User;
+import com.cloud.vm.UserVmManager;
+import com.cloud.vm.VirtualMachine;
 import junit.framework.Assert;
 import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
 import org.junit.After;
@@ -101,6 +108,8 @@ public class VolumeApiServiceImplTest {
     VolumeService volService;
     @Mock
     CreateVolumeCmd createVol;
+    @Mock
+    UserVmManager _userVmMgr;
 
     DetachVolumeCmd detachCmd = new DetachVolumeCmd();
     Class<?> _detachCmdClass = detachCmd.getClass();
@@ -118,6 +127,8 @@ public class VolumeApiServiceImplTest {
         _svc._jobMgr = _jobMgr;
         _svc.volFactory = _volFactory;
         _svc.volService = volService;
+        _svc._userVmMgr = _userVmMgr;
+        _svc._gson = GsonHelper.getGsonLogger();
 
         // mock caller context
         AccountVO account = new AccountVO("admin", 1L, "networkDomain", Account.ACCOUNT_TYPE_NORMAL,
"uuid");
@@ -383,6 +394,20 @@ public class VolumeApiServiceImplTest {
         Assert.assertSame(_svc.getVolumeNameFromCommand(createVol), "abc");
     }
 
+    @Test
+    public void testUpdateMissingRootDiskControllerWithNullChainInfo() {
+        _svc.updateMissingRootDiskController(null, null);
+        verify(_svc._userVmMgr, times(0)).persistDeviceBusInfo(any(UserVmVO.class), anyString());
+    }
+
+    @Test
+    public void testUpdateMissingRootDiskControllerWithValidChainInfo() {
+        UserVmVO vm = _svc._userVmDao.findById(1L);
+        assert vm.getType() == VirtualMachine.Type.User;
+        _svc.updateMissingRootDiskController(vm, "{\"diskDeviceBusName\":\"scsi0:0\",\"diskChain\":[\"[somedatastore]
i-3-VM-somePath/ROOT-1.vmdk\"]}");
+        verify(_svc._userVmMgr, times(1)).persistDeviceBusInfo(any(UserVmVO.class), eq("scsi"));
+    }
+
     @After
     public void tearDown() {
         CallContext.unregister();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/server/test/com/cloud/vm/UserVmManagerTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/vm/UserVmManagerTest.java b/server/test/com/cloud/vm/UserVmManagerTest.java
index cffb506..637a309 100644
--- a/server/test/com/cloud/vm/UserVmManagerTest.java
+++ b/server/test/com/cloud/vm/UserVmManagerTest.java
@@ -928,4 +928,25 @@ public class UserVmManagerTest {
             CallContext.unregister();
         }
     }
+
+    @Test
+    public void testPersistDeviceBusInfoWithNullController() {
+        when(_vmMock.getDetail(any(String.class))).thenReturn(null);
+        _userVmMgr.persistDeviceBusInfo(_vmMock, null);
+        verify(_vmDao, times(0)).saveDetails(any(UserVmVO.class));
+    }
+
+    @Test
+    public void testPersistDeviceBusInfoWithEmptyController() {
+        when(_vmMock.getDetail(any(String.class))).thenReturn("");
+        _userVmMgr.persistDeviceBusInfo(_vmMock, "");
+        verify(_vmDao, times(0)).saveDetails(any(UserVmVO.class));
+    }
+
+    @Test
+    public void testPersistDeviceBusInfo() {
+        when(_vmMock.getDetail(any(String.class))).thenReturn(null);
+        _userVmMgr.persistDeviceBusInfo(_vmMock, "lsilogic");
+        verify(_vmDao, times(1)).saveDetails(any(UserVmVO.class));
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/utils/src/main/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfo.java
----------------------------------------------------------------------
diff --git a/utils/src/main/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfo.java
b/utils/src/main/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfo.java
new file mode 100644
index 0000000..c158f10
--- /dev/null
+++ b/utils/src/main/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfo.java
@@ -0,0 +1,48 @@
+// 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 org.apache.cloudstack.utils.volume;
+
+import org.apache.commons.lang.StringUtils;
+
+public class VirtualMachineDiskInfo {
+    private String diskDeviceBusName;
+    private String[] diskChain;
+
+    public String getDiskDeviceBusName() {
+        return diskDeviceBusName;
+    }
+
+    public void setDiskDeviceBusName(String diskDeviceBusName) {
+        this.diskDeviceBusName = diskDeviceBusName;
+    }
+
+    public String[] getDiskChain() {
+        return diskChain;
+    }
+
+    public void setDiskChain(String[] diskChain) {
+        this.diskChain = diskChain;
+    }
+
+    public String getControllerFromDeviceBusName() {
+        if (StringUtils.isEmpty(diskDeviceBusName) || !diskDeviceBusName.contains(":")) {
+            return null;
+        }
+        return diskDeviceBusName.substring(0, diskDeviceBusName.indexOf(":") - 1);
+    }
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/utils/src/test/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfoTest.java
----------------------------------------------------------------------
diff --git a/utils/src/test/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfoTest.java
b/utils/src/test/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfoTest.java
new file mode 100644
index 0000000..8b858d4
--- /dev/null
+++ b/utils/src/test/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfoTest.java
@@ -0,0 +1,55 @@
+//
+// 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 org.apache.cloudstack.utils.volume;
+
+import com.google.gson.GsonBuilder;
+import com.google.gson.JsonParseException;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.runners.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class VirtualMachineDiskInfoTest {
+
+    @Test
+    public void testGetControllerFromDeviceBusName() {
+        VirtualMachineDiskInfo vmDiskInfo = new VirtualMachineDiskInfo();
+        vmDiskInfo.setDiskDeviceBusName("scsi0:0");
+        String[] diskChain = new String[]{"[somedatastore] i-3-VM-somePath/ROOT-1.vmdk"};
+        vmDiskInfo.setDiskChain(diskChain);
+        Assert.assertEquals(vmDiskInfo.getControllerFromDeviceBusName(), "scsi");
+        Assert.assertArrayEquals(vmDiskInfo.getDiskChain(), diskChain);
+    }
+
+    @Test
+    public void testGetControllerFromDeviceBusNameWithInvalidBusName() {
+        VirtualMachineDiskInfo vmDiskInfo = new VirtualMachineDiskInfo();
+        vmDiskInfo.setDiskDeviceBusName("scsi0");
+        Assert.assertEquals(vmDiskInfo.getControllerFromDeviceBusName(), null);
+    }
+
+    @Test
+    public void testGSonDeserialization() throws JsonParseException {
+        VirtualMachineDiskInfo infoInChain = new GsonBuilder().create().fromJson("{\"diskDeviceBusName\":\"scsi0:0\",\"diskChain\":[\"[somedatastore]
i-3-VM-somePath/ROOT-1.vmdk\"]}", VirtualMachineDiskInfo.class);
+        Assert.assertEquals(infoInChain.getDiskDeviceBusName(), "scsi0:0");
+        Assert.assertEquals(infoInChain.getControllerFromDeviceBusName(), "scsi");
+    }
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfo.java
----------------------------------------------------------------------
diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfo.java b/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfo.java
deleted file mode 100644
index 30746fe..0000000
--- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfo.java
+++ /dev/null
@@ -1,42 +0,0 @@
-// 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 class VirtualMachineDiskInfo {
-    String diskDeviceBusName;
-    String[] diskChain;
-
-    public VirtualMachineDiskInfo() {
-    }
-
-    public String getDiskDeviceBusName() {
-        return diskDeviceBusName;
-    }
-
-    public void setDiskDeviceBusName(String diskDeviceBusName) {
-        this.diskDeviceBusName = diskDeviceBusName;
-    }
-
-    public String[] getDiskChain() {
-        return diskChain;
-    }
-
-    public void setDiskChain(String[] diskChain) {
-        this.diskChain = diskChain;
-    }
-}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfoBuilder.java
----------------------------------------------------------------------
diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfoBuilder.java
b/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfoBuilder.java
index ba0a3ea..3b310fb 100644
--- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfoBuilder.java
+++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfoBuilder.java
@@ -17,6 +17,8 @@
 
 package com.cloud.hypervisor.vmware.mo;
 
+import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
+
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/8e4644e4/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java
----------------------------------------------------------------------
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 eeb0b4d..8b9d4e7 100644
--- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java
+++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java
@@ -2137,7 +2137,7 @@ public class VirtualMachineMO extends BaseMO {
         }
 
         assert (false);
-        throw new Exception(diskController + " Controller Not Found");
+        throw new IllegalStateException("Scsi disk controller of type " + diskController
+ " not found among configured devices.");
     }
 
     public int getScsiDiskControllerKeyNoException(String diskController) throws Exception
{


Mime
View raw message