From ba0503d000250e233701a71bd8e406e52b6ad110 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Maharana Date: Mon, 14 Sep 2015 22:02:37 +0530 Subject: [PATCH 1/4] CLOUDSTACK-8847: ListServiceOfferings is returning incompatible tagged offerings when called with VM id Fixed the subset and superset issue. Added unit test for the same. --- .../cloud/vm/VirtualMachineManagerImpl.java | 8 +++--- .../vm/VirtualMachineManagerImplTest.java | 28 +++++++++++++++++++ .../com/cloud/api/query/QueryManagerImpl.java | 6 ++-- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java index 5c5838951f98..cda695dc8ad4 100644 --- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2908,12 +2908,12 @@ public void checkIfCanUpgrade(final VirtualMachine vmInstance, final ServiceOffe newServiceOffering.getCpu() + " cpu(s) at " + newServiceOffering.getSpeed() + " Mhz, and " + newServiceOffering.getRamSize() + " MB of memory"); } - // Check that the service offering being upgraded to has storage tags subset of the current service offering storage tags, since volume is not migrated. + // Check that the service offering being upgraded to has all the tags of the current service offering. final List currentTags = StringUtils.csvTagsToList(currentServiceOffering.getTags()); final List newTags = StringUtils.csvTagsToList(newServiceOffering.getTags()); - if (!currentTags.containsAll(newTags)) { - throw new InvalidParameterValueException("Unable to upgrade virtual machine; the new service offering " + " should have tags as subset of " + - "current service offering tags. Current service offering tags: " + currentTags + "; " + "new service " + "offering tags: " + newTags); + if (!newTags.containsAll(currentTags)) { + throw new InvalidParameterValueException("Unable to upgrade virtual machine; the current service offering " + " should have tags as subset of " + + "the new service offering tags. Current service offering tags: " + currentTags + "; " + "new service " + "offering tags: " + newTags); } } diff --git a/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java index 865b0662ec05..d849eabaaf4a 100644 --- a/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -29,7 +29,9 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.ArrayList; +import com.cloud.service.dao.ServiceOfferingDao; import junit.framework.Assert; import org.junit.Before; @@ -135,6 +137,8 @@ public class VirtualMachineManagerImplTest { @Mock VMInstanceDao _vmInstanceDao; @Mock + ServiceOfferingDao _offeringDao; + @Mock VMTemplateDao _templateDao; @Mock VolumeDao _volsDao; @@ -149,6 +153,8 @@ public class VirtualMachineManagerImplTest { @Mock VMInstanceVO _vmInstance; @Mock + ServiceOfferingVO _serviceOfferingMock; + @Mock HostVO _host; @Mock VMTemplateVO _templateMock; @@ -227,6 +233,8 @@ public void setup() { _vmMgr._uservmDetailsDao = _vmDetailsDao; _vmMgr._entityMgr = _entityMgr; _vmMgr._configDepot = _configDepot; + _vmMgr._offeringDao = _offeringDao; + _vmMgr.hostAllocators = new ArrayList<>(); when(_vmMock.getId()).thenReturn(314l); when(_vmInstance.getId()).thenReturn(1L); @@ -505,4 +513,24 @@ public void testSendStopWithNullAnswer() throws Exception { Assert.assertFalse(actual); } + + @Test + public void testCheckIfCanUpgrade() throws Exception { + when(_vmInstance.getState()).thenReturn(State.Stopped); + when(_serviceOfferingMock.isDynamic()).thenReturn(true); + when(_vmInstance.getServiceOfferingId()).thenReturn(1l); + when(_serviceOfferingMock.getId()).thenReturn(2l); + + ServiceOfferingVO mockCurrentServiceOffering = mock(ServiceOfferingVO.class); + + when(_offeringDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(mockCurrentServiceOffering); + when(mockCurrentServiceOffering.getUseLocalStorage()).thenReturn(true); + when(_serviceOfferingMock.getUseLocalStorage()).thenReturn(true); + when(mockCurrentServiceOffering.getSystemUse()).thenReturn(true); + when(_serviceOfferingMock.getSystemUse()).thenReturn(true); + when(mockCurrentServiceOffering.getTags()).thenReturn("x,y"); + when(_serviceOfferingMock.getTags()).thenReturn("z,x,y"); + + _vmMgr.checkIfCanUpgrade(_vmInstance, _serviceOfferingMock); + } } diff --git a/server/src/com/cloud/api/query/QueryManagerImpl.java b/server/src/com/cloud/api/query/QueryManagerImpl.java index a87d9fb1f0d6..94929577a399 100644 --- a/server/src/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/com/cloud/api/query/QueryManagerImpl.java @@ -2620,11 +2620,11 @@ private List filterOfferingsOnCurrentTags(List currentTagsList = StringUtils.csvTagsToList(currentVmOffering.getTags()); - // New offerings should be a subset of existing storage tags. Discard offerings who are not. + // New service offering should have all the tags of the current service offering. List filteredOfferings = new ArrayList<>(); for (ServiceOfferingJoinVO offering : offerings){ - List tags = StringUtils.csvTagsToList(offering.getTags()); - if(currentTagsList.containsAll(tags)){ + List newTagsList = StringUtils.csvTagsToList(offering.getTags()); + if(newTagsList.containsAll(currentTagsList)){ filteredOfferings.add(offering); } } From c40a1ae0edf50e6d4a2477e53e309a5fc21d1712 Mon Sep 17 00:00:00 2001 From: Sverrir Berg Date: Wed, 13 Apr 2016 10:35:27 +0000 Subject: [PATCH 2/4] Installing bzip2 since it is required for extracting templates. --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index c391325dcacb..6c101ce1375b 100644 --- a/debian/control +++ b/debian/control @@ -15,7 +15,7 @@ Description: A common package which contains files which are shared by several C Package: cloudstack-management Architecture: all -Depends: ${misc:Depends}, ${python:Depends}, cloudstack-common (= ${source:Version}), tomcat6, sudo, jsvc, python-mysqldb, libmysql-java, augeas-tools, mysql-client, adduser +Depends: ${misc:Depends}, ${python:Depends}, cloudstack-common (= ${source:Version}), tomcat6, sudo, jsvc, python-mysqldb, libmysql-java, augeas-tools, mysql-client, adduser, bzip2 Conflicts: cloud-server, cloud-client, cloud-client-ui Description: CloudStack server library The CloudStack management server From 8e4644e413777d0a58edd5405a928df8256fdbd9 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Fri, 5 Feb 2016 19:09:03 +0100 Subject: [PATCH 3/4] 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 --- 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 +- .../cloud/storage/VolumeApiServiceImpl.java | 36 +++++++++++- server/src/com/cloud/vm/UserVmManager.java | 2 + .../src/com/cloud/vm/UserVmManagerImpl.java | 18 +++++- .../storage/VolumeApiServiceImplTest.java | 25 +++++++++ .../test/com/cloud/vm/UserVmManagerTest.java | 21 +++++++ .../utils/volume}/VirtualMachineDiskInfo.java | 18 ++++-- .../volume/VirtualMachineDiskInfoTest.java | 55 +++++++++++++++++++ .../mo/VirtualMachineDiskInfoBuilder.java | 2 + .../vmware/mo/VirtualMachineMO.java | 2 +- 14 files changed, 190 insertions(+), 42 deletions(-) rename {vmware-base/src/com/cloud/hypervisor/vmware/mo => utils/src/main/java/org/apache/cloudstack/utils/volume}/VirtualMachineDiskInfo.java (74%) create mode 100644 utils/src/test/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfoTest.java diff --git a/api/src/com/cloud/vm/VmDetailConstants.java b/api/src/com/cloud/vm/VmDetailConstants.java index d06ad67197eb..d34afc13a169 100644 --- a/api/src/com/cloud/vm/VmDetailConstants.java +++ b/api/src/com/cloud/vm/VmDetailConstants.java @@ -19,7 +19,7 @@ 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"; 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 fd96bc8357cc..986000aa9389 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 VirtualMachineTO implement(VirtualMachineProfile vm) { } } - 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); 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 fdbc244997d0..dd419f2574ff 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.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 @@ protected StartAnswer execute(StartCommand cmd) { 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 @@ protected StartAnswer execute(StartCommand cmd) { 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 @@ protected StartAnswer execute(StartCommand cmd) { 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 @@ protected StartAnswer execute(StartCommand cmd) { } 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 @@ private int getDiskController(VirtualMachineDiskInfo matchingExistingDisk, DiskT if (vol.getType() == Volume.Type.ROOT) { Map 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 { 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 fa2f369afaf6..567f8576d7be 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.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.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 @@ private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean 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); diff --git a/pom.xml b/pom.xml index 3530569e6096..af9eb765665e 100644 --- a/pom.xml +++ b/pom.xml @@ -92,7 +92,7 @@ 2.5 1.2 1.0-20081010.060147 - 5.5 + 6.0 3.2.12.RELEASE 1.9.5 1.5.3 diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java index 5a53f9c531d0..fdd237cecaf3 100644 --- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java @@ -30,8 +30,10 @@ 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.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.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.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 _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 @@ private Volume orchestrateDetachVolumeFromVM(long vmId, long volumeId) { } } + 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 Volume migrateVolume(MigrateVolumeCmd cmd) { 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 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, L } _userVmDao.loadDetails(vm); Map controllerInfo = new HashMap(); - 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); diff --git a/server/src/com/cloud/vm/UserVmManager.java b/server/src/com/cloud/vm/UserVmManager.java index 5d9a6614f2c1..fe0e98c8cd59 100644 --- a/server/src/com/cloud/vm/UserVmManager.java +++ b/server/src/com/cloud/vm/UserVmManager.java @@ -114,4 +114,6 @@ UserVm updateVirtualMachine(long id, String displayName, String group, Boolean h public void removeCustomOfferingDetails(long vmId); void generateUsageEvent(VirtualMachine vm, boolean isDisplay, String eventType); + + void persistDeviceBusInfo(UserVmVO paramUserVmVO, String paramString); } diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index c586e14a8194..832a94881765 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.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 UserVmVO doInTransaction(TransactionStatus status) throws InsufficientCap 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 @@ private void encryptAndStorePassword(UserVmVO vm, String password) { } } + 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(); diff --git a/server/test/com/cloud/storage/VolumeApiServiceImplTest.java b/server/test/com/cloud/storage/VolumeApiServiceImplTest.java index 4b502a404dc9..71f6deddf60c 100644 --- a/server/test/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/test/com/cloud/storage/VolumeApiServiceImplTest.java @@ -18,7 +18,11 @@ 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 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 void setup() throws Exception { _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 void testNonEmptyGetVolumeNameFromCmd() { 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(); diff --git a/server/test/com/cloud/vm/UserVmManagerTest.java b/server/test/com/cloud/vm/UserVmManagerTest.java index cffb506285fa..637a30922190 100644 --- a/server/test/com/cloud/vm/UserVmManagerTest.java +++ b/server/test/com/cloud/vm/UserVmManagerTest.java @@ -928,4 +928,25 @@ public void testUpdateVmNicIpFailure3() throws Exception { 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)); + } } diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfo.java b/utils/src/main/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfo.java similarity index 74% rename from vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfo.java rename to utils/src/main/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfo.java index 30746fed2296..c158f10d3a39 100644 --- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineDiskInfo.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/volume/VirtualMachineDiskInfo.java @@ -15,14 +15,13 @@ // specific language governing permissions and limitations // under the License. -package com.cloud.hypervisor.vmware.mo; +package org.apache.cloudstack.utils.volume; -public class VirtualMachineDiskInfo { - String diskDeviceBusName; - String[] diskChain; +import org.apache.commons.lang.StringUtils; - public VirtualMachineDiskInfo() { - } +public class VirtualMachineDiskInfo { + private String diskDeviceBusName; + private String[] diskChain; public String getDiskDeviceBusName() { return diskDeviceBusName; @@ -39,4 +38,11 @@ public String[] getDiskChain() { 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); + } } 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 000000000000..8b858d4c9ab6 --- /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"); + } +} 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 ba0a3ea9f25f..3b310fb586e3 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; 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 eeb0b4da2783..8b9d4e73beaa 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 int getScsiDiskControllerKey(String diskController) throws Exception { } 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 { From b32c0569c52a614db44b04f277dcf9b7745041c4 Mon Sep 17 00:00:00 2001 From: Remi Bergsma Date: Sun, 21 Feb 2016 19:19:42 +0100 Subject: [PATCH 4/4] bump ssh retries to prevent false positives of test_loadbalance --- test/integration/smoke/test_loadbalance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/smoke/test_loadbalance.py b/test/integration/smoke/test_loadbalance.py index 2202e559a53c..7ca5cf870a59 100644 --- a/test/integration/smoke/test_loadbalance.py +++ b/test/integration/smoke/test_loadbalance.py @@ -134,7 +134,7 @@ def try_ssh(self, ip_addr, unameCmd): self.services['lbrule']["publicport"], self.vm_1.username, self.vm_1.password, - retries=5 + retries=10 ) unameCmd.append(ssh_1.execute("uname")[0]) self.debug(unameCmd)