You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by mike-tutkowski <gi...@git.apache.org> on 2015/08/04 00:44:04 UTC
[GitHub] cloudstack pull request: Reduce the lowest hypervisor snapshot res...
GitHub user mike-tutkowski opened a pull request:
https://github.com/apache/cloudstack/pull/653
Reduce the lowest hypervisor snapshot reserve value from 50% to 10%
If the admin specifies a hypervisor snapshot reserve value that is below a particular number, we want to just use that number.
For example, if the admin specifies 5% for hypervisor snapshot reserve, we want to use - at least - 10% now (it used to be 50%, but that can make the backend volume overly large in certain situations).
This only impacts the SolidFire storage plug-in.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mike-tutkowski/cloudstack master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/653.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 #653
----
commit 7c6cda55aef501705888c3fa629d411e8fca793b
Author: Mike Tutkowski <mi...@solidfire.com>
Date: 2015-08-03T02:17:55Z
Reduce the default hypervisor snapshot reserve from 50% to 10%
----
---
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: Reduce the lowest hypervisor snapshot res...
Posted by mike-tutkowski <gi...@git.apache.org>.
GitHub user mike-tutkowski reopened a pull request:
https://github.com/apache/cloudstack/pull/653
Reduce the lowest hypervisor snapshot reserve value from 50% to 10%
If the admin specifies a hypervisor snapshot reserve value that is below a particular number, we want to just use that number.
For example, if the admin specifies 5% for hypervisor snapshot reserve, we want to use - at least - 10% now (it used to be 50%, but that can make the backend volume overly large in certain situations).
This only impacts the SolidFire storage plug-in.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mike-tutkowski/cloudstack master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/653.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 #653
----
commit 2cbc1688220b6af086ea4a367f2132d892a97cb9
Author: Mike Tutkowski <mi...@solidfire.com>
Date: 2015-08-05T21:47:57Z
The lowest the hypervisor snapshot reserve value can be is 10 (down from 50).
----
---
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: Reduce the lowest hypervisor snapshot res...
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski closed the pull request at:
https://github.com/apache/cloudstack/pull/653
---
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: Reduce the lowest hypervisor snapshot res...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/653
---
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: Reduce the lowest hypervisor snapshot res...
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/653#issuecomment-127472162
Good points...I went ahead and used Math.max (I also replaced the magic number with a private static final...it doesn't need to be user configurable at the time being)
---
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: Reduce the lowest hypervisor snapshot res...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/653#issuecomment-127471522
:+1: with/without the minor change I suggested
---
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: Reduce the lowest hypervisor snapshot res...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the pull request:
https://github.com/apache/cloudstack/pull/653#issuecomment-127530000
great. thanks for the update @mike-tutkowski. :+1:
---
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: Reduce the lowest hypervisor snapshot res...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/653#discussion_r36140697
--- Diff: plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java ---
@@ -352,8 +352,8 @@ public long getVolumeSizeIncludingHypervisorSnapshotReserve(Volume volume, Stora
Integer hypervisorSnapshotReserve = volume.getHypervisorSnapshotReserve();
if (hypervisorSnapshotReserve != null) {
- if (hypervisorSnapshotReserve < 50) {
- hypervisorSnapshotReserve = 50;
+ if (hypervisorSnapshotReserve < 10) {
+ hypervisorSnapshotReserve = 10;
--- End diff --
why not a ConfigKey<Integer>?
---
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: Reduce the lowest hypervisor snapshot res...
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/653#issuecomment-127426349
Definitely a valid point. It's something on my radar, but I just wanted to
put this "quick fix" in per a customer request. :)
On Mon, Aug 3, 2015 at 4:50 PM, Daan Hoogland <no...@github.com>
wrote:
> Of course I have something to nag about but LGTM
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cloudstack/pull/653#issuecomment-127425881>.
>
--
*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>*™*
---
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: Reduce the lowest hypervisor snapshot res...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/653#issuecomment-127425881
Of course I have something to nag about but 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: Reduce the lowest hypervisor snapshot res...
Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the pull request:
https://github.com/apache/cloudstack/pull/653#issuecomment-128159614
Sorry...I accidentally closed this PR without getting the code properly into upstream (so re-opening it for a bit).
---
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: Reduce the lowest hypervisor snapshot res...
Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/653#discussion_r36155878
--- Diff: plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java ---
@@ -352,8 +352,8 @@ public long getVolumeSizeIncludingHypervisorSnapshotReserve(Volume volume, Stora
Integer hypervisorSnapshotReserve = volume.getHypervisorSnapshotReserve();
if (hypervisorSnapshotReserve != null) {
- if (hypervisorSnapshotReserve < 50) {
- hypervisorSnapshotReserve = 50;
+ if (hypervisorSnapshotReserve < 10) {
+ hypervisorSnapshotReserve = 10;
--- End diff --
Why not use Math.max(hypervisorSnapshotReserve, 10)? That way we will be using the const only once and its more readable( 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: Reduce the lowest hypervisor snapshot res...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/653#issuecomment-127532378
Changes 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.
---