Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Content-Type: multipart/alternative;
 boundary="===============1558554896846011272=="
MIME-Version: 1.0
Subject: Re: Review Request 11479: SolidFire storage plug-in and enhancements
 to the
 storage framework and GUI
From: "John Burwell" <jburwell@basho.com>
To: "edison su" <edison.su@citrix.com>, "John Burwell" <jburwell@basho.com>
Cc: "cloudstack" <cloudstack-dev@incubator.apache.org>,
 "Mike Tutkowski" <mike.tutkowski@solidfire.com>
Date: Thu, 18 Jul 2013 14:33:34 -0000
Message-ID: <20130718143334.1154.90843@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "John Burwell" <noreply@reviews.apache.org>
References: <20130628154223.23541.18737@reviews.apache.org>
In-Reply-To: <20130628154223.23541.18737@reviews.apache.org>
Reply-To: "John Burwell" <jburwell@basho.com>

--===============1558554896846011272==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit


> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > api/src/com/cloud/offering/DiskOffering.java, line 60
> > <https://reviews.apache.org/r/11479/diff/4/?file=310081#file310081line60>
> >
> >     When would it be valid for the value of this property to be null?  Seems like it should be boolean, not Boolean.
> 
> Mike Tutkowski wrote:
>     null is used if this feature is not in use.

What is the difference between false and null?


- John


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


On June 21, 2013, 4:35 p.m., Mike Tutkowski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
> 
> (Updated June 21, 2013, 4:35 p.m.)
> 
> 
> Review request for cloudstack, edison su and John Burwell.
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> This patch implements a storage plug-in for SolidFire. The plug-in is based on the new storage framework that went in with 4.2, as well.
> 
> In addition, there are GUI (and related) changes to enable admins and end users to specify a Min, Max, and Burst number of IOPS for a Disk Offering. These fields (although optional) tend to follow the pattern previously established for the Disk Size field.
> 
> Also, the storage framework itself has been enhanced. For example, it now supports creating and deleting storage repositories as is necessary for a dynamic type of zone-wide primary storage (such as the SolidFire plug-in is).
> 
> The desired behavior of the software is as such:
> 
> * Allow an admin to invoke the CloudStack API to add Primary Storage based on the SolidFire plug-in.
> 
> * Allow an admin to create a Disk Offering that specifies a Min, Max, and Burst number of IOPS or allows the admin to pass this ability on to the end user.
> 
> * Allow an end user to execute such a Disk Offering. As is the case for any Disk Offering, this leads to the creation of a row in the volumes table.
> 
> * Allow an end user to attach the resultant volume (noted in the DB) to a VM. The storage framework invokes logic in the plug-in and the plug-in creates a volume on its SAN with the correct size and IOPS values. The agent software for XenServer detects that such an attach is being requested and creates a Storage Repository (and single VDI within the SR) based on the storage IP address of the SAN and the IQN of the volume. The VDI is then hooked up to the VM.
> 
> * Allow an end user to detach the volume. This leads to the destruction of the SR, but the SAN volume remains intact and can be reattached later to any VM running in a XenServer resource pool in the zone.
> 
> * Allow an end user to delete the volume. This leads to the volume being deleted on the SAN and being marked in the CloudStack DB as deleted.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/offering/DiskOffering.java ae4528c 
>   api/src/com/cloud/storage/StoragePool.java 8b95383 
>   api/src/com/cloud/storage/Volume.java 4903594 
>   api/src/org/apache/cloudstack/api/ApiConstants.java 1704ca3 
>   api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java a2c5f77 
>   api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java 74eb2b9 
>   api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java 86a494b 
>   api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java 35cf21a 
>   api/src/org/apache/cloudstack/api/response/StoragePoolResponse.java 965407d 
>   api/src/org/apache/cloudstack/api/response/VolumeResponse.java e3463bd 
>   client/WEB-INF/classes/resources/messages.properties a0a36c8 
>   client/pom.xml ab758eb 
>   client/tomcatconf/applicationContext.xml.in 049e483 
>   core/src/com/cloud/agent/api/AttachVolumeAnswer.java b377b7c 
>   core/src/com/cloud/agent/api/AttachVolumeCommand.java 2658262 
>   core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java 251a6cb 
>   core/test/org/apache/cloudstack/api/agent/test/AttachVolumeCommandTest.java 1ec416a 
>   core/test/org/apache/cloudstack/api/agent/test/BackupSnapshotCommandTest.java 44d53aa 
>   core/test/org/apache/cloudstack/api/agent/test/SnapshotCommandTest.java c2d69c0 
>   core/test/src/com/cloud/agent/api/test/ResizeVolumeCommandTest.java 02085f5 
>   engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreDriver.java cf5759b 
>   engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java b2b787c 
>   engine/api/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java d461d58 
>   engine/api/src/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java 0262f65 
>   engine/schema/src/com/cloud/storage/DiskOfferingVO.java 44f9e8f 
>   engine/schema/src/com/cloud/storage/VolumeVO.java 1699afd 
>   engine/schema/src/com/cloud/storage/dao/VolumeDao.java 2513181 
>   engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java 12ca3c7 
>   engine/storage/image/src/org/apache/cloudstack/storage/image/ImageServiceImpl.java 99b1013 
>   engine/storage/image/src/org/apache/cloudstack/storage/image/driver/AncientImageDataStoreDriverImpl.java 4c16f2f 
>   engine/storage/image/src/org/apache/cloudstack/storage/image/driver/DefaultImageDataStoreDriverImpl.java 3d46c73 
>   engine/storage/integration-test/test/org/apache/cloudstack/storage/allocator/StorageAllocatorTest.java 9444fa5 
>   engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/strategy/AncientSnapshotStrategy.java 4aba3d9 
>   engine/storage/src/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java 5326701 
>   engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java d8d4132 
>   engine/storage/src/org/apache/cloudstack/storage/datastore/DataObjectManagerImpl.java 9d1afbe 
>   engine/storage/src/org/apache/cloudstack/storage/datastore/PrimaryDataStoreEntityImpl.java 2dc3e25 
>   engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 349f6ba 
>   engine/storage/volume/src/org/apache/cloudstack/storage/datastore/DefaultPrimaryDataStore.java 31e6908 
>   engine/storage/volume/src/org/apache/cloudstack/storage/datastore/driver/DefaultPrimaryDataStoreDriverImpl.java e5ee742 
>   engine/storage/volume/src/org/apache/cloudstack/storage/datastore/lifecycle/DefaultPrimaryDataStoreLifeCycleImpl.java cffa1ce 
>   engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java ea31be3 
>   engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java 54dcbd2 
>   plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java 7d90f6a 
>   plugins/hypervisors/simulator/src/com/cloud/agent/manager/MockStorageManagerImpl.java a50dff6 
>   plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 1af4239 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 5e8283a 
>   plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java 0486902 
>   plugins/storage/volume/solidfire/pom.xml 9db0685 
>   plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java f31126c 
>   plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java PRE-CREATION 
>   plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/provider/SolidfirePrimaryDataStoreProvider.java 650cac8 
>   plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java PRE-CREATION 
>   server/src/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java 7022ee6 
>   server/src/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java f2b9525 
>   server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java 453e82e 
>   server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 2336a48 
>   server/src/com/cloud/api/query/vo/StoragePoolJoinVO.java 29e97f4 
>   server/src/com/cloud/api/query/vo/VolumeJoinVO.java 735cf9a 
>   server/src/com/cloud/configuration/ConfigurationManager.java 93cadfa 
>   server/src/com/cloud/configuration/ConfigurationManagerImpl.java e7e3f74 
>   server/src/com/cloud/server/ConfigurationServerImpl.java 510455b 
>   server/src/com/cloud/storage/StorageManager.java 29c7ebc 
>   server/src/com/cloud/storage/StorageManagerImpl.java 20b435c 
>   server/src/com/cloud/storage/VolumeManager.java 56de408 
>   server/src/com/cloud/storage/VolumeManagerImpl.java e5868d3 
>   server/src/com/cloud/test/DatabaseConfig.java ef0259d 
>   server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 6e3d187 
>   setup/db/db/schema-410to420.sql bcdf2d9 
>   tools/marvin/marvin/cloudstackConnection.py b092ef0 
>   ui/dictionary.jsp 7809cdb 
>   ui/scripts/configuration.js 150f244 
>   ui/scripts/docs.js 5aa352a 
>   ui/scripts/sharedFunctions.js d87f0dc 
>   ui/scripts/storage.js 2c03d39 
>   ui/scripts/system.js df37f31 
>   utils/src/com/cloud/utils/StringUtils.java 359b169 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/HostDatastoreSystemMO.java 3dcd724 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java a866fdc 
>   vmware-base/src/com/cloud/hypervisor/vmware/mo/HostStorageSystemMO.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/11479/diff/
> 
> 
> Testing
> -------
> 
> Manual testing has been performed:
> 
> A plug-in can be added to CloudStack via an API call.
> 
> Create a Disk Offering where the admin specifies both Disk Size and Disk IOPS. Disk IOPS are left blank.
> Create a Disk Offering where the admin allows the end user to customize both Disk Size and Disk IOPS. Disk IOPS are left blank.
> Create a Disk Offering where the admin specifies Disk Size, but allows the end user to customize Disk IOPS. Disk IOPS are left blank.
> Create a Disk Offering where the admin specifies Disk IOPS, but allows the end user to customize Disk Size. Disk IOPS are left blank.
> 
> Create a Disk Offering where the admin specifies both Disk Size and Disk IOPS. Disk IOPS are all filled in.
> Create a Disk Offering where the admin allows the end user to customize both Disk Size and Disk IOPS. Disk IOPS are all filled in.
> Create a Disk Offering where the admin specifies Disk Size, but allows the end user to customize Disk IOPS. Disk IOPS are all filled in.
> Create a Disk Offering where the admin specifies Disk IOPS, but allows the end user to customize Disk Size. Disk IOPS are all filled in.
> 
> A newly created volume is attached to a VM for the first time and an SR and VDI are created.
> This volume is detached and the VDI and SR are deleted.
> The volume is reattached and is properly introduced into the SR and VDI ("introduced" in the sense that the data that was previously on the volume is not destroyed upon reattach).
> This volume is detached and the VDI and SR are deleted.
> 
> 
> Thanks,
> 
> Mike Tutkowski
> 
>


--===============1558554896846011272==--