DaanHoogland commented on code in PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#discussion_r1302736984
##########
plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java:
##########
@@ -96,14 +95,7 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- List<HostVO> clusterHosts =
hostDao.listByClusterAndHypervisorType(host.getClusterId(),
host.getHypervisorType());
- HostVO hostWithMinSocket =
clusterHosts.stream().min(Comparator.comparing(HostVO::getCpuSockets)).orElse(null);
Review Comment:
this is looking for sockets instead of vcpu, i.e. cores. might this be the
problem?
##########
plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java:
##########
@@ -96,14 +95,7 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- List<HostVO> clusterHosts =
hostDao.listByClusterAndHypervisorType(host.getClusterId(),
host.getHypervisorType());
- HostVO hostWithMinSocket =
clusterHosts.stream().min(Comparator.comparing(HostVO::getCpuSockets)).orElse(null);
- Integer vCpus =
MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId());
- if (hostWithMinSocket != null &&
hostWithMinSocket.getCpuSockets() != null &&
- hostWithMinSocket.getCpuSockets() < vCpus) {
- vCpus = hostWithMinSocket.getCpuSockets();
- }
Review Comment:
again sockets instead of cores
##########
plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/XenServerGuru.java:
##########
@@ -96,14 +95,7 @@ public VirtualMachineTO implement(VirtualMachineProfile vm) {
if (userVmVO != null) {
HostVO host = hostDao.findById(userVmVO.getHostId());
if (host != null) {
- List<HostVO> clusterHosts =
hostDao.listByClusterAndHypervisorType(host.getClusterId(),
host.getHypervisorType());
- HostVO hostWithMinSocket =
clusterHosts.stream().min(Comparator.comparing(HostVO::getCpuSockets)).orElse(null);
- Integer vCpus =
MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId());
- if (hostWithMinSocket != null &&
hostWithMinSocket.getCpuSockets() != null &&
- hostWithMinSocket.getCpuSockets() < vCpus) {
- vCpus = hostWithMinSocket.getCpuSockets();
- }
- to.setVcpuMaxLimit(vCpus);
+
to.setVcpuMaxLimit(MaxNumberOfVCPUSPerVM.valueIn(host.getClusterId()));
Review Comment:
What I see this code do is if there is a host in the cluster with less VCPU
than the setting for this cluster, this is the maximum a VM can have.
I see why this needs reverting, but it might also give issues in clusters
where some hosts have less cores than others.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
