-
Notifications
You must be signed in to change notification settings - Fork 1.2k
KVM memballooning requires free page reporting and autodeflate #11932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.22
Are you sure you want to change the base?
Conversation
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
Outdated
Show resolved
Hide resolved
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11932 +/- ##
============================================
+ Coverage 17.36% 17.56% +0.20%
- Complexity 15245 15541 +296
============================================
Files 5888 5909 +21
Lines 525831 529061 +3230
Branches 64183 64618 +435
============================================
+ Hits 91298 92924 +1626
- Misses 424227 425683 +1456
- Partials 10306 10454 +148
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
pretty sure the simulator failure is unrelated to my changes |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15585 |
weizhouapache
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm, not tested
thanks @bradh352
|
@blueorangutan test keepEnv |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14747)
|
|
@weizhouapache looks like the smoke test passed |
good @bradh352 |
|
@weizhouapache is there a package artifact generated by this I can download and use to test? I'm still getting familiar with cloudstack, I'd rather not figure out how to build .deb packages for it if I don't have to :) |
we do not provide the packages publicly, as the cloudstack-management-xx.deb package is huge |
| memBalloonBuilder.append("<memballoon model='" + memBalloonModel + "'"); | ||
| /* Version integer format: major * 1,000,000 + minor * 1,000 + release. | ||
| * Require: libvirt 6.9.0, qemu 5.1.0 */ | ||
| if (memBalloonModel != MemBalloonModel.NONE && s_qemuVersion >= 5001000 && s_libvirtVersion >= 6009000) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bradh352, thanks for the PR, code looks good. Just one minor suggestion, could you extract this comment into a method's Javadoc string? For instance:
Lines 2178 to 2183 in 3d6cafe
| /** | |
| * Updates the parent on the {@code checkpointXml} to {@code snapshotParent}. If {@code snapshotParent} is null, removes the parent. | |
| * @param checkpointXml the checkpoint XML to be updated | |
| * @param snapshotParent the snapshot parent. Inform null if no parent. | |
| * */ | |
| private void updateParent(String snapshotParent, Document checkpointXml, XPath xPath) throws XPathExpressionException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bernardodemarco I'm unfortunately not familiar with the specifics of javadoc so its not clear to me how to apply this to something other than a method itself. I can update it if you let me know how it needs to be formatted.
@bradh352, I personally use this DEB builder: https://github.com/scclouds/cloudstack-deb-builder. By default, it does not populate the To build the packages, go to the Apache CloudStack source code root directory and execute: Where:
For example: It may take a little bit longer for the first build process to finish, due to the download process of some dependencies. After it is finished, the DEB packages will be located inside the Apache CloudStack source code directory, at the Hope it helps! |
Thanks, I'm going to test this out tomorrow! |
|
@bernardodemarco I did get .deb files generated using your method above like |
|
Ok, I have a custom built 4.22 with this PR installed in my test environment. virsh dumpxml of a running vm shows: And in turn the generated qemu command line has this: VMs start up just fine. I'm going to now try to allocate a bunch of memory within a series of VMs and release it and inspect if the host is able to reclaim the memory. |
|
Well, honestly the first test was enough to convince me it is working well: Fresh VM (wihtin VM): In Hypervisor: In VM, allocate a tmpfs mount and a large file: In Hypervisor: Cleanup: In Hypervisor: |
|
@blueorangutan package |
|
@rajujith a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15913 |
Description
As per #11930 KVM's memory ballooning does not auto-inflate and auto-deflate without the Free Page Reporting and autodeflate attributes on the memballoon configuration.
This is a change similar to OpenStack's NOVA:
openstack/nova@cd401c5
Basically if memballooning is enabled for KVM, these features are also always enabled. Having memory ballooning on in cloudstack otherwise does nothing as there is no tooling to inflate or deflate the balloon.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Not yet tested, hoping CI/CD runs through some self-tests, then I'll try a test deployment.
How did you try to break this feature and the system with this change?
N/A