Skip to content

Commit 4297857

Browse files
committed
CLOUDSTACK-9428: Fix for CLOUDSTACK-9211 - Improve performance
1 parent 2875af7 commit 4297857

File tree

3 files changed

+80
-35
lines changed

3 files changed

+80
-35
lines changed

api/src/com/cloud/vm/VmDetailConstants.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,5 @@ public interface VmDetailConstants {
2323
public static final String NESTED_VIRTUALIZATION_FLAG = "nestedVirtualizationFlag";
2424
public static final String HYPERVISOR_TOOLS_VERSION = "hypervisortoolsversion";
2525
public static final String DATA_DISK_CONTROLLER = "dataDiskController";
26+
public static final String SVGA_VRAM_SIZE = "svga.vramSize";
2627
}

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1946,6 +1946,9 @@ protected StartAnswer execute(StartCommand cmd) {
19461946
vmConfigSpec.getExtraConfig().addAll(
19471947
Arrays.asList(configureVnc(extraOptions.toArray(new OptionValue[0]), hyperHost, vmInternalCSName, vmSpec.getVncPassword(), keyboardLayout)));
19481948

1949+
// config video card
1950+
configureVideoCard(vmMo, vmSpec, vmConfigSpec);
1951+
19491952
//
19501953
// Configure VM
19511954
//
@@ -1964,8 +1967,6 @@ protected StartAnswer execute(StartCommand cmd) {
19641967

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

1967-
postVideoCardMemoryConfigBeforeStart(vmMo, vmSpec);
1968-
19691970
//
19701971
// Power-on VM
19711972
//
@@ -2015,76 +2016,71 @@ protected StartAnswer execute(StartCommand cmd) {
20152016
}
20162017

20172018
/**
2018-
* Sets video card memory to the one provided in detail svga.vramSize (if provided).
2019+
* Sets video card memory to the one provided in detail svga.vramSize (if provided) on {@code vmConfigSpec}.
20192020
* 64MB was always set before.
20202021
* Size must be in KB.
20212022
* @param vmMo virtual machine mo
20222023
* @param vmSpec virtual machine specs
2024+
* @param vmConfigSpec virtual machine config spec
2025+
* @throws Exception exception
20232026
*/
2024-
protected void postVideoCardMemoryConfigBeforeStart(VirtualMachineMO vmMo, VirtualMachineTO vmSpec) {
2025-
String paramVRamSize = "svga.vramSize";
2026-
if (vmSpec.getDetails().containsKey(paramVRamSize)){
2027-
String value = vmSpec.getDetails().get(paramVRamSize);
2027+
protected void configureVideoCard(VirtualMachineMO vmMo, VirtualMachineTO vmSpec, VirtualMachineConfigSpec vmConfigSpec) throws Exception {
2028+
if (vmSpec.getDetails().containsKey(VmDetailConstants.SVGA_VRAM_SIZE)){
2029+
String value = vmSpec.getDetails().get(VmDetailConstants.SVGA_VRAM_SIZE);
20282030
try {
20292031
long svgaVmramSize = Long.parseLong(value);
2030-
setNewVRamSizeVmVideoCard(vmMo, svgaVmramSize);
2032+
setNewVRamSizeVmVideoCard(vmMo, svgaVmramSize, vmConfigSpec);
20312033
}
20322034
catch (NumberFormatException e){
20332035
s_logger.error("Unexpected value, cannot parse " + value + " to long due to: " + e.getMessage());
20342036
}
2035-
catch (Exception e){
2036-
s_logger.error("Error while reconfiguring vm due to: " + e.getMessage());
2037-
}
20382037
}
20392038
}
20402039

20412040
/**
20422041
* Search for vm video card iterating through vm device list
20432042
* @param vmMo virtual machine mo
20442043
* @param svgaVmramSize new svga vram size (in KB)
2044+
* @param vmConfigSpec virtual machine config spec
20452045
*/
2046-
private void setNewVRamSizeVmVideoCard(VirtualMachineMO vmMo, long svgaVmramSize) throws Exception {
2046+
protected void setNewVRamSizeVmVideoCard(VirtualMachineMO vmMo, long svgaVmramSize, VirtualMachineConfigSpec vmConfigSpec) throws Exception {
20472047
for (VirtualDevice device : vmMo.getAllDeviceList()){
20482048
if (device instanceof VirtualMachineVideoCard){
20492049
VirtualMachineVideoCard videoCard = (VirtualMachineVideoCard) device;
2050-
modifyVmVideoCardVRamSize(videoCard, vmMo, svgaVmramSize);
2050+
modifyVmVideoCardVRamSize(videoCard, vmMo, svgaVmramSize, vmConfigSpec);
20512051
}
20522052
}
20532053
}
20542054

20552055
/**
2056-
* 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)
2056+
* 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}
20572057
* @param videoCard vm's video card device
20582058
* @param vmMo virtual machine mo
20592059
* @param svgaVmramSize new svga vram size (in KB)
2060+
* @param vmConfigSpec virtual machine config spec
20602061
*/
2061-
private void modifyVmVideoCardVRamSize(VirtualMachineVideoCard videoCard, VirtualMachineMO vmMo, long svgaVmramSize) throws Exception {
2062+
protected void modifyVmVideoCardVRamSize(VirtualMachineVideoCard videoCard, VirtualMachineMO vmMo, long svgaVmramSize, VirtualMachineConfigSpec vmConfigSpec) {
20622063
if (videoCard.getVideoRamSizeInKB().longValue() != svgaVmramSize){
20632064
s_logger.info("Video card memory was set " + videoCard.getVideoRamSizeInKB().longValue() + "kb instead of " + svgaVmramSize + "kb");
2064-
VirtualMachineConfigSpec newSizeSpecs = configSpecVideoCardNewVRamSize(videoCard, svgaVmramSize);
2065-
boolean res = vmMo.configureVm(newSizeSpecs);
2066-
if (res) {
2067-
s_logger.info("Video card memory successfully updated to " + svgaVmramSize + "kb");
2068-
}
2065+
configureSpecVideoCardNewVRamSize(videoCard, svgaVmramSize, vmConfigSpec);
20692066
}
20702067
}
20712068

20722069
/**
2073-
* Returns a VirtualMachineConfigSpec to edit its svga vram size
2070+
* Add edit spec on {@code vmConfigSpec} to modify svga vram size
20742071
* @param videoCard video card device to edit providing the svga vram size
20752072
* @param svgaVmramSize new svga vram size (in KB)
2073+
* @param vmConfigSpec virtual machine spec
20762074
*/
2077-
private VirtualMachineConfigSpec configSpecVideoCardNewVRamSize(VirtualMachineVideoCard videoCard, long svgaVmramSize){
2075+
protected void configureSpecVideoCardNewVRamSize(VirtualMachineVideoCard videoCard, long svgaVmramSize, VirtualMachineConfigSpec vmConfigSpec){
20782076
videoCard.setVideoRamSizeInKB(svgaVmramSize);
20792077
videoCard.setUseAutoDetect(false);
20802078

20812079
VirtualDeviceConfigSpec arrayVideoCardConfigSpecs = new VirtualDeviceConfigSpec();
20822080
arrayVideoCardConfigSpecs.setDevice(videoCard);
20832081
arrayVideoCardConfigSpecs.setOperation(VirtualDeviceConfigSpecOperation.EDIT);
20842082

2085-
VirtualMachineConfigSpec changeVideoCardSpecs = new VirtualMachineConfigSpec();
2086-
changeVideoCardSpecs.getDeviceChange().add(arrayVideoCardConfigSpecs);
2087-
return changeVideoCardSpecs;
2083+
vmConfigSpec.getDeviceChange().add(arrayVideoCardConfigSpecs);
20882084
}
20892085

20902086
private void tearDownVm(VirtualMachineMO vmMo) throws Exception{

plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@
1919
import static org.mockito.Mockito.doReturn;
2020
import static org.mockito.Mockito.verify;
2121
import static org.mockito.Mockito.when;
22-
import static org.mockito.Mockito.mock;
2322
import static org.junit.Assert.assertEquals;
2423
import static org.junit.Assert.assertFalse;
2524
import static org.junit.Assert.assertTrue;
2625
import static org.mockito.Mockito.any;
2726
import static org.mockito.Mockito.never;
27+
import static org.mockito.Matchers.eq;
2828

29+
import java.util.ArrayList;
2930
import java.util.Arrays;
3031
import java.util.HashMap;
3132
import java.util.Map;
@@ -34,15 +35,18 @@
3435
import org.junit.Before;
3536
import org.junit.Test;
3637
import org.junit.runner.RunWith;
38+
import org.mockito.InOrder;
3739
import org.mockito.InjectMocks;
3840
import org.mockito.Mock;
41+
import org.mockito.Mockito;
3942
import org.mockito.MockitoAnnotations;
4043
import org.mockito.Spy;
4144
import org.powermock.api.mockito.PowerMockito;
4245
import org.powermock.core.classloader.annotations.PrepareForTest;
4346
import org.powermock.modules.junit4.PowerMockRunner;
4447

4548
import com.vmware.vim25.VirtualDevice;
49+
import com.vmware.vim25.VirtualDeviceConfigSpec;
4650
import com.vmware.vim25.VirtualMachineConfigSpec;
4751
import com.vmware.vim25.VirtualMachineVideoCard;
4852
import com.cloud.agent.api.Command;
@@ -99,6 +103,10 @@ public VmwareHypervisorHost getHyperHost(VmwareContext context, Command cmd) {
99103
@Mock
100104
VirtualMachineTO vmSpec3dgpu;
101105
@Mock
106+
VirtualMachineVideoCard videoCard;
107+
@Mock
108+
VirtualDevice virtualDevice;
109+
@Mock
102110
DataTO srcDataTO;
103111
@Mock
104112
NfsTO srcDataNfsTO;
@@ -107,16 +115,19 @@ public VmwareHypervisorHost getHyperHost(VmwareContext context, Command cmd) {
107115

108116
private static final Integer NFS_VERSION = Integer.valueOf(3);
109117
private static final Integer NFS_VERSION_NOT_PRESENT = null;
118+
private static final long VRAM_MEMORY_SIZE = 131072l;
119+
private static final long VIDEO_CARD_MEMORY_SIZE = 65536l;
110120

111121
@Before
112-
public void setup() {
122+
public void setup() throws Exception {
113123
MockitoAnnotations.initMocks(this);
114124
storageCmd = PowerMockito.mock(CopyCommand.class);
115125
doReturn(context).when(_resource).getServiceContext(null);
116126
when(cmd.getVirtualMachine()).thenReturn(vmSpec);
117127
when(storageCmd.getSrcTO()).thenReturn(srcDataTO);
118128
when(srcDataTO.getDataStore()).thenReturn(srcDataNfsTO);
119129
when(srcDataNfsTO.getNfsVersion()).thenReturn(NFS_VERSION);
130+
when(videoCard.getVideoRamSizeInKB()).thenReturn(VIDEO_CARD_MEMORY_SIZE);
120131
}
121132

122133
//Test successful scaling up the vm
@@ -138,19 +149,56 @@ public void testScaleVMF1() throws Exception {
138149
}
139150

140151
@Test
141-
public void testStartVm3dgpuEnabled() throws Exception{
152+
public void testConfigureVideoCardSvgaVramProvided() throws Exception {
142153
Map<String, String> specDetails = new HashMap<String, String>();
143-
specDetails.put("svga.vramSize", "131072");
154+
specDetails.put("svga.vramSize", String.valueOf(VRAM_MEMORY_SIZE));
144155
when(vmSpec3dgpu.getDetails()).thenReturn(specDetails);
145156

146-
VirtualMachineVideoCard videoCard = mock(VirtualMachineVideoCard.class);
147-
when(videoCard.getVideoRamSizeInKB()).thenReturn(65536l);
148-
when(vmMo3dgpu.getAllDeviceList()).thenReturn(Arrays.asList((VirtualDevice) videoCard));
157+
_resource.configureVideoCard(vmMo3dgpu, vmSpec3dgpu, vmConfigSpec);
158+
verify(_resource).setNewVRamSizeVmVideoCard(vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec);
159+
}
160+
161+
@Test
162+
public void testConfigureVideoCardNotSvgaVramProvided() throws Exception {
163+
_resource.configureVideoCard(vmMo3dgpu, vmSpec3dgpu, vmConfigSpec);
164+
verify(_resource, never()).setNewVRamSizeVmVideoCard(vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec);
165+
}
166+
167+
@Test
168+
public void testModifyVmVideoCardVRamSizeDifferentVramSizes() {
169+
_resource.modifyVmVideoCardVRamSize(videoCard, vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec);
170+
verify(_resource).configureSpecVideoCardNewVRamSize(videoCard, VRAM_MEMORY_SIZE, vmConfigSpec);
171+
}
172+
173+
@Test
174+
public void testModifyVmVideoCardVRamSizeEqualSizes() {
175+
_resource.modifyVmVideoCardVRamSize(videoCard, vmMo3dgpu, VIDEO_CARD_MEMORY_SIZE, vmConfigSpec);
176+
verify(_resource, never()).configureSpecVideoCardNewVRamSize(videoCard, VIDEO_CARD_MEMORY_SIZE, vmConfigSpec);
177+
}
149178

150-
when(vmMo3dgpu.configureVm(any(VirtualMachineConfigSpec.class))).thenReturn(true);
179+
@Test
180+
public void testSetNewVRamSizeVmVideoCardPresent() throws Exception {
181+
when(vmMo3dgpu.getAllDeviceList()).thenReturn(Arrays.asList(videoCard, virtualDevice));
182+
_resource.setNewVRamSizeVmVideoCard(vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec);
183+
verify(_resource).modifyVmVideoCardVRamSize(videoCard, vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec);
184+
}
151185

152-
_resource.postVideoCardMemoryConfigBeforeStart(vmMo3dgpu, vmSpec3dgpu);
153-
verify(vmMo3dgpu).configureVm(any(VirtualMachineConfigSpec.class));
186+
@Test
187+
public void testSetNewVRamSizeVmVideoCardNotPresent() throws Exception {
188+
when(vmMo3dgpu.getAllDeviceList()).thenReturn(Arrays.asList(virtualDevice));
189+
_resource.setNewVRamSizeVmVideoCard(vmMo3dgpu, VRAM_MEMORY_SIZE, vmConfigSpec);
190+
verify(_resource, never()).modifyVmVideoCardVRamSize(any(VirtualMachineVideoCard.class), eq(vmMo3dgpu), eq(VRAM_MEMORY_SIZE), eq(vmConfigSpec));
191+
}
192+
193+
@Test
194+
public void testConfigureSpecVideoCardNewVRamSize() {
195+
when(vmConfigSpec.getDeviceChange()).thenReturn(new ArrayList<VirtualDeviceConfigSpec>());
196+
_resource.configureSpecVideoCardNewVRamSize(videoCard, VRAM_MEMORY_SIZE, vmConfigSpec);
197+
198+
InOrder inOrder = Mockito.inOrder(videoCard, vmConfigSpec);
199+
inOrder.verify(videoCard).setVideoRamSizeInKB(VRAM_MEMORY_SIZE);
200+
inOrder.verify(videoCard).setUseAutoDetect(false);
201+
inOrder.verify(vmConfigSpec).getDeviceChange();
154202
}
155203

156204
// ---------------------------------------------------------------------------------------------------

0 commit comments

Comments
 (0)