Skip to content

CLOUDSTACK-9428: Fix for CLOUDSTACK-9211 - Improve performance of 3D GPU support in cloud-plugin-hypervisor-vmware #1605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
asfgit merged 2 commits into from
Sep 11, 2016
Merged

CLOUDSTACK-9428: Fix for CLOUDSTACK-9211 - Improve performance of 3D GPU support in cloud-plugin-hypervisor-vmware #1605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4297857
CLOUDSTACK-9428: Fix for CLOUDSTACK-9211 - Improve performance
nvazquez Jul 5, 2016
2de5b0d
CLOUDSTACK-9428: Add marvin test
nvazquez Sep 1, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/src/com/cloud/vm/VmDetailConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
46 changes: 21 additions & 25 deletions plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1946,6 +1946,9 @@ protected StartAnswer execute(StartCommand cmd) {
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
//
Expand All @@ -1964,8 +1967,6 @@ protected StartAnswer execute(StartCommand cmd) {

postDiskConfigBeforeStart(vmMo, vmSpec, sortedDisks, ideControllerKey, scsiControllerKey, iqnToPath, hyperHost, context);

postVideoCardMemoryConfigBeforeStart(vmMo, vmSpec);

//
// Power-on VM
//
Expand Down Expand Up @@ -2015,76 +2016,71 @@ protected StartAnswer execute(StartCommand cmd) {
}

/**
* 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());
}
}
}

/**
* 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);

VirtualDeviceConfigSpec arrayVideoCardConfigSpecs = new VirtualDeviceConfigSpec();
arrayVideoCardConfigSpecs.setDevice(videoCard);
arrayVideoCardConfigSpecs.setOperation(VirtualDeviceConfigSpecOperation.EDIT);

VirtualMachineConfigSpec changeVideoCardSpecs = new VirtualMachineConfigSpec();
changeVideoCardSpecs.getDeviceChange().add(arrayVideoCardConfigSpecs);
return changeVideoCardSpecs;
vmConfigSpec.getDeviceChange().add(arrayVideoCardConfigSpecs);
Copy link
Contributor

@jburwell jburwell Aug 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for getDeviceChange to return null?

Copy link

@serg38 serg38 Aug 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. Device change done in the structure locally and exception occurs during the VM reconfiguration on the hypervisor if anything in the config is wrong.

}

private void tearDownVm(VirtualMachineMO vmMo) throws Exception{
Expand Down
68 changes: 58 additions & 10 deletions plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@
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;
Expand All @@ -34,15 +35,18 @@
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;
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;
Expand Down Expand Up @@ -99,6 +103,10 @@ public VmwareHypervisorHost getHyperHost(VmwareContext context, Command cmd) {
@Mock
VirtualMachineTO vmSpec3dgpu;
@Mock
VirtualMachineVideoCard videoCard;
@Mock
VirtualDevice virtualDevice;
@Mock
DataTO srcDataTO;
@Mock
NfsTO srcDataNfsTO;
Expand All @@ -107,16 +115,19 @@ public VmwareHypervisorHost getHyperHost(VmwareContext context, Command cmd) {

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);
when(cmd.getVirtualMachine()).thenReturn(vmSpec);
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
Expand All @@ -138,19 +149,56 @@ public void testScaleVMF1() throws Exception {
}

@Test
public void testStartVm3dgpuEnabled() throws Exception{
public void testConfigureVideoCardSvgaVramProvided() throws Exception {
Map<String, String> specDetails = new HashMap<String, String>();
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);
}

@Test
public void testConfigureVideoCardNotSvgaVramProvided() throws Exception {
_resource.configureVideoCard(vmMo3dgpu, vmSpec3dgpu, vmConfigSpec);
verify(_resource, never()).setNewVRamSizeVmVideoCard(vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec);
}

@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);
}

when(vmMo3dgpu.configureVm(any(VirtualMachineConfigSpec.class))).thenReturn(true);
@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);
}

_resource.postVideoCardMemoryConfigBeforeStart(vmMo3dgpu, vmSpec3dgpu);
verify(vmMo3dgpu).configureVm(any(VirtualMachineConfigSpec.class));
@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<VirtualDeviceConfigSpec>());
_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();
}

// ---------------------------------------------------------------------------------------------------
Expand Down
Loading