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 2023/01/16 10:54:53 UTC

[GitHub] [cloudstack] soreana opened a new pull request, #6809: Reserve memory for host

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

   ### Description
   
   This pr is a follow up to the https://github.com/apache/cloudstack/pull/4259 quoted the following from the pr:
   
   > By default cloudstack reserves 1Gb of RAM in hosts 
   > using _dom0_memory field. Add a global setting
   > "host.reserved.mem.mb" which can used to either
   > increase or decrese the amount of memory which can be reserved
   
   <!--- 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)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [x] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [x] Minor
   
   
   
   ### How Has This Been Tested?
   
   Please refer to the original pr at https://github.com/apache/cloudstack/pull/4259
   
   
   <!-- 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] blueorangutan commented on pull request #6809: Reserve memory for host

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

   <b>[SF] Trillian Build Failed (tid-6592)<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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280520332

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6809 (SL-JID-2519)


-- 
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] wido commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
wido commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1053509434


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +472,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
             "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
             true, ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",
+            "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.", true);

Review Comment:
   Yes, I think so. As hardware might be different and maybe you have other strategies per cluster you might want to set this differently.



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1365801988

   > > @wido That is a good point. What would be the best? Should I make it cluster-level or add another cluster-level setting and keep the global one?
   > 
   > We can add granularity, right? So it can be global, but also per cluster, correct? If not, I'd suggest cluster only.
   
   @wido @soreana , we donΒ΄t have true granularity, but we can, if the scope is cluster, edit at global level a vaule that will then function as a default on cluster level. No need to keep the global level to be able to set it globally. I think though, that the global level will only function as default to those clusters that have not set it yet. I am not sure about the reset.


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1357416083

   @nvazquez @weizhouapache @GutoVeronezi @wido we need more reviews here, please


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1386905360

   @soreana a 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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1386905086

   @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 #6809: Reserve memory for host

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

   @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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1270158758

   @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


Re: [PR] Reserve memory for host [cloudstack]

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

   The situation is exactly the same as I commented on Jan 23, changing the global setting works properly (updating the database and agent.properties files) but the reset functionality (reset configuration) resets only the global setting, not the database and agent.properties files.
   
   The intial value is 1024:
   ```
   (localcloud) 🐱 > list configurations name=host.reserved.mem.mb
   {
     "configuration": [
       {
         "category": "Advanced",
         "component": "ConfigurationManagerImpl",
         "defaultvalue": "1024",
         "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
         "displaytext": "Host reserved mem mb",
         "group": "Hypervisor",
         "isdynamic": true,
         "name": "host.reserved.mem.mb",
         "subgroup": "Hypervisor",
         "type": "Number",
         "value": "1024"
       }
     ],
     "count": 1
   }
   ```
   We change it to 2048:
   ```
   (localcloud) 🐱 > update configuration name=host.reserved.mem.mb value=2048
   {
     "configuration": {
       "category": "Advanced",
       "component": "ConfigurationManagerImpl",
       "defaultvalue": "1024",
       "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
       "displaytext": "Host reserved mem mb",
       "group": "Hypervisor",
       "isdynamic": true,
       "name": "host.reserved.mem.mb",
       "subgroup": "Hypervisor",
       "type": "Number",
       "value": "2048"
     }
   ```
   
   The database is properly set:
   ```
   mysql> select dom0_memory,ram from host where type="Routing" and removed is null\G
   *************************** 1. row ***************************
   dom0_memory: 2147483648
           ram: 6154563584
   *************************** 2. row ***************************
   dom0_memory: 2147483648
           ram: 6154563584
   2 rows in set (0.00 sec)
   ```
   
   and the agent.properties files:
   ```
   # cat /etc/cloudstack/agent/agent.properties | grep reserved
   host.reserved.mem.mb=2048
   ```
   
   Now we reset the global configuration parameter:
   ```
   (localcloud) 🐱 > reset configuration name=host.reserved.mem.mb
   {
     "configuration": {
       "category": "Advanced",
       "component": "ConfigurationManagerImpl",
       "defaultvalue": "1024",
       "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
       "displaytext": "Host reserved mem mb",
       "group": "Hypervisor",
       "isdynamic": true,
       "name": "host.reserved.mem.mb",
       "subgroup": "Hypervisor",
       "type": "Number",
       "value": "1024"
     }
   }
   ```
   
   It properly shows the original value (1024). The database is not updated though:
   ```
   mysql> select dom0_memory,ram from host where type="Routing" and removed is null\G
   *************************** 1. row ***************************
   dom0_memory: 2147483648
           ram: 6154563584
   *************************** 2. row ***************************
   dom0_memory: 2147483648
           ram: 6154563584
   2 rows in set (0.00 sec)
   ```
   
   and the agent.properties files are not updated too:
   ```
   # cat /etc/cloudstack/agent/agent.properties | grep reserved
   host.reserved.mem.mb=2048
   ```


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


Re: [PR] Reserve memory for host [cloudstack]

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

   @soreana can you comment on @vladimirpetrov 's findings?


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


Re: [PR] Reserve memory for host [cloudstack]

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

   > I thought `need-love` means the PR has been ignored for long time and need some attentions. πŸ˜„
   
   oh, I interpret it as the code needs love ;)


-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280469630

   @DaanHoogland Sure, I'm on it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1387528688

   @soreana a 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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1387527090

   @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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1396762393

   @wido That in my view 
   
   > Overall it is looking good.
   > 
   > What I am missing is a validation that nobody can set a value <0 or maybe a super large like 1TB or example. Do we maybe need to set some lower and upper limits?
   
   @wido That is a good idea, but I'm not sure if there is a good way to validate the settings. Do you have any suggestion, where I can run those validations?


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1387299037

   @soreana a 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1271289930

   Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 4369


-- 
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 diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r996048501


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1371,11 +1373,19 @@ public boolean configureHostParams(final Map<String, String> params) {
         }
 
         if (params.get(Config.MigrateWait.toString()) != null) {
-            String value = (String)params.get(Config.MigrateWait.toString());
+            String value = (String) params.get(Config.MigrateWait.toString());
             Integer intValue = NumbersUtil.parseInt(value, -1);
             storage.persist("vm.migrate.wait", String.valueOf(intValue));
             _migrateWait = intValue;
         }
+        if (params.get(HOST_RESERVED_MEM_MB_STRING) != null) {
+            long value = Long.parseLong(params.get(HOST_RESERVED_MEM_MB_STRING));
+            s_logger.info("Reserved memory for host is " + value + "MB");
+            _dom0MinMem = value * 1024L * 1024L;
+            if (!String.valueOf(value).equals("")) {

Review Comment:
   ```suggestion
               if (!StringUtils.isEmpty(String.valueOf(value))) {
   ```



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278778933

   @soreana a 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278788332

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6809 (SL-JID-2506)


-- 
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] acs-robot commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278779047

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278946473

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6809 (SL-JID-2509)


-- 
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] sonarcloud[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278994856

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6809)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL)
   
   [![2.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '2.2%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list) [2.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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 #6809: Reserve memory for host

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

   @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


Re: [PR] Reserve memory for host [cloudstack]

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

   @shwstppr a [SL] 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1272367905

   <b>Trillian test result (tid-5082)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40461 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5082-kvm-centos7.zip
   Smoke tests completed. 103 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 563.98 | test_kubernetes_clusters.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] DaanHoogland commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1274795353

   > @DaanHoogland Can I also have this one on 4.18, please ? :)
   
   marked for 4.18 @soreana 


-- 
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 diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r996043314


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1862,4 +1870,50 @@ public void propagateChangeToAgents(Map<String, String> params) {
             sendCommandToAgents(hostsPerZone, params);
         }
     }
+
+    @Override
+    public void updateCapacityOfHosts() {
+        Map<Long, List<Long>> hostsByZone = new HashMap<>();
+        Map<String, String> params = new HashMap<>();
+        boolean status = true;
+        List<HostVO> allHosts = _resourceMgr.listAllHostsInAllZonesByType(Host.Type.Routing);
+        if (allHosts == null) {

Review Comment:
   ```suggestion
           if (CollectionUtils.isEmpty(allHosts)) {
   ```



-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r995486867


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +469,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
             "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
             true, ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<Integer>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",

Review Comment:
   Good catch. Btw, the other the configs have type in angle brackets. Should I remove those, as well? Maybe in separate pull request?



-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r995484827


##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -203,6 +156,52 @@
 import com.cloud.vm.dao.UserVmDetailsDao;
 import com.cloud.vm.dao.VMInstanceDao;
 import com.google.gson.Gson;
+import org.apache.cloudstack.annotation.AnnotationService;

Review Comment:
   Done!



-- 
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] acs-robot commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278664665

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
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] sonarcloud[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278870064

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6809)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL)
   
   [![2.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '2.2%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list) [2.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073392085


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -121,7 +124,10 @@
 import com.cloud.utils.nio.Task;
 import com.cloud.utils.time.InaccurateClock;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.collections.CollectionUtils;
 
+import static com.cloud.configuration.ConfigurationManagerImpl.HOST_RESERVED_MEM_MB;
+import static com.cloud.configuration.ConfigurationManagerImpl.HOST_RESERVED_MEM_MB_STRING;

Review Comment:
   Done πŸ‘ 



-- 
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] sonarcloud[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1386974576

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6809)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL)
   
   [![1.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '1.4%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list) [1.4% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1081127968


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -990,7 +992,16 @@ public boolean configure(final String name, final Map<String, Object> params) th
         _videoRam = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_VIDEO_RAM);
 
         // Reserve 1GB unless admin overrides
-        _dom0MinMem = ByteScaleUtils.mebibytesToBytes(AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_RESERVED_MEM_MB));
+        long reservedMemory = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_RESERVED_MEM_MB);
+
+        value = (String)params.get(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key());

Review Comment:
   these lines (997 to 1002) are not needed I think.
   
   @GutoVeronezi 
   can you confirm ?
   



-- 
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 #6809: Reserve memory for host

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

   > Trillian test result (tid-5952) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 43104 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5952-kvm-centos7.zip Smoke tests completed. 105 look OK, 2 have errors, 0 did not run Only failed and skipped tests results shown below:
   > Test 	Result 	Time (s) 	Test File
   > test_create_pvlan_network 	`Error` 	0.05 	test_pvlan.py
   > test_01_redundant_vpc_site2site_vpn 	`Failure` 	618.32 	test_vpc_vpn.py
   
   @soreana , and at this please?


-- 
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] soreana commented on pull request #6809: Reserve memory for host

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

   > @soreana , will you work on this before RC cutting? (I might postpone another week, but really want to more forward)
   
   @DaanHoogland Yes, I'm working on this PR. If nothing goes wrong, I can manage to fix it 🀞 
   Due to the VM reinstall issue last week, I couldn't work on this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1354716726

   @DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1356010195

   <b>Trillian test result (tid-5527)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 52062 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5527-vmware-65u2.zip
   Smoke tests completed. 105 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


Re: [PR] Reserve memory for host [cloudstack]

Posted by "soreana (via GitHub)" <gi...@apache.org>.
soreana closed pull request #6809: Reserve memory for host
URL: https://github.com/apache/cloudstack/pull/6809


-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1376916451

   @DaanHoogland Sure, I'm working on that in this sprint.


-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073398094


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1862,4 +1871,50 @@ public void propagateChangeToAgents(Map<String, String> params) {
             sendCommandToAgents(hostsPerZone, params);
         }
     }
+
+    @Override
+    public void updateCapacityOfHosts() {
+        Map<Long, List<Long>> hostsByZone = new HashMap<>();
+        Map<String, String> params = new HashMap<>();
+        boolean status = true;
+        List<HostVO> allHosts = _resourceMgr.listAllHostsInAllZonesByType(Host.Type.Routing);
+        if (CollectionUtils.isEmpty(allHosts)) {
+            return;
+        }
+
+        String value = HOST_RESERVED_MEM_MB.value().toString();
+        params.put(HOST_RESERVED_MEM_MB.key(), value);
+        for (HostVO host : allHosts) {
+            Long zoneId = host.getDataCenterId();
+            try {
+                // Update the "ram" for all hosts
+                long updatedHostRam = (host.getTotalMemory() + host.getDom0MinMemory()) - (Integer.parseInt(value) * 1024L * 1024L);
+                if (updatedHostRam > 0) {
+                    host.setTotalMemory(updatedHostRam);
+                    // Update "dom0_memory" in host table
+                    host.setDom0MinMemory(Integer.parseInt(value) * 1024L * 1024L);

Review Comment:
   Done πŸ‘ 



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073470091


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1346,11 +1357,19 @@ public boolean configureHostParams(final Map<String, String> params) {
         }
 
         if (params.get(Config.MigrateWait.toString()) != null) {
-            String value = (String)params.get(Config.MigrateWait.toString());
+            String value = (String) params.get(Config.MigrateWait.toString());
             Integer intValue = NumbersUtil.parseInt(value, -1);
             storage.persist("vm.migrate.wait", String.valueOf(intValue));
             _migrateWait = intValue;
         }
+        if (params.get(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key()) != null) {
+            long value = Long.parseLong(params.get(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key()));
+            s_logger.info("Reserved memory for host is " + value + "MB");
+            _dom0MinMem = ByteScaleUtils.mebibytesToBytes(value);
+            if (!StringUtils.isEmpty(String.valueOf(value))) {
+                storage.persist(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key(), String.valueOf(value));
+            }
+        }

Review Comment:
   can you create a new method for this?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

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


[GitHub] [cloudstack] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1386984310

   > @soreana will you still change the new setting to be cluste scoped?
   
   Yes, @DaanHoogland I'm working on it. If the schedule is tight, we can merge this one, and I will create a follow-up PR to move it to the cluster level.


-- 
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] wido commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
wido commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1396473638

   Overall it is looking good.
   
   What I am missing is a validation that nobody can set a value <0 or maybe a super large like 1TB or example. Do we maybe need to set some lower and upper limits?


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1053517021


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +472,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
             "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
             true, ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",
+            "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.", true);

Review Comment:
   +1.
   would be good to change to a cluster setting



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1355870140

   <b>Trillian test result (tid-5525)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40347 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5525-xenserver-71.zip
   Smoke tests completed. 105 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] github-actions[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1362760289

   This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1270160816

   @soreana a 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278645713

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6809 (SL-JID-2503)


-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278671752

   @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] codecov[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278845967

   # [Codecov](https://codecov.io/gh/apache/cloudstack/pull/6809?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6809](https://codecov.io/gh/apache/cloudstack/pull/6809?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b35afa0) into [main](https://codecov.io/gh/apache/cloudstack/commit/713a236843c0c51f248d1bf05d198f935e6bf13d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (713a236) will **increase** coverage by `0.03%`.
   > The diff coverage is `3.17%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##               main    #6809      +/-   ##
   ============================================
   + Coverage     10.60%   10.63%   +0.03%     
   - Complexity     6849     6887      +38     
   ============================================
     Files          2466     2466              
     Lines        244549   244623      +74     
     Branches      38262    38289      +27     
   ============================================
   + Hits          25936    26019      +83     
   + Misses       215331   215320      -11     
   - Partials       3282     3284       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/6809?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ” | |
   |---|---|---|
   | [...pi/src/main/java/com/cloud/agent/AgentManager.java](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL2NvbXBvbmVudHMtYXBpL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FnZW50L0FnZW50TWFuYWdlci5qYXZh) | `33.33% <ΓΈ> (ΓΈ)` | |
   | [...java/com/cloud/agent/manager/AgentManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL29yY2hlc3RyYXRpb24vc3JjL21haW4vamF2YS9jb20vY2xvdWQvYWdlbnQvbWFuYWdlci9BZ2VudE1hbmFnZXJJbXBsLmphdmE=) | `0.00% <0.00%> (ΓΈ)` | |
   | [...ne/schema/src/main/java/com/cloud/host/HostVO.java](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9ob3N0L0hvc3RWTy5qYXZh) | `42.10% <0.00%> (-0.62%)` | :arrow_down: |
   | [...ervisor/kvm/resource/LibvirtComputingResource.java](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vcmVzb3VyY2UvTGlidmlydENvbXB1dGluZ1Jlc291cmNlLmphdmE=) | `15.65% <0.00%> (-0.04%)` | :arrow_down: |
   | [...n/java/com/cloud/resource/ResourceManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3Jlc291cmNlL1Jlc291cmNlTWFuYWdlckltcGwuamF2YQ==) | `0.00% <0.00%> (ΓΈ)` | |
   | [.../cloud/configuration/ConfigurationManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2NvbmZpZ3VyYXRpb24vQ29uZmlndXJhdGlvbk1hbmFnZXJJbXBsLmphdmE=) | `11.24% <12.50%> (+0.01%)` | :arrow_up: |
   | [...rg/apache/cloudstack/quota/QuotaStatementImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZnJhbWV3b3JrL3F1b3RhL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL3F1b3RhL1F1b3RhU3RhdGVtZW50SW1wbC5qYXZh) | `36.28% <0.00%> (-3.99%)` | :arrow_down: |
   | [...rg/apache/cloudstack/backup/veeam/VeeamClient.java](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9iYWNrdXAvdmVlYW0vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svYmFja3VwL3ZlZWFtL1ZlZWFtQ2xpZW50LmphdmE=) | `16.40% <0.00%> (-0.18%)` | :arrow_down: |
   | [...om/cloud/hypervisor/kvm/resource/LibvirtVMDef.java](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vcmVzb3VyY2UvTGlidmlydFZNRGVmLmphdmE=) | `64.15% <0.00%> (ΓΈ)` | |
   | ... and [19 more](https://codecov.io/gh/apache/cloudstack/pull/6809/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :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=The+Apache+Software+Foundation)
   


-- 
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] sonarcloud[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280585966

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6809)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL)
   
   [![2.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '2.2%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list) [2.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278929672

   @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] acs-robot commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280506525

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
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 #6809: Reserve memory for host

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

   @soreana 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 #6809: Reserve memory for host

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

   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 6150


-- 
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 #6809: Reserve memory for host

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

   @DaanHoogland a [LL] 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


Re: [PR] Reserve memory for host [cloudstack]

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

   <b>[SF] Trillian test result (tid-8076)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43953 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t8076-kvm-centos7.zip
   Smoke tests completed. 113 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


Re: [PR] Reserve memory for host [cloudstack]

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

   @soreana I am moving this forwards to 4.20. If you see chance to work on it, feel free to readjust the milestone.


-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r995480793


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -120,7 +88,43 @@
 import com.cloud.utils.nio.NioServer;
 import com.cloud.utils.nio.Task;
 import com.cloud.utils.time.InaccurateClock;
+import org.apache.cloudstack.agent.lb.IndirectAgentLB;

Review Comment:
   Done!



-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278776063

   > @soreana i see an error during build:
   > 
   > ```
   > [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.18.0.0-SNAPSHOT/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:101:8: Unused import - org.apache.cloudstack.framework.messagebus.MessageSubscriber. [UnusedImports]
   > ```
   
   @DaanHoogland fixed


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1279020890

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4470


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280504313

   @soreana a 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278779965

   @acs-robot a Jenkins job has been kicked to build UI QA env. 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] DaanHoogland commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1279092217

   @blueorangutan test matrix


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278922772

   @acs-robot a Jenkins job has been kicked to build UI QA env. 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1271407040

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4370


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1362836204

   @soreana can you look at the conflicts, please?


-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1358288122

   > > @nvazquez @weizhouapache @GutoVeronezi @wido we need more reviews here, please
   > 
   > Yes, sure.
   > 
   > However, looking at this, doesn't this make the config setting in the KVM agent.properties redundant?
   
   I don't think so. The config in the agent.properties applied to one host, although this global setting applies to all 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] GutoVeronezi commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1366626422

   > @wido @soreana , we donΒ΄t have true granularity, but we can, if the scope is cluster, edit at global level a vaule that will then function as a default on cluster level. No need to keep the global level to be able to set it globally. I think though, that the global level will only function as default to those clusters that have not set it yet. I am not sure about the reset.
   
   @DaanHoogland, regarding the behavior of the configurations, once we change a configuration on cluster level or another scope (except global), an entry in `cloud.<X>_details` (`cluster_details` for `Cluster`, `data_center_details` for `Zone`, and so on) is persisted, and then, for that scope, this entry will guide the behavior instead of the entry in `cloud.configurations` (global level). For resetting those configurations to the global (default) value, we can use the API [resetConfiguration](https://cloudstack.apache.org/api/apidocs-4.17/apis/resetConfiguration.html).
   
   


-- 
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] GutoVeronezi commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1366678641

   > Exactly, which should be good enough. Editing the global level should set the default on a scoped level. It might be that a scoped level that has not set this setting, keeps using the default value from the global level though, instead of the edited global value. I have not tested that and if it does that would be a bug.
   > 
   > a work around for this would be to edit the global default.
   
   It tries to retrieve the value from the scope:
   
   https://github.com/apache/cloudstack/blob/ca7e1ac1abbe78513bfdd57b990d58e018f8d65d/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java#L153-L164
   
   If it returns null (meaning the value is not defined for the scope), it will retrieve the value of the global configuration:
   
   https://github.com/apache/cloudstack/blob/ca7e1ac1abbe78513bfdd57b990d58e018f8d65d/framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java#L143-L151
   
   It will only use the default value if the global configuration is null (which should not happen) or its value is null.
   
   


-- 
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] github-actions[bot] commented on pull request #6809: Reserve memory for host

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

   This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073468223


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1862,4 +1871,49 @@ public void propagateChangeToAgents(Map<String, String> params) {
             sendCommandToAgents(hostsPerZone, params);
         }
     }
+
+    @Override
+    public void updateCapacityOfHosts() {

Review Comment:
   @soreana can you simplify this method; i.e. extract some code to smaller methods.



-- 
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 #6809: Reserve memory for host

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

   > Manual setting of values works fine but the reset functionality doesn't change the value in database:
   > 
   > ```
   > (localcloud) 🐱 > update configuration name=host.reserved.mem.mb value=2048
   > {
   >   "configuration": {
   >     "category": "Advanced",
   >     "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
   >     "isdynamic": true,
   >     "name": "host.reserved.mem.mb",
   >     "value": "2048"
   >   }
   > }
   > 
   > (localcloud) 🐱 > list configurations name=host.reserved.mem.mb
   > {
   >   "configuration": [
   >     {
   >       "category": "Advanced",
   >       "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
   >       "isdynamic": true,
   >       "name": "host.reserved.mem.mb",
   >       "value": "2048"
   >     }
   >   ],
   >   "count": 1
   > }
   > 
   > mysql> select dom0_memory,ram from host where type="Routing" and removed is null\G
   > *************************** 1. row ***************************
   > dom0_memory: 1073741824
   >         ram: 7234621440
   > *************************** 2. row ***************************
   > dom0_memory: 1073741824
   >         ram: 7234629632
   > 2 rows in set (0,00 sec)
   > ```
   > 
   > But when the 'reset' functionality is used:
   > 
   > ```
   > (localcloud) 🐱 > reset configuration name=host.reserved.mem.mb
   > {
   >   "configuration": {
   >     "category": "Advanced",
   >     "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
   >     "isdynamic": true,
   >     "name": "host.reserved.mem.mb",
   >     "value": "1024"
   >   }
   > }
   > ```
   > 
   > The value is changed only in the global setting, but not updated in the database:
   > 
   > ```
   > (localcloud) 🐱 > list configurations name=host.reserved.mem.mb
   > {
   >   "configuration": [
   >     {
   >       "category": "Advanced",
   >       "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
   >       "isdynamic": true,
   >       "name": "host.reserved.mem.mb",
   >       "value": "1024"
   >     }
   >   ],
   >   "count": 1
   > }
   > 
   > mysql> select dom0_memory,ram from host where type="Routing" and removed is null\G
   > *************************** 1. row ***************************
   > dom0_memory: 2147483648
   >         ram: 6160879616
   > *************************** 2. row ***************************
   > dom0_memory: 2147483648
   >         ram: 6160887808
   > 2 rows in set (0,00 sec)
   > ```
   
   @soreana can you look at this?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

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


[GitHub] [cloudstack] vladimirpetrov commented on pull request #6809: Reserve memory for host

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

   Manual setting of values works fine but the reset functionality doesn't change the value in database:
   
   ```
   (localcloud) 🐱 > update configuration name=host.reserved.mem.mb value=2048
   {
     "configuration": {
       "category": "Advanced",
       "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
       "isdynamic": true,
       "name": "host.reserved.mem.mb",
       "value": "2048"
     }
   }
   
   (localcloud) 🐱 > list configurations name=host.reserved.mem.mb
   {
     "configuration": [
       {
         "category": "Advanced",
         "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
         "isdynamic": true,
         "name": "host.reserved.mem.mb",
         "value": "2048"
       }
     ],
     "count": 1
   }
   
   mysql> select dom0_memory,ram from host where type="Routing" and removed is null\G
   *************************** 1. row ***************************
   dom0_memory: 1073741824
           ram: 7234621440
   *************************** 2. row ***************************
   dom0_memory: 1073741824
           ram: 7234629632
   2 rows in set (0,00 sec)
   ```
   But when the 'reset' functionality is used:
   ```
   (localcloud) 🐱 > reset configuration name=host.reserved.mem.mb
   {
     "configuration": {
       "category": "Advanced",
       "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
       "isdynamic": true,
       "name": "host.reserved.mem.mb",
       "value": "1024"
     }
   }
   ```
   The value is changed only in the global setting, but not updated in the database:
   ```
   (localcloud) 🐱 > list configurations name=host.reserved.mem.mb
   {
     "configuration": [
       {
         "category": "Advanced",
         "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
         "isdynamic": true,
         "name": "host.reserved.mem.mb",
         "value": "1024"
       }
     ],
     "count": 1
   }
   
   mysql> select dom0_memory,ram from host where type="Routing" and removed is null\G
   *************************** 1. row ***************************
   dom0_memory: 2147483648
           ram: 6160879616
   *************************** 2. row ***************************
   dom0_memory: 2147483648
           ram: 6160887808
   2 rows in set (0,00 sec)
   ```


-- 
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 #6809: Reserve memory for host

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

   <b>Trillian test result (tid-5952)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43104 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5952-kvm-centos7.zip
   Smoke tests completed. 105 look OK, 2 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_create_pvlan_network | `Error` | 0.05 | test_pvlan.py
   test_01_redundant_vpc_site2site_vpn | `Failure` | 618.32 | test_vpc_vpn.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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073721282


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1346,11 +1357,19 @@ public boolean configureHostParams(final Map<String, String> params) {
         }
 
         if (params.get(Config.MigrateWait.toString()) != null) {
-            String value = (String)params.get(Config.MigrateWait.toString());
+            String value = (String) params.get(Config.MigrateWait.toString());
             Integer intValue = NumbersUtil.parseInt(value, -1);
             storage.persist("vm.migrate.wait", String.valueOf(intValue));
             _migrateWait = intValue;
         }
+        if (params.get(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key()) != null) {
+            long value = Long.parseLong(params.get(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key()));
+            s_logger.info("Reserved memory for host is " + value + "MB");
+            _dom0MinMem = ByteScaleUtils.mebibytesToBytes(value);
+            if (!StringUtils.isEmpty(String.valueOf(value))) {
+                storage.persist(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key(), String.valueOf(value));
+            }
+        }

Review Comment:
   Done πŸ‘ 



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1081135154


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -990,7 +992,16 @@ public boolean configure(final String name, final Map<String, Object> params) th
         _videoRam = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_VIDEO_RAM);
 
         // Reserve 1GB unless admin overrides
-        _dom0MinMem = ByteScaleUtils.mebibytesToBytes(AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_RESERVED_MEM_MB));
+        long reservedMemory = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_RESERVED_MEM_MB);
+
+        value = (String)params.get(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key());

Review Comment:
   @soreana 
   if the value in agent.properties is 0 or less than 0, use the default (1024) ?



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1398358906

   > @wido That in my view
   > 
   > > Overall it is looking good.
   > > What I am missing is a validation that nobody can set a value <0 or maybe a super large like 1TB or example. Do we maybe need to set some lower and upper limits?
   > 
   > @wido That is a good idea, but I'm not sure if there is a good way to validate the settings. Do you have any suggestion, where I can run those validations?
   
   @soreana can you add a setting for that?
   
   @wido @weizhouapache is this really needed? this is an operator action anyway, isnΒ΄t it? they should know how to tune their cloud....!?!


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


Re: [PR] Reserve memory for host [cloudstack]

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

   > The situation is exactly the same as I commented on Jan 23, changing the global setting works properly (updating the database and agent.properties files) but the reset functionality (reset configuration) resets only the global setting, not the database and agent.properties files.
   > 
   
   good point @vladimirpetrov 
   this needs some changes indeed.
   
   the solution may be similar as
   ```
   messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT
   ```
   
   cc @soreana 


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


Re: [PR] Reserve memory for host [cloudstack]

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

   @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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1354532291

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4980


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1354715876

   @blueorangutan test matrix


-- 
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] wido commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
wido commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1365793297

   > > > > > @nvazquez @weizhouapache @GutoVeronezi @wido we need more reviews here, please
   > > > > 
   > > > > 
   > > > > Yes, sure.
   > > > > However, looking at this, doesn't this make the config setting in the KVM agent.properties redundant?
   > > > 
   > > > 
   > > > I don't think so. The config in the agent.properties applied to one host, although this global setting applies to all hosts.
   > > 
   > > 
   > > Get it. But I don't think this can be just a global setting. You also want to be able to set this per cluster. As clusters will defer in size and configuration.
   > 
   > @wido That is a good point. What would be the best? Should I make it cluster-level or add another cluster-level setting and keep the global one?
   
   We can add granularity, right? So it can be global, but also per cluster, correct? If not, I'd suggest cluster only.


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1270173172

   Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 4364


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1347997166

   @DaanHoogland a 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] DaanHoogland commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1347995958

   @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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1349388870

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 4905


-- 
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 #6809: Reserve memory for host

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

   @soreana , will you work on this before RC cutting? (I might postpone another week, but really want to more forward)
   


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r992405345


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -286,6 +176,112 @@
 import com.google.common.collect.Sets;
 import com.googlecode.ipv6.IPv6Address;
 import com.googlecode.ipv6.IPv6Network;
+import org.apache.cloudstack.acl.SecurityChecker;

Review Comment:
   why the order of imports change? can you change that back please?



##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +469,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
             "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
             true, ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<Integer>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",

Review Comment:
   ```suggestion
       public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",
   ```



##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -120,7 +88,43 @@
 import com.cloud.utils.nio.NioServer;
 import com.cloud.utils.nio.Task;
 import com.cloud.utils.time.InaccurateClock;
+import org.apache.cloudstack.agent.lb.IndirectAgentLB;

Review Comment:
   can you undo the import re-order?



##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -203,6 +156,52 @@
 import com.cloud.vm.dao.UserVmDetailsDao;
 import com.cloud.vm.dao.VMInstanceDao;
 import com.google.gson.Gson;
+import org.apache.cloudstack.annotation.AnnotationService;

Review Comment:
   can you undo the import re-order please?



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278748701

   @soreana i see an error during build:
   ```
   [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.18.0.0-SNAPSHOT/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:101:8: Unused import - org.apache.cloudstack.framework.messagebus.MessageSubscriber. [UnusedImports]
   ```


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278858727

   Packaging result: :heavy_check_mark: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_check_mark: suse15. SL-JID 4466


-- 
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] acs-robot commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278930022

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1273061265

   @DaanHoogland Can I also have this one on 4.18, please ? :)


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


Re: [PR] Reserve memory for host [cloudstack]

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

   @soreana can you please address merge conflicts and any outstanding comments? Should this be considered for 4.19.0.0?


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


Re: [PR] Reserve memory for host [cloudstack]

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

   > @soreana can you please address merge conflicts and any outstanding comments? Should this be considered for 4.19.0.0?
   
   @shwstppr please consider it for 4.19.0.0, I will address the merge conflict later tomorrow.


-- 
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 #6809: Reserve memory for host

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

   <b>[LL]Trillian test result (tid-6551)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 21178 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t6551-kvm-centos7.zip
   Smoke tests completed. 86 look OK, 0 have errors, 24 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   all_test_safe_shutdown | `Skipped` | --- | test_safe_shutdown.py
   all_test_metrics_api | `Skipped` | --- | test_metrics_api.py
   all_test_outofbandmanagement | `Skipped` | --- | test_outofbandmanagement.py
   all_test_outofbandmanagement_nestedplugin | `Skipped` | --- | test_outofbandmanagement_nestedplugin.py
   all_test_routers_iptables_default_policy | `Skipped` | --- | test_routers_iptables_default_policy.py
   all_test_secondary_storage | `Skipped` | --- | test_secondary_storage.py
   all_test_service_offerings | `Skipped` | --- | test_service_offerings.py
   all_test_storage_policy | `Skipped` | --- | test_storage_policy.py
   all_test_templates | `Skipped` | --- | test_templates.py
   all_test_update_security_group | `Skipped` | --- | test_update_security_group.py
   all_test_usage_events | `Skipped` | --- | test_usage_events.py
   all_test_vm_autoscaling | `Skipped` | --- | test_vm_autoscaling.py
   all_test_vm_deployment_planner | `Skipped` | --- | test_vm_deployment_planner.py
   all_test_vm_life_cycle | `Skipped` | --- | test_vm_life_cycle.py
   all_test_vm_lifecycle_unmanage_import | `Skipped` | --- | test_vm_lifecycle_unmanage_import.py
   all_test_vm_snapshot_kvm | `Skipped` | --- | test_vm_snapshot_kvm.py
   all_test_vm_snapshots | `Skipped` | --- | test_vm_snapshots.py
   all_test_volumes | `Skipped` | --- | test_volumes.py
   all_test_vpc_ipv6 | `Skipped` | --- | test_vpc_ipv6.py
   all_test_vpc_redundant | `Skipped` | --- | test_vpc_redundant.py
   all_test_vpc_router_nics | `Skipped` | --- | test_vpc_router_nics.py
   all_test_vpc_vpn | `Skipped` | --- | test_vpc_vpn.py
   all_test_host_maintenance | `Skipped` | --- | test_host_maintenance.py
   all_test_hostha_kvm | `Skipped` | --- | 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


Re: [PR] Reserve memory for host [cloudstack]

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

   > The situation is exactly the same as I commented on Jan 23, changing the global setting works properly (updating the database and agent.properties files) but the reset functionality (reset configuration) resets only the global setting, not the database and agent.properties files.
   > 
   > The intial value is 1024:
   > 
   > ```
   > (localcloud) 🐱 > list configurations name=host.reserved.mem.mb
   > {
   >   "configuration": [
   >     {
   >       "category": "Advanced",
   >       "component": "ConfigurationManagerImpl",
   >       "defaultvalue": "1024",
   >       "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
   >       "displaytext": "Host reserved mem mb",
   >       "group": "Hypervisor",
   >       "isdynamic": true,
   >       "name": "host.reserved.mem.mb",
   >       "subgroup": "Hypervisor",
   >       "type": "Number",
   >       "value": "1024"
   >     }
   >   ],
   >   "count": 1
   > }
   > ```
   > 
   > We change it to 2048:
   > 
   > ```
   > (localcloud) 🐱 > update configuration name=host.reserved.mem.mb value=2048
   > {
   >   "configuration": {
   >     "category": "Advanced",
   >     "component": "ConfigurationManagerImpl",
   >     "defaultvalue": "1024",
   >     "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
   >     "displaytext": "Host reserved mem mb",
   >     "group": "Hypervisor",
   >     "isdynamic": true,
   >     "name": "host.reserved.mem.mb",
   >     "subgroup": "Hypervisor",
   >     "type": "Number",
   >     "value": "2048"
   >   }
   > ```
   > 
   > The database is properly set:
   > 
   > ```
   > mysql> select dom0_memory,ram from host where type="Routing" and removed is null\G
   > *************************** 1. row ***************************
   > dom0_memory: 2147483648
   >         ram: 6154563584
   > *************************** 2. row ***************************
   > dom0_memory: 2147483648
   >         ram: 6154563584
   > 2 rows in set (0.00 sec)
   > ```
   > 
   > and the agent.properties files:
   > 
   > ```
   > # cat /etc/cloudstack/agent/agent.properties | grep reserved
   > host.reserved.mem.mb=2048
   > ```
   > 
   > Now we reset the global configuration parameter:
   > 
   > ```
   > (localcloud) 🐱 > reset configuration name=host.reserved.mem.mb
   > {
   >   "configuration": {
   >     "category": "Advanced",
   >     "component": "ConfigurationManagerImpl",
   >     "defaultvalue": "1024",
   >     "description": "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.",
   >     "displaytext": "Host reserved mem mb",
   >     "group": "Hypervisor",
   >     "isdynamic": true,
   >     "name": "host.reserved.mem.mb",
   >     "subgroup": "Hypervisor",
   >     "type": "Number",
   >     "value": "1024"
   >   }
   > }
   > ```
   > 
   > It properly shows the original value (1024). The database is not updated though:
   > 
   > ```
   > mysql> select dom0_memory,ram from host where type="Routing" and removed is null\G
   > *************************** 1. row ***************************
   > dom0_memory: 2147483648
   >         ram: 6154563584
   > *************************** 2. row ***************************
   > dom0_memory: 2147483648
   >         ram: 6154563584
   > 2 rows in set (0.00 sec)
   > ```
   > 
   > and the agent.properties files are not updated too:
   > 
   > ```
   > # cat /etc/cloudstack/agent/agent.properties | grep reserved
   > host.reserved.mem.mb=2048
   > ```
   
   @vladimirpetrov Thanks for testing the PR, I found the root cause, I'm working on that. 


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


Re: [PR] Reserve memory for host [cloudstack]

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

   Hi @soreana Can you please resolve the conflicts, and let me know if this PR can be targeted for 4.19.1. Thanks.


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280581620

   @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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1281546070

   <b>Trillian test result (tid-5152)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 42880 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5152-kvm-centos7.zip
   Smoke tests completed. 104 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1279629765

   <b>Trillian test result (tid-5134)</b>
   Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 39501 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5134-xenserver-71.zip
   Smoke tests completed. 104 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278672223

   @soreana a 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1279093529

   @DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280507967

   @acs-robot a Jenkins job has been kicked to build UI QA env. 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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1271342724

   @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] GutoVeronezi commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1052398464


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -450,6 +449,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     private Set<String> overprovisioningFactorsForValidation;
     public static final String VM_USERDATA_MAX_LENGTH_STRING = "vm.userdata.max.length";
 
+    public static final String HOST_RESERVED_MEM_MB_STRING = "host.reserved.mem.mb";

Review Comment:
   We could use `HOST_RESERVED_MEM_MB.key()` instead of declaring a constant with it.



##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +472,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
             "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
             true, ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",
+            "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.", true);

Review Comment:
   As hosts can behave differently between clusters, might the operators want to set a different value for each  cluster?



##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -2190,6 +2191,12 @@ protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource
                     hostTags = implicitHostTags;
                 }
             }
+
+            // Update host memory reported by agent
+            if (ssCmd.getHypervisorType().equals(HypervisorType.KVM) ||
+                    ssCmd.getHypervisorType().equals(HypervisorType.LXC)) {
+                host.setDom0MinMemory(Long.parseLong(_configDao.getValue(HOST_RESERVED_MEM_MB_STRING)) * 1024L * 1024L);

Review Comment:
   We could use `HOST_RESERVED_MEM_MB.value()` here, instead of retrieving with `_configDao`.



##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1862,4 +1871,50 @@ public void propagateChangeToAgents(Map<String, String> params) {
             sendCommandToAgents(hostsPerZone, params);
         }
     }
+
+    @Override
+    public void updateCapacityOfHosts() {
+        Map<Long, List<Long>> hostsByZone = new HashMap<>();
+        Map<String, String> params = new HashMap<>();
+        boolean status = true;
+        List<HostVO> allHosts = _resourceMgr.listAllHostsInAllZonesByType(Host.Type.Routing);
+        if (CollectionUtils.isEmpty(allHosts)) {
+            return;
+        }
+
+        String value = HOST_RESERVED_MEM_MB.value().toString();
+        params.put(HOST_RESERVED_MEM_MB.key(), value);
+        for (HostVO host : allHosts) {
+            Long zoneId = host.getDataCenterId();
+            try {
+                // Update the "ram" for all hosts
+                long updatedHostRam = (host.getTotalMemory() + host.getDom0MinMemory()) - (Integer.parseInt(value) * 1024L * 1024L);
+                if (updatedHostRam > 0) {
+                    host.setTotalMemory(updatedHostRam);
+                    // Update "dom0_memory" in host table
+                    host.setDom0MinMemory(Integer.parseInt(value) * 1024L * 1024L);

Review Comment:
   We can use `ByteScaleUtils` for scale conversions.



##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1862,4 +1871,50 @@ public void propagateChangeToAgents(Map<String, String> params) {
             sendCommandToAgents(hostsPerZone, params);
         }
     }
+
+    @Override
+    public void updateCapacityOfHosts() {
+        Map<Long, List<Long>> hostsByZone = new HashMap<>();
+        Map<String, String> params = new HashMap<>();
+        boolean status = true;
+        List<HostVO> allHosts = _resourceMgr.listAllHostsInAllZonesByType(Host.Type.Routing);
+        if (CollectionUtils.isEmpty(allHosts)) {
+            return;
+        }
+
+        String value = HOST_RESERVED_MEM_MB.value().toString();
+        params.put(HOST_RESERVED_MEM_MB.key(), value);
+        for (HostVO host : allHosts) {
+            Long zoneId = host.getDataCenterId();
+            try {
+                // Update the "ram" for all hosts
+                long updatedHostRam = (host.getTotalMemory() + host.getDom0MinMemory()) - (Integer.parseInt(value) * 1024L * 1024L);

Review Comment:
   We can use `ByteScaleUtils` for scale conversions.



##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1783,6 +1791,7 @@ public void processConnect(final Host host, final StartupCommand cmd, final bool
                     Map<String, String> params = new HashMap<String, String>();
                     params.put(Config.RouterAggregationCommandEachTimeout.toString(), _configDao.getValue(Config.RouterAggregationCommandEachTimeout.toString()));
                     params.put(Config.MigrateWait.toString(), _configDao.getValue(Config.MigrateWait.toString()));
+                    params.put(HOST_RESERVED_MEM_MB_STRING, _configDao.getValue(HOST_RESERVED_MEM_MB_STRING));

Review Comment:
   ```suggestion
                       params.put(HOST_RESERVED_MEM_MB.key(), HOST_RESERVED_MEM_MB.value());
   ```



##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -121,7 +124,10 @@
 import com.cloud.utils.nio.Task;
 import com.cloud.utils.time.InaccurateClock;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.collections.CollectionUtils;
 
+import static com.cloud.configuration.ConfigurationManagerImpl.HOST_RESERVED_MEM_MB;
+import static com.cloud.configuration.ConfigurationManagerImpl.HOST_RESERVED_MEM_MB_STRING;

Review Comment:
   IMHO, using the normal import would let the code more readable than using `import static`, as we would know from where the attribute is just by looking at the class qualification.



##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -2190,6 +2191,12 @@ protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource
                     hostTags = implicitHostTags;
                 }
             }
+
+            // Update host memory reported by agent
+            if (ssCmd.getHypervisorType().equals(HypervisorType.KVM) ||
+                    ssCmd.getHypervisorType().equals(HypervisorType.LXC)) {
+                host.setDom0MinMemory(Long.parseLong(_configDao.getValue(HOST_RESERVED_MEM_MB_STRING)) * 1024L * 1024L);

Review Comment:
   We can use `ByteScaleUtils` for scale conversions.



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1371,11 +1373,19 @@ public boolean configureHostParams(final Map<String, String> params) {
         }
 
         if (params.get(Config.MigrateWait.toString()) != null) {
-            String value = (String)params.get(Config.MigrateWait.toString());
+            String value = (String) params.get(Config.MigrateWait.toString());
             Integer intValue = NumbersUtil.parseInt(value, -1);
             storage.persist("vm.migrate.wait", String.valueOf(intValue));
             _migrateWait = intValue;
         }
+        if (params.get(HOST_RESERVED_MEM_MB_STRING) != null) {
+            long value = Long.parseLong(params.get(HOST_RESERVED_MEM_MB_STRING));
+            s_logger.info("Reserved memory for host is " + value + "MB");
+            _dom0MinMem = value * 1024L * 1024L;

Review Comment:
   We can use `ByteScaleUtils` for scale conversions.



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1386977827

   @soreana will you still change the new setting to be cluste scoped?


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1386973896

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5309


-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073709406


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1862,4 +1871,49 @@ public void propagateChangeToAgents(Map<String, String> params) {
             sendCommandToAgents(hostsPerZone, params);
         }
     }
+
+    @Override
+    public void updateCapacityOfHosts() {

Review Comment:
   Done. :+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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073409987


##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -2190,6 +2191,12 @@ protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource
                     hostTags = implicitHostTags;
                 }
             }
+
+            // Update host memory reported by agent
+            if (ssCmd.getHypervisorType().equals(HypervisorType.KVM) ||
+                    ssCmd.getHypervisorType().equals(HypervisorType.LXC)) {
+                host.setDom0MinMemory(Long.parseLong(_configDao.getValue(HOST_RESERVED_MEM_MB_STRING)) * 1024L * 1024L);

Review Comment:
   Done πŸ‘ 



-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073409078


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -450,6 +449,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     private Set<String> overprovisioningFactorsForValidation;
     public static final String VM_USERDATA_MAX_LENGTH_STRING = "vm.userdata.max.length";
 
+    public static final String HOST_RESERVED_MEM_MB_STRING = "host.reserved.mem.mb";

Review Comment:
   Done πŸ‘ 



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1081127968


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -990,7 +992,16 @@ public boolean configure(final String name, final Map<String, Object> params) th
         _videoRam = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_VIDEO_RAM);
 
         // Reserve 1GB unless admin overrides
-        _dom0MinMem = ByteScaleUtils.mebibytesToBytes(AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_RESERVED_MEM_MB));
+        long reservedMemory = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_RESERVED_MEM_MB);
+
+        value = (String)params.get(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key());

Review Comment:
   these lines are not needed I think.
   
   @GutoVeronezi 
   can you confirm ?
   



-- 
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] github-actions[bot] commented on pull request #6809: Reserve memory for host

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

   This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.


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


Re: [PR] Reserve memory for host [cloudstack]

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

   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 7485


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


Re: [PR] Reserve memory for host [cloudstack]

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

   > > @soreana I am moving this forwards to 4.20. If you see chance to work on it, feel free to readjust the milestone.
   > 
   > @DaanHoogland The PR is ready for the review, and I wish we can keep it in 4.19. I would ask for review let's see if we can get enough approval to merge it in the 4.19. (The PR was here since 4.16 πŸ˜„)
   
   Than why did you add the label 'needs-love'? It just needs review, right?


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


Re: [PR] Reserve memory for host [cloudstack]

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

   @soreana a [SL] 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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1387319769

   @DaanHoogland I've addressed your comments and moved the setting to cluster level.
   The pr is ready for further testing.


-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073396749


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1862,4 +1871,50 @@ public void propagateChangeToAgents(Map<String, String> params) {
             sendCommandToAgents(hostsPerZone, params);
         }
     }
+
+    @Override
+    public void updateCapacityOfHosts() {
+        Map<Long, List<Long>> hostsByZone = new HashMap<>();
+        Map<String, String> params = new HashMap<>();
+        boolean status = true;
+        List<HostVO> allHosts = _resourceMgr.listAllHostsInAllZonesByType(Host.Type.Routing);
+        if (CollectionUtils.isEmpty(allHosts)) {
+            return;
+        }
+
+        String value = HOST_RESERVED_MEM_MB.value().toString();
+        params.put(HOST_RESERVED_MEM_MB.key(), value);
+        for (HostVO host : allHosts) {
+            Long zoneId = host.getDataCenterId();
+            try {
+                // Update the "ram" for all hosts
+                long updatedHostRam = (host.getTotalMemory() + host.getDom0MinMemory()) - (Integer.parseInt(value) * 1024L * 1024L);

Review Comment:
   Done πŸ‘ 



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1396829906

   > @wido That in my view
   > 
   > > Overall it is looking good.
   > > What I am missing is a validation that nobody can set a value <0 or maybe a super large like 1TB or example. Do we maybe need to set some lower and upper limits?
   > 
   > @wido That is a good idea, but I'm not sure if there is a good way to validate the settings. Do you have any suggestion, where I can run those validations?
   
   @soreana 
   you can add the validation to `validateConfigurationValue` method in `ConfigurationManagerImpl` class.
   
   
   a upper limit makes sense.
   a lower limit might be unnecessary if there is a validation to make sure the value is greater than 0.


-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1387298035

   @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] soreana commented on pull request #6809: Reserve memory for host

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

   > > @wido That in my view
   > > > Overall it is looking good.
   > > > What I am missing is a validation that nobody can set a value <0 or maybe a super large like 1TB or example. Do we maybe need to set some lower and upper limits?
   > > 
   > > 
   > > @wido That is a good idea, but I'm not sure if there is a good way to validate the settings. Do you have any suggestion, where I can run those validations?
   > 
   > @soreana can you add a setting for that? πŸ˜‹
   > 
   > @wido @weizhouapache is this really needed? this is an operator action anyway, isnΒ΄t it? they should know how to tune their cloud....!?!
   
   @DaanHoogland I have mixed feelings about new global settings. On one hand, I like to prevent any damage. On the other hand, I love to let admins who don't know how to tune their data center screw it. 😈 
   
   Anyway, It would be better to merge this on 4.18 and work on the new global setting in a different milestone. I've created the #7129 to track that. Please assign it to me and plan it for the 4.18.1 milestone.
   
   
   @DaanHoogland @wido, @weizhouapache, @vladimirpetrov Thanks for testing and commenting. I'm working on a fix to see what went wrong with the database value and the test 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] DaanHoogland commented on pull request #6809: Reserve memory for host

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

   > @DaanHoogland Yes, I'm working on this PR. If nothing goes wrong, I can manage to fix it crossed_fingers
   > Due to the VM reinstall issue last week, I couldn't work on this.
   
   So when are you ready, +-? keep in mind that we need time to test and merge as well)


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1366666453

   > > @wido @soreana , we donΒ΄t have true granularity, but we can, if the scope is cluster, edit at global level a vaule that will then function as a default on cluster level. No need to keep the global level to be able to set it globally. I think though, that the global level will only function as default to those clusters that have not set it yet. I am not sure about the reset.
   > 
   > @DaanHoogland, regarding the behavior of the configurations, once we change a configuration on cluster level or another scope (except global), an entry in `cloud.<X>_details` (`cluster_details` for `Cluster`, `data_center_details` for `Zone`, and so on) is persisted, and then, for that scope, this entry will guide the behavior instead of the entry in `cloud.configurations` (global level). For resetting those configurations to the global (default) value, we can use the API [resetConfiguration](https://cloudstack.apache.org/api/apidocs-4.17/apis/resetConfiguration.html).
   
   Exactly, which should be good enough. Editing the global level should set the default on a scoped level. It might be that a scoped level that has not set this setting, keeps using the default value from the global level though, instead of the edited global value. I have not tested that and if it does that would be a bug.


-- 
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] wido commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
wido commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1366593083

   > > > @wido That is a good point. What would be the best? Should I make it cluster-level or add another cluster-level setting and keep the global one?
   > > 
   > > 
   > > We can add granularity, right? So it can be global, but also per cluster, correct? If not, I'd suggest cluster only.
   > 
   > @wido @soreana , we donΒ΄t have true granularity, but we can, if the scope is cluster, edit at global level a vaule that will then function as a default on cluster level. No need to keep the global level to be able to set it globally. I think though, that the global level will only function as default to those clusters that have not set it yet. I am not sure about the reset.
   
   Yes, that seem fine to me. I think you want to do this per cluster, but a global default sounds great.


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278671978

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6809 (SL-JID-2504)


-- 
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] sonarcloud[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278847256

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6809)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL)
   
   [![2.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '2.2%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list) [2.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278736934

   Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 4464


-- 
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] acs-robot commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278921629

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
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] acs-robot commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278788318

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278943782

   @soreana a 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278638186

   @acs-robot a Jenkins job has been kicked to build UI QA env. 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] acs-robot commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278637209

   Found UI changes, kicking a new UI QA build
   @blueorangutan ui


-- 
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] sonarcloud[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278996435

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6809)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL)
   
   [![2.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '2.2%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list) [2.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1271343651

   @soreana a 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] sonarcloud[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1271405762

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6809)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL)
   
   [![2.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '2.2%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list) [2.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list)
   
   


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


Re: [PR] Reserve memory for host [cloudstack]

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

   @blueorangutan packge


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


Re: [PR] Reserve memory for host [cloudstack]

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

   @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] GutoVeronezi commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1053229339


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +472,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
             "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
             true, ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",
+            "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.", true);

Review Comment:
   cc: @wido @soreana 



-- 
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 #6809: Reserve memory for host

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

   @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] soreana commented on pull request #6809: Reserve memory for host

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

   @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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073399576


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1371,11 +1373,19 @@ public boolean configureHostParams(final Map<String, String> params) {
         }
 
         if (params.get(Config.MigrateWait.toString()) != null) {
-            String value = (String)params.get(Config.MigrateWait.toString());
+            String value = (String) params.get(Config.MigrateWait.toString());
             Integer intValue = NumbersUtil.parseInt(value, -1);
             storage.persist("vm.migrate.wait", String.valueOf(intValue));
             _migrateWait = intValue;
         }
+        if (params.get(HOST_RESERVED_MEM_MB_STRING) != null) {
+            long value = Long.parseLong(params.get(HOST_RESERVED_MEM_MB_STRING));
+            s_logger.info("Reserved memory for host is " + value + "MB");
+            _dom0MinMem = value * 1024L * 1024L;

Review Comment:
   Done πŸ‘ 



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1387380322

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5317


-- 
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] wido commented on pull request #6809: Reserve memory for host

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

   > > @wido That in my view
   > > > Overall it is looking good.
   > > > What I am missing is a validation that nobody can set a value <0 or maybe a super large like 1TB or example. Do we maybe need to set some lower and upper limits?
   > > 
   > > 
   > > @wido That is a good idea, but I'm not sure if there is a good way to validate the settings. Do you have any suggestion, where I can run those validations?
   > 
   > @soreana can you add a setting for that? πŸ˜‹
   > 
   > @wido @weizhouapache is this really needed? this is an operator action anyway, isnΒ΄t it? they should know how to tune their cloud....!?!
   
   No, not really needed. New PR is fine.


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1081135154


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -990,7 +992,16 @@ public boolean configure(final String name, final Map<String, Object> params) th
         _videoRam = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_VIDEO_RAM);
 
         // Reserve 1GB unless admin overrides
-        _dom0MinMem = ByteScaleUtils.mebibytesToBytes(AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_RESERVED_MEM_MB));
+        long reservedMemory = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_RESERVED_MEM_MB);
+
+        value = (String)params.get(ConfigurationManagerImpl.HOST_RESERVED_MEM_MB.key());

Review Comment:
   @soreana 
   if the value is 0 or less than 0, use the default (1024) ?



-- 
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 #6809: Reserve memory for host

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

   @DaanHoogland 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] blueorangutan commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1271281755

   @soreana a 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1272239715

   @rohityadavcloud 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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1271280424

   @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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1279641643

   <b>Trillian test result (tid-5136)</b>
   Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43302 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5136-vmware-65u2.zip
   Smoke tests completed. 104 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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r995479471


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -286,6 +176,112 @@
 import com.google.common.collect.Sets;
 import com.googlecode.ipv6.IPv6Address;
 import com.googlecode.ipv6.IPv6Network;
+import org.apache.cloudstack.acl.SecurityChecker;

Review Comment:
   I have got an OCD alert when I saw them. That is why I reorganised them :D 
   + I redo the changes.



-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r995479471


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -286,6 +176,112 @@
 import com.google.common.collect.Sets;
 import com.googlecode.ipv6.IPv6Address;
 import com.googlecode.ipv6.IPv6Network;
+import org.apache.cloudstack.acl.SecurityChecker;

Review Comment:
   I have got an OCD alert when I saw them. :D
   I redo the changes.



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278930828

   @acs-robot a Jenkins job has been kicked to build UI QA env. 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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r995486867


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +469,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
             "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
             true, ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<Integer>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",

Review Comment:
   Good catch. Btw, the other the configs have type in angle brackets, should I remove those, as well? Maybe in separate pull request?



-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278778425

   @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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278939295

   UI build: :heavy_check_mark:
   Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6809 (SL-JID-2508)


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280465856

   @soreana I think nothing serious left. Can you answer the outstanding comments. I'll re-run the packaging/tests afterwards.


-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280503322

   @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


Re: [PR] Reserve memory for host [cloudstack]

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

   @blueorangutan packge


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


Re: [PR] Reserve memory for host [cloudstack]

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

   > > > @soreana I am moving this forwards to 4.20. If you see chance to work on it, feel free to readjust the milestone.
   > > 
   > > 
   > > @DaanHoogland The PR is ready for the review, and I wish we can keep it in 4.19. I would ask for review let's see if we can get enough approval to merge it in the 4.19. (The PR was here since 4.16 πŸ˜„)
   > 
   > Than why did you add the label 'needs-love'? It just needs review, right?
   
   I thought `need-love` means the PR has been ignored for long time and need some attentions. πŸ˜„


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


Re: [PR] Reserve memory for host [cloudstack]

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

   > @soreana I am moving this forwards to 4.20. If you see chance to work on it, feel free to readjust the milestone.
   
   @DaanHoogland The PR is ready for the review, and I wish we can keep it in 4.19. I would ask for review let's see if we can get enough approval to merge it in the 4.19. (The PR was here since 4.16 πŸ˜„)


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


Re: [PR] Reserve memory for host [cloudstack]

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

   This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278665289

   @acs-robot a Jenkins job has been kicked to build UI QA env. 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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1279634887

   <b>Trillian test result (tid-5135)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40850 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5135-kvm-centos7.zip
   Smoke tests completed. 103 look OK, 1 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_08_upgrade_kubernetes_ha_cluster | `Failure` | 838.50 | test_kubernetes_clusters.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] DaanHoogland commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r996743833


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +469,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
             "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
             true, ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<Integer>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",

Review Comment:
   can do in this one or a separate one. doesnΒ΄t matter.



-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r996776870


##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +469,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
             "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
             true, ConfigKey.Scope.Global, null);
+    public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<Integer>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",

Review Comment:
   Cool, will do that in separate one. In case someone want to back port it, he/she doesn't have to get unnecessary code changes.



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280566143

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4482


-- 
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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1278943145

   @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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1280582171

   @DaanHoogland 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] rohityadavcloud commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1272239544

   @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] wido commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
wido commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1358090765

   > @nvazquez @weizhouapache @GutoVeronezi @wido we need more reviews here, please
   
   Yes, sure.
   
   However, looking at this, doesn't this make the config setting in the KVM agent.properties redundant?
   
   


-- 
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] wido commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
wido commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1359052834

   > > > @nvazquez @weizhouapache @GutoVeronezi @wido we need more reviews here, please
   > > 
   > > 
   > > Yes, sure.
   > > However, looking at this, doesn't this make the config setting in the KVM agent.properties redundant?
   > 
   > I don't think so. The config in the agent.properties applied to one host, although this global setting applies to all hosts.
   
   Get it. But I don't think this can be just a global setting. You also want to be able to set this per cluster. As clusters will defer in size and configuration.


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1355927411

   <b>Trillian test result (tid-5526)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 43959 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6809-t5526-kvm-centos7.zip
   Smoke tests completed. 105 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] soreana commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1365781931

   > > > > @nvazquez @weizhouapache @GutoVeronezi @wido we need more reviews here, please
   > > > 
   > > > 
   > > > Yes, sure.
   > > > However, looking at this, doesn't this make the config setting in the KVM agent.properties redundant?
   > > 
   > > 
   > > I don't think so. The config in the agent.properties applied to one host, although this global setting applies to all hosts.
   > 
   > Get it. But I don't think this can be just a global setting. You also want to be able to set this per cluster. As clusters will defer in size and configuration.
   
   That is a good point. What would be the best? Should I make it cluster-level or add another cluster-level setting and keep the global one?


-- 
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 #6809: Reserve memory for host

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

   @DaanHoogland a [SF] 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] DaanHoogland commented on pull request #6809: Reserve memory for host

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

   @blueorangutan LLtest


-- 
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 #6809: Reserve memory for host

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

   <b>[SF] Trillian Build Failed (tid-6597)<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] DaanHoogland commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1376871901

   @soreana can you fix the conflicts and address @GutoVeronezi 's comments please?


-- 
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 closed pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland closed pull request #6809: Reserve memory for host
URL: https://github.com/apache/cloudstack/pull/6809


-- 
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] sonarcloud[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1387380192

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6809)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL)
   
   [![1.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '1.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list) [1.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1398224805

   @weizhouapache @wido @GutoVeronezi is this ready?/are you satisfied?/ did any of you 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] weizhouapache commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1398330107

   > @weizhouapache @wido @GutoVeronezi is this ready?/are you satisfied?/ did any of you test?
   
   @DaanHoogland 
   overall lgtm
   only Wido's comments need to be addressed. can we do it in separated PR ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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

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


[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1387667236

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6809)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6809&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6809&resolved=false&types=CODE_SMELL)
   
   [![1.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '1.2%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list) [1.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6809&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073409798


##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -2190,6 +2191,12 @@ protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource
                     hostTags = implicitHostTags;
                 }
             }
+
+            // Update host memory reported by agent
+            if (ssCmd.getHypervisorType().equals(HypervisorType.KVM) ||
+                    ssCmd.getHypervisorType().equals(HypervisorType.LXC)) {
+                host.setDom0MinMemory(Long.parseLong(_configDao.getValue(HOST_RESERVED_MEM_MB_STRING)) * 1024L * 1024L);

Review Comment:
   Done πŸ‘ 



-- 
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] soreana commented on a diff in pull request #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
soreana commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1073394316


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1783,6 +1791,7 @@ public void processConnect(final Host host, final StartupCommand cmd, final bool
                     Map<String, String> params = new HashMap<String, String>();
                     params.put(Config.RouterAggregationCommandEachTimeout.toString(), _configDao.getValue(Config.RouterAggregationCommandEachTimeout.toString()));
                     params.put(Config.MigrateWait.toString(), _configDao.getValue(Config.MigrateWait.toString()));
+                    params.put(HOST_RESERVED_MEM_MB_STRING, _configDao.getValue(HOST_RESERVED_MEM_MB_STRING));

Review Comment:
   Done πŸ‘ 



-- 
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 #6809: Reserve memory for host

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#issuecomment-1387641440

   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5318


-- 
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 #6809: Reserve memory for host

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

   @DaanHoogland a [SF] 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


Re: [PR] Reserve memory for host [cloudstack]

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

   @DaanHoogland a [SL] 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


Re: [PR] Reserve memory for host [cloudstack]

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

   > ```
   > messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT
   > ```
   
   @weizhouapache I saw that the message bus only sends out `EventTypes.EVENT_CONFIGURATION_VALUE_EDIT` when the configuration is updated globally. I'm currently working on resolving this so that it will also be emitted for cluster level updates and reset as well.
   
   https://github.com/apache/cloudstack/blob/d3cad4266af5b57482526d88a9407c5434857cb3/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java#L875-L878


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