incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "edison su" <edison...@citrix.com>
Subject Re: Review Request: Support for local data disk feature. (CS-14277)
Date Mon, 27 Aug 2012 23:07:00 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message