> On Aug. 20, 2012, 8:49 p.m., edison su wrote:
> > server/src/com/cloud/storage/allocator/FirstFitStoragePoolAllocator.java, 
> > line 87
> > <https://reviews.apache.org/r/6431/diff/3/?file=142936#file142936line87>
> >
> >     Why don't use LocalStoragePoolAllocator?
> 
> Koushik Das wrote:
>     LocalStoragePoolAllocator allocateToPool internally calls 
> super.allocateToPool, so have made the changes in FirstFitAllocator
> 
> edison su wrote:
>     That doesn't tell me why you need to make the changes in the abstract 
> class for your specific logic.  You can change LocalStoragePoolAllocator or 
> whatever, but please don't add the specific logic in an abstract class.
> 
> Koushik Das wrote:
>     Moved logic to LocalStoragePoolAllocator

Ok to me, Nitin, could you help to check it in? If the patch is ok for you.


- edison


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


On Aug. 27, 2012, 12:05 p.m., Koushik Das wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6431/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2012, 12:05 p.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek and Nitin Mehta.
> 
> 
> Description
> -------
> 
> Support for local data disk. Currently enable/disable config is at zone 
> level, in subsequent checkins it can be made more granular.
>     Following changes are made:
>     - Create disk offering API now takes an extra parameter to denote storage 
> type (local or shared). This is similar to storage type in service offering.
>     - Create/delete of data volume on local storage
>     - Attach/detach for local data volumes. Re-attach is allowed as long as 
> vm host and data volume storage pool host is same.
>     - Migration of VM instance is not supported if it uses local root or data 
> volumes.
>     - Migrate is not supported for local volumes.
>     - Zone level config to enable/disable local storage usage for service and 
> disk offerings.
>     - Local storage gets discovered when a host is added/reconnected if zone 
> level config is enabled. When disabled existing local storages are not 
> removed but any new local storage is not added.
>     - Deploy VM command validates service and disk offerings based on local 
> storage config.
>     - Upgrade uses the global config 'use.local.storage' to set the zone 
> level config for local storage.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/api/ApiConstants.java 825e276 
>   api/src/com/cloud/api/commands/CreateDiskOfferingCmd.java b3d9962 
>   api/src/com/cloud/api/commands/CreateZoneCmd.java b36c721 
>   api/src/com/cloud/api/commands/DeployVMCmd.java 9e2bc24 
>   api/src/com/cloud/api/commands/UpdateZoneCmd.java c22bff7 
>   api/src/com/cloud/api/response/DiskOfferingResponse.java 9b4f891 
>   api/src/com/cloud/api/response/ZoneResponse.java f591d70 
>   api/src/com/cloud/dc/DataCenter.java 2d3064f 
>   client/WEB-INF/classes/resources/messages.properties 13517ab 
>   server/src/com/cloud/api/ApiResponseHelper.java 0340a94 
>   server/src/com/cloud/configuration/Config.java 5ee66ff 
>   server/src/com/cloud/configuration/ConfigurationManager.java df28251 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3b07707 
>   server/src/com/cloud/dc/DataCenterVO.java a2b7d5f 
>   server/src/com/cloud/storage/LocalStoragePoolListener.java 1be7a55 
>   server/src/com/cloud/storage/StorageManagerImpl.java 5a256dc 
>   server/src/com/cloud/storage/allocator/AbstractStoragePoolAllocator.java 
> 87cb065 
>   server/src/com/cloud/storage/allocator/FirstFitStoragePoolAllocator.java 
> 006931d 
>   server/src/com/cloud/storage/allocator/LocalStoragePoolAllocator.java 
> 991baad 
>   server/src/com/cloud/storage/allocator/StoragePoolAllocator.java 13f44e7 
>   server/src/com/cloud/storage/allocator/UseLocalForRootAllocator.java 
> 38e116a 
>   server/src/com/cloud/test/DatabaseConfig.java a6aa094 
>   server/src/com/cloud/vm/UserVmManagerImpl.java a9474ba 
>   setup/db/create-schema.sql fa933e3 
>   setup/db/db/schema-303to40.sql 9f24966 
>   ui/index.jsp 2d8b8d6 
>   ui/scripts/configuration.js 85a169b 
>   ui/scripts/storage.js e75244f 
>   ui/scripts/system.js 015f491 
> 
> Diff: https://reviews.apache.org/r/6431/diff/
> 
> 
> Testing
> -------
> 
> Tested on XS.
> 
> 
> Thanks,
> 
> Koushik Das
> 
>

Reply via email to