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 2022/05/04 19:04:24 UTC
[GitHub] [cloudstack] joseflauzino opened a new pull request, #6358: Fix memory stats for KVM
joseflauzino opened a new pull request, #6358:
URL: https://github.com/apache/cloudstack/pull/6358
### Description
Using KVM hypervisor, the free memory stats of the user VMs that are returned by ACS do not correspond to those observed directly in the VMs (when using the `free -m` command, for example).
It was identified that the problem was in the memory stats collection process performed by Libvirt. This happens because the period that Libvirt would get memory stats updates for each VM was never set. Thus, even if ACS periodically requested the stats for the hosts, the data obtained were always the same.
This PR solves the mentioned problem. A new Agent configuration parameter called `vm.memballoon.stats.period` was created. This parameter allows operatos to set the time interval in which Libvirt will get memory stats updates. The default value of this parameter is 60 seconds, the same default value as the `vm.stats.interval` parameter. However, in a cloud containing multiple Management Servers, operators can set the value of `vm.memballoon.stats.period` lower than the `vm.stats.interval`, so that each Management Server always gets updated memory stats for all user VMs (since each Management Server can request stats at different instants).
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
### How Has This Been Tested?
In a local lab, I tested multiple combinations for all the related Agent properties (`vm.memballoon.stats.period`, `vm.stats.interval`, and `vm.memballoon.disable`). In all cases tested, the memory stats of the user VMs were obtained properly.
Also, I added unit 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] acs-robot commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1133644987
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] GutoVeronezi commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1015483402
##########
agent/conf/agent.properties:
##########
@@ -202,6 +202,11 @@ hypervisor.type=kvm
# Disable memory ballooning on vm guests for overcommit, by default overcommit
# feature enables balloon and sets currentMemory to a minimum value.
#
+# The time interval (in seconds) at which the balloon driver will get memory stats updates.
+# This is equivalent to Libvirt's --period parameter when using the dommemstat command.
+# Default value: 0
Review Comment:
As the commented property already has the default value, the line `# Default value: 0` is not necessary.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1298,9 +1308,85 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Gets the ID list of the VMs to set memory balloon stats period.
+ * @param conn the Libvirt connection.
+ * @return the list of VM IDs.
+ */
+ protected List<Integer> getVmsToSetMemoryBalloonStatsPeriod(Connect conn) {
+ List<Integer> vmIdList = new ArrayList<Integer>();
+ Integer[] vmIds = null;
+ try {
+ vmIds = ArrayUtils.toObject(conn.listDomains());
+ } catch (final LibvirtException e) {
+ s_logger.error("Unable to get the list of Libvirt domains on this host.", e);
+ return vmIdList;
+ }
+ vmIdList.addAll(Arrays.asList(vmIds));
+ s_logger.debug(String.format("We have found a total of [%s] VMs (Libvirt domains) on this host: [%s].", vmIdList.size(), vmIdList.toString()));
+
+ if (vmIdList.isEmpty()) {
+ s_logger.info("Skipping the memory balloon stats period setting, since there are no VMs (active Libvirt domains) on this host.");
+ }
+ return vmIdList;
+ }
+
+ /**
+ * Gets the current VM balloon stats period from the agent.properties file.
+ * @return the current VM balloon stats period.
+ */
+ protected Integer getCurrentVmBalloonStatsPeriod() {
+ if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE)) {
+ s_logger.info(String.format("The [%s] property is set to 'true', so the memory balloon stats period will be set to 0 for all VMs.",
+ AgentProperties.VM_MEMBALLOON_DISABLE.getName()));
+ return 0;
+ }
+ Integer vmBalloonStatsPeriod = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_STATS_PERIOD);
+ if(vmBalloonStatsPeriod == 0) {
Review Comment:
```suggestion
if (vmBalloonStatsPeriod == 0) {
```
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -440,6 +440,19 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
protected Boolean enableManuallySettingCpuTopologyOnKvmVm = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM);
+ /**
+ * Virsh command to set the memory balloon stats period.<br><br>
+ * 1st parameter: the VM name;<br>
+ * 2nd parameter: the period (in seconds).
+ */
+ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live";
Review Comment:
ping @joseflauzino
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1298,9 +1308,85 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Gets the ID list of the VMs to set memory balloon stats period.
+ * @param conn the Libvirt connection.
+ * @return the list of VM IDs.
+ */
+ protected List<Integer> getVmsToSetMemoryBalloonStatsPeriod(Connect conn) {
+ List<Integer> vmIdList = new ArrayList<Integer>();
+ Integer[] vmIds = null;
+ try {
+ vmIds = ArrayUtils.toObject(conn.listDomains());
+ } catch (final LibvirtException e) {
+ s_logger.error("Unable to get the list of Libvirt domains on this host.", e);
+ return vmIdList;
+ }
+ vmIdList.addAll(Arrays.asList(vmIds));
+ s_logger.debug(String.format("We have found a total of [%s] VMs (Libvirt domains) on this host: [%s].", vmIdList.size(), vmIdList.toString()));
+
+ if (vmIdList.isEmpty()) {
+ s_logger.info("Skipping the memory balloon stats period setting, since there are no VMs (active Libvirt domains) on this host.");
+ }
+ return vmIdList;
+ }
+
+ /**
+ * Gets the current VM balloon stats period from the agent.properties file.
+ * @return the current VM balloon stats period.
+ */
+ protected Integer getCurrentVmBalloonStatsPeriod() {
+ if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE)) {
+ s_logger.info(String.format("The [%s] property is set to 'true', so the memory balloon stats period will be set to 0 for all VMs.",
+ AgentProperties.VM_MEMBALLOON_DISABLE.getName()));
+ return 0;
+ }
+ Integer vmBalloonStatsPeriod = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_STATS_PERIOD);
+ if(vmBalloonStatsPeriod == 0) {
+ s_logger.info(String.format("The [%s] property is set to '0', this prevents memory statistics from being displayed correctly. "
+ + "Adjust (increase) the value of this parameter to correct this.", AgentProperties.VM_MEMBALLOON_STATS_PERIOD.getName()));
+ }
+ return vmBalloonStatsPeriod;
+ }
+
+ /**
+ * Sets the balloon driver of each VM to get the memory stats at the time interval defined in the agent.properties file.
+ * @param conn the Libvirt connection.
+ */
+ protected void setupMemoryBalloonStatsPeriod(Connect conn) {
+ List<Integer> vmIdList = getVmsToSetMemoryBalloonStatsPeriod(conn);
+ Integer currentVmBalloonStatsPeriod = getCurrentVmBalloonStatsPeriod();
+ for (Integer vmId : vmIdList) {
+ Domain dm = null;
+ try {
+ dm = conn.domainLookupByID(vmId);
+ parser.parseDomainXML(dm.getXMLDesc(0));
+ MemBalloonDef memBalloon = parser.getMemBalloon();
+ if (MemBalloonDef.MemBalloonModel.VIRTIO.equals(memBalloon.getMemBalloonModel())) {
+ String setMemBalloonStatsPeriodCommand = String.format(COMMAND_SET_MEM_BALLOON_STATS_PERIOD, vmId, currentVmBalloonStatsPeriod);
+ String setMemBalloonStatsPeriodResult = Script.runSimpleBashScript(setMemBalloonStatsPeriodCommand);
+ if (StringUtils.isNotBlank(setMemBalloonStatsPeriodResult)) {
+ s_logger.error(String.format("Unable to set up memory balloon stats period for VM (Libvirt Domain) with ID [%s] due to an error when running the [%s] "
+ + "command. Output: [%s].", vmId, setMemBalloonStatsPeriodCommand, setMemBalloonStatsPeriodResult));
+ continue;
+ }
+ s_logger.debug(String.format("The memory balloon stats period [%s] has been set successfully for the VM (Libvirt Domain) with ID [%s] and name [%s].",
+ currentVmBalloonStatsPeriod, vmId, dm.getName()));
+ continue;
+ }
+ s_logger.debug(String.format("Skipping the memory balloon stats period setting for the VM (Libvirt Domain) with ID [%s] and name [%s] because this VM has no memory"
+ + " balloon.", vmId, dm.getName()));
Review Comment:
We could invert those statements to avoid hadouken ifs.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java:
##########
@@ -342,6 +346,25 @@ public boolean parseDomainXML(String domXML) {
return false;
}
+ /**
+ * Parse the memballoon tag.
+ * @param devices the devices tag.
+ * @return the MemBalloonDef.
+ */
+ private MemBalloonDef parseMemBalloonTag(Element devices) {
+ MemBalloonDef def = new MemBalloonDef();
+ NodeList memBalloons = devices.getElementsByTagName("memballoon");
+ if ((memBalloons != null) && (memBalloons.getLength() != 0)) {
Review Comment:
```suggestion
if (memBalloons != null && memBalloons.getLength() != 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] blueorangutan commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1308083256
<b>Trillian test result (tid-46)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44501 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6358-t46-kvm-centos7.zip
Smoke tests completed. 102 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_upgrade_kubernetes_cluster | `Failure` | 264.05 | test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster | `Failure` | 492.99 | test_kubernetes_clusters.py
test_01_redundant_vpc_site2site_vpn | `Failure` | 749.85 | test_vpc_vpn.py
test_01_vpc_site2site_vpn_multiple_options | `Error` | 1275.47 | test_vpc_vpn.py
test_01_vpc_site2site_vpn | `Error` | 546.30 | 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] weizhouapache commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1293203372
@joseflauzino
I use some your changes in https://github.com/apache/cloudstack/pull/6571/commits/fb868986a290030332052561605a0903ac005fce
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1212113780
@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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1212120731
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6358 (SL-JID-2130)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1146018584
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [70.9% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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] GabrielBrascher commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r932761069
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -440,6 +440,19 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
protected Boolean enableManuallySettingCpuTopologyOnKvmVm = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM);
+ /**
+ * Virsh command to set the memory balloon stats period.<br><br>
+ * 1st parameter: the VM name;<br>
+ * 2nd parameter: the period (in seconds).
+ */
+ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live";
+
+ /**
+ * Virsh command to get the memory balloon tag from the VM's XML file.<br><br>
+ * 1st parameter: the VM name;<br>
+ */
+ private static final String COMMAND_GET_MEM_BALLOON_FROM_XML_FILE = "virsh dumpxml %s | grep '<memballoon model='";
Review Comment:
@joseflauzino there are some examples around the codebase. Maybe [this one](https://github.com/apache/cloudstack/blob/81b7e6e750a581375612b78fbf04ce94da573839/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java#L1184) can help :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1305708338
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [16 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [75.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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 merged pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1286911092
@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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1212125533
@joseflauzino 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] joseflauzino commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1212124728
@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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1301533853
@joseflauzino 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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1272285031
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_check_mark: suse15. SL-JID 4389
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1305885784
@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 a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1016607418
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1298,9 +1308,84 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Gets the ID list of the VMs to set memory balloon stats period.
+ * @param conn the Libvirt connection.
+ * @return the list of VM IDs.
+ */
+ protected List<Integer> getVmsToSetMemoryBalloonStatsPeriod(Connect conn) {
+ List<Integer> vmIdList = new ArrayList<Integer>();
Review Comment:
```suggestion
List<Integer> vmIdList = new ArrayList<>();
```
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1298,9 +1308,84 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Gets the ID list of the VMs to set memory balloon stats period.
+ * @param conn the Libvirt connection.
+ * @return the list of VM IDs.
+ */
+ protected List<Integer> getVmsToSetMemoryBalloonStatsPeriod(Connect conn) {
+ List<Integer> vmIdList = new ArrayList<Integer>();
+ Integer[] vmIds = null;
+ try {
+ vmIds = ArrayUtils.toObject(conn.listDomains());
+ } catch (final LibvirtException e) {
+ s_logger.error("Unable to get the list of Libvirt domains on this host.", e);
+ return vmIdList;
+ }
+ vmIdList.addAll(Arrays.asList(vmIds));
+ s_logger.debug(String.format("We have found a total of [%s] VMs (Libvirt domains) on this host: [%s].", vmIdList.size(), vmIdList.toString()));
+
+ if (vmIdList.isEmpty()) {
+ s_logger.info("Skipping the memory balloon stats period setting, since there are no VMs (active Libvirt domains) on this host.");
+ }
+ return vmIdList;
+ }
+
+ /**
+ * Gets the current VM balloon stats period from the agent.properties file.
+ * @return the current VM balloon stats period.
+ */
+ protected Integer getCurrentVmBalloonStatsPeriod() {
+ if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE)) {
Review Comment:
```suggestion
if (Boolean.TRUE.equals(AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE))) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1146024461
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3525
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1117848257
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [69.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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] sonarcloud[bot] commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1212188316
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [22 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [70.9% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r926714356
##########
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##########
@@ -54,6 +54,28 @@
*/
public static final Property<Boolean> ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM = new Property<Boolean>("enable.manually.setting.cpu.topology.on.kvm.vm", true);
+ /**
+ * Enable manually IO driver on KVM's VM. <br>
+ * Data type: boolean.<br>
+ * Default value: true.
+ */
+ public static final Property<Boolean> ENABLE_IO_URING = new Property<Boolean>("enable.io.uring", true);
Review Comment:
@joseflauzino
`ENABLE_IO_URING` is not needed I think.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1288,9 +1301,67 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Sets the balloon driver of each VM to get the memory stats at the time interval defined in the agent.properties file.
+ * @param conn the Libvirt connection.
+ */
+ protected void setupMemoryBalloonStatsPeriod(Connect conn) {
+ Integer[] vmIds = null;
+ try {
+ vmIds = ArrayUtils.toObject(conn.listDomains());
+ } catch (final LibvirtException e) {
+ s_logger.error("Unable to get the list of Libvirt domains on this host.", e);
+ return;
+ }
+ List<Integer> vmIdList = Arrays.asList(vmIds);
+ s_logger.debug(String.format("We have found a total of [%s] VMs (Libvirt domains) on this host: [%s].", vmIdList.size(), vmIdList.toString()));
+
+ if (vmIdList.isEmpty()) {
+ s_logger.debug("Skipping the memory balloon stats period setting, since there are no VMs (active Libvirt domains) on this host.");
+ return;
+ }
+
+ if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE)) {
+ s_logger.debug(String.format("Skipping the memory balloon stats period setting because the [%s] property is set to 'true'.",
+ AgentProperties.VM_MEMBALLOON_DISABLE.getName()));
+ return;
+ }
+
+ Integer currentVmBalloonStatsPeriod = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_STATS_PERIOD);
+
+ for (Integer vmId : vmIdList) {
+ String getMemBalloonFromXmlFileCommand = String.format(COMMAND_GET_MEM_BALLOON_FROM_XML_FILE, vmId);
Review Comment:
it is better to update `LibvirtDomainXMLParser` to support `GuestResourceDef` , instead of
`COMMAND_GET_MEM_BALLOON_FROM_XML_FILE = "virsh dumpxml %s | grep '<memballoon model='";`
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java:
##########
@@ -275,7 +279,14 @@ public String toString() {
response.append(String.format("<cpu> <numa> <cell id='0' cpus='0-%s' memory='%s' unit='KiB'/> </numa> </cpu>\n", this.maxVcpu - 1, this.currentMemory));
}
- response.append(String.format("<devices>\n<memballoon model='%s'/>\n</devices>\n", this.memoryBalloning ? "virtio" : "none"));
+ String memBalloonModel = "none";
+ int statsPeriod = 0;
+ if (this.memoryBalloning) {
+ memBalloonModel = "virtio";
+ statsPeriod = memoryBalloonStatsPeriod;
+ }
+ response.append(String.format("<devices>\n<memballoon model='%s'>\n<stats period='%s'/>\n</memballoon>\n</devices>\n", memBalloonModel, statsPeriod));
Review Comment:
if model is 'none', we do not need to add `<stats period='xxx'/>`
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1288,9 +1301,67 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Sets the balloon driver of each VM to get the memory stats at the time interval defined in the agent.properties file.
+ * @param conn the Libvirt connection.
+ */
+ protected void setupMemoryBalloonStatsPeriod(Connect conn) {
+ Integer[] vmIds = null;
+ try {
+ vmIds = ArrayUtils.toObject(conn.listDomains());
+ } catch (final LibvirtException e) {
+ s_logger.error("Unable to get the list of Libvirt domains on this host.", e);
+ return;
+ }
+ List<Integer> vmIdList = Arrays.asList(vmIds);
+ s_logger.debug(String.format("We have found a total of [%s] VMs (Libvirt domains) on this host: [%s].", vmIdList.size(), vmIdList.toString()));
+
+ if (vmIdList.isEmpty()) {
+ s_logger.debug("Skipping the memory balloon stats period setting, since there are no VMs (active Libvirt domains) on this host.");
+ return;
+ }
+
+ if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE)) {
+ s_logger.debug(String.format("Skipping the memory balloon stats period setting because the [%s] property is set to 'true'.",
+ AgentProperties.VM_MEMBALLOON_DISABLE.getName()));
Review Comment:
what about the existing vms which has memory ballooning enabled ?
should memory ballooning be disabled ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1286851600
@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 a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1002680696
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1288,9 +1298,66 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Sets the balloon driver of each VM to get the memory stats at the time interval defined in the agent.properties file.
+ * @param conn the Libvirt connection.
+ */
+ protected void setupMemoryBalloonStatsPeriod(Connect conn) {
Review Comment:
50 line method. Can you disect it into smaller parts, 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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1305939951
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. LL-JID 212
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r865642146
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -150,6 +153,29 @@ public int value() {
private String _keystoreSetupPath;
private String _keystoreCertImportPath;
+ /**
+ * Virsh command to list the name of all VM instances starting with "i-".
+ */
+ protected static final String COMMAND_LIST_ALL_VMS = "virsh list --all --name | grep '^i-*'";
+
+ /**
+ * Virsh command to set the memory balloon stats period.<br><br>
+ * 1st parameter: the VM name;<br>
+ * 2nd parameter: the period (in seconds).
+ */
+ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live";
+
+ /**
+ * Virsh command to get the memory balloon tag from the VM's XML file.<br><br>
+ * 1st parameter: the VM name;<br>
+ */
+ private static final String COMMAND_GET_MEM_BALLOON_FROM_XML_FILE = "virsh dumpxml %s | grep '<memballoon model='";
Review Comment:
(comment for all virsh usages)
Is there a libvirt equivalent to do this? Forking to shell isn't a good idea with arbitrary strings `%s` that may be passed to it.
In case libvirt doesn't support that, you may also raise a MR with https://gitlab.com/libvirt/libvirt-java and then use those 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] joseflauzino commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1119526817
@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] joseflauzino commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1133644968
I moved the code to a more appropriate location. Also, I adjusted the code to get the VM list (Libvirt domains) by using libvirt-java instead of a virsh command.
Thanks for the reviews.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1212160271
# [Codecov](https://codecov.io/gh/apache/cloudstack/pull/6358?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 [#6358](https://codecov.io/gh/apache/cloudstack/pull/6358?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (daa193a) into [main](https://codecov.io/gh/apache/cloudstack/commit/840c3f6a7adf0593a769ab52cf62b33d85de53c6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (840c3f6) will **increase** coverage by `0.02%`.
> The diff coverage is `89.74%`.
```diff
@@ Coverage Diff @@
## main #6358 +/- ##
============================================
+ Coverage 5.87% 5.90% +0.02%
- Complexity 3935 3942 +7
============================================
Files 2454 2454
Lines 242575 242651 +76
Branches 37970 37978 +8
============================================
+ Hits 14250 14319 +69
- Misses 226749 226755 +6
- Partials 1576 1577 +1
```
| [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/6358?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ypervisor/kvm/resource/LibvirtDomainXMLParser.java](https://codecov.io/gh/apache/cloudstack/pull/6358/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-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vcmVzb3VyY2UvTGlidmlydERvbWFpblhNTFBhcnNlci5qYXZh) | `48.52% <81.81%> (+1.62%)` | :arrow_up: |
| [...om/cloud/hypervisor/kvm/resource/LibvirtVMDef.java](https://codecov.io/gh/apache/cloudstack/pull/6358/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=) | `62.94% <88.46%> (+0.64%)` | :arrow_up: |
| [...ervisor/kvm/resource/LibvirtComputingResource.java](https://codecov.io/gh/apache/cloudstack/pull/6358/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=) | `17.17% <92.68%> (+1.20%)` | :arrow_up: |
: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] joseflauzino commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1212112323
Thanks for the review, @weizhouapache and @GabrielBrascher.
I have just committed the new version of the code. These are the relevant changes:
1. Now the stats parameter of the memballoon tag will be set to 0 when `vm.memballoon.disable=true`.
2. Virsh commands have been replaced by methods from the Domain class. The only exception is the command to set the stats parameter of the memballoon tag (`virsh dommemstat <vm-name-or-id> --period <value-in-seconds> --live`), which cannot be replaced by the `memoryStats(int number)` method. The reason is that this method only allows us to get the memory stats and not to set the new gathering period in the VM XML (as desired).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r992215798
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -440,6 +440,19 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
protected Boolean enableManuallySettingCpuTopologyOnKvmVm = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM);
+ /**
+ * Virsh command to set the memory balloon stats period.<br><br>
+ * 1st parameter: the VM name;<br>
+ * 2nd parameter: the period (in seconds).
+ */
+ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live";
Review Comment:
Can you answer this one @joseflauzino , did you investigate the java binding?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1286940917
<b>Trillian Build Failed (tid-5185)<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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1287547417
<b>Trillian test result (tid-5184)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39770 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6358-t5184-kvm-centos7.zip
Smoke tests completed. 99 look OK, 5 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_add_primary_storage_disabled_host | `Error` | 0.60 | test_primary_storage.py
test_01_primary_storage_nfs | `Error` | 0.10 | test_primary_storage.py
ContextSuite context=TestStorageTags>:setup | `Error` | 0.18 | test_primary_storage.py
test_01_secure_vm_migration | `Error` | 156.36 | test_vm_life_cycle.py
test_02_unsecure_vm_migration | `Error` | 270.83 | test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration | `Error` | 141.97 | test_vm_life_cycle.py
test_08_migrate_vm | `Error` | 43.85 | test_vm_life_cycle.py
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 33.00 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 60.73 | test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster | `Failure` | 38.07 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 39.99 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 130.33 | test_kubernetes_clusters.py
test_02_list_snapshots_with_removed_data_store | `Error` | 8.56 | test_snapshots.py
test_02_list_snapshots_with_removed_data_store | `Error` | 8.56 | test_snapshots.py
test_hostha_enable_ha_when_host_in_maintenance | `Error` | 304.80 | test_hostha_kvm.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1219246854
@joseflauzino it is alright not to agree with each comment, but please don't resolve them unanswered. please answer them and leave it to the commenter to resolve them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r932759440
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -440,6 +440,19 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
protected Boolean enableManuallySettingCpuTopologyOnKvmVm = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM);
+ /**
+ * Virsh command to set the memory balloon stats period.<br><br>
+ * 1st parameter: the VM name;<br>
+ * 2nd parameter: the period (in seconds).
+ */
+ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live";
+
+ /**
+ * Virsh command to get the memory balloon tag from the VM's XML file.<br><br>
+ * 1st parameter: the VM name;<br>
+ */
+ private static final String COMMAND_GET_MEM_BALLOON_FROM_XML_FILE = "virsh dumpxml %s | grep '<memballoon model='";
Review Comment:
Same here, there is already a Java bind for the `virsh dumpxml` via [Domain.getXMLDesc(int flags)](https://gitlab.com/libvirt/libvirt-java/-/blob/master/src/main/java/org/libvirt/Domain.java#L1296)
##########
agent/conf/agent.properties:
##########
@@ -202,6 +202,11 @@ hypervisor.type=kvm
# Disable memory ballooning on vm guests for overcommit, by default overcommit
# feature enables balloon and sets currentMemory to a minimum value.
#
+# The time interval (in seconds) at which the balloon driver will get memory stats updates.
+# This is equivalent to Libvirt's --period parameter when using the dommemstat command.
+# Default value: 60
+#vm.memballoon.stats.period=60
Review Comment:
```suggestion
# vm.memballoon.stats.period=60
```
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -440,6 +440,19 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
protected Boolean enableManuallySettingCpuTopologyOnKvmVm = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM);
+ /**
+ * Virsh command to set the memory balloon stats period.<br><br>
+ * 1st parameter: the VM name;<br>
+ * 2nd parameter: the period (in seconds).
+ */
+ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live";
Review Comment:
Why use `virsh` commands "by hand" if there is already Java bind for it via [Libvirt-Java Domain.memoryStats(int number)](https://gitlab.com/libvirt/libvirt-java/-/blob/master/src/main/java/org/libvirt/Domain.java#L1509)?
Is there something that the `memoryStats` does not support?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1301551291
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. LL-JID 184
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1301564257
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [26 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [75.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1286911708
@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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1286913818
@weizhouapache a Trillian-Jenkins test job (ubuntu22 mgmt + kvm-ubuntu22) 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 a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1002935441
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java:
##########
@@ -275,7 +280,14 @@ public String toString() {
response.append(String.format("<cpu> <numa> <cell id='0' cpus='0-%s' memory='%s' unit='KiB'/> </numa> </cpu>\n", this.maxVcpu - 1, this.currentMemory));
}
- response.append(String.format("<devices>\n<memballoon model='%s'/>\n</devices>\n", this.memoryBalloning ? "virtio" : "none"));
+ MemBalloonDef memBalloonDef = new MemBalloonDef();
+ if (this.memoryBalloning) {
+ memBalloonDef.defVirtioMemBalloon(String.valueOf(memoryBalloonStatsPeriod));
+ } else {
+ memBalloonDef.defNoneMemBalloon();
+ }
+ response.append(String.format("<devices>\n%s\n</devices>\n", memBalloonDef.toString()));
+
Review Comment:
can this go in a separate method, 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 a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1002935154
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java:
##########
@@ -196,6 +198,18 @@ public boolean parseDomainXML(String domXML) {
diskDefs.add(def);
}
+ NodeList memBalloons = devices.getElementsByTagName("memballoon");
+ if ((memBalloons != null) && (memBalloons.getLength() != 0)) {
+ Element memBalloon = (Element)memBalloons.item(0);
+ String model = memBalloon.getAttribute("model");
+ MemBalloonDef def = new MemBalloonDef();
+ if (model.equalsIgnoreCase("virtio")) {
+ String statsPeriod = getAttrValue("stats", "period", memBalloon);
+ def.defVirtioMemBalloon(statsPeriod);
+ }
+ memBalloonDef = def;
+ }
+
Review Comment:
can this go in a separate method, 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] rohityadavcloud commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1272245450
@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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1133643898
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] github-actions[bot] commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1134684269
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] shwstppr commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
shwstppr commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1216577772
@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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1219166275
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4014
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1012410782
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java:
##########
@@ -196,6 +198,18 @@ public boolean parseDomainXML(String domXML) {
diskDefs.add(def);
}
+ NodeList memBalloons = devices.getElementsByTagName("memballoon");
+ if ((memBalloons != null) && (memBalloons.getLength() != 0)) {
+ Element memBalloon = (Element)memBalloons.item(0);
+ String model = memBalloon.getAttribute("model");
+ MemBalloonDef def = new MemBalloonDef();
+ if (model.equalsIgnoreCase("virtio")) {
+ String statsPeriod = getAttrValue("stats", "period", memBalloon);
+ def.defVirtioMemBalloon(statsPeriod);
+ }
+ memBalloonDef = def;
+ }
+
Review Comment:
Done.
##########
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##########
@@ -54,6 +54,21 @@
*/
public static final Property<Boolean> ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM = new Property<Boolean>("enable.manually.setting.cpu.topology.on.kvm.vm", true);
+ /**
+ * Disables memory ballooning on VM guests for overcommit.<br>
+ * By default overcommit feature enables balloon and sets currentMemory to a minimum value.<br>
+ * Data type: Boolean.<br>
+ * Default value: <code>false</code>
+ */
+ public static final Property<Boolean> VM_MEMBALLOON_DISABLE = new Property<>("vm.memballoon.disable", false);
+
+ /**
+ * The time interval (in seconds) at which the balloon driver will get memory stats updates. This is equivalent to Libvirt's <code>--period</code> parameter when using the dommemstat command.
+ * Data type: Integer.<br>
+ * Default value: <code>60</code>
+ */
+ public static final Property<Integer> VM_MEMBALLOON_STATS_PERIOD = new Property<>("vm.memballoon.stats.period", 60);
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] joseflauzino commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1015649147
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1298,9 +1308,85 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Gets the ID list of the VMs to set memory balloon stats period.
+ * @param conn the Libvirt connection.
+ * @return the list of VM IDs.
+ */
+ protected List<Integer> getVmsToSetMemoryBalloonStatsPeriod(Connect conn) {
+ List<Integer> vmIdList = new ArrayList<Integer>();
+ Integer[] vmIds = null;
+ try {
+ vmIds = ArrayUtils.toObject(conn.listDomains());
+ } catch (final LibvirtException e) {
+ s_logger.error("Unable to get the list of Libvirt domains on this host.", e);
+ return vmIdList;
+ }
+ vmIdList.addAll(Arrays.asList(vmIds));
+ s_logger.debug(String.format("We have found a total of [%s] VMs (Libvirt domains) on this host: [%s].", vmIdList.size(), vmIdList.toString()));
+
+ if (vmIdList.isEmpty()) {
+ s_logger.info("Skipping the memory balloon stats period setting, since there are no VMs (active Libvirt domains) on this host.");
+ }
+ return vmIdList;
+ }
+
+ /**
+ * Gets the current VM balloon stats period from the agent.properties file.
+ * @return the current VM balloon stats period.
+ */
+ protected Integer getCurrentVmBalloonStatsPeriod() {
+ if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE)) {
+ s_logger.info(String.format("The [%s] property is set to 'true', so the memory balloon stats period will be set to 0 for all VMs.",
+ AgentProperties.VM_MEMBALLOON_DISABLE.getName()));
+ return 0;
+ }
+ Integer vmBalloonStatsPeriod = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_STATS_PERIOD);
+ if(vmBalloonStatsPeriod == 0) {
Review Comment:
Done.
##########
agent/conf/agent.properties:
##########
@@ -202,6 +202,11 @@ hypervisor.type=kvm
# Disable memory ballooning on vm guests for overcommit, by default overcommit
# feature enables balloon and sets currentMemory to a minimum value.
#
+# The time interval (in seconds) at which the balloon driver will get memory stats updates.
+# This is equivalent to Libvirt's --period parameter when using the dommemstat command.
+# Default value: 0
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] joseflauzino commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1015453624
##########
agent/conf/agent.properties:
##########
@@ -202,6 +202,11 @@ hypervisor.type=kvm
# Disable memory ballooning on vm guests for overcommit, by default overcommit
# feature enables balloon and sets currentMemory to a minimum value.
#
+# The time interval (in seconds) at which the balloon driver will get memory stats updates.
+# This is equivalent to Libvirt's --period parameter when using the dommemstat command.
+# Default value: 60
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r868151148
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -150,6 +153,29 @@ public int value() {
private String _keystoreSetupPath;
private String _keystoreCertImportPath;
+ /**
+ * Virsh command to list the name of all VM instances starting with "i-".
+ */
+ protected static final String COMMAND_LIST_ALL_VMS = "virsh list --all --name | grep '^i-*'";
+
+ /**
+ * Virsh command to set the memory balloon stats period.<br><br>
+ * 1st parameter: the VM name;<br>
+ * 2nd parameter: the period (in seconds).
+ */
+ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live";
+
+ /**
+ * Virsh command to get the memory balloon tag from the VM's XML file.<br><br>
+ * 1st parameter: the VM name;<br>
+ */
+ private static final String COMMAND_GET_MEM_BALLOON_FROM_XML_FILE = "virsh dumpxml %s | grep '<memballoon model='";
Review Comment:
@joseflauzino
at the least the first can be replaced with `LibvirtConnection.getConnection().listDomains`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1196486985
ping @joseflauzino can you address the review comments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1212171944
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3964
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1133645093
@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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1133646008
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6358 (SL-JID-1609)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1145977150
@joseflauzino 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] ravening commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
ravening commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r872333367
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -700,6 +728,62 @@ protected void processRequest(final Request request, final Link link) {
}
}
+ /**
+ * Performs tasks after Agent startup. The tasks performed here should not interfere with the Agent's operation.
+ */
+ protected void runTasksAfterAgentStartup() {
+ setupMemoryBalloonStatsPeriod();
+ }
+
+ /**
+ * Sets the balloon driver of each VM to get the memory stats at the time interval defined in the agent.properties file.
+ */
+ protected void setupMemoryBalloonStatsPeriod() {
+ String listAllVmsResult = Script.runSimpleBashScript(COMMAND_LIST_ALL_VMS);
+ if (StringUtils.isBlank(listAllVmsResult)) {
+ s_logger.debug("Skipping the memory balloon stats period setting, since there are no guest VMs.");
+ return;
+ }
+
+ if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE)) {
+ s_logger.debug(String.format("Skipping the memory balloon stats period setting because the [%s] property is set to 'true'.",
Review Comment:
same thing for this line also... probably set level to trace?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1216578446
@shwstppr 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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1216618861
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 4003
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
ravening commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r872331359
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -700,6 +728,62 @@ protected void processRequest(final Request request, final Link link) {
}
}
+ /**
+ * Performs tasks after Agent startup. The tasks performed here should not interfere with the Agent's operation.
+ */
+ protected void runTasksAfterAgentStartup() {
+ setupMemoryBalloonStatsPeriod();
+ }
+
+ /**
+ * Sets the balloon driver of each VM to get the memory stats at the time interval defined in the agent.properties file.
+ */
+ protected void setupMemoryBalloonStatsPeriod() {
+ String listAllVmsResult = Script.runSimpleBashScript(COMMAND_LIST_ALL_VMS);
+ if (StringUtils.isBlank(listAllVmsResult)) {
+ s_logger.debug("Skipping the memory balloon stats period setting, since there are no guest VMs.");
Review Comment:
if period is set to 30 seconds, dont you think this line will populate the log file if there are no vm's?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1012410683
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1288,9 +1298,66 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Sets the balloon driver of each VM to get the memory stats at the time interval defined in the agent.properties file.
+ * @param conn the Libvirt connection.
+ */
+ protected void setupMemoryBalloonStatsPeriod(Connect conn) {
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1301529724
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] weizhouapache commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1004238267
##########
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##########
@@ -54,6 +54,21 @@
*/
public static final Property<Boolean> ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM = new Property<Boolean>("enable.manually.setting.cpu.topology.on.kvm.vm", true);
+ /**
+ * Disables memory ballooning on VM guests for overcommit.<br>
+ * By default overcommit feature enables balloon and sets currentMemory to a minimum value.<br>
+ * Data type: Boolean.<br>
+ * Default value: <code>false</code>
+ */
+ public static final Property<Boolean> VM_MEMBALLOON_DISABLE = new Property<>("vm.memballoon.disable", false);
+
+ /**
+ * The time interval (in seconds) at which the balloon driver will get memory stats updates. This is equivalent to Libvirt's <code>--period</code> parameter when using the dommemstat command.
+ * Data type: Integer.<br>
+ * Default value: <code>60</code>
+ */
+ public static final Property<Integer> VM_MEMBALLOON_STATS_PERIOD = new Property<>("vm.memballoon.stats.period", 60);
Review Comment:
@joseflauzino
This will change the default behavior, should the default value be 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] sonarcloud[bot] commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1197190343
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [69.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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] joseflauzino commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1197269264
@weizhouapache, thanks for the review. I addressed the comments with the most straightforward solutions and will address the remaining ASAP.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1286904725
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 4519
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1286913463
@blueorangutan test ubuntu22 kvm-ubuntu22
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1015648357
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java:
##########
@@ -342,6 +346,25 @@ public boolean parseDomainXML(String domXML) {
return false;
}
+ /**
+ * Parse the memballoon tag.
+ * @param devices the devices tag.
+ * @return the MemBalloonDef.
+ */
+ private MemBalloonDef parseMemBalloonTag(Element devices) {
+ MemBalloonDef def = new MemBalloonDef();
+ NodeList memBalloons = devices.getElementsByTagName("memballoon");
+ if ((memBalloons != null) && (memBalloons.getLength() != 0)) {
Review Comment:
Done.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1298,9 +1308,85 @@ public boolean configure(final String name, final Map<String, Object> params) th
s_logger.info("iscsi session clean up is disabled");
}
+ setupMemoryBalloonStatsPeriod(conn);
+
return true;
}
+ /**
+ * Gets the ID list of the VMs to set memory balloon stats period.
+ * @param conn the Libvirt connection.
+ * @return the list of VM IDs.
+ */
+ protected List<Integer> getVmsToSetMemoryBalloonStatsPeriod(Connect conn) {
+ List<Integer> vmIdList = new ArrayList<Integer>();
+ Integer[] vmIds = null;
+ try {
+ vmIds = ArrayUtils.toObject(conn.listDomains());
+ } catch (final LibvirtException e) {
+ s_logger.error("Unable to get the list of Libvirt domains on this host.", e);
+ return vmIdList;
+ }
+ vmIdList.addAll(Arrays.asList(vmIds));
+ s_logger.debug(String.format("We have found a total of [%s] VMs (Libvirt domains) on this host: [%s].", vmIdList.size(), vmIdList.toString()));
+
+ if (vmIdList.isEmpty()) {
+ s_logger.info("Skipping the memory balloon stats period setting, since there are no VMs (active Libvirt domains) on this host.");
+ }
+ return vmIdList;
+ }
+
+ /**
+ * Gets the current VM balloon stats period from the agent.properties file.
+ * @return the current VM balloon stats period.
+ */
+ protected Integer getCurrentVmBalloonStatsPeriod() {
+ if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE)) {
+ s_logger.info(String.format("The [%s] property is set to 'true', so the memory balloon stats period will be set to 0 for all VMs.",
+ AgentProperties.VM_MEMBALLOON_DISABLE.getName()));
+ return 0;
+ }
+ Integer vmBalloonStatsPeriod = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_STATS_PERIOD);
+ if(vmBalloonStatsPeriod == 0) {
+ s_logger.info(String.format("The [%s] property is set to '0', this prevents memory statistics from being displayed correctly. "
+ + "Adjust (increase) the value of this parameter to correct this.", AgentProperties.VM_MEMBALLOON_STATS_PERIOD.getName()));
+ }
+ return vmBalloonStatsPeriod;
+ }
+
+ /**
+ * Sets the balloon driver of each VM to get the memory stats at the time interval defined in the agent.properties file.
+ * @param conn the Libvirt connection.
+ */
+ protected void setupMemoryBalloonStatsPeriod(Connect conn) {
+ List<Integer> vmIdList = getVmsToSetMemoryBalloonStatsPeriod(conn);
+ Integer currentVmBalloonStatsPeriod = getCurrentVmBalloonStatsPeriod();
+ for (Integer vmId : vmIdList) {
+ Domain dm = null;
+ try {
+ dm = conn.domainLookupByID(vmId);
+ parser.parseDomainXML(dm.getXMLDesc(0));
+ MemBalloonDef memBalloon = parser.getMemBalloon();
+ if (MemBalloonDef.MemBalloonModel.VIRTIO.equals(memBalloon.getMemBalloonModel())) {
+ String setMemBalloonStatsPeriodCommand = String.format(COMMAND_SET_MEM_BALLOON_STATS_PERIOD, vmId, currentVmBalloonStatsPeriod);
+ String setMemBalloonStatsPeriodResult = Script.runSimpleBashScript(setMemBalloonStatsPeriodCommand);
+ if (StringUtils.isNotBlank(setMemBalloonStatsPeriodResult)) {
+ s_logger.error(String.format("Unable to set up memory balloon stats period for VM (Libvirt Domain) with ID [%s] due to an error when running the [%s] "
+ + "command. Output: [%s].", vmId, setMemBalloonStatsPeriodCommand, setMemBalloonStatsPeriodResult));
+ continue;
+ }
+ s_logger.debug(String.format("The memory balloon stats period [%s] has been set successfully for the VM (Libvirt Domain) with ID [%s] and name [%s].",
+ currentVmBalloonStatsPeriod, vmId, dm.getName()));
+ continue;
+ }
+ s_logger.debug(String.format("Skipping the memory balloon stats period setting for the VM (Libvirt Domain) with ID [%s] and name [%s] because this VM has no memory"
+ + " balloon.", vmId, dm.getName()));
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1305887241
@joseflauzino 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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1305965033
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [15 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [75.1% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1307177068
@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] sonarcloud[bot] commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1307333779
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [75.1% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1272245488
@rohityadavcloud 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] ravening commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
ravening commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r872331359
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -700,6 +728,62 @@ protected void processRequest(final Request request, final Link link) {
}
}
+ /**
+ * Performs tasks after Agent startup. The tasks performed here should not interfere with the Agent's operation.
+ */
+ protected void runTasksAfterAgentStartup() {
+ setupMemoryBalloonStatsPeriod();
+ }
+
+ /**
+ * Sets the balloon driver of each VM to get the memory stats at the time interval defined in the agent.properties file.
+ */
+ protected void setupMemoryBalloonStatsPeriod() {
+ String listAllVmsResult = Script.runSimpleBashScript(COMMAND_LIST_ALL_VMS);
+ if (StringUtils.isBlank(listAllVmsResult)) {
+ s_logger.debug("Skipping the memory balloon stats period setting, since there are no guest VMs.");
Review Comment:
if period is set to 30 seconds, dont you think this line will populate the log file if there are no vm's?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r879150633
##########
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##########
@@ -61,6 +61,21 @@
*/
public static final Property<Boolean> ENABLE_IO_URING = new Property<Boolean>("enable.io.uring", true);
+ /**
+ * Disables memory ballooning on VM guests for overcommit.<br>
+ * By default overcommit feature enables balloon and sets currentMemory to a minimum value.<br>
+ * Data type: Boolean.<br>
+ * Default value: <code>false</code>
+ */
+ public static final Property<Boolean> VM_MEMBALLOON_DISABLE = new Property<Boolean>("vm.memballoon.disable", false);
Review Comment:
```suggestion
public static final Property<Boolean> VM_MEMBALLOON_DISABLE = new Property<>("vm.memballoon.disable", false);
```
##########
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##########
@@ -61,6 +61,21 @@
*/
public static final Property<Boolean> ENABLE_IO_URING = new Property<Boolean>("enable.io.uring", true);
+ /**
+ * Disables memory ballooning on VM guests for overcommit.<br>
+ * By default overcommit feature enables balloon and sets currentMemory to a minimum value.<br>
+ * Data type: Boolean.<br>
+ * Default value: <code>false</code>
+ */
+ public static final Property<Boolean> VM_MEMBALLOON_DISABLE = new Property<Boolean>("vm.memballoon.disable", false);
+
+ /**
+ * The time interval (in seconds) at which the balloon driver will get memory stats updates. This is equivalent to Libvirt's <code>--period</code> parameter when using the dommemstat command.
+ * Data type: Integer.<br>
+ * Default value: <code>60</code>
+ */
+ public static final Property<Integer> VM_MEMBALLOON_STATS_PERIOD = new Property<Integer>("vm.memballoon.stats.period", 60);
Review Comment:
```suggestion
public static final Property<Integer> VM_MEMBALLOON_STATS_PERIOD = new Property<>("vm.memballoon.stats.period", 60);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1133643795
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] sonarcloud[bot] commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1197219721
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [12 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [69.8% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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] acs-robot commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1212112707
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1119527575
@joseflauzino 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] weizhouapache commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1013379803
##########
agent/conf/agent.properties:
##########
@@ -202,6 +202,11 @@ hypervisor.type=kvm
# Disable memory ballooning on vm guests for overcommit, by default overcommit
# feature enables balloon and sets currentMemory to a minimum value.
#
+# The time interval (in seconds) at which the balloon driver will get memory stats updates.
+# This is equivalent to Libvirt's --period parameter when using the dommemstat command.
+# Default value: 60
Review Comment:
thanks @joseflauzino
can you update the default value to 0 in agent/conf/agent.properties ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] joseflauzino commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1012413128
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java:
##########
@@ -275,7 +280,14 @@ public String toString() {
response.append(String.format("<cpu> <numa> <cell id='0' cpus='0-%s' memory='%s' unit='KiB'/> </numa> </cpu>\n", this.maxVcpu - 1, this.currentMemory));
}
- response.append(String.format("<devices>\n<memballoon model='%s'/>\n</devices>\n", this.memoryBalloning ? "virtio" : "none"));
+ MemBalloonDef memBalloonDef = new MemBalloonDef();
+ if (this.memoryBalloning) {
+ memBalloonDef.defVirtioMemBalloon(String.valueOf(memoryBalloonStatsPeriod));
+ } else {
+ memBalloonDef.defNoneMemBalloon();
+ }
+ response.append(String.format("<devices>\n%s\n</devices>\n", memBalloonDef.toString()));
+
Review Comment:
I think there is no need in this case. I've only added a condition before appending the string. I also created a test case that validates 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] joseflauzino commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1301533470
@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] sonarcloud[bot] commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1304004348
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [26 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [75.2% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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] joseflauzino commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r1015650145
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -440,6 +440,19 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
protected Boolean enableManuallySettingCpuTopologyOnKvmVm = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.ENABLE_MANUALLY_SETTING_CPU_TOPOLOGY_ON_KVM_VM);
+ /**
+ * Virsh command to set the memory balloon stats period.<br><br>
+ * 1st parameter: the VM name;<br>
+ * 2nd parameter: the period (in seconds).
+ */
+ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live";
Review Comment:
@DaanHoogland and @GutoVeronezi
I answered this one in [this other comment](https://github.com/apache/cloudstack/pull/6358#issuecomment-1212112323). That's why I marked it as resolved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1307178587
@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] joseflauzino commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1145976149
@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] joseflauzino commented on a diff in pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on code in PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#discussion_r866744709
##########
agent/src/main/java/com/cloud/agent/Agent.java:
##########
@@ -150,6 +153,29 @@ public int value() {
private String _keystoreSetupPath;
private String _keystoreCertImportPath;
+ /**
+ * Virsh command to list the name of all VM instances starting with "i-".
+ */
+ protected static final String COMMAND_LIST_ALL_VMS = "virsh list --all --name | grep '^i-*'";
+
+ /**
+ * Virsh command to set the memory balloon stats period.<br><br>
+ * 1st parameter: the VM name;<br>
+ * 2nd parameter: the period (in seconds).
+ */
+ private static final String COMMAND_SET_MEM_BALLOON_STATS_PERIOD = "virsh dommemstat %s --period %s --live";
+
+ /**
+ * Virsh command to get the memory balloon tag from the VM's XML file.<br><br>
+ * 1st parameter: the VM name;<br>
+ */
+ private static final String COMMAND_GET_MEM_BALLOON_FROM_XML_FILE = "virsh dumpxml %s | grep '<memballoon model='";
Review Comment:
@rohityadavcloud Unfortunately, there are no corresponding methods for these specific virsh commands.
I think it would be better if we solved the collection of memory stats for now (even if it is using virsh commands). At another time we could add into libvirt-java corresponding methods for these and other commands already used in ACS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1119554488
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3358
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1133655075
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6358)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6358&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL) [12 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6358&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list) [71.6% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6358&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] joseflauzino commented on pull request #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
joseflauzino commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1201048165
Thanks for the review, @GabrielBrascher. I will address your comments as well. I will be pushing a new version of the code including all the revisions in the next few days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1286851927
@weizhouapache 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 #6358: Fix memory stats for KVM
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6358:
URL: https://github.com/apache/cloudstack/pull/6358#issuecomment-1287110641
> Trillian Build Failed (tid-5185)
it's yours that failed @weizhouapache :gnagna:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org