diff --git a/api/src/com/cloud/vm/VmDetailConstants.java b/api/src/com/cloud/vm/VmDetailConstants.java index d34afc13a16..c3c6db7a296 100644 --- a/api/src/com/cloud/vm/VmDetailConstants.java +++ b/api/src/com/cloud/vm/VmDetailConstants.java @@ -23,4 +23,5 @@ public interface VmDetailConstants { public static final String NESTED_VIRTUALIZATION_FLAG = "nestedVirtualizationFlag"; public static final String HYPERVISOR_TOOLS_VERSION = "hypervisortoolsversion"; public static final String DATA_DISK_CONTROLLER = "dataDiskController"; + public static final String SVGA_VRAM_SIZE = "svga.vramSize"; } 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 238ba3ed864..9b7885a74e1 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 @@ -1946,6 +1946,9 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa vmConfigSpec.getExtraConfig().addAll( Arrays.asList(configureVnc(extraOptions.toArray(new OptionValue[0]), hyperHost, vmInternalCSName, vmSpec.getVncPassword(), keyboardLayout))); + // config video card + configureVideoCard(vmMo, vmSpec, vmConfigSpec); + // // Configure VM // @@ -1964,8 +1967,6 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa postDiskConfigBeforeStart(vmMo, vmSpec, sortedDisks, ideControllerKey, scsiControllerKey, iqnToPath, hyperHost, context); - postVideoCardMemoryConfigBeforeStart(vmMo, vmSpec); - // // Power-on VM // @@ -2015,26 +2016,24 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa } /** - * Sets video card memory to the one provided in detail svga.vramSize (if provided). + * Sets video card memory to the one provided in detail svga.vramSize (if provided) on {@code vmConfigSpec}. * 64MB was always set before. * Size must be in KB. * @param vmMo virtual machine mo * @param vmSpec virtual machine specs + * @param vmConfigSpec virtual machine config spec + * @throws Exception exception */ - protected void postVideoCardMemoryConfigBeforeStart(VirtualMachineMO vmMo, VirtualMachineTO vmSpec) { - String paramVRamSize = "svga.vramSize"; - if (vmSpec.getDetails().containsKey(paramVRamSize)){ - String value = vmSpec.getDetails().get(paramVRamSize); + protected void configureVideoCard(VirtualMachineMO vmMo, VirtualMachineTO vmSpec, VirtualMachineConfigSpec vmConfigSpec) throws Exception { + if (vmSpec.getDetails().containsKey(VmDetailConstants.SVGA_VRAM_SIZE)){ + String value = vmSpec.getDetails().get(VmDetailConstants.SVGA_VRAM_SIZE); try { long svgaVmramSize = Long.parseLong(value); - setNewVRamSizeVmVideoCard(vmMo, svgaVmramSize); + setNewVRamSizeVmVideoCard(vmMo, svgaVmramSize, vmConfigSpec); } catch (NumberFormatException e){ s_logger.error("Unexpected value, cannot parse " + value + " to long due to: " + e.getMessage()); } - catch (Exception e){ - s_logger.error("Error while reconfiguring vm due to: " + e.getMessage()); - } } } @@ -2042,39 +2041,38 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa * Search for vm video card iterating through vm device list * @param vmMo virtual machine mo * @param svgaVmramSize new svga vram size (in KB) + * @param vmConfigSpec virtual machine config spec */ - private void setNewVRamSizeVmVideoCard(VirtualMachineMO vmMo, long svgaVmramSize) throws Exception { + protected void setNewVRamSizeVmVideoCard(VirtualMachineMO vmMo, long svgaVmramSize, VirtualMachineConfigSpec vmConfigSpec) throws Exception { for (VirtualDevice device : vmMo.getAllDeviceList()){ if (device instanceof VirtualMachineVideoCard){ VirtualMachineVideoCard videoCard = (VirtualMachineVideoCard) device; - modifyVmVideoCardVRamSize(videoCard, vmMo, svgaVmramSize); + modifyVmVideoCardVRamSize(videoCard, vmMo, svgaVmramSize, vmConfigSpec); } } } /** - * Modifies vm vram size if it was set to a different size to the one provided in svga.vramSize (user_vm_details or template_vm_details) + * Modifies vm vram size if it was set to a different size to the one provided in svga.vramSize (user_vm_details or template_vm_details) on {@code vmConfigSpec} * @param videoCard vm's video card device * @param vmMo virtual machine mo * @param svgaVmramSize new svga vram size (in KB) + * @param vmConfigSpec virtual machine config spec */ - private void modifyVmVideoCardVRamSize(VirtualMachineVideoCard videoCard, VirtualMachineMO vmMo, long svgaVmramSize) throws Exception { + protected void modifyVmVideoCardVRamSize(VirtualMachineVideoCard videoCard, VirtualMachineMO vmMo, long svgaVmramSize, VirtualMachineConfigSpec vmConfigSpec) { if (videoCard.getVideoRamSizeInKB().longValue() != svgaVmramSize){ s_logger.info("Video card memory was set " + videoCard.getVideoRamSizeInKB().longValue() + "kb instead of " + svgaVmramSize + "kb"); - VirtualMachineConfigSpec newSizeSpecs = configSpecVideoCardNewVRamSize(videoCard, svgaVmramSize); - boolean res = vmMo.configureVm(newSizeSpecs); - if (res) { - s_logger.info("Video card memory successfully updated to " + svgaVmramSize + "kb"); - } + configureSpecVideoCardNewVRamSize(videoCard, svgaVmramSize, vmConfigSpec); } } /** - * Returns a VirtualMachineConfigSpec to edit its svga vram size + * Add edit spec on {@code vmConfigSpec} to modify svga vram size * @param videoCard video card device to edit providing the svga vram size * @param svgaVmramSize new svga vram size (in KB) + * @param vmConfigSpec virtual machine spec */ - private VirtualMachineConfigSpec configSpecVideoCardNewVRamSize(VirtualMachineVideoCard videoCard, long svgaVmramSize){ + protected void configureSpecVideoCardNewVRamSize(VirtualMachineVideoCard videoCard, long svgaVmramSize, VirtualMachineConfigSpec vmConfigSpec){ videoCard.setVideoRamSizeInKB(svgaVmramSize); videoCard.setUseAutoDetect(false); @@ -2082,9 +2080,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa arrayVideoCardConfigSpecs.setDevice(videoCard); arrayVideoCardConfigSpecs.setOperation(VirtualDeviceConfigSpecOperation.EDIT); - VirtualMachineConfigSpec changeVideoCardSpecs = new VirtualMachineConfigSpec(); - changeVideoCardSpecs.getDeviceChange().add(arrayVideoCardConfigSpecs); - return changeVideoCardSpecs; + vmConfigSpec.getDeviceChange().add(arrayVideoCardConfigSpecs); } private void tearDownVm(VirtualMachineMO vmMo) throws Exception{ diff --git a/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java b/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java index 22388a8a69c..33e7cd2fc70 100644 --- a/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java +++ b/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java @@ -19,13 +19,14 @@ package com.cloud.hypervisor.vmware.resource; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.mockito.Mockito.mock; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.never; +import static org.mockito.Matchers.eq; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -34,8 +35,10 @@ import org.apache.cloudstack.storage.command.CopyCommand; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InOrder; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.mockito.Spy; import org.powermock.api.mockito.PowerMockito; @@ -43,6 +46,7 @@ import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; import com.vmware.vim25.VirtualDevice; +import com.vmware.vim25.VirtualDeviceConfigSpec; import com.vmware.vim25.VirtualMachineConfigSpec; import com.vmware.vim25.VirtualMachineVideoCard; import com.cloud.agent.api.Command; @@ -99,6 +103,10 @@ public class VmwareResourceTest { @Mock VirtualMachineTO vmSpec3dgpu; @Mock + VirtualMachineVideoCard videoCard; + @Mock + VirtualDevice virtualDevice; + @Mock DataTO srcDataTO; @Mock NfsTO srcDataNfsTO; @@ -107,9 +115,11 @@ public class VmwareResourceTest { private static final Integer NFS_VERSION = Integer.valueOf(3); private static final Integer NFS_VERSION_NOT_PRESENT = null; + private static final long VRAM_MEMORY_SIZE = 131072l; + private static final long VIDEO_CARD_MEMORY_SIZE = 65536l; @Before - public void setup() { + public void setup() throws Exception { MockitoAnnotations.initMocks(this); storageCmd = PowerMockito.mock(CopyCommand.class); doReturn(context).when(_resource).getServiceContext(null); @@ -117,6 +127,7 @@ public class VmwareResourceTest { when(storageCmd.getSrcTO()).thenReturn(srcDataTO); when(srcDataTO.getDataStore()).thenReturn(srcDataNfsTO); when(srcDataNfsTO.getNfsVersion()).thenReturn(NFS_VERSION); + when(videoCard.getVideoRamSizeInKB()).thenReturn(VIDEO_CARD_MEMORY_SIZE); } //Test successful scaling up the vm @@ -138,19 +149,56 @@ public class VmwareResourceTest { } @Test - public void testStartVm3dgpuEnabled() throws Exception{ + public void testConfigureVideoCardSvgaVramProvided() throws Exception { Map specDetails = new HashMap(); - specDetails.put("svga.vramSize", "131072"); + specDetails.put("svga.vramSize", String.valueOf(VRAM_MEMORY_SIZE)); when(vmSpec3dgpu.getDetails()).thenReturn(specDetails); - VirtualMachineVideoCard videoCard = mock(VirtualMachineVideoCard.class); - when(videoCard.getVideoRamSizeInKB()).thenReturn(65536l); - when(vmMo3dgpu.getAllDeviceList()).thenReturn(Arrays.asList((VirtualDevice) videoCard)); + _resource.configureVideoCard(vmMo3dgpu, vmSpec3dgpu, vmConfigSpec); + verify(_resource).setNewVRamSizeVmVideoCard(vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec); + } - when(vmMo3dgpu.configureVm(any(VirtualMachineConfigSpec.class))).thenReturn(true); + @Test + public void testConfigureVideoCardNotSvgaVramProvided() throws Exception { + _resource.configureVideoCard(vmMo3dgpu, vmSpec3dgpu, vmConfigSpec); + verify(_resource, never()).setNewVRamSizeVmVideoCard(vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec); + } - _resource.postVideoCardMemoryConfigBeforeStart(vmMo3dgpu, vmSpec3dgpu); - verify(vmMo3dgpu).configureVm(any(VirtualMachineConfigSpec.class)); + @Test + public void testModifyVmVideoCardVRamSizeDifferentVramSizes() { + _resource.modifyVmVideoCardVRamSize(videoCard, vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec); + verify(_resource).configureSpecVideoCardNewVRamSize(videoCard, VRAM_MEMORY_SIZE, vmConfigSpec); + } + + @Test + public void testModifyVmVideoCardVRamSizeEqualSizes() { + _resource.modifyVmVideoCardVRamSize(videoCard, vmMo3dgpu, VIDEO_CARD_MEMORY_SIZE, vmConfigSpec); + verify(_resource, never()).configureSpecVideoCardNewVRamSize(videoCard, VIDEO_CARD_MEMORY_SIZE, vmConfigSpec); + } + + @Test + public void testSetNewVRamSizeVmVideoCardPresent() throws Exception { + when(vmMo3dgpu.getAllDeviceList()).thenReturn(Arrays.asList(videoCard, virtualDevice)); + _resource.setNewVRamSizeVmVideoCard(vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec); + verify(_resource).modifyVmVideoCardVRamSize(videoCard, vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec); + } + + @Test + public void testSetNewVRamSizeVmVideoCardNotPresent() throws Exception { + when(vmMo3dgpu.getAllDeviceList()).thenReturn(Arrays.asList(virtualDevice)); + _resource.setNewVRamSizeVmVideoCard(vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec); + verify(_resource, never()).modifyVmVideoCardVRamSize(any(VirtualMachineVideoCard.class), eq(vmMo3dgpu), eq(VRAM_MEMORY_SIZE), eq(vmConfigSpec)); + } + + @Test + public void testConfigureSpecVideoCardNewVRamSize() { + when(vmConfigSpec.getDeviceChange()).thenReturn(new ArrayList()); + _resource.configureSpecVideoCardNewVRamSize(videoCard, VRAM_MEMORY_SIZE, vmConfigSpec); + + InOrder inOrder = Mockito.inOrder(videoCard, vmConfigSpec); + inOrder.verify(videoCard).setVideoRamSizeInKB(VRAM_MEMORY_SIZE); + inOrder.verify(videoCard).setUseAutoDetect(false); + inOrder.verify(vmConfigSpec).getDeviceChange(); } // ---------------------------------------------------------------------------------------------------