You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/11/05 08:55:04 UTC
[GitHub] [cloudstack] Spaceman1984 opened a new pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Spaceman1984 opened a new pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447
## Description
<!--- Describe your changes in detail -->
When trying to start an instance with dynamic scaling on Xenserver with a Hypervisor with less than 16 cores, the VM fails to start. This is due to the default max value being set to 16. This PR limits the max vCPUs to what the host is capable of.
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Set enable.dynamic.scale.vm to true
Try to start an instance on a Xenserver host with less than 16 cores.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-725255291
@DaanHoogland I have determined this PR is related to #4443, I will merge this branch into that one and then close this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-723145992
looking by the discussion here, we should move this to 4.16 and make a good functional definition of what is and isn't possible on different xen platforms.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518537865
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
@Spaceman1984 got it, thanks for the article and yes it seems over committing CPU is disabled.
Do you think we have to also provide some global config "vcpu-unrestricted" to allow overcommit of CPU like xenserver does this way "xe vm-param-set uuid=<uuid> platform:vcpu-unrestricted=true". This gives admins to choose whether to overcommit or not.
Otherwise If we have to go with by setting only actual pCPUs, we need to remove this configuration parameter "xen.vm.vcpu.max" as we dont need it now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722967266
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-724960656
> looking by the discussion here, we should move this to 4.16 and make a good functional definition of what is and isn't possible on different xen platforms.
ping @Spaceman1984 @harikrishna-patnala . cc @rhtyd
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518515461
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
I tested XCP-NG 7.6 and Xenserver 7.6, I was unable to start a VM with more vCPUs than pCPUs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722967508
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518553838
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
please put a log message which explans why we are setting that value, otherwise user may think why a different value is set other than "xen.vm.vcpu.max"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722982952
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2337
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722794316
@blueorangutan test centos7 xcpng76
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722996985
@blueorangutan test centos7 xcpng76
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518549353
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
I kept the global var in to allow things like if you have a host with 32 pCPUs but want to set a lower max vCPU limit for a VM for the purpose of dynamic scaling.
As for the ability to force overcommit - This is not reccommended in the article and I would think not best practice, that's why I would vote no.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518524633
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
It might also depend on the supported limits on guest OS. but setting the max vcpu limit less than or equal to host cpus does not give complete leverage to scale the VM. Also this setting was there since long time so I think this is something to do with OS type of XCP ng version limitation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-723082050
<b>Trillian test result (tid-3130)</b>
Environment: xcpng76 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33232 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4447-t3130-xcpng76.zip
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_scale_vm | `Failure` | 10.39 | test_scale_vm.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-725273457
> > looking by the discussion here, we should move this to 4.16 and make a good functional definition of what is and isn't possible on different xen platforms.
>
> ping @Spaceman1984 @harikrishna-patnala . cc @rhtyd
@DaanHoogland agreed to keep it for 4.16 since this makes a substantial change in the existing flow or values assigned.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722794685
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + xcpng76) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518508407
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
I think host.getCpus() get the actual CPU on the physical host, but xenserver host is capable of allocating virtual CPUs more than the actual count.
Here is the list of configuration limits for XS 7.1 version. https://docs.citrix.com/en-us/xenserver/7-1/system-requirements/configuration-limits.html
It says "Virtual CPUs per VM (Linux)" 32. So we might need to set the value according to this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722261045
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2336
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722997243
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + xcpng76) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722238554
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518521764
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
Seems like overcommitting CPU on Xenserver was removed according to this article @harikrishna-patnala https://support.citrix.com/article/CTX236977
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-723276471
<b>Trillian test result (tid-3131)</b>
Environment: xcpng76 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33933 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4447-t3131-xcpng76.zip
Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_scale_vm | `Failure` | 9.32 | test_scale_vm.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722243657
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518549353
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
I kept the global var in to allow things like if you have a host with 32 pCPUs but want to set a lower max vCPU limit for a VM for the purpose of dynamic scaling.
As for the ability to force overcommit - This is not recommended in the article and I would think not best practice, that's why I would vote no.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518552551
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
Okay LGTM.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518509360
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
I tested this with a host with 8 CPUs and it failed trying to allocate 16 on XCP-ng 7.6
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518510415
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
I'll do some testing on Xenserver
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#issuecomment-722966536
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518521764
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
Seems like overcommit on xenserver was removed according to this article @harikrishna-patnala https://support.citrix.com/article/CTX236977
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4447: Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus.
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4447:
URL: https://github.com/apache/cloudstack/pull/4447#discussion_r518557625
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
##########
@@ -98,7 +98,12 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
+ // Max cpu cannot be more than host capability
+ if (host.getCpus() < MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()))
+ to.setVcpuMaxLimit(host.getCpus());
Review comment:
Alright, awesome, on it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org