cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mike Tutkowski" <mike.tutkow...@solidfire.com>
Subject Re: Review Request 11479: SolidFire storage plug-in and enhancements to the storage framework and GUI
Date Sun, 14 Jul 2013 05:58:16 GMT


> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java,
line 194
> > <https://reviews.apache.org/r/11479/diff/4/?file=310131#file310131line194>
> >
> >     Consider using Guava's Splitter class because it much more expressive, and will
greatly shorten this method.  It also a tested utility ...

Noted in my to-do document as a potential future activity to implement.


> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java,
line 210
> > <https://reviews.apache.org/r/11479/diff/4/?file=310131#file310131line210>
> >
> >     Consider using Guava's Splitter class because it much more expressive, and will
greatly shorten this method.  It also a tested utility ...

Noted in my to-do document as a potential future activity to implement.


> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java,
line 237
> > <https://reviews.apache.org/r/11479/diff/4/?file=310131#file310131line237>
> >
> >     Why is URL parsing logic being implemented?  Use java.net.URI ...

Noted in my to-do document as a potential future activity to implement.


> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java,
line 145
> > <https://reviews.apache.org/r/11479/diff/4/?file=310131#file310131line145>
> >
> >     Why are we parsing a URL here?  Use the java.net.URI class.  It will perform
this operation, and handle a number of corner cases this method handle such as escaped characters.

Noted in my to-do document as a potential future activity to implement.


> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/provider/SolidfirePrimaryDataStoreProvider.java,
line 83
> > <https://reviews.apache.org/r/11479/diff/4/?file=310132#file310132line83>
> >
> >     Consider using Guava's ImmutableSet .. this method can be reduced to ImmutableSet.of(DataStoreProviderType.PRIMARY).
 Also, why prevents someone from using a SolidFire for secondary storage?

Noted in my to-do document as a potential future activity to implement.

Users can make use of SolidFire storage for secondary storage, but they would not do so via
the SolidFire plug-in (they would just use the default plug-in).


- Mike


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


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