You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Mice Xia <mi...@tcloudcomputing.com> on 2012/07/25 03:18:25 UTC

答复: Review Request: CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage

Hi Edison,

Do you mean I should add a new static public method (storagepoolhasenoughspace) in AbstractStoragePoolAllocator, and call it to check against storage space in findSuitablePoolsForVolumes after the for loop?

Regards
Mice

-----邮件原件-----
发件人: disheng su [mailto:noreply@reviews.apache.org] 代表 disheng su
发送时间: 2012年7月25日 6:28
收件人: Prachi Damle; Nitin Mehta
抄送: cloudstack; Mice Xia; disheng su
主题: Re: Review Request: CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6028/#review9421
-----------------------------------------------------------



server/src/com/cloud/deploy/FirstFitPlanner.java
<https://reviews.apache.org/r/6028/#comment20158>

    1. The logic should be added into findSuitablePoolsForVolumes, as findSuitablePoolsForVolumes means given a bunch of volumes and its corresponding requirements, find the proper storage pool for me. 
    2. The actual code should be put into AbstractStoragePoolAllocator, e.g. adding a new method, called storagepoolhasenoughspace, the input parameter is a map<volume, storagepool>, then calculate the storagepool has enough space or not.


- disheng su


On July 18, 2012, 1:29 p.m., mice xia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6028/
> -----------------------------------------------------------
> 
> (Updated July 18, 2012, 1:29 p.m.)
> 
> 
> Review request for cloudstack, Prachi Damle and Nitin Mehta.
> 
> 
> Description
> -------
> 
> fix CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage
> 
> check the availability of un-allocated primary storage space during planning stage, for multiple-volume VM creation scenario
> 
> modification in findPotentialDeploymentResources:
> for each <volume, storagePool>, test if ( requestedVolumeSize + storagePool.usedCapacity > storagePool.totalCapacity * overprovisioningFactor * allocatedStorageThredshold
> 
> 
> This addresses bug CS-15609.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/deploy/FirstFitPlanner.java eb82c75 
> 
> Diff: https://reviews.apache.org/r/6028/diff/
> 
> 
> Testing
> -------
> 
> perform following tests: (overprovisioning.factor=1)
> 1) create vm with 20G root + 20G data on one NFS PS, with allocation state 7.82GB/36.72GB, VM creation failed as expected
> 2) create vm with 20G root + 5G  data on one NFS PS, with allocation state 7.82GB/36.72GB, threshold=0.85, VM creation failed as expected
> 3) create vm with 20G root + 5G  data on one NFS PS, with allocation state 7.82GB/36.72GB, threshold=0.99, VM creation succeed as expected
> 
> 
> Thanks,
> 
> mice xia
> 
>


Re: Review Request: CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage

Posted by Edison Su <Ed...@citrix.com>.
How about put it in storagemanagerimpl? It's the place you can put all the storage related query or operation.

Sent from my iPhone

On Jul 24, 2012, at 10:48 PM, "Mice Xia" <mi...@tcloudcomputing.com> wrote:

> Making a public static method requires a lot of the fields in it to be static, I'm not sure if it has some side-effect..
> Is it OK to extract the related logic into another private method in the planner?
> 
> Regards
> Mice
> 
> -----Original Message-----
> From: Edison Su [mailto:Edison.su@citrix.com] 
> Sent: Wednesday, July 25, 2012 1:09 PM
> To: Mice Xia
> Cc: cloudstack-dev@incubator.apache.org; Prachi Damle; Nitin Mehta
> Subject: Re: 答复: Review Request: CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage
> 
> Yes, exactly. I don't want mess planner with storage allocator. The planner itself is already complicated enough. How do you think?
> 
> Sent from my iPhone
> 
> On Jul 24, 2012, at 6:18 PM, "Mice Xia" <mi...@tcloudcomputing.com> wrote:
> 
>> Hi Edison,
>> 
>> Do you mean I should add a new static public method (storagepoolhasenoughspace) in AbstractStoragePoolAllocator, and call it to check against storage space in findSuitablePoolsForVolumes after the for loop?
>> 
>> Regards
>> Mice
> 

Re: Re: Review Request: CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage

Posted by Mice Xia <mi...@tcloudcomputing.com>.
Making a public static method requires a lot of the fields in it to be static, I'm not sure if it has some side-effect..
Is it OK to extract the related logic into another private method in the planner?

Regards
Mice

-----Original Message-----
From: Edison Su [mailto:Edison.su@citrix.com] 
Sent: Wednesday, July 25, 2012 1:09 PM
To: Mice Xia
Cc: cloudstack-dev@incubator.apache.org; Prachi Damle; Nitin Mehta
Subject: Re: 答复: Review Request: CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage

Yes, exactly. I don't want mess planner with storage allocator. The planner itself is already complicated enough. How do you think?

Sent from my iPhone

On Jul 24, 2012, at 6:18 PM, "Mice Xia" <mi...@tcloudcomputing.com> wrote:

> Hi Edison,
> 
> Do you mean I should add a new static public method (storagepoolhasenoughspace) in AbstractStoragePoolAllocator, and call it to check against storage space in findSuitablePoolsForVolumes after the for loop?
> 
> Regards
> Mice
 

Re: 答复: Review Request: CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage

Posted by Edison Su <Ed...@citrix.com>.
Yes, exactly. I don't want mess planner with storage allocator. The planner itself is already complicated enough. How do you think?

Sent from my iPhone

On Jul 24, 2012, at 6:18 PM, "Mice Xia" <mi...@tcloudcomputing.com> wrote:

> Hi Edison,
> 
> Do you mean I should add a new static public method (storagepoolhasenoughspace) in AbstractStoragePoolAllocator, and call it to check against storage space in findSuitablePoolsForVolumes after the for loop?
> 
> Regards
> Mice
> 
> -----邮件原件-----
> 发件人: disheng su [mailto:noreply@reviews.apache.org] 代表 disheng su
> 发送时间: 2012年7月25日 6:28
> 收件人: Prachi Damle; Nitin Mehta
> 抄送: cloudstack; Mice Xia; disheng su
> 主题: Re: Review Request: CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6028/#review9421
> -----------------------------------------------------------
> 
> 
> 
> server/src/com/cloud/deploy/FirstFitPlanner.java
> <https://reviews.apache.org/r/6028/#comment20158>
> 
>    1. The logic should be added into findSuitablePoolsForVolumes, as findSuitablePoolsForVolumes means given a bunch of volumes and its corresponding requirements, find the proper storage pool for me. 
>    2. The actual code should be put into AbstractStoragePoolAllocator, e.g. adding a new method, called storagepoolhasenoughspace, the input parameter is a map<volume, storagepool>, then calculate the storagepool has enough space or not.
> 
> 
> - disheng su
> 
> 
> On July 18, 2012, 1:29 p.m., mice xia wrote:
>> 
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/6028/
>> -----------------------------------------------------------
>> 
>> (Updated July 18, 2012, 1:29 p.m.)
>> 
>> 
>> Review request for cloudstack, Prachi Damle and Nitin Mehta.
>> 
>> 
>> Description
>> -------
>> 
>> fix CS-15609 Volumes can be created as a part of VM creation when un-allocated space is insufficient on primary storage
>> 
>> check the availability of un-allocated primary storage space during planning stage, for multiple-volume VM creation scenario
>> 
>> modification in findPotentialDeploymentResources:
>> for each <volume, storagePool>, test if ( requestedVolumeSize + storagePool.usedCapacity > storagePool.totalCapacity * overprovisioningFactor * allocatedStorageThredshold
>> 
>> 
>> This addresses bug CS-15609.
>> 
>> 
>> Diffs
>> -----
>> 
>>  server/src/com/cloud/deploy/FirstFitPlanner.java eb82c75 
>> 
>> Diff: https://reviews.apache.org/r/6028/diff/
>> 
>> 
>> Testing
>> -------
>> 
>> perform following tests: (overprovisioning.factor=1)
>> 1) create vm with 20G root + 20G data on one NFS PS, with allocation state 7.82GB/36.72GB, VM creation failed as expected
>> 2) create vm with 20G root + 5G  data on one NFS PS, with allocation state 7.82GB/36.72GB, threshold=0.85, VM creation failed as expected
>> 3) create vm with 20G root + 5G  data on one NFS PS, with allocation state 7.82GB/36.72GB, threshold=0.99, VM creation succeed as expected
>> 
>> 
>> Thanks,
>> 
>> mice xia
>> 
>> 
>