Repository navigation
add kvm vcpu socket and threads config - #4178
div8cn wants to merge 4 commits into
Conversation
Sorry, something went wrong.
add kvm support sockets and threads config
|
This feature aims to allow user to specify how many CPU sockets the target VM would have by using the 'cores per socket' parameter. Assume user assign 16 vCPUs to a new VM, without manually specify the 'cores per socket', CloudStack would try to divide the 16 vCPUs by 6, 4 and 1 in order. In this case, CloudStack would assign 4 sockets (16vCPUs /4) to this VM and every scoket has 4 vCPUs. In most cases, it shall casue no issue, however if there's sort of CPU limits due to Guest OS or application, e.g. SQL Sever version, it could turn into a big performance issue. In our real case, user deployed an Windows 10 guest OS with 16 vCPUs, and CloudStack assgins 4 virtual CPU sockets by default. Due to Windows 10 only supports 2 CPU sockets (https://answers.microsoft.com/en-us/windows/forum/windows_10-win_upgrade/windows-10-versions-cpu-limits/905c24ad-ad54-4122-b730-b9e7519c823f?auth=1) , it turns out this guest OS is only able to utilize two sockets (8 vCPUs) even though we have assigned 16 vCPUs in toal. If user has CPU intensive application running on this VM, this could introduce bottelneck. |
Sorry, something went wrong.
GabrielBrascher
left a comment
There was a problem hiding this comment.
Thanks for the PR, @div8cn. Overal code LGTM (I have not tested it); however, I see some improvements in terms of extracting a few lines to a method(s) and then adding unit test cases.
This is not a -1, it is a tip of what (I think) can be improved. We are trying to cover tests as much as possible on CloudStack codebase and breaking big methods into smaller is a great way of doing it.
Sorry, something went wrong.
| } else if (vcpus % 4 == 0) { | ||
| final int sockets = vcpus / 4; | ||
| cmd.setTopology(4, sockets); | ||
| } |
There was a problem hiding this comment.
What do you think of extracting lines 2300 - 2325 into a method, document, and then create JUnit tests for the new method(s)?
Sorry, something went wrong.
|
@GabrielBrascher @div8cn @luhaijiao what is the status of this PR? |
Sorry, something went wrong.
Sorry, something went wrong.
Description
Supplement the vcpu cores Per Socket configuration of KVM VM
Increase the vcpu hyperthreading configuration of KVM VM
Use vm.details for configuration
Types of changes
Screenshots (if appropriate):
How Has This Been Tested?