You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "weizhouapache (via GitHub)" <gi...@apache.org> on 2023/08/21 11:52:35 UTC

[GitHub] [cloudstack] weizhouapache opened a new pull request, #7887: xen/xcpng: revert java changes in PR #4672

weizhouapache opened a new pull request, #7887:
URL: https://github.com/apache/cloudstack/pull/7887

   This fixes #7375
   
   ### Description
   
   This PR...
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- 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: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### Types of changes
   
   - [ ] 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)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   
   ### 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. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#discussion_r1302740294


##########
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:
   `getCpus()` instead of `getCpuSockets()`?



-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1693157345

   @weizhouapache a [SF] Trillian-Jenkins test job (centos7 mgmt + xcpng82) 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1694510275

   @blueorangutan test centos7 xcpng81


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695144271

   @harikrishna-patnala 
   let's use cpuCores instead of cpuSockets, as @DaanHoogland suggested.


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1687550663

   <b>[SF] Trillian test result (tid-7499)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 39997 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7887-t7499-xenserver-71.zip
   Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: set vmr.VCPUsMax to minimum of global setting and host cpu cores

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695527445

   @blueorangutan test rocky8 xenserver-71


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686348023

   
   @blueorangutan test rocky8 xcpng82
   
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695326166

   @weizhouapache a [SF] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: set vmr.VCPUsMax to minimum of global setting and host cpu cores

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695411291

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6914


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695153723

   @harikrishna-patnala @DaanHoogland 
   I thought about it again, I think we should not set the vcpu max limit to the minimum cpu cores of host in the cluster.
   it might be possible that some hosts have enough capacity (cpu cores and cpu speed), but vms cannot be deployed.


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686324726

   @weizhouapache [SF] unsupported parameters provided. Supported mgmt server os are: `centos7, centos6, suse15, alma8, ubuntu18, ubuntu22, ubuntu20, rocky8, alma9`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-rocky8, kvm-alma8, kvm-alma9, kvm-ubuntu18, kvm-ubuntu20, kvm-ubuntu22, kvm-suse15, vmware-55u3, vmware-60u2, vmware-65u2, vmware-67u3, vmware-70u1, vmware-70u2, vmware-70u3, vmware-80, vmware-80u1, xenserver-65sp1, xenserver-71, xenserver-74, xcpng74, xcpng76, xcpng80, xcpng81, xcpng82`


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686906413

   <b>[SF] Trillian Build Failed (tid-7498)<b/>


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1689212133

   <b>[SF] Trillian test result (tid-7505)</b>
   Environment: xcpng82 (x2), Advanced Networking with Mgmt server r8
   Total time taken: 65076 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7887-t7505-xcpng82.zip
   Smoke tests completed. 103 look OK, 5 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_non_strict_host_anti_affinity | `Error` | 206.32 | test_nonstrict_affinity_group.py
   test_02_non_strict_host_affinity | `Error` | 115.60 | test_nonstrict_affinity_group.py
   test_deploy_vm_with_registered_userdata | `Error` | 827.37 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_allow | `Error` | 811.66 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_append | `Error` | 812.47 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_params | `Error` | 844.17 | test_register_userdata.py
   test_01_scale_vm | `Error` | 130.45 | test_scale_vm.py
   test_04_scale_vm_with_user_account | `Error` | 144.92 | test_scale_vm.py
   test_05_scale_vm_dont_allow_disk_offering_change | `Error` | 33.10 | test_scale_vm.py
   test_list_vms_metrics_admin | `Error` | 0.22 | test_metrics_api.py
   test_list_vms_metrics_history | `Error` | 0.23 | test_metrics_api.py
   test_list_volumes_metrics_history | `Error` | 0.18 | test_metrics_api.py
   test_01_deploy_vm_on_specific_host | `Error` | 0.10 | test_vm_deployment_planner.py
   test_02_deploy_vm_on_specific_cluster | `Error` | 0.09 | test_vm_deployment_planner.py
   test_03_deploy_vm_on_specific_pod | `Error` | 0.12 | test_vm_deployment_planner.py
   test_04_deploy_vm_on_host_override_pod_and_cluster | `Error` | 0.14 | test_vm_deployment_planner.py
   test_05_deploy_vm_on_cluster_override_pod | `Error` | 0.10 | test_vm_deployment_planner.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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1689671655

   @DaanHoogland @shwstppr 
   this PR breaks the smoke tests on xcpng82, but xenserver-71 works fine.
   I have no idea what caused it. 
   I'd like to close this PR and suggest @shwstppr @DaanHoogland to have to look at the issue #7375 .
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#discussion_r1307161147


##########
plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java:
##########
@@ -1338,7 +1338,7 @@ public VM createVmFromTemplate(final Connection conn, final VirtualMachineTO vmS
                 vmr.VCPUsMax = (long)vmSpec.getCpus();
             } else {
                 if (vmSpec.getVcpuMaxLimit() != null) {
-                    vmr.VCPUsMax = (long)vmSpec.getVcpuMaxLimit();
+                    vmr.VCPUsMax = Math.min(vmSpec.getVcpuMaxLimit(), _host.getCpus());

Review Comment:
   ```suggestion
                       vmr.VCPUsMax = Math.min(vmSpec.getVcpuMaxLimit(), (long) _host.getCpus());
   ```



-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695325358

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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686187449

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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1693153127

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6893


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686348796

   @weizhouapache a [SF] Trillian-Jenkins test job (rocky8 mgmt + xcpng82) 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1687667648

   
   @blueorangutan test centos7 xcpng82 keepEnv
   
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695233981

   Packaging result [SF]: :heavy_multiplication_x: el7 :heavy_check_mark: debian. SL-JID 6909


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695283738

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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland merged pull request #7887: xen/xcpng: set vmr.VCPUsMax to minimum of global setting and host cpu cores

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland merged PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: set vmr.VCPUsMax to minimum of global setting and host cpu cores

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695530339

   @weizhouapache a [SF] Trillian-Jenkins test job (rocky8 mgmt + xenserver-71) 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686189028

   @weizhouapache a [SF] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686323457

   
   @blueorangutan test rocky8 xcpng-82
   
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1694770393

   @weizhouapache a [SF] Trillian-Jenkins test job (centos7 mgmt + xcpng81) 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: set vmr.VCPUsMax to minimum of global setting and host cpu cores

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695526689

   @DaanHoogland @harikrishna-patnala @shwstppr 
   I have manually tested ok
   
   - The env have 2 hosts (3 sockets, 1 cpu per socket)
   ```
   MariaDB [cloud]> select cpus,cpu_sockets from host where type='Routing';
   +------+-------------+
   | cpus | cpu_sockets |
   +------+-------------+
   |    3 |           3 |
   |    3 |           3 |
   +------+-------------+
   2 rows in set (0.00 sec)
   ```
   
   - Updated db manually to simulate hosts with 1 socket and 3 cpus per socket.
   ```
   MariaDB [cloud]> update host set cpu_sockets  = 1 where type='Routing';
   Query OK, 2 rows affected (0.01 sec)
   Rows matched: 2  Changed: 2  Warnings: 0
   
   MariaDB [cloud]> 
   MariaDB [cloud]> select cpus,cpu_sockets from host where type='Routing';
   +------+-------------+
   | cpus | cpu_sockets |
   +------+-------------+
   |    3 |           1 |
   |    3 |           1 |
   +------+-------------+
   2 rows in set (0.00 sec)
   ```
   
   - changed global/zone configuration to enable dynamic scale
   enable.dynamic.scale.vm = true
   
   - changed template to "Dynamically scalable"
   ![image](https://github.com/apache/cloudstack/assets/57355700/1f0b9715-496a-4fdc-8a83-98cc52b0b072)
   
   - created a service offering with 2 cpus and 1GB memory
   
   - create a vm with the new service offering and template
   
   without this PR, it failed
   ```
   com.cloud.utils.exception.CloudRuntimeException: Unable to start VM(i-2-21-VM) on host(4282a91e-dc93-4305-a21a-d87ce4fe7e87) due to Task failed! Task record:                 uuid: 5f732b21-7e74-cf31-2cf4-ddc4f48517bc
              nameLabel: Async.VM.start_on
        nameDescription:
      allowedOperations: []
      currentOperations: {}
                created: Mon Aug 28 10:39:13 UTC 2023
               finished: Mon Aug 28 10:39:13 UTC 2023
                 status: failure
             residentOn: com.xensource.xenapi.Host@b83408c7
               progress: 1.0
                   type: <none/>
                 result:
              errorInfo: [VALUE_NOT_SUPPORTED, VCPUs-at-startup, 2, value greater than VCPUs-max]
            otherConfig: {}
              subtaskOf: com.xensource.xenapi.Task@aaf13f6f
               subtasks: []
   ```
   
   with this PR, succeed to create a vm
   
   
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1694733496

   <b>[SF] Trillian test result (tid-7574)</b>
   Environment: xcpng81 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 72407 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7887-t7574-xcpng81.zip
   Smoke tests completed. 102 look OK, 6 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_02_list_cpvm_vm | `Failure` | 0.04 | test_ssvm.py
   test_04_cpvm_internals | `Failure` | 0.04 | test_ssvm.py
   test_01_non_strict_host_anti_affinity | `Error` | 227.27 | test_nonstrict_affinity_group.py
   test_02_non_strict_host_affinity | `Error` | 115.91 | test_nonstrict_affinity_group.py
   test_deploy_vm_with_registered_userdata | `Error` | 832.07 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_allow | `Error` | 847.73 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_append | `Error` | 824.29 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_params | `Error` | 902.95 | test_register_userdata.py
   test_01_scale_vm | `Error` | 156.53 | test_scale_vm.py
   test_04_scale_vm_with_user_account | `Error` | 163.44 | test_scale_vm.py
   test_05_scale_vm_dont_allow_disk_offering_change | `Error` | 46.22 | test_scale_vm.py
   test_list_vms_metrics_admin | `Error` | 0.16 | test_metrics_api.py
   test_list_vms_metrics_history | `Error` | 0.15 | test_metrics_api.py
   test_list_volumes_metrics_history | `Error` | 0.13 | test_metrics_api.py
   test_01_deploy_vm_on_specific_host | `Error` | 0.08 | test_vm_deployment_planner.py
   test_02_deploy_vm_on_specific_cluster | `Error` | 0.05 | test_vm_deployment_planner.py
   test_03_deploy_vm_on_specific_pod | `Error` | 0.08 | test_vm_deployment_planner.py
   test_04_deploy_vm_on_host_override_pod_and_cluster | `Error` | 0.10 | test_vm_deployment_planner.py
   test_05_deploy_vm_on_cluster_override_pod | `Error` | 0.07 | test_vm_deployment_planner.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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1687665914

   @weizhouapache [SF] unsupported parameters provided. Supported mgmt server os are: `centos7, centos6, suse15, alma8, ubuntu18, ubuntu22, ubuntu20, rocky8, alma9`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-rocky8, kvm-alma8, kvm-alma9, kvm-ubuntu18, kvm-ubuntu20, kvm-ubuntu22, kvm-suse15, vmware-55u3, vmware-60u2, vmware-65u2, vmware-67u3, vmware-70u1, vmware-70u2, vmware-70u3, vmware-80, vmware-80u1, xenserver-65sp1, xenserver-71, xenserver-74, xcpng74, xcpng76, xcpng80, xcpng81, xcpng82`


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686440009

   <b>[SF] Trillian Build Failed (tid-7491)<b/>


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#discussion_r1307160295


##########
plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java:
##########
@@ -1338,7 +1338,7 @@ public VM createVmFromTemplate(final Connection conn, final VirtualMachineTO vmS
                 vmr.VCPUsMax = (long)vmSpec.getCpus();
             } else {
                 if (vmSpec.getVcpuMaxLimit() != null) {
-                    vmr.VCPUsMax = (long)vmSpec.getVcpuMaxLimit();
+                    vmr.VCPUsMax = Math.min(vcpuMaxLimit, _host.getCpus());

Review Comment:
   ```suggestion
                       vmr.VCPUsMax = Math.min(vmSpec.getVcpuMaxLimit(), _host.getCpus());
   ```



-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695313826

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6913


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: set vmr.VCPUsMax to minimum of global setting and host cpu cores

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695461117

   @blueorangutan test rocky8 xenserver-71


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695227602

   > > > @harikrishna-patnala @DaanHoogland I thought about it again, I think we should not set the vcpu max limit to the minimum cpu cores of host in the cluster. it might be possible that some hosts have enough capacity (cpu cores and cpu speed), but vms cannot be deployed.
   > > 
   > > 
   > > you are right @weizhouapache , we should actually set it to the maximum for any host in the cluster, if at all. I wonder if the setting is meant for operators to have some control over the kind of VMs that can be deployed...
   > 
   > My opinion is we either use the setting or specific host cpu details (decided by the deployment planner). Using maximum of any host in the cluster may result in using higher value in host which has lower capacity in the cluster (host which is decided by the deployment planner)
   
   @harikrishna-patnala 
   good discussion.
   
   Based on it, I updated the PR to set the vmr.VCPUsMax to minimum of global setting and cpu cores of specific hosts.


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695228722

   @weizhouapache a [SF] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695284176

   @weizhouapache a [SF] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache closed pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache closed pull request #7887: xen/xcpng: revert java changes in PR #4672
URL: https://github.com/apache/cloudstack/pull/7887


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686264089

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6841


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686359143

   
   @blueorangutan test rocky8 xenserver-71


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1688896245

   <b>[SF] Trillian test result (tid-7504)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server r8
   Total time taken: 41108 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7887-t7504-xenserver-71.zip
   Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1693085746

   @weizhouapache a [SF] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1693083158

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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1694770241

   @blueorangutan test centos7 xcpng81


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1693089989

   > @DaanHoogland @shwstppr this PR breaks the smoke tests on xcpng82, but xenserver-71 works fine. I have no idea what caused it. I'd like to close this PR and suggest @shwstppr @DaanHoogland to have to look at the issue #7375 .
   
   update: 
   I tested rocky8/xcpng82 with PR7345 (health check PR), it looks the test failures also exist.
   Therefore I reopened this issue, I will run another round of 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1693156823

   
   @blueorangutan test centos7 xcpng82
   
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] harikrishna-patnala commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695195930

   > > @harikrishna-patnala @DaanHoogland I thought about it again, I think we should not set the vcpu max limit to the minimum cpu cores of host in the cluster. it might be possible that some hosts have enough capacity (cpu cores and cpu speed), but vms cannot be deployed.
   > 
   > you are right @weizhouapache , we should actually set it to the maximum for any host in the cluster, if at all. I wonder if the setting is meant for operators to have some control over the kind of VMs that can be deployed...
   
   My opinion is we either use the setting or specific host cpu details (decided by the deployment planner). Using maximum of any host in the cluster may result in using higher value in host which has lower capacity in the cluster (host which is decided by the deployment planner)


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: set vmr.VCPUsMax to minimum of global setting and host cpu cores

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695465565

   @weizhouapache a [SF] Trillian-Jenkins test job (rocky8 mgmt + xenserver-71) 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695227895

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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695157440

   > @harikrishna-patnala @DaanHoogland I thought about it again, I think we should not set the vcpu max limit to the minimum cpu cores of host in the cluster. it might be possible that some hosts have enough capacity (cpu cores and cpu speed), but vms cannot be deployed.
   
   you are right @weizhouapache , we should actually set it to the maximum for any host in the cluster, if at all. I wonder if the setting is meant for operators to have some control over the kind of VMs that can be deployed...


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: check hosts cpu cores intead of cpu sockets

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695148639

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

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1694155208

   <b>[SF] Trillian test result (tid-7561)</b>
   Environment: xcpng82 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 62521 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7887-t7561-xcpng82.zip
   Smoke tests completed. 103 look OK, 5 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_non_strict_host_anti_affinity | `Error` | 198.03 | test_nonstrict_affinity_group.py
   test_02_non_strict_host_affinity | `Error` | 113.03 | test_nonstrict_affinity_group.py
   test_deploy_vm_with_registered_userdata | `Error` | 808.42 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_allow | `Error` | 812.76 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_append | `Error` | 822.84 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_params | `Error` | 815.66 | test_register_userdata.py
   test_01_scale_vm | `Error` | 134.01 | test_scale_vm.py
   test_04_scale_vm_with_user_account | `Error` | 142.20 | test_scale_vm.py
   test_05_scale_vm_dont_allow_disk_offering_change | `Error` | 35.01 | test_scale_vm.py
   test_list_vms_metrics_admin | `Error` | 0.19 | test_metrics_api.py
   test_list_vms_metrics_history | `Error` | 0.16 | test_metrics_api.py
   test_list_volumes_metrics_history | `Error` | 0.15 | test_metrics_api.py
   test_01_deploy_vm_on_specific_host | `Error` | 0.08 | test_vm_deployment_planner.py
   test_02_deploy_vm_on_specific_cluster | `Error` | 0.07 | test_vm_deployment_planner.py
   test_03_deploy_vm_on_specific_pod | `Error` | 0.09 | test_vm_deployment_planner.py
   test_04_deploy_vm_on_host_override_pod_and_cluster | `Error` | 0.09 | test_vm_deployment_planner.py
   test_05_deploy_vm_on_cluster_override_pod | `Error` | 0.06 | test_vm_deployment_planner.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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686518097

   <b>[SF] Trillian Build Failed (tid-7492)<b/>


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] codecov[bot] commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686264291

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/7887?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#7887](https://app.codecov.io/gh/apache/cloudstack/pull/7887?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (a83b902) into [4.18](https://app.codecov.io/gh/apache/cloudstack/commit/ddc2a362a88eb5f4347693a9c1ec9ba8922a4b77?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (ddc2a36) will **increase** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@            Coverage Diff            @@
   ##               4.18    #7887   +/-   ##
   =========================================
     Coverage     13.04%   13.04%           
   - Complexity     9067     9068    +1     
   =========================================
     Files          2720     2720           
     Lines        257236   257230    -6     
     Branches      40103    40101    -2     
   =========================================
   + Hits          33552    33554    +2     
   + Misses       219474   219465    -9     
   - Partials       4210     4211    +1     
   ```
   
   
   | [Files Changed](https://app.codecov.io/gh/apache/cloudstack/pull/7887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [.../main/java/com/cloud/hypervisor/XenServerGuru.java](https://app.codecov.io/gh/apache/cloudstack/pull/7887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGx1Z2lucy9oeXBlcnZpc29ycy94ZW5zZXJ2ZXIvc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9YZW5TZXJ2ZXJHdXJ1LmphdmE=) | `49.45% <0.00%> (+3.05%)` | :arrow_up: |
   
   ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/cloudstack/pull/7887/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686362944

   @weizhouapache a [SF] Trillian-Jenkins test job (rocky8 mgmt + xenserver-71) 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1694510601

   @weizhouapache a [SF] Trillian-Jenkins test job (centos7 mgmt + xcpng81) 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] harikrishna-patnala commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695189018

   Reposting my comment here again (which is on the issue #7375 ) regarding cpuCores or cpuSockets.
   
   https://support.citrix.com/article/CTX236977/overcommitting-pcpus-on-individual-xenserver-vms this document says "Citrix recommends that you do not run a VM with more virtual CPUs (vCPUs) than the number physical CPUs (pCPUs) available on the XenServer host"
   I think this is the reason we are setting based on host details.
   
   Admin anyways have the control of setting vCPU max using the cluster/global level setting, so this should be fine, unless this won't break any existing 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695303442

   Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 6911


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: check hosts cpu cores intead of cpu sockets

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695149321

   @weizhouapache a [SF] Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: set vmr.VCPUsMax to minimum of global setting and host cpu cores

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1696547050

   <b>[SF] Trillian test result (tid-7590)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server r8
   Total time taken: 40529 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7887-t7590-xenserver-71.zip
   Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on pull request #7887: xen/xcpng: set vmr.VCPUsMax to minimum of global setting and host cpu cores

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1695607184

   reproduced and validated :+1: 


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686323185

   @blueorangutan test rocky xcpng-82


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1686629565

   <b>[SF] Trillian Build Failed (tid-7493)<b/>


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] blueorangutan commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1687612667

   <b>[SF] Trillian test result (tid-7490)</b>
   Environment: xcpng82 (x2), Advanced Networking with Mgmt server r8
   Total time taken: 62077 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7887-t7490-xcpng82.zip
   Smoke tests completed. 103 look OK, 5 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_non_strict_host_anti_affinity | `Error` | 195.62 | test_nonstrict_affinity_group.py
   test_02_non_strict_host_affinity | `Error` | 120.50 | test_nonstrict_affinity_group.py
   test_deploy_vm_with_registered_userdata | `Error` | 809.85 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_allow | `Error` | 834.18 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_override_policy_append | `Error` | 812.00 | test_register_userdata.py
   test_deploy_vm_with_registered_userdata_with_params | `Error` | 857.08 | test_register_userdata.py
   test_01_scale_vm | `Error` | 136.66 | test_scale_vm.py
   test_04_scale_vm_with_user_account | `Error` | 153.96 | test_scale_vm.py
   test_05_scale_vm_dont_allow_disk_offering_change | `Error` | 34.13 | test_scale_vm.py
   test_list_vms_metrics_admin | `Error` | 0.22 | test_metrics_api.py
   test_list_vms_metrics_history | `Error` | 0.18 | test_metrics_api.py
   test_list_volumes_metrics_history | `Error` | 0.17 | test_metrics_api.py
   test_01_deploy_vm_on_specific_host | `Error` | 0.11 | test_vm_deployment_planner.py
   test_02_deploy_vm_on_specific_cluster | `Error` | 0.09 | test_vm_deployment_planner.py
   test_03_deploy_vm_on_specific_pod | `Error` | 0.10 | test_vm_deployment_planner.py
   test_04_deploy_vm_on_host_override_pod_and_cluster | `Error` | 0.15 | test_vm_deployment_planner.py
   test_05_deploy_vm_on_cluster_override_pod | `Error` | 0.10 | test_vm_deployment_planner.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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#issuecomment-1687663565

   
   @blueorangutan test rocky8 xcpng-82 keepEnv
   
   


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache closed pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache closed pull request #7887: xen/xcpng: revert java changes in PR #4672
URL: https://github.com/apache/cloudstack/pull/7887


-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#discussion_r1302788363


##########
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:
   @DaanHoogland 
   you should add comment to #4672 , not this PR :-D
    



-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7887: xen/xcpng: revert java changes in PR #4672

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7887:
URL: https://github.com/apache/cloudstack/pull/7887#discussion_r1305769377


##########
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:
   ah, I see



-- 
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: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org