[
https://issues.apache.org/jira/browse/CLOUDSTACK-9428?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15435695#comment-15435695
]
ASF GitHub Bot commented on CLOUDSTACK-9428:
--------------------------------------------
Github user rafaelweingartner commented on the issue:
https://github.com/apache/cloudstack/pull/1605
@nv, I am sorry, my bad; last time I read the code, I overlooked the method
com.cloud.hypervisor.vmware.mo.VirtualMachineMO.getAllDeviceList(). Having said
that, how do you feel about extracting the try/catch block to
“getAllDeviceList”, then this method would not need to throw a checked
exception. It could re-throw a runtime exception such as the CloudRuntime.
From my understanding the test method “testStartVm3dgpuEnabled” is used to
test the method “configureVideoCard”, right? What about changing that method
name to “testConfigureVideoCard”?
Moreover, do not you think that the method “configureVideoCard” needs at
least two test cases? One that is already written, and the other to test the
condition in which the call
“vmSpec.getDetails().containsKey(VmDetailConstants.SVGA_VRAM_SIZE)” returns
false, and then the method “setNewVRamSizeVmVideoCard” is never called.
The same happens for the method “modifyVmVideoCardVRamSize”, I think it has
another condition “videoCard.getVideoRamSizeInKB().longValue() ==
svgaVmramSize” to be tested.
And finally, for the test method “testConfigureSpecVideoCardNewVRamSize”,
what about using the “Mockito.InOrder” to assure that the methods are being
called in the proper order. If someone changes that order of call in the
future, that can affect the method behavior.
I also have one small comment about the title of the PR and the commit
message. “Improve performance” seems a little too vague for a title. The idea
is very well explained on the PR, but the title does not reflect much. What
about, “improve the performance of cloud-plugin-hypervisor-vmware” or something
like that.
> Fix for CLOUDSTACK-9211 - Improve performance
> ---------------------------------------------
>
> Key: CLOUDSTACK-9428
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9428
> Project: CloudStack
> Issue Type: Improvement
> Security Level: Public(Anyone can view this level - this is the
> default.)
> Components: VMware
> Reporter: Nicolas Vazquez
> Assignee: Nicolas Vazquez
>
> h3. Introduction
> On [CLOUDSTACK-9211|https://issues.apache.org/jira/browse/CLOUDSTACK-9211]
> passing vRAM size to support 3D GPU problem was addressed on VMware. It was
> found out that it could be improved to increase performance by reducing extra
> API calls, as we'll describe later
> h3. Improvement
> On WMware, {{VmwareResource}} manages execution of {{StartCommand}}. Before
> sending power on command to ESXi hypervisor, vm is configured by calling
> {{reconfigVMTask}} web method on vSphere's client {{VimPortType}} web service.
> It was found out that we were using this method 2 times when passing vRAM
> size, as it implied creating a new vm config spec only editing video card
> specs and making an extra call to {{reconfigVMTask}}.
> We propose reducing the extra web service call by adjusting vm's config spec.
> This way video card gets properly configured (when passing vRAM size) in the
> same configure call, increasing performance.
> h3. Use case (passing vRAM size)
> # Deploy a new VM, let its id be X
> # Stop VM
> # Execute SQL, where X is vm's id and Z is vRAM size (in kB): {code:sql}
> INSERT INTO cloud.user_vm_details (vm_id, name, value) VALUES (X,
> 'mks.enable3d', 'true');
> INSERT INTO cloud.user_vm_details (vm_id, name, value) VALUES (X,
> 'mks.use3dRenderer', 'automatic');
> INSERT INTO cloud.user_vm_details (vm_id, name, value) VALUES (X,
> 'svga.autodetect', 'false');
> INSERT INTO cloud.user_vm_details (vm_id, name, value) VALUES (X,
> 'svga.vramSize', Z);
> {code}
> # Start VM
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
