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 2021/02/08 08:40:54 UTC
[GitHub] [cloudstack] DK101010 opened a new pull request #4662: Feat/ram reservation
DK101010 opened a new pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662
### Description
WIth this PR user can manuelly modify the ram reservation for a specific vm in vmware context. User set it via vm settings as a float value between 0 and 1
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] New feature (non-breaking change which adds functionality)
- [ ] 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
- [x] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
tested in own vmware environment
<!-- 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] nvazquez commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r686490135
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
Review comment:
@DK101010 I have examined the `setBasicVmConfig(..)` method is used on:
- `VmwareResource.execute(StartCommand cmd)` -> the values set on these removed lines are not relevant, are overridden later (https://github.com/apache/cloudstack/blob/main/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L1995-L2005)
- `HypervisorHostHelper.createBlankVm(..)` -> possible regression at this point: blank/worker VMs are always created with `cpuHotAdd = false` and `memHotAdd = false`. I'm not sure how/if this would affect deployments, any thoughts?
cc. @sureshanaparti @rhtyd @DaanHoogland
--
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889151832
@blueorangutan test centos7 vmware-67u3
--
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] rhtyd commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r660343877
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
I've my doubts on these changes, pinged vmware experts @harikrishna-patnala @nvazquez @sureshanaparti
--
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] davidjumani commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r674526440
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java
##########
@@ -139,6 +141,9 @@ VirtualMachineTO implement(VirtualMachineProfile vm, VirtualMachineTO to, long c
details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString());
}
}
+ if(StringUtils.isEmpty(details.get(VmDetailConstants.RAM_RESERVATION))){
Review comment:
Nope, passing 0 ensures that no memory is reserved, but 0.0 does not. When 0.0 is passed, all the memory is reserved
--
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-896820318
@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] nvazquez merged pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez merged pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662
--
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 change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r629136908
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -2905,7 +2905,7 @@ private void tearDownVm(VirtualMachineMO vmMo) throws Exception {
int getReservedMemoryMb(VirtualMachineTO vmSpec) {
if (vmSpec.getDetails().get(VMwareGuru.VmwareReserveMemory.key()).equalsIgnoreCase("true")) {
- if(!vmSpec.getDetails().get(VmDetailConstants.RAM_RESERVATION).equals("0.0")){
+ if(vmSpec.getDetails().get(VmDetailConstants.RAM_RESERVATION) != null && !vmSpec.getDetails().get(VmDetailConstants.RAM_RESERVATION).equals("0.0")){
Review comment:
how about
```suggestion
if(!"0.0".equals(vmSpec.getDetails().get(VmDetailConstants.RAM_RESERVATION))){
```
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-896820690
@nvazquez 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.
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-836519538
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 550
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-872617724
<b>Trillian test result (tid-1187)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33599 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4662-t1187-kvm-centos7.zip
Smoke tests completed. 88 look OK, 0 have error(s)
Only failed 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] davidjumani commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r673873927
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java
##########
@@ -139,6 +141,9 @@ VirtualMachineTO implement(VirtualMachineProfile vm, VirtualMachineTO to, long c
details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString());
}
}
+ if(StringUtils.isEmpty(details.get(VmDetailConstants.RAM_RESERVATION))){
Review comment:
Could cause an issue if the user does not want to reserve memory for a VM, better to leave it unset and check whether the key exists in getReservedMemoryMb
--
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 change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r575132757
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
ok @DK101010 , if that's verified I'm fine with the code.
----------------------------------------------------------------
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] nvazquez commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r689214743
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
Review comment:
Thanks @sureshanaparti
--
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] sureshanaparti commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r688315471
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
Review comment:
@nvazquez Hot-adding cpu & memory are set / overridden based on guest OS support. Also, the reservation and hot add are different things. So, I think, the hot add config here seems not relevant 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.
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889133604
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 695
--
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 change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r571964241
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
thsi removal does not seem relevant to your feature description. can you explain @DK101010 ?
----------------------------------------------------------------
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-778604000
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889835761
@nvazquez 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.
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-872271155
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-904332397
@blueorangutan test centos7 vmware-67u3
--
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] nvazquez closed pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez closed pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662
--
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-883013656
@davidjumani can you please test this PR? I think this will only apply for non-deploy-as-is VMs, but not for deploy-as-is
--
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 change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r574379685
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
not sure if that is needed and/or suffices; How about when people want to hot-add and don't care about reservation?
----------------------------------------------------------------
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-861036568
<b>Trillian test result (tid-937)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38713 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4662-t937-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 87 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_disable_oobm_ha_state_ineligible | `Error` | 1513.49 | test_hostha_kvm.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] GabrielBrascher commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r631882117
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java
##########
@@ -139,6 +140,9 @@ VirtualMachineTO implement(VirtualMachineProfile vm, VirtualMachineTO to, long c
details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString());
}
}
+ if(StringUtils.isEmpty(details.get(VmDetailConstants.RAM_RESERVATION))){
+ details.put(VmDetailConstants.RAM_RESERVATION, "0.0");
Review comment:
There are two occurrences of the "0.0" String.
What do you think of replacing these `"0.0"` with `NumberUtils.DOUBLE_ZERO`? The NumberUtils will return a `Double` object of `0.0d` and, therefore, it has a `toString()` that returns the String `"0.0"`.
Other option would be to create a constant with a self-describing name.
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
I have not much experience with the codebase for VMware to comment on this piece of code. Good that it has been verified. I will rely on the contributor 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] blueorangutan commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-904760222
<b>Trillian test result (tid-1759)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36206 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4662-t1759-vmware-67u3.zip
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed 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] davidjumani commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r673873927
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java
##########
@@ -139,6 +141,9 @@ VirtualMachineTO implement(VirtualMachineProfile vm, VirtualMachineTO to, long c
details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString());
}
}
+ if(StringUtils.isEmpty(details.get(VmDetailConstants.RAM_RESERVATION))){
Review comment:
Could cause an issue if the user does not want to reserve memory for a VM and passes 0.0, better to leave it unset and check whether the key exists in getReservedMemoryMb (Tested, passing 0.0 causes memory to be reserved)
--
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] DK101010 commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r681473223
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
Hi @nvazquez not sure what do you mean with regression.
Reservation and hot add are two different things. I can use reservation in it doesn't matter if hot add enabled/disbaled and it makes for me not sense to enable this when I'm reserve memory .
Also you can't more reserve as you have for the current vm.
I may be missing something, but currently I don't see a problem here.
--
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] davidjumani commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r673873927
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java
##########
@@ -139,6 +141,9 @@ VirtualMachineTO implement(VirtualMachineProfile vm, VirtualMachineTO to, long c
details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString());
}
}
+ if(StringUtils.isEmpty(details.get(VmDetailConstants.RAM_RESERVATION))){
Review comment:
Could cause an issue if the user does not want to reserve memory for a VM and passes 0.0, better to leave it unset and check whether the key exists in getReservedMemoryMb
--
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] davidjumani commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889110663
@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] DK101010 commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r574474281
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
@DaanHoogland hot add and reservation are two different features and they are independent of each other. I talked with my team and test it in my environment to get sure and in my env it works. In my point of view this code lines makes no sense.
----------------------------------------------------------------
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-883013656
@davidjumani can you please test this PR? I think this will only apply for non-deploy-as-is VMs, but not for deploy-as-is
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-896840210
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 843
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-778118787
@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] blueorangutan commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-778602671
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2702
----------------------------------------------------------------
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] DK101010 commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r573782520
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
@DaanHoogland sorry for my late answer. In my opinion I think yes and no.
Hot add memory and cpu will be enabled when reserved memory and cpu is in use. But this is not correct. Both features are independent, therefore I thought it is good to remove it in this PR. But if you want I can create a separate PR for this commit.
----------------------------------------------------------------
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889152436
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889862829
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] davidjumani commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889054840
@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] nvazquez commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r686490135
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
Review comment:
@DK101010 I have examined the `setBasicVmConfig(..)` method is used on:
- `VmwareResource.execute(StartCommand cmd)` -> the values set on these removed lines are not relevant, are overridden later (https://github.com/apache/cloudstack/blob/main/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L1995-L2005)
- `HypervisorHostHelper.createBlankVm(..)` -> possible regression at this point: blank VMs are always created with `cpuHotAdd = false` and `memHotAdd = false`. I'm not sure how/if this would affect deployments, any thoughts?
cc. @sureshanaparti @rhtyd @DaanHoogland
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-904331462
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 987
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889111822
@davidjumani 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] davidjumani commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r679083478
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java
##########
@@ -57,6 +57,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang3.math.NumberUtils;
Review comment:
```suggestion
```
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889135750
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 696
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889090180
Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_multiplication_x: debian. SL-JID 692
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-778604120
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] blueorangutan commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-778119003
@DaanHoogland 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] blueorangutan commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-778694744
<b>Trillian test result (tid-3537)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42405 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4662-t3537-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 84 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 779.98 | test_kubernetes_clusters.py
test_02_redundant_VPC_default_routes | `Failure` | 262.17 | test_vpc_redundant.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] rhtyd commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r660343877
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
I've my doubts on these changes, pinged vmware experts @harikrishna-patnala @nvazquez @sureshanaparti
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-904332815
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] DaanHoogland commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-837933293
@GabrielBrascher @sureshanaparti @nvazquez (not sure who else to bother), can you guys look at this?
tnx
--
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-872270954
@blueorangutan test
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-901607570
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] nvazquez commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r680624974
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
- vmConfig.setCpuHotAddEnabled(true);
- }
- if (memoryMB != memoryReserveMB) {
- vmConfig.setMemoryHotAddEnabled(true);
- }
Review comment:
@DK101010 I think removing these lines will cause a regression on cpu and memory overcommit, have you tested those cases?
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889722680
<b>Trillian test result (tid-1417)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 65614 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4662-t1417-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 81 look OK, 8 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_deployvm_userconcentrated | `Error` | 93.84 | test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userdispersing | `Error` | 6.26 | test_deploy_vms_with_varied_deploymentplanners.py
test_01_primary_storage_nfs | `Error` | 0.10 | test_primary_storage.py
ContextSuite context=TestStorageTags>:setup | `Error` | 0.18 | test_primary_storage.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 708.23 | test_internal_lb.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 708.25 | test_internal_lb.py
test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3606.42 | test_kubernetes_clusters.py
test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 3603.31 | test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.05 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.04 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 47.16 | test_kubernetes_clusters.py
test_04_rvpc_privategw_static_routes | `Failure` | 916.43 | test_privategw_acl.py
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 409.76 | test_routers_network_ops.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 835.79 | test_vpc_redundant.py
test_02_redundant_VPC_default_routes | `Failure` | 399.26 | test_vpc_redundant.py
test_02_redundant_VPC_default_routes | `Error` | 399.27 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 628.53 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Failure` | 361.98 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Error` | 361.99 | test_vpc_redundant.py
test_02_cancel_host_maintenace_with_migration_jobs | `Error` | 20.75 | test_host_maintenance.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] DK101010 commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r649735655
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java
##########
@@ -139,6 +140,9 @@ VirtualMachineTO implement(VirtualMachineProfile vm, VirtualMachineTO to, long c
details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString());
}
}
+ if(StringUtils.isEmpty(details.get(VmDetailConstants.RAM_RESERVATION))){
+ details.put(VmDetailConstants.RAM_RESERVATION, "0.0");
Review comment:
Hi @GabrielBrascher, sorry for my late answer, I must have missed the notification. I think there are no benefits regards reusable of this code, but I agree it's better to use parameter instead of strings. I'm prefer your first suggestion and will adapt 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
[GitHub] [cloudstack] DK101010 commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r683167140
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
Review comment:
Hi @nvazquez not sure what do you mean with regression.
Reservation and hot add are two different things. I can use reservation in it doesn't matter if hot add enabled/disbaled and it makes for me not sense to enable this when I'm reserve memory .
Also you can't more reserve as you have for the current vm.
I may be missing something, but currently I don't see a problem here.
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-896847960
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) 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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889862421
@blueorangutan test
--
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-896847123
@blueorangutan test centos7 vmware-67u3
--
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] DK101010 commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r674532662
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java
##########
@@ -139,6 +141,9 @@ VirtualMachineTO implement(VirtualMachineProfile vm, VirtualMachineTO to, long c
details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString());
}
}
+ if(StringUtils.isEmpty(details.get(VmDetailConstants.RAM_RESERVATION))){
Review comment:
Strange, I'll take care of it. Thanks for the infos.
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-871921478
@nvazquez 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.
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-778158344
Packaging result: ✖centos7 ✔centos8 ✖debian. JID-2687
----------------------------------------------------------------
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-859583262
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 214
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-778593654
@DaanHoogland 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] blueorangutan commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-890268450
<b>Trillian test result (tid-1432)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42843 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4662-t1432-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 85 look OK, 4 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_add_primary_storage_disabled_host | `Error` | 0.54 | test_primary_storage.py
test_01_primary_storage_nfs | `Error` | 0.11 | test_primary_storage.py
ContextSuite context=TestStorageTags>:setup | `Error` | 0.19 | test_primary_storage.py
test_02_list_snapshots_with_removed_data_store | `Error` | 1.17 | test_snapshots.py
test_01_secure_vm_migration | `Error` | 156.12 | test_vm_life_cycle.py
test_02_unsecure_vm_migration | `Error` | 278.68 | test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration | `Error` | 145.71 | test_vm_life_cycle.py
test_08_migrate_vm | `Error` | 43.80 | test_vm_life_cycle.py
test_hostha_kvm_host_degraded | `Error` | 705.74 | test_hostha_kvm.py
test_hostha_kvm_host_fencing | `Error` | 681.02 | test_hostha_kvm.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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-871945149
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 440
--
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-871921021
@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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889850622
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 711
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-837524335
<b>Trillian test result (tid-638)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44049 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4662-t638-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 88 look OK, 0 have error(s)
Only failed 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DK101010 commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r674520599
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VmwareVmImplementer.java
##########
@@ -139,6 +141,9 @@ VirtualMachineTO implement(VirtualMachineProfile vm, VirtualMachineTO to, long c
details.put(VmDetailConstants.NIC_ADAPTER, VirtualEthernetCardType.E1000.toString());
}
}
+ if(StringUtils.isEmpty(details.get(VmDetailConstants.RAM_RESERVATION))){
Review comment:
Hi @davidjumani , thanks for testing. It strange that it not work this line in getReservedMemory should prevent this
```java
if(!NumberUtils.DOUBLE_ZERO.toString().equals(vmSpec.getDetails().get(VmDetailConstants.RAM_RESERVATION))){
```
But the Method returns every time the minimum ram.
```java
return (int) (vmSpec.getMinRam() / ResourceType.bytesToMiB);
```
Could it be in your test case that the reserved memory = minimum ram is.
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-778593591
@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] blueorangutan commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889055001
@davidjumani 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889835517
@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] DK101010 commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r683167140
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
Review comment:
Hi @nvazquez not sure what do you mean with regression.
Reservation and hot add are two different things. I can use reservation in it doesn't matter if hot add enabled/disbaled and it makes for me not sense to enable this when I'm reserve memory .
Also you can't more reserve as you have for the current vm.
I may be missing something, but currently I don't see a problem here.
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889114538
Packaging result: :heavy_check_mark: el7 :heavy_multiplication_x: debian. SL-JID 694
--
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-883013656
@davidjumani can you please test this PR? I think this will only apply for non-deploy-as-is VMs, but not for deploy-as-is
--
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] nvazquez commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r681076778
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
##########
@@ -549,12 +549,6 @@ public static void setBasicVmConfig(VirtualMachineConfigSpec vmConfig, int cpuCo
cpuInfo.setReservation((long)cpuReservedMhz);
vmConfig.setCpuAllocation(cpuInfo);
- if (cpuSpeedMHz != cpuReservedMhz) {
Review comment:
@DK101010 I think removing these lines will cause a regression on cpu and memory overcommit, have you tested those cases?
--
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] davidjumani commented on a change in pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#discussion_r679629433
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -2906,7 +2905,10 @@ private void tearDownVm(VirtualMachineMO vmMo) throws Exception {
int getReservedMemoryMb(VirtualMachineTO vmSpec) {
if (vmSpec.getDetails().get(VMwareGuru.VmwareReserveMemory.key()).equalsIgnoreCase("true")) {
- return (int) (vmSpec.getMinRam() / ResourceType.bytesToMiB);
+ if(vmSpec.getDetails().get(VmDetailConstants.RAM_RESERVATION) != null){
+ float reservedMemory = (vmSpec.getMaxRam() * Float.parseFloat(vmSpec.getDetails().get(VmDetailConstants.RAM_RESERVATION)));
+ return (int) (reservedMemory / ResourceType.bytesToMiB);
+ }
Review comment:
```suggestion
}
return (int) (vmSpec.getMinRam() / ResourceType.bytesToMiB);
```
By default reserve all memory
--
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-899182795
@blueorangutan test centos7 vmware-67u3
--
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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-904318812
@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] nvazquez commented on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-901607227
@blueorangutan test centos7 vmware-67u3
--
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 removed a comment on pull request #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-889114538
Packaging result: :heavy_check_mark: el7 :heavy_multiplication_x: debian. SL-JID 694
--
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 #4662: Feat/ram reservation
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4662:
URL: https://github.com/apache/cloudstack/pull/4662#issuecomment-904318885
@nvazquez 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org