-
Notifications
You must be signed in to change notification settings - Fork 1.2k
kvm: Properly report available memory to Management Server #2795
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
Conversation
|
@blueorangutan package |
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class HostInfo { |
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.
Can you rename this class to KVMHostInfo, that way it will be more obvious what this class is expected of?
|
|
||
| public class HostInfo { | ||
|
|
||
| private static final Logger s_logger = Logger.getLogger(HostInfo.class); |
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.
Remove the s_, instead using LOG or LOGGER?
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2237 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
16f0739 to
4cb5c9f
Compare
|
@rhtyd Done, I've renamed the class and the logger variable. How does this look like? |
|
Ping @RPDiep |
|
Trillian test result (tid-2925)
|
|
@blueorangutan package |
|
@dhlaluku a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2238 |
|
@blueorangutan test |
|
@dhlaluku a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2927)
|
| final LibvirtCapXMLParser parser = new LibvirtCapXMLParser(); | ||
| parser.parseCapabilitiesXML(conn.getCapabilities()); | ||
| final ArrayList<String> oss = parser.getGuestOsType(); | ||
| for (final String s : oss) { |
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.
Why doing all of this process if you only report the host supporting HVM? I mean, KVM does not run if the CPU does not support HVM. Therefore, all KVM hosts will always have HVM support, right?
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.
Yes, they will. But what I did is take the original code and use it. I didn't modify the logic behind the capabilities.
I would make that a different PR if we want to do that.
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.
I understand that you are only moving the old code around, but why not remove useless code now?
I mean, why would we keep a code that does not aggregate value, and can be misleading to newcomers?
|
@rhtyd do you know if the failing tests are false positives? I have been testing some actions such as VM migrations and adding/removing hosts. I had no problem when using primary storage with nfs and a cluster with KVM servers. |
|
@GabrielBrascher most of the failing tests have already been addressed and merged into master with this PRs #2763 #2756 #2754 @wido have this changes been rebased against the latest master? |
4cb5c9f to
bc90f30
Compare
|
@dhlaluku I've rebased against master |
|
@wido thank you |
|
@dhlaluku a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2256 |
|
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-2517 |
|
@blueorangutan package |
|
@dhlaluku a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
@wido can you check and fix build failures, see job 1 of travis |
|
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-2518 |
48482b0 to
004e785
Compare
|
I just force pushed. This seems to be a thing (Unit Test) which fails on Travis, but works on my local system. Could be a problem where tests are running inside a Container? |
004e785 to
31ad338
Compare
The KVM Agent had two mechanisms for reporting its capabilities and memory to the Management Server. On startup it would ask libvirt the amount of Memory the Host has and subtract and add the reserved and overcommit memory. When the HostStats were however reported to the Management Server these two configured values on the Agent were no longer reported in the statistics thus showing all the available memory in the Agent/Host to the Management Server. This commit unifies this by using the same logic on Agent Startup and during statistics reporting. memory=3069636608, reservedMemory=1073741824 This was reported by a 4GB Hypervisor with this setting: host.reserved.mem.mb=1024 The GUI (thus API) would then show: Memory Total 2.86 GB This way the Agent properly 'lies' to the Management Server about its capabilities in terms of Memory. This is very helpful if you want to overprovision or undercommit machines for various reasons. Overcommitting can be done when KSM or ZSwap or a fast SWAP device is installed in the machine. Underprovisioning is done when the Host might run other tasks then a KVM hypervisor, for example when it runs in a hyperconverged setup with Ceph. In addition internally many values have been changed from a Double to a Long and also store the amount of bytes instead of Kilobytes. Signed-off-by: Wido den Hollander <[email protected]>
31ad338 to
e58f4b1
Compare
|
I just rebased against master and everything seems fine to me. Are there any objections on this PR? |
|
@blueorangutan package |
|
@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2551 |
|
@blueorangutan test |
|
@GabrielBrascher a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
@wido we need to check the integration test results before being able to merge it. |
|
Trillian test result (tid-3333)
|
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-3335)
|
|
@blueorangutan package |
|
@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2555 |
|
@blueorangutan test |
|
@GabrielBrascher a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-3338)
|
Description
The KVM Agent had two mechanisms for reporting its capabilities
and memory to the Management Server.
On startup it would ask libvirt the amount of Memory the Host has
and subtract and add the reserved and overcommit memory.
When the HostStats were however reported to the Management Server
these two configured values on the Agent were no longer reported
in the statistics thus showing all the available memory in the
Agent/Host to the Management Server.
This commit unifies this by using the same logic on Agent Startup
and during statistics reporting.
memory=3069636608, reservedMemory=1073741824
This was reported by a 4GB Hypervisor with this setting:
host.reserved.mem.mb=1024
The GUI (thus API) would then show:
Memory Total 2.86 GB
This way the Agent properly 'lies' to the Management Server about its
capabilities in terms of Memory.
This is very helpful if you want to overprovision or undercommit machines
for various reasons.
Overcommitting can be done when KSM or ZSwap or a fast SWAP device is
installed in the machine.
Underprovisioning is done when the Host might run other tasks then a KVM
hypervisor, for example when it runs in a hyperconverged setup with Ceph.
In addition internally many values have been changed from a Double to a Long
and also store the amount of bytes instead of Kilobytes.
Types of changes
How Has This Been Tested?
I tested this on a CloudStack 4.12 test setup running on my local system.
The KVM Agent will report the amount of memory it has minus any reservations. This way the Mgmt server will never know the exact amount of memory the Agent has.
Checklist:
Testing