You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by kmccormick <gi...@git.apache.org> on 2015/08/31 20:20:20 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
GitHub user kmccormick opened a pull request:
https://github.com/apache/cloudstack/pull/766
CLOUDSTACK-8678: Reserve RAM for KVM host OS
Use host.reserved.ram.mb agent property to modify total system RAM
before reporting to management server.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kmccormick/cloudstack CLOUDSTACK-8678
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/766.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #766
----
commit 414136637e2fd115c781e3b3d6c8d83f40191b31
Author: Kevin McCormick <ke...@intrinium.com>
Date: 2015-08-31T18:11:17Z
CLOUDSTACK-8678: Reserve RAM for KVM host OS
Use host.reserved.ram.mb agent property to modify total system RAM
before reporting to management server.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/766#issuecomment-137854742
LGTM Tested it, works great (even without single change to agent.properties). It will reserve 1GB by default.
![before]
(http://s29.postimg.org/uoxtrihhz/Screen_Shot_2015_09_04_at_22_51_02.png)
![with new agent from this PR]
(http://s15.postimg.org/rbin8ipff/Screen_Shot_2015_09_04_at_23_04_01.png)
Thanks for fixing this @kmccormick !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/766#issuecomment-137243367
@kmccormick Agreed. A release note would be enough since running VMs will not be affected.
CPU is a different problem indeed. Stuff becomes slow, but going Out of Memory is a bigger problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/766
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by kmccormick <gi...@git.apache.org>.
Github user kmccormick commented on the pull request:
https://github.com/apache/cloudstack/pull/766#issuecomment-137155599
Regarding the upgrade scenario, I believe that this would not affect any running VMs. Can anyone confirm? Besides just running VMs, it is still a potential problem for an installation that is already near capacity, as the upgrade would remove a little bit of capacity. For both issues, I think a mention in the release notes would be sufficient. To keep the old behavior, admins just need to add host.reserved.ram.mb=0 to their agent.properties file.
CPU I don't think is an issue, as memory is typically the limiting factor in virtualization. If the CPU gets overly taxed, things slow down, where if all the memory gets used up, VMs or other processes get killed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by securityvoodoo <gi...@git.apache.org>.
Github user securityvoodoo commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/766#discussion_r38436231
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -258,7 +258,7 @@
private String _updateHostPasswdPath;
- private int _dom0MinMem;
+ private long _dom0MinMem;
--- End diff --
I see no reason not to default it to something sane.
Wido -
The way it is setup right now, admins would not be able to change it to something lower than whatever we default to unless still greater than 768mb or 10% of system ram. At least not by specifying it via the host.reserved.mem.mb property. That being said, the default should be greater than 768mb. A 1G default sounds sane to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/766#issuecomment-136632587
except for the initialisation issue, LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by kmccormick <gi...@git.apache.org>.
Github user kmccormick commented on the pull request:
https://github.com/apache/cloudstack/pull/766#issuecomment-136810812
OK, reworked this a bit. Removed dom0ram and using only _dom0MinMem now.
This line should cause a 1GB default if the value isn't specified in the file:
_dom0MinMem = NumbersUtil.parseInt(value, 1024) * 1024 * 1024L;
No longer doing the odd min/max calculation, just using either the fixed default or the admin specified value.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/766#issuecomment-136922701
LGTM, in case my previous comment is not already interpreted as such.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/766#discussion_r38409667
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -258,7 +258,7 @@
private String _updateHostPasswdPath;
- private int _dom0MinMem;
+ private long _dom0MinMem;
--- End diff --
Agree, I think we should always reserve at least 1GB to keep a system alive. Admins can always set it so something lower.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/766#issuecomment-137495603
@kmccormick thanks for the PR, will test it soon! Had a bit of a rough week at the office with some emergencies going on.
Please rebase with current master, as we cannot merge it with conflicts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/766#issuecomment-137004807
What happens in case of an upgrade? Looks like there is a possibility that the host may get over-provisioned. Should there be some scripts to let the user know if some VMs needs to be destroyed before upgrade?
Also do we need a similar behaviour for cpu?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/766#issuecomment-137923198
No Travis test results? Both Apache builds are green and I verified the function to work properly so merging it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM
for KVM host...
Posted by Mike Tutkowski <mi...@solidfire.com>.
Member variables of type int or long are auto initialized to 0 in Java (as
opposed to an unknown value).
That being said, that might not be a good default value for this situation.
On Tuesday, September 1, 2015, DaanHoogland <gi...@git.apache.org> wrote:
> Github user DaanHoogland commented on a diff in the pull request:
>
> https://github.com/apache/cloudstack/pull/766#discussion_r38394939
>
> --- Diff:
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
> ---
> @@ -258,7 +258,7 @@
>
> private String _updateHostPasswdPath;
>
> - private int _dom0MinMem;
> + private long _dom0MinMem;
> --- End diff --
>
> I think we need to initialize to a reasonable default or 0L. Not sure
> if it is possible by installation but otherwise the properties file might
> be corrupted or empty.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org <javascript:;> or
> file a JIRA ticket
> with INFRA.
> ---
>
--
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the cloud
<http://solidfire.com/solution/overview/?video=play>*™*
[GitHub] cloudstack pull request: CLOUDSTACK-8678: Reserve RAM for KVM host...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/766#discussion_r38394939
--- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java ---
@@ -258,7 +258,7 @@
private String _updateHostPasswdPath;
- private int _dom0MinMem;
+ private long _dom0MinMem;
--- End diff --
I think we need to initialize to a reasonable default or 0L. Not sure if it is possible by installation but otherwise the properties file might be corrupted or empty.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---