Skip to content

Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus. #4447

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

Closed
Spaceman1984 wants to merge 2 commits into from
Closed

Fixed failed to start instances on Xenserver with dynamic scaling due to default max cpus. #4447

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
27ceb7c
Limiting max cpus to max host value
Spaceman1984 Nov 5, 2020
e0cc29e
Added log message
Spaceman1984 Nov 6, 2020
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
14 changes: 13 additions & 1 deletion plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,19 @@ 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())) {
String message = String.valueOf(new StringBuilder()
.append("Ignoring global max vCPUs limit (from global setting xen.vm.vcpu.max) on vm ")
.append(vm.getUuid()).append(" and setting VM max vCPU to host pCPU max: ")
.append(host.getCpus())
.append(". Requested number of virtual CPUs exceeds number of host physical CPUs"));
logger.info(message);
to.setVcpuMaxLimit(host.getCpus());

@harikrishna-patnala harikrishna-patnala Nov 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

@Spaceman1984 Spaceman1984 Nov 6, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tested this with a host with 8 CPUs and it failed trying to allocate 16 on XCP-ng 7.6

@Spaceman1984 Spaceman1984 Nov 6, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll do some testing on Xenserver

@Spaceman1984 Spaceman1984 Nov 6, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tested XCP-NG 7.6 and Xenserver 7.6, I was unable to start a VM with more vCPUs than pCPUs.
XCP-NG
Xenserver

@Spaceman1984 Spaceman1984 Nov 6, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Seems like overcommitting CPU on Xenserver was removed according to this article @harikrishna-patnala https://support.citrix.com/article/CTX236977

@harikrishna-patnala harikrishna-patnala Nov 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@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= 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.

@Spaceman1984 Spaceman1984 Nov 6, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

@harikrishna-patnala harikrishna-patnala Nov 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay LGTM.

@harikrishna-patnala harikrishna-patnala Nov 6, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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"

@Spaceman1984 Spaceman1984 Nov 6, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alright, awesome, on it.

} else {
logger.info("Setting vm: " + vm.getUuid() + " max vCPU limit (from global setting xen.vm.vcpu.max) to: " + MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
}
}
}

Expand Down