You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Mike Tutkowski <mi...@solidfire.com> on 2013/05/29 00:49:11 UTC
Review Request: SolidFire storage plug-in and enhancements to the storage
framework and GUI
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/
-----------------------------------------------------------
Review request for cloudstack and edison su.
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 dd77c70
api/src/com/cloud/storage/Storage.java c130fe2
api/src/com/cloud/storage/Volume.java 4903594
api/src/org/apache/cloudstack/api/ApiConstants.java 1e9435f
api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java aa11599
api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java 86a494b
api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java 377e66e
api/src/org/apache/cloudstack/api/response/VolumeResponse.java 21d7d1a
client/WEB-INF/classes/resources/messages.properties 1638be1
client/pom.xml 0c38ecb
client/tomcatconf/applicationContext.xml.in edf83a9
core/src/com/cloud/agent/api/AttachVolumeCommand.java 302b8f8
core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java 251a6cb
core/test/org/apache/cloudstack/api/agent/test/AttachVolumeCommandTest.java 1ec416a
engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java b2b787c
engine/schema/src/com/cloud/storage/DiskOfferingVO.java 909d7fe
engine/schema/src/com/cloud/storage/VolumeVO.java 1699afd
engine/storage/integration-test/test/org/apache/cloudstack/storage/allocator/StorageAllocatorTest.java 9444fa5
engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java e976980
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 5f8daf4
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java ea31be3
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4680fde
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 283181f
server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java e27e2d9
server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 6d3cdcb
server/src/com/cloud/api/query/vo/VolumeJoinVO.java 6ef8c91
server/src/com/cloud/configuration/ConfigurationManager.java 8db037b
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 8baf1fb
server/src/com/cloud/server/ConfigurationServerImpl.java bc52e9a
server/src/com/cloud/storage/VolumeManager.java d198e5d
server/src/com/cloud/storage/VolumeManagerImpl.java 4b654eb
server/src/com/cloud/test/DatabaseConfig.java 70c8178
server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 21b3590
setup/db/db/schema-410to420.sql cf4e98d
tools/marvin/marvin/cloudstackConnection.py b092ef0
ui/dictionary.jsp ded9ea0
ui/scripts/configuration.js 211d7b7
ui/scripts/docs.js 7c1aaf8
ui/scripts/storage.js e816334
ui/scripts/system.js 8b9a81f
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 reattach 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
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review23471
-----------------------------------------------------------
Updated diff files related to Review Board comments can be found here (under the check-in message of "Changes related to Review Board comments"):
https://git-wip-us.apache.org/repos/asf?p=cloudstack.git&a=search&h=refs%2Fheads%2F4.2&st=author&s=Tutkowski
- Mike Tutkowski
On June 21, 2013, 10:35 a.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated June 21, 2013, 10:35 a.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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by "Jenkins Cloudstack.org" <hu...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review23613
-----------------------------------------------------------
Review 11479 failed the build test : FAILURE
The url of build cloudstack-master-with-patch #3 is : http://jenkins.cloudstack.org/job/cloudstack-master-with-patch/3/
- Jenkins Cloudstack.org
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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
> 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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
> On June 28, 2013, 9:42 a.m., John Burwell wrote:
> > plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java, line 3927
> > <https://reviews.apache.org/r/11479/diff/4/?file=310126#file310126line3927>
> >
> > Use an ExceutorService. Don't implement your own thread pool. Also, why are we multi-threading? Are there performance numbers to justify this complexity?
It often takes on the order of seconds to rescan HBAs and VMFS datastores and - due to certain VMware issues - these kinds of scans need to happen several times while I'm attaching and detaching volumes. Instead of taking, say, 3 seconds per scan times the number of hosts and making the user wait that long, I perform these scans in parallel. This can be the difference between the user waiting only 10 - 15 seconds or having to wait over a minute.
I have converted the custom threading "pool" to use the ExecutorService.
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review22508
-----------------------------------------------------------
On June 21, 2013, 10:35 a.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated June 21, 2013, 10:35 a.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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > core/src/com/cloud/agent/api/AttachVolumeCommand.java, line 61
> > <https://reviews.apache.org/r/11479/diff/4/?file=310095#file310095line61>
> >
> > Fix formatting/continuation from the previous line
I think I fixed all formatting issues in this file.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > engine/storage/src/org/apache/cloudstack/storage/datastore/PrimaryDataStoreEntityImpl.java, line 181
> > <https://reviews.apache.org/r/11479/diff/4/?file=310117#file310117line181>
> >
> > What is the resolution of this TODO? It appears that this getter is broken as it doesn't return a parameter value.
It appears this class has been removed from the codebase.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > engine/storage/volume/src/org/apache/cloudstack/storage/datastore/lifecycle/DefaultPrimaryDataStoreLifeCycleImpl.java, line 111
> > <https://reviews.apache.org/r/11479/diff/4/?file=310121#file310121line111>
> >
> > I know it is out of the scope of this patch, but please correct the name of this method on the interface and all implementations to conform to JavaBean property standard. Accessors for boolean properties should be named is<PropertyName>. Failure to conform to this convention will interfere with any reflection tools we elect to use.
This method no longer seems to be present in the codebase.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java, line 3789
> > <https://reviews.apache.org/r/11479/diff/4/?file=310126#file310126line3789>
> >
> > 1) Don't throw Exception generically. Throw out the specific exceptions that can occur.
> > 2) What is the error handling here? This method has no recovery provisions. A failure could leave dangling resources. Seems that we should be catching Exception, and attempting to undo the operations performed, and then re-throwing the exception as a CloudRuntimeException.
I agree exceptions should be more specific. The problem here is that this VMware code (the VmwareResource class itself) was written with generic exception handling all over the place. I think if we want to correct this, it will take a larger effort (therefore I'm thinking this change is out of scope for this release).
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java, line 3813
> > <https://reviews.apache.org/r/11479/diff/4/?file=310126#file310126line3813>
> >
> > Exception is too broad. Throw a CloudRuntimeException.
I agree exceptions should be more specific. The problem here is that this VMware code (the VmwareResource class itself) was written with generic exception handling all over the place. I think if we want to correct this, it will take a larger effort (therefore I'm thinking this change is out of scope for this release).
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java, line 3816
> > <https://reviews.apache.org/r/11479/diff/4/?file=310126#file310126line3816>
> >
> > Casting down from a long to int seems dangerous. Why are casting this value down?
The VMware API is expecting an int referencing the size in MBs. This code here is copied/pasted from another location in the VmwareResource file where the same conversion happens.
I have made a utility function for this.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java, line 3891
> > <https://reviews.apache.org/r/11479/diff/4/?file=310126#file310126line3891>
> >
> > 1) Strip out tab characters
> > 2) Fix indentation
> > 3) Don't throw Exception -- it's too generic. Seems that we should be catching Exception, and attempting to undo any operations performed in order to ensure that we don't leave the hypervisor in an inconsistent state then rethrowing the exception as a CloudRuntimeException.
I agree exceptions should be more specific. The problem here is that this VMware code (the VmwareResource class itself) was written with generic exception handling all over the place. I think if we want to correct this, it will take a larger effort (therefore I'm thinking this change is out of scope for this release).
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java, line 3955
> > <https://reviews.apache.org/r/11479/diff/4/?file=310126#file310126line3955>
> >
> > Why are we locking a variable with local method scope? There should be *no* need to synchronize here, and it is very expensive.
It is actually a local variable that has been passed into multiple threads to record any exceptions that might happen. Since more than one thread can call the "add" method at the same time, I have protected the data structure with synchronization.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java, line 126
> > <https://reviews.apache.org/r/11479/diff/4/?file=310130#file310130line126>
> >
> > Set defaults here. Also, as noted earlier, don't throw Exception ...
I now throw an IllegalArgumentException if the values are not correct.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java, line 193
> > <https://reviews.apache.org/r/11479/diff/4/?file=310130#file310130line193>
> >
> > Lines 190-208 look very similar to lines 72-90 createSolidFireVolume. Can this work be extracted to a common method used by both methods?
I believe this has been fixed since the review came out (when I performed a merge and made other updates).
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java, line 246
> > <https://reviews.apache.org/r/11479/diff/4/?file=310130#file310130line246>
> >
> > I would collapse lines 242-247 into something such as the following to make things a little more readable:
> >
> > storagePool.setAvailableBytes(availableBytes >= 0 : availableBytes ? 0);
Implemented with ternary operator now...originally I didn't because people have told me they find ternary operators less readable. :)
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java, line 253
> > <https://reviews.apache.org/r/11479/diff/4/?file=310130#file310130line253>
> >
> > Shouldn't this exception be passed up to higher layers? Seems like no storage available should be a stopper for clients of this method.
I believe the exception is passed up to higher layers this way:
result.setResult(errMsg);
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java, line 301
> > <https://reviews.apache.org/r/11479/diff/4/?file=310130#file310130line301>
> >
> > Shouldn't this exception be passed up to higher layers? Seems like no storage available should be a stopper for clients of this method.
I believe the exception is passed up to higher layers this way:
result.setResult(errMsg);
> 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 263
> > <https://reviews.apache.org/r/11479/diff/4/?file=310131#file310131line263>
> >
> > It seems like this method is lying to the client. The host was not attached, but we are telling them that it occurred. Why are we lying here?
Method no longer exists.
> 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 269
> > <https://reviews.apache.org/r/11479/diff/4/?file=310131#file310131line269>
> >
> > It seems like this method is lying to the client. The cluster was not attached, but we are telling them that it occurred. Why are we lying here?
Method no longer exists.
> 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 280
> > <https://reviews.apache.org/r/11479/diff/4/?file=310131#file310131line280>
> >
> > because?
Method no longer exists.
> 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 282
> > <https://reviews.apache.org/r/11479/diff/4/?file=310131#file310131line282>
> >
> > While it is out of the scope of this patch, fix the spelling error, and change the name to conform with the JavaBean property specification.
Method no longer exists.
> 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 36
> > <https://reviews.apache.org/r/11479/diff/4/?file=310132#file310132line36>
> >
> > Why are these attributes protected and not private?
It was a copy/paste/modify from a "template." I have changed them to be private.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 646
> > <https://reviews.apache.org/r/11479/diff/4/?file=310133#file310133line646>
> >
> > This method seems like it should be in a common utility class ...
Something to consider perhaps going forward.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > vmware-base/src/com/cloud/hypervisor/vmware/mo/HostStorageSystemMO.java, line 36
> > <https://reviews.apache.org/r/11479/diff/4/?file=310160#file310160line36>
> >
> > Throwing Exception is too broad. Can the client recover from the checked exception encountered by this method? If not, catch and re-throw as CloudRuntimeException. Otherwise, declare as throwing only the expected checked exceptions.
This is a general-purpose problem with VMware-related code. It would be nice if we fixed this in a comprehensive manner at some point.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > vmware-base/src/com/cloud/hypervisor/vmware/mo/HostStorageSystemMO.java, line 40
> > <https://reviews.apache.org/r/11479/diff/4/?file=310160#file310160line40>
> >
> > Throwing Exception is too broad. Can the client recover from the checked exception encountered by this method? If not, catch and re-throw as CloudRuntimeException. Otherwise, declare as throwing only the expected checked exceptions.
This is a general-purpose problem with VMware-related code. It would be nice if we fixed this in a comprehensive manner at some point.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > vmware-base/src/com/cloud/hypervisor/vmware/mo/HostStorageSystemMO.java, line 44
> > <https://reviews.apache.org/r/11479/diff/4/?file=310160#file310160line44>
> >
> > Throwing Exception is too broad. Can the client recover from the checked exception encountered by this method? If not, catch and re-throw as CloudRuntimeException. Otherwise, declare as throwing only the expected checked exceptions.
This is a general-purpose problem with VMware-related code. It would be nice if we fixed this in a comprehensive manner at some point.
- 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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
> On June 28, 2013, 9:42 a.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.
>
> John Burwell wrote:
> What is the difference between false and null?
null means storage QoS is not in use.
false means storage QoS is in use and the admin has selected the min and max IOPS values.
true means storage QoS is in use and the admin has not selected the min and max IOPS values (the end user can select them).
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review22508
-----------------------------------------------------------
On June 21, 2013, 10:35 a.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated June 21, 2013, 10:35 a.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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by John Burwell <jb...@basho.com>.
> 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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
> 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.
null is used if this feature is not in use.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java, line 79
> > <https://reviews.apache.org/r/11479/diff/4/?file=310122#file310122line79>
> >
> > Change of this method to conform with JavaBean property specifications -- get<PropertyName>. In this case, getISCIName would likely be most appropriate. As mentioned previously, these conventions are critical for proper reflection operation.
Member variable is named "_iScsiName", so getter is named get_iScsiName() (which is kind of nice because it allows us to have more standard casing for "iSCSI", where the "i" is small and the "S" is capital)
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 291
> > <https://reviews.apache.org/r/11479/diff/4/?file=310133#file310133line291>
> >
> > If this class is unused, why not remove the cruft rather ignore the warning?
It is required for JSON parsing (using the Gson library), but the code-checking logic believes one or more properties are not in use, so it would generate one or more unnecessary warnings.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 339
> > <https://reviews.apache.org/r/11479/diff/4/?file=310133#file310133line339>
> >
> > If this class is unused, why not remove the cruft rather ignore the warning?
It is required for JSON parsing (using the Gson library), but the code-checking logic believes one or more properties are not in use, so it would generate one or more unnecessary warnings.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 361
> > <https://reviews.apache.org/r/11479/diff/4/?file=310133#file310133line361>
> >
> > If this class is unused, why not remove the cruft rather ignore the warning?
It is required for JSON parsing (using the Gson library), but the code-checking logic believes one or more properties are not in use, so it would generate one or more unnecessary warnings.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 384
> > <https://reviews.apache.org/r/11479/diff/4/?file=310133#file310133line384>
> >
> > If this class is unused, why not remove the cruft rather ignore the warning?
It is required for JSON parsing (using the Gson library), but the code-checking logic believes one or more properties are not in use, so it would generate one or more unnecessary warnings.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 406
> > <https://reviews.apache.org/r/11479/diff/4/?file=310133#file310133line406>
> >
> > If this class is unused, why not remove the cruft rather ignore the warning?
It is required for JSON parsing (using the Gson library), but the code-checking logic believes one or more properties are not in use, so it would generate one or more unnecessary warnings.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 428
> > <https://reviews.apache.org/r/11479/diff/4/?file=310133#file310133line428>
> >
> > If this class is unused, why not remove the cruft rather ignore the warning?
It is required for JSON parsing (using the Gson library), but the code-checking logic believes one or more properties are not in use, so it would generate one or more unnecessary warnings.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 450
> > <https://reviews.apache.org/r/11479/diff/4/?file=310133#file310133line450>
> >
> > If this class is unused, why not remove the cruft rather ignore the warning?
It is required for JSON parsing (using the Gson library), but the code-checking logic believes one or more properties are not in use, so it would generate one or more unnecessary warnings.
> On June 28, 2013, 3:42 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 472
> > <https://reviews.apache.org/r/11479/diff/4/?file=310133#file310133line472>
> >
> > If this class is unused, why not remove the cruft rather ignore the warning?
It is required for JSON parsing (using the Gson library), but the code-checking logic believes one or more properties are not in use, so it would generate one or more unnecessary warnings.
- 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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review22508
-----------------------------------------------------------
api/src/com/cloud/offering/DiskOffering.java
<https://reviews.apache.org/r/11479/#comment46044>
Strip out additional tab charcters
api/src/com/cloud/offering/DiskOffering.java
<https://reviews.apache.org/r/11479/#comment46045>
When would it be valid for the value of this property to be null? Seems like it should be boolean, not Boolean.
api/src/com/cloud/storage/Volume.java
<https://reviews.apache.org/r/11479/#comment46046>
Strip out tab charcaters
api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java
<https://reviews.apache.org/r/11479/#comment46047>
Strip out tab characters
api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java
<https://reviews.apache.org/r/11479/#comment46048>
Strip out tab characters
api/src/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java
<https://reviews.apache.org/r/11479/#comment46049>
Strip out tab characters
api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java
<https://reviews.apache.org/r/11479/#comment46050>
Strip out tab characters
api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java
<https://reviews.apache.org/r/11479/#comment46051>
Strip out tab characters
api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java
<https://reviews.apache.org/r/11479/#comment46052>
Strip out tab characters
api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java
<https://reviews.apache.org/r/11479/#comment46053>
Strip out tab characters
api/src/org/apache/cloudstack/api/response/StoragePoolResponse.java
<https://reviews.apache.org/r/11479/#comment46054>
Strip out tab characters
api/src/org/apache/cloudstack/api/response/VolumeResponse.java
<https://reviews.apache.org/r/11479/#comment46055>
Strip out tab characters
api/src/org/apache/cloudstack/api/response/VolumeResponse.java
<https://reviews.apache.org/r/11479/#comment46056>
Strip out tab characters
core/src/com/cloud/agent/api/AttachVolumeAnswer.java
<https://reviews.apache.org/r/11479/#comment46057>
Strip out tab characters
core/src/com/cloud/agent/api/AttachVolumeCommand.java
<https://reviews.apache.org/r/11479/#comment46058>
Fix formatting/continuation from the previous line
core/src/com/cloud/agent/api/AttachVolumeCommand.java
<https://reviews.apache.org/r/11479/#comment46059>
Fix indentation
core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java
<https://reviews.apache.org/r/11479/#comment46060>
Create a new AttachVolumeAnswer constructor that only takes two parameters and encapsulates the default value. Leaky abstraction that negatively impacts maintenance.
engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java
<https://reviews.apache.org/r/11479/#comment46032>
Strip out tab characters
engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java
<https://reviews.apache.org/r/11479/#comment46031>
Strip out tab characters
engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java
<https://reviews.apache.org/r/11479/#comment46033>
Strip out tab characters
engine/api/src/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java
<https://reviews.apache.org/r/11479/#comment46034>
Strip out tab characters
engine/schema/src/com/cloud/storage/DiskOfferingVO.java
<https://reviews.apache.org/r/11479/#comment46035>
1) The minIops and maxIops attributes should be private.
2) Strip out tab characters
engine/schema/src/com/cloud/storage/DiskOfferingVO.java
<https://reviews.apache.org/r/11479/#comment46036>
Strip out tab characters
engine/schema/src/com/cloud/storage/DiskOfferingVO.java
<https://reviews.apache.org/r/11479/#comment46037>
1) Fix identation
2) Strip out tab characters
engine/schema/src/com/cloud/storage/VolumeVO.java
<https://reviews.apache.org/r/11479/#comment46038>
1) Strip out tab characters
2) Make minIops and maxIops attributes private
engine/schema/src/com/cloud/storage/VolumeVO.java
<https://reviews.apache.org/r/11479/#comment46039>
Strip out tab characters
engine/schema/src/com/cloud/storage/VolumeVO.java
<https://reviews.apache.org/r/11479/#comment46040>
1) Strip out tab characters
2) Fix formatting from the previous line
engine/schema/src/com/cloud/storage/VolumeVO.java
<https://reviews.apache.org/r/11479/#comment46041>
Strip out additional tab characters
engine/storage/src/org/apache/cloudstack/storage/datastore/PrimaryDataStoreEntityImpl.java
<https://reviews.apache.org/r/11479/#comment46042>
What is the resolution of this TODO? It appears that this getter is broken as it doesn't return a parameter value.
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java
<https://reviews.apache.org/r/11479/#comment46043>
Strip out tab characters
engine/storage/volume/src/org/apache/cloudstack/storage/datastore/lifecycle/DefaultPrimaryDataStoreLifeCycleImpl.java
<https://reviews.apache.org/r/11479/#comment46061>
I know it is out of the scope of this patch, but please correct the name of this method on the interface and all implementations to conform to JavaBean property standard. Accessors for boolean properties should be named is<PropertyName>. Failure to conform to this convention will interfere with any reflection tools we elect to use.
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
<https://reviews.apache.org/r/11479/#comment46062>
Change of this method to conform with JavaBean property specifications -- get<PropertyName>. In this case, getISCIName would likely be most appropriate. As mentioned previously, these conventions are critical for proper reflection operation.
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
<https://reviews.apache.org/r/11479/#comment46066>
1) Don't throw Exception generically. Throw out the specific exceptions that can occur.
2) What is the error handling here? This method has no recovery provisions. A failure could leave dangling resources. Seems that we should be catching Exception, and attempting to undo the operations performed, and then re-throwing the exception as a CloudRuntimeException.
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
<https://reviews.apache.org/r/11479/#comment46063>
1) Strip out tab characters
2) Fix indentation
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
<https://reviews.apache.org/r/11479/#comment46064>
Exception is too broad. Throw a CloudRuntimeException.
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
<https://reviews.apache.org/r/11479/#comment46065>
Casting down from a long to int seems dangerous. Why are casting this value down?
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
<https://reviews.apache.org/r/11479/#comment46067>
String out tab characters
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
<https://reviews.apache.org/r/11479/#comment46068>
1) Strip out tab characters
2) Fix indentation
3) Don't throw Exception -- it's too generic. Seems that we should be catching Exception, and attempting to undo any operations performed in order to ensure that we don't leave the hypervisor in an inconsistent state then rethrowing the exception as a CloudRuntimeException.
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
<https://reviews.apache.org/r/11479/#comment46070>
Use an ExceutorService. Don't implement your own thread pool. Also, why are we multi-threading? Are there performance numbers to justify this complexity?
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
<https://reviews.apache.org/r/11479/#comment46069>
Why are we locking a variable with local method scope? There should be *no* need to synchronize here, and it is very expensive.
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment46071>
Strip out tab characters
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment46072>
Strip out tab characters
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment46073>
Strip out tab characters
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment46074>
Strip out tab characters
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment46076>
Do not generically throw Exception -- it is too broad. Implement error handling, and ensure the hypervisor is returned to a consistent state.
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment46075>
Don't throw exception -- throw a runtime exception. In CloudStack, use CloudRuntimeException.
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment46077>
Strip out tab characters
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46078>
Strip out tab characters
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46079>
1) Strip out tab characters
2) Fix indentation
3) Don't throw Exception -- it is too broad. Implement error handling to undo operations and attempt to return the hypervisor to a consistent state.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46080>
Seems like these default values should be configurable by device. At minimum they should be constants. Also, if they are constants, push them down to the Iops class, and encapsulate the values and tests for setting them.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46081>
Set defaults here. Also, as noted earlier, don't throw Exception ...
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46082>
Throw only the checked exceptions expected to be thrown. Throwing Exception causes us to miss error handling later introduced by dependencies that we should account for in this method. The compiler will catch it, and cause us to realize and address it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46098>
Lines 190-208 look very similar to lines 72-90 createSolidFireVolume. Can this work be extracted to a common method used by both methods?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46083>
Strip out tab characters
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46084>
I would collapse lines 242-247 into something such as the following to make things a little more readable:
storagePool.setAvailableBytes(availableBytes >= 0 : availableBytes ? 0);
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46090>
Shouldn't this exception be passed up to higher layers? Seems like no storage available should be a stopper for clients of this method.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46085>
Too broad a catch statement as it will swallow RuntimeExceptions that should percolate up to higher layers.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46086>
Strip out tab characters
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46087>
I would change the structure of this block to check if the type is *not* a VOLUME, handle it, and return. The remainder of the method will be the success path -- reducing an indentation level and making things much more readable.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46089>
Shouldn't this exception be passed up to higher layers? Seems like no storage available should be a stopper for clients of this method.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46088>
As mentioned above, don't catch exception.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46092>
Why is there no implementation? If this method is not supported, throw an UnsupportedOperationException. If this methods are used, the client will under the mistaken impression that the copy was performed.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46091>
Why is there no implementation? If this method is not supported, throw an UnsupportedOperationException. If this methods are used, the client will under the mistaken impression that a snapshot was taken.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46093>
Why is there no implementation? If this method is not supported, throw an UnsupportedOperationException. If this methods are used, the client will under the mistaken impression that a snapshot was reverted.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment46094>
Why is there no implementation? If this method is not supported, throw an UnsupportedOperationException. If this methods are used, the client will under the mistaken impression that a resize was performed.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46095>
Strip out tab characters
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46097>
This a very broad catch to discover an illegal argument. Why can't the underlying runtime exception just bubble up, and this method proactively check the argument before attempting to operate?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46096>
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.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46099>
Consider using Guava's Splitter class because it much more expressive, and will greatly shorten this method. It also a tested utility ...
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46100>
Consider using Guava's Splitter class because it much more expressive, and will greatly shorten this method. It also a tested utility ...
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46101>
Why is URL parsing logic being implemented? Use java.net.URI ...
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46102>
It seems like this method is lying to the client. The host was not attached, but we are telling them that it occurred. Why are we lying here?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46103>
It seems like this method is lying to the client. The cluster was not attached, but we are telling them that it occurred. Why are we lying here?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46105>
because?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46104>
While it is out of the scope of this patch, fix the spelling error, and change the name to conform with the JavaBean property specification.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46106>
Lines 313-331 seems very similar to code in create and delete. Consolidate into a common private method.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment46107>
Too broad a catch. Only catch the checked exceptions expected.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/provider/SolidfirePrimaryDataStoreProvider.java
<https://reviews.apache.org/r/11479/#comment46109>
Strip out tab characters
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/provider/SolidfirePrimaryDataStoreProvider.java
<https://reviews.apache.org/r/11479/#comment46108>
Why are these attributes protected and not private?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/provider/SolidfirePrimaryDataStoreProvider.java
<https://reviews.apache.org/r/11479/#comment46110>
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?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46111>
Strip out tab characters
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46117>
1) Strip out tab characters
2) Fix indentation
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46112>
As mentioned elsewhere, too broad a throws. Refine and add error handling as necessary to ensure the system is in a consistent state.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46113>
As mentioned elsewhere, too broad a throws. Refine and add error handling as necessary to ensure the system is in a consistent state.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46114>
As mentioned elsewhere, too broad a throws. Refine and add error handling as necessary to ensure the system is in a consistent state.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46115>
Inspecific hashCode. All attributes must be calculated for proper determinism.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46116>
1) Check that obj is not null
2) Check getClass().equals rather than instanceof to avoid class version mismatch issues cause by Classloader hinkiness
3) Check all attributes to ensure determinism
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46118>
Inspecific hashCode. All attributes must be calculated for proper determinism.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46119>
1) Check that obj is not null
2) Check getClass().equals rather than instanceof to avoid class version mismatch issues cause by Classloader hinkiness
3) Check all attributes to ensure determinism
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46120>
If this class is unused, why not remove the cruft rather ignore the warning?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46121>
If this class is unused, why not remove the cruft rather ignore the warning?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46122>
If this class is unused, why not remove the cruft rather ignore the warning?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46123>
If this class is unused, why not remove the cruft rather ignore the warning?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46124>
If this class is unused, why not remove the cruft rather ignore the warning?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46125>
If this class is unused, why not remove the cruft rather ignore the warning?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46126>
If this class is unused, why not remove the cruft rather ignore the warning?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46127>
If this class is unused, why not remove the cruft rather ignore the warning?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46128>
Clients of this method can not recover from these exceptions. Catch these exception and rethrow them as CloudRuntimeException.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46129>
Seems like we should be using the pooled connection manager. However, I don't know how it would be properly managed in the static context. Likely needs to be a ticket opened to be addressed later.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46131>
Use CloudRuntimeException
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46130>
Don't catch throwable as it can swallow Errors that should abend the JVM. Only catch the checked exception expected. Also, log the ignored exception to WARN.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46132>
This method seems like it should be in a common utility class ...
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46133>
Throw CloudRuntimeException not Exception.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment46134>
Throw CloudRuntimeException not Exception.
server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java
<https://reviews.apache.org/r/11479/#comment46135>
Strip out tab characters
server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java
<https://reviews.apache.org/r/11479/#comment46136>
Strip out tab characters
server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java
<https://reviews.apache.org/r/11479/#comment46137>
Strip out tab characters
server/src/com/cloud/api/query/vo/VolumeJoinVO.java
<https://reviews.apache.org/r/11479/#comment46138>
Strip out tab characters
server/src/com/cloud/api/query/vo/VolumeJoinVO.java
<https://reviews.apache.org/r/11479/#comment46139>
Strip out tab characters
server/src/com/cloud/configuration/ConfigurationManager.java
<https://reviews.apache.org/r/11479/#comment46140>
Strip out tab characters
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46142>
Fix formatting/indentation overflow
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46141>
Strip out tab characters
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46143>
Strip out tab characters
server/src/com/cloud/storage/StorageManager.java
<https://reviews.apache.org/r/11479/#comment46146>
Strip out tab characters
server/src/com/cloud/storage/StorageManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46148>
Coding convention: Surround wit curly braces
server/src/com/cloud/storage/StorageManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46147>
Strip out tab characters
server/src/com/cloud/storage/VolumeManager.java
<https://reviews.apache.org/r/11479/#comment46149>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46150>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46151>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46152>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46153>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46154>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46155>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46156>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46157>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46158>
Strip out tab characters
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46159>
Strip out tab characters
server/src/com/cloud/test/DatabaseConfig.java
<https://reviews.apache.org/r/11479/#comment46160>
Leaky abstraction: Create a construction on DiskOfferingVO that support passing the six parameters and encapsulates this default behavior.
server/test/com/cloud/vpc/MockConfigurationManagerImpl.java
<https://reviews.apache.org/r/11479/#comment46161>
Strip out tab characters
ui/scripts/configuration.js
<https://reviews.apache.org/r/11479/#comment46162>
Strip out tab characters
ui/scripts/configuration.js
<https://reviews.apache.org/r/11479/#comment46163>
Strip out tab characters
ui/scripts/configuration.js
<https://reviews.apache.org/r/11479/#comment46164>
Strip out tab characters
ui/scripts/configuration.js
<https://reviews.apache.org/r/11479/#comment46165>
Strip out tab characters
ui/scripts/configuration.js
<https://reviews.apache.org/r/11479/#comment46166>
Strip out tab characters
ui/scripts/configuration.js
<https://reviews.apache.org/r/11479/#comment46167>
Strip out tab characters
ui/scripts/configuration.js
<https://reviews.apache.org/r/11479/#comment46168>
Strip out tab characters
ui/scripts/configuration.js
<https://reviews.apache.org/r/11479/#comment46169>
Strip out tab characters
ui/scripts/configuration.js
<https://reviews.apache.org/r/11479/#comment46170>
Strip out tab characters
ui/scripts/storage.js
<https://reviews.apache.org/r/11479/#comment46171>
Strip out tab characters
ui/scripts/storage.js
<https://reviews.apache.org/r/11479/#comment46172>
Strip out tab characters
utils/src/com/cloud/utils/StringUtils.java
<https://reviews.apache.org/r/11479/#comment46173>
Strip out tab characters
vmware-base/src/com/cloud/hypervisor/vmware/mo/HostDatastoreSystemMO.java
<https://reviews.apache.org/r/11479/#comment46174>
Strip out tab characters
vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java
<https://reviews.apache.org/r/11479/#comment46175>
Strip out tab characters
vmware-base/src/com/cloud/hypervisor/vmware/mo/HostStorageSystemMO.java
<https://reviews.apache.org/r/11479/#comment46176>
Strip out tab characters
vmware-base/src/com/cloud/hypervisor/vmware/mo/HostStorageSystemMO.java
<https://reviews.apache.org/r/11479/#comment46177>
Throwing Exception is too broad. Can the client recover from the checked exception encountered by this method? If not, catch and re-throw as CloudRuntimeException. Otherwise, declare as throwing only the expected checked exceptions.
vmware-base/src/com/cloud/hypervisor/vmware/mo/HostStorageSystemMO.java
<https://reviews.apache.org/r/11479/#comment46178>
Throwing Exception is too broad. Can the client recover from the checked exception encountered by this method? If not, catch and re-throw as CloudRuntimeException. Otherwise, declare as throwing only the expected checked exceptions.
vmware-base/src/com/cloud/hypervisor/vmware/mo/HostStorageSystemMO.java
<https://reviews.apache.org/r/11479/#comment46179>
Throwing Exception is too broad. Can the client recover from the checked exception encountered by this method? If not, catch and re-throw as CloudRuntimeException. Otherwise, declare as throwing only the expected checked exceptions.
- John Burwell
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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review23472
-----------------------------------------------------------
Updated diff files related to Review Board comments can be found here (under the check-in message of "Changes related to Review Board comments"):
https://git-wip-us.apache.org/repos/asf?p=cloudstack.git&a=search&h=refs%2Fheads%2F4.2&st=author&s=Tutkowski
- Mike Tutkowski
On June 21, 2013, 10:35 a.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated June 21, 2013, 10:35 a.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
>
>
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
-----------------------------------------------------------
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.
Changes
-------
Made a small change and re-uploaded the entire diff of my changes for this feature
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 (updated)
-----
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
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/
-----------------------------------------------------------
(Updated June 20, 2013, 5:35 p.m.)
Review request for cloudstack, edison su and John Burwell.
Changes
-------
Updated changes that include mutual exclusion of the storage and hypervisor QoS features
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 (updated)
-----
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
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/
-----------------------------------------------------------
(Updated June 19, 2013, 12:06 a.m.)
Review request for cloudstack, edison su and John Burwell.
Changes
-------
I uploaded a new diff file.
The only remaining items are that we need a mechanism in the GUI and API for mutual exclusion of mine and Wei's features and that we haven't decided yet how to handle the hypervisor to use being required for a storage pool. For the time being at least, I have let my plug-in take on the hypervisor type of "Any" and I've placed some logic elsewhere to deal with this Any type in an allocator.
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 (updated)
-----
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/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
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > api/src/com/cloud/offering/DiskOffering.java, line 70
> > <https://reviews.apache.org/r/11479/diff/1/?file=297861#file297861line70>
> >
> > Is the notion of burst IOPS standard across QoS storage systems or specific to a product?
We decided to defer Burst IOPS to CS 4.3, where a more general-purpose approach can be implemented.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > api/src/com/cloud/storage/Storage.java, line 107
> > <https://reviews.apache.org/r/11479/diff/1/?file=297862#file297862line107>
> >
> > Coding standards are that enumeration values are all caps.
I have removed this field from the codebase in favor of another approach.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java, line 6182
> > <https://reviews.apache.org/r/11479/diff/1/?file=297882#file297882line6182>
> >
> > Why did this method change from taking a domain object to a taking discrete values? Is this information not available on the StorageFilerTO?
getIscsiSR was initially written with the assumption that it would create an SR based on information that is exclusively contained in a row in the storage_pool table. With the new storage plug-in framework and a 1:1 mapping between a SAN volume and an SR, information from the volumes table needed to be passed in when required.
I investigated what getIscsiSR was using from StorageFilerTO and passed those values in instead. This way I could re-use getIscsiSR with info from the volumes table when needed.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java, line 6486
> > <https://reviews.apache.org/r/11479/diff/1/?file=297882#file297882line6486>
> >
> > As discussed on the list, the intention is to remove the StoragePoolType.Dynamic in favor of a VolumeTO#isManaged check. When this method becomes available, extracting the allocation of the underlying storage to a private method would make things a good bit more readable.
No need for StoragePoolType.Dynamic anymore
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java, line 6500
> > <https://reviews.apache.org/r/11479/diff/1/?file=297882#file297882line6500>
> >
> > 1. Extract the 100000 value to a constant
> > 2. Should this slack value be a global configuration setting?
I have been able to determine approximately how much metadata per gigabyte Xen uses for an SR. I've removed this value and replaced it with a method that performs a more general calculation.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java, line 80
> > <https://reviews.apache.org/r/11479/diff/1/?file=297884#file297884line80>
> >
> > Why is the UUID being prepended with the provider name? Smells of magic value ...
Edison recommended I create any value for this UUID that - given the same inputs - would lead to the same value.
I just prepended it with "SolidFire_" to make it easier to locate such plug-ins in the storage_pool table.
After submitting this code, I did realize admins might want more than one SolidFire SAN in a CS zone, so I've changed this UUID to permit that.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java, line 300
> > <https://reviews.apache.org/r/11479/diff/1/?file=297884#file297884line300>
> >
> > What's the resolution of this TODO?
Removed the line
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java, line 307
> > <https://reviews.apache.org/r/11479/diff/1/?file=297884#file297884line307>
> >
> > What's the resolution of this TODO?
Removed the line
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java, line 314
> > <https://reviews.apache.org/r/11479/diff/1/?file=297884#file297884line314>
> >
> > What's the resolution of this TODO?
Removed the line
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java, line 72
> > <https://reviews.apache.org/r/11479/diff/1/?file=297885#file297885line72>
> >
> > Why is the UUID being prepended with the provider name? Smells of magic value.
Edison recommended I create any value for this UUID that - given the same inputs - would lead to the same value.
I just prepended it with "SolidFire_" to make it easier to locate such plug-ins in the storage_pool table.
After submitting this code, I did realize admins might want more than one SolidFire SAN in a CS zone, so I've changed this UUID to permit that.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 434
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line434>
> >
> > What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
It is used by Gson to parse JSON into a Java class that is set up to accept the appropriate format.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 444
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line444>
> >
> > What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
It is used by Gson to parse JSON into a Java class that is set up to accept the appropriate format.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 461
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line461>
> >
> > What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
It is used by Gson to parse JSON into a Java class that is set up to accept the appropriate format.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 471
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line471>
> >
> > What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
It is used by Gson to parse JSON into a Java class that is set up to accept the appropriate format.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 488
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line488>
> >
> > What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
It is used by Gson to parse JSON into a Java class that is set up to accept the appropriate format.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 498
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line498>
> >
> > What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
It is used by Gson to parse JSON into a Java class that is set up to accept the appropriate format.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 570
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line570>
> >
> > This method looks like it will also throw an exception. Is that correct?
That is correct. It has a "throws Exception" clause.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > server/src/com/cloud/configuration/ConfigurationManagerImpl.java, line 2216
> > <https://reviews.apache.org/r/11479/diff/1/?file=297893#file297893line2216>
> >
> > Is it permissable for minIOPS to be equal to maxIOPS?
Yes
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > server/src/com/cloud/storage/VolumeManagerImpl.java, line 897
> > <https://reviews.apache.org/r/11479/diff/1/?file=297896#file297896line897>
> >
> > Check >= 0 to be completely defensive.
I believe you meant minIops <= 0 to be completely defensive.
Either way, this logic had been changed since I submitted the code for review and this issue is no longer applicable.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > server/src/com/cloud/storage/VolumeManagerImpl.java, line 905
> > <https://reviews.apache.org/r/11479/diff/1/?file=297896#file297896line905>
> >
> > Check >= 0 to be completely defensive.
I believe you meant minIops <= 0 to be completely defensive.
Either way, this logic had been changed since I submitted the code for review and this issue is no longer applicable.
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review21347
-----------------------------------------------------------
On May 31, 2013, 9:18 p.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated May 31, 2013, 9:18 p.m.)
>
>
> Review request for cloudstack, edison su and John Burwell.
>
>
> 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 dd77c70
> api/src/com/cloud/storage/Storage.java c130fe2
> api/src/com/cloud/storage/Volume.java 4903594
> api/src/org/apache/cloudstack/api/ApiConstants.java 1e9435f
> api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java aa11599
> api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java 86a494b
> api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java 377e66e
> api/src/org/apache/cloudstack/api/response/VolumeResponse.java 21d7d1a
> client/WEB-INF/classes/resources/messages.properties 1638be1
> client/pom.xml 0c38ecb
> client/tomcatconf/applicationContext.xml.in edf83a9
> core/src/com/cloud/agent/api/AttachVolumeCommand.java 302b8f8
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java 251a6cb
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeCommandTest.java 1ec416a
> engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java b2b787c
> engine/schema/src/com/cloud/storage/DiskOfferingVO.java 909d7fe
> engine/schema/src/com/cloud/storage/VolumeVO.java 1699afd
> engine/storage/integration-test/test/org/apache/cloudstack/storage/allocator/StorageAllocatorTest.java 9444fa5
> engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java e976980
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 5f8daf4
> engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java ea31be3
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4680fde
> 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 283181f
> server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java e27e2d9
> server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 6d3cdcb
> server/src/com/cloud/api/query/vo/VolumeJoinVO.java 6ef8c91
> server/src/com/cloud/configuration/ConfigurationManager.java 8db037b
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 8baf1fb
> server/src/com/cloud/server/ConfigurationServerImpl.java bc52e9a
> server/src/com/cloud/storage/VolumeManager.java d198e5d
> server/src/com/cloud/storage/VolumeManagerImpl.java 4b654eb
> server/src/com/cloud/test/DatabaseConfig.java 70c8178
> server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 21b3590
> setup/db/db/schema-410to420.sql cf4e98d
> tools/marvin/marvin/cloudstackConnection.py b092ef0
> ui/dictionary.jsp ded9ea0
> ui/scripts/configuration.js 211d7b7
> ui/scripts/docs.js 7c1aaf8
> ui/scripts/storage.js e816334
> ui/scripts/system.js 8b9a81f
>
> 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
>
>
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java, line 6494
> > <https://reviews.apache.org/r/11479/diff/1/?file=297882#file297882line6494>
> >
> > To reduce the conditional nesting, check if (!created[0]), perform the associated work, and return -- then proceed with the true case.
I did some refactoring before the code review was available on Review Board. I think this issue has been addressed.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 232
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line232>
> >
> > Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
This class is used by the Gson library for JSON purposes.
Since Eclipse cannot see certain variables being used, it marks them as not in use. They actually are needed by Gson, so I put in the @SuppressWarnings.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 280
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line280>
> >
> > Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
This class is used by the Gson library for JSON purposes.
Since Eclipse cannot see certain variables being used, it marks them as not in use. They actually are needed by Gson, so I put in the @SuppressWarnings.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 325
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line325>
> >
> > Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
This class is used by the Gson library for JSON purposes.
Since Eclipse cannot see certain variables being used, it marks them as not in use. They actually are needed by Gson, so I put in the @SuppressWarnings.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 347
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line347>
> >
> > Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
This class is used by the Gson library for JSON purposes.
Since Eclipse cannot see certain variables being used, it marks them as not in use. They actually are needed by Gson, so I put in the @SuppressWarnings.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 369
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line369>
> >
> > Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
This class is used by the Gson library for JSON purposes.
Since Eclipse cannot see certain variables being used, it marks them as not in use. They actually are needed by Gson, so I put in the @SuppressWarnings.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 391
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line391>
> >
> > Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
This class is used by the Gson library for JSON purposes.
Since Eclipse cannot see certain variables being used, it marks them as not in use. They actually are needed by Gson, so I put in the @SuppressWarnings.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java, line 413
> > <https://reviews.apache.org/r/11479/diff/1/?file=297887#file297887line413>
> >
> > Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
This class is used by the Gson library for JSON purposes.
Since Eclipse cannot see certain variables being used, it marks them as not in use. They actually are needed by Gson, so I put in the @SuppressWarnings.
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review21347
-----------------------------------------------------------
On May 31, 2013, 9:18 p.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated May 31, 2013, 9:18 p.m.)
>
>
> Review request for cloudstack, edison su and John Burwell.
>
>
> 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 dd77c70
> api/src/com/cloud/storage/Storage.java c130fe2
> api/src/com/cloud/storage/Volume.java 4903594
> api/src/org/apache/cloudstack/api/ApiConstants.java 1e9435f
> api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java aa11599
> api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java 86a494b
> api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java 377e66e
> api/src/org/apache/cloudstack/api/response/VolumeResponse.java 21d7d1a
> client/WEB-INF/classes/resources/messages.properties 1638be1
> client/pom.xml 0c38ecb
> client/tomcatconf/applicationContext.xml.in edf83a9
> core/src/com/cloud/agent/api/AttachVolumeCommand.java 302b8f8
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java 251a6cb
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeCommandTest.java 1ec416a
> engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java b2b787c
> engine/schema/src/com/cloud/storage/DiskOfferingVO.java 909d7fe
> engine/schema/src/com/cloud/storage/VolumeVO.java 1699afd
> engine/storage/integration-test/test/org/apache/cloudstack/storage/allocator/StorageAllocatorTest.java 9444fa5
> engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java e976980
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 5f8daf4
> engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java ea31be3
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4680fde
> 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 283181f
> server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java e27e2d9
> server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 6d3cdcb
> server/src/com/cloud/api/query/vo/VolumeJoinVO.java 6ef8c91
> server/src/com/cloud/configuration/ConfigurationManager.java 8db037b
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 8baf1fb
> server/src/com/cloud/server/ConfigurationServerImpl.java bc52e9a
> server/src/com/cloud/storage/VolumeManager.java d198e5d
> server/src/com/cloud/storage/VolumeManagerImpl.java 4b654eb
> server/src/com/cloud/test/DatabaseConfig.java 70c8178
> server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 21b3590
> setup/db/db/schema-410to420.sql cf4e98d
> tools/marvin/marvin/cloudstackConnection.py b092ef0
> ui/dictionary.jsp ded9ea0
> ui/scripts/configuration.js 211d7b7
> ui/scripts/docs.js 7c1aaf8
> ui/scripts/storage.js e816334
> ui/scripts/system.js 8b9a81f
>
> 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
>
>
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java, line 131
> > <https://reviews.apache.org/r/11479/diff/1/?file=297885#file297885line131>
> >
> > Why not use the URI class to parse out the parameters? This code could be broken by a malformed URL/URI. I recommend parsing into a java.net.URI on line 60.
Yeah, it's called a URL, but it's really not a URL. The parameter where you pass in vendor-specific info to your plug-in from the API is called url and its contents are what I'm parsing here. Its contents are really just name/value pairs.
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java, line 184
> > <https://reviews.apache.org/r/11479/diff/1/?file=297885#file297885line184>
> >
> > Consider using Guava's Splitter. It will make this code much more concise ...
Noted to investigate
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java, line 196
> > <https://reviews.apache.org/r/11479/diff/1/?file=297885#file297885line196>
> >
> > Consider using Guava's Splitter to grab out the value.
Noted to investigate
> On June 12, 2013, 11:15 p.m., John Burwell wrote:
> > plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java, line 218
> > <https://reviews.apache.org/r/11479/diff/1/?file=297885#file297885line218>
> >
> > See previous comments about using java.net.URI.
Really just key/value pairs. The name url comes from the API where the plug-in-specific info that's passed to the driver is called this.
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review21347
-----------------------------------------------------------
On May 31, 2013, 9:18 p.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated May 31, 2013, 9:18 p.m.)
>
>
> Review request for cloudstack, edison su and John Burwell.
>
>
> 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 dd77c70
> api/src/com/cloud/storage/Storage.java c130fe2
> api/src/com/cloud/storage/Volume.java 4903594
> api/src/org/apache/cloudstack/api/ApiConstants.java 1e9435f
> api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java aa11599
> api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java 86a494b
> api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java 377e66e
> api/src/org/apache/cloudstack/api/response/VolumeResponse.java 21d7d1a
> client/WEB-INF/classes/resources/messages.properties 1638be1
> client/pom.xml 0c38ecb
> client/tomcatconf/applicationContext.xml.in edf83a9
> core/src/com/cloud/agent/api/AttachVolumeCommand.java 302b8f8
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java 251a6cb
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeCommandTest.java 1ec416a
> engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java b2b787c
> engine/schema/src/com/cloud/storage/DiskOfferingVO.java 909d7fe
> engine/schema/src/com/cloud/storage/VolumeVO.java 1699afd
> engine/storage/integration-test/test/org/apache/cloudstack/storage/allocator/StorageAllocatorTest.java 9444fa5
> engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java e976980
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 5f8daf4
> engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java ea31be3
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4680fde
> 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 283181f
> server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java e27e2d9
> server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 6d3cdcb
> server/src/com/cloud/api/query/vo/VolumeJoinVO.java 6ef8c91
> server/src/com/cloud/configuration/ConfigurationManager.java 8db037b
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 8baf1fb
> server/src/com/cloud/server/ConfigurationServerImpl.java bc52e9a
> server/src/com/cloud/storage/VolumeManager.java d198e5d
> server/src/com/cloud/storage/VolumeManagerImpl.java 4b654eb
> server/src/com/cloud/test/DatabaseConfig.java 70c8178
> server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 21b3590
> setup/db/db/schema-410to420.sql cf4e98d
> tools/marvin/marvin/cloudstackConnection.py b092ef0
> ui/dictionary.jsp ded9ea0
> ui/scripts/configuration.js 211d7b7
> ui/scripts/docs.js 7c1aaf8
> ui/scripts/storage.js e816334
> ui/scripts/system.js 8b9a81f
>
> 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
>
>
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by John Burwell <jb...@basho.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review21347
-----------------------------------------------------------
api/src/com/cloud/offering/DiskOffering.java
<https://reviews.apache.org/r/11479/#comment44377>
Is the notion of burst IOPS standard across QoS storage systems or specific to a product?
api/src/com/cloud/storage/Storage.java
<https://reviews.apache.org/r/11479/#comment44252>
Coding standards are that enumeration values are all caps.
core/src/com/cloud/agent/api/AttachVolumeCommand.java
<https://reviews.apache.org/r/11479/#comment44253>
Strip leading spaces on blank lines and remove tab characters.
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
<https://reviews.apache.org/r/11479/#comment45019>
As discussed on the mailing list, Burst IOPS are implementation specific. Defer implementation to a later release where we have a more robust mechanism for capturing extended data.
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment45020>
Why did this method change from taking a domain object to a taking discrete values? Is this information not available on the StorageFilerTO?
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment44260>
Is it valid to configure CHAP with just a username? While odd, just curious if there is a scenario where it is done. If so, the
Also, check that the strings are not blank which covers both null, empty strings, and strings containing only spaces. (StringUtils#isNotBlank).
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment44279>
Why not use a for-each loop to reduce/simplify the code?
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment44261>
Fix indentation in this block to match coding standards -- 4 spaces per level.
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment45022>
As discussed on the list, the intention is to remove the StoragePoolType.Dynamic in favor of a VolumeTO#isManaged check. When this method becomes available, extracting the allocation of the underlying storage to a private method would make things a good bit more readable.
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment44262>
To reduce the conditional nesting, check if (!created[0]), perform the associated work, and return -- then proceed with the true case.
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/11479/#comment45024>
1. Extract the 100000 value to a constant
2. Should this slack value be a global configuration setting?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment45025>
Why is the UUID being prepended with the provider name? Smells of magic value ...
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment45029>
1. I would implement two constructors -- a noarg and this one. The noarg calls this constructor with the default values where this constructor applies the values passed. This approach is more explicit, and more clearly conveys to the calls the expected behavior. Otherwise, to get default values, the caller has to know to call with magic 0 values.
2. Hard-coded like these seem a bit inflexible. Can they be made configurable as a global configuration option or the DataStore configuration?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment45031>
Why is the exception being logged to debug and not error?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment45032>
Why is this exception being logged to debug and not error?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment45030>
Readability: check this condition at the top of the method and bail out early to avoid large nested blocks.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment45033>
What's the resolution of this TODO?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment45034>
What's the resolution of this TODO?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
<https://reviews.apache.org/r/11479/#comment45035>
What's the resolution of this TODO?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment45037>
Why is the UUID being prepended with the provider name? Smells of magic value.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment45038>
Prefer IllegalStateException for this condition
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment45040>
Why not use the URI class to parse out the parameters? This code could be broken by a malformed URL/URI. I recommend parsing into a java.net.URI on line 60.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment45041>
Consider using Guava's Splitter. It will make this code much more concise ...
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment45042>
Consider using Guava's Splitter to grab out the value.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment45043>
Prefer IllegalStateException in this situation
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment45044>
See previous comments about using java.net.URI.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFirePrimaryDataStoreLifeCycle.java
<https://reviews.apache.org/r/11479/#comment45045>
Prefer an explicit throws list to be more declarative
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/provider/SolidfirePrimaryDataStoreProvider.java
<https://reviews.apache.org/r/11479/#comment45046>
Why aren't these values being injected via @Inject?
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45047>
Implement equals, hashCode, and toString
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45050>
Make these attributes as final since they can not be modified
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45048>
Implement equals, hashCode, and toString
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45049>
Mark attributes as final since they can not be modified
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45051>
Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45052>
Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45053>
Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45054>
Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45055>
Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45056>
Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45057>
Cruft Prevention: If this class is unused (infered from the @SupressWarnings annotation) why it is in the codebase? Delete it if we don't need it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45058>
What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45059>
What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45060>
What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45061>
What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45062>
What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45063>
What is the purpose of this class? All of the data is private and no methods are provided to access or modify it.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45066>
Explicitly list the exceptions thrown
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45064>
Extract 515-525 to a private factory method
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45065>
By spec, any 20x status code represents success. Why is success being constrainted to 200? By spec, we should check >= 200 && < 300. I recommend creating a general utility method, isSuccess, to implement this check.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45068>
1) This resource management block should be in a finally block to ensure execution. Also, we want to ignore any exceptions from it.
2) We should be setting up an HttpConnection pool through HttpConnectionManager. The shutdown method kills the pool. This class should be fitted to create an HttpConnectionManager at startup and simply release the HttpConnections back to pool.
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45069>
Prefer an IllegalStateException here
plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/util/SolidFireUtil.java
<https://reviews.apache.org/r/11479/#comment45071>
This method looks like it will also throw an exception. Is that correct?
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/11479/#comment45072>
Is it permissable for minIOPS to be equal to maxIOPS?
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment45074>
Don't use the Long constructor because it avoid the constant pool resulting in memory bloat and garbage collector stress. Use Long.valueOf or, even better, just use 0L and let the compiler autobox the value.
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment45075>
Check >= 0 to be completely defensive.
server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/11479/#comment45076>
Check >= 0 to be completely defensive.
- John Burwell
On May 31, 2013, 9:18 p.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated May 31, 2013, 9:18 p.m.)
>
>
> Review request for cloudstack, edison su and John Burwell.
>
>
> 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 dd77c70
> api/src/com/cloud/storage/Storage.java c130fe2
> api/src/com/cloud/storage/Volume.java 4903594
> api/src/org/apache/cloudstack/api/ApiConstants.java 1e9435f
> api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java aa11599
> api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java 86a494b
> api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java 377e66e
> api/src/org/apache/cloudstack/api/response/VolumeResponse.java 21d7d1a
> client/WEB-INF/classes/resources/messages.properties 1638be1
> client/pom.xml 0c38ecb
> client/tomcatconf/applicationContext.xml.in edf83a9
> core/src/com/cloud/agent/api/AttachVolumeCommand.java 302b8f8
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java 251a6cb
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeCommandTest.java 1ec416a
> engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java b2b787c
> engine/schema/src/com/cloud/storage/DiskOfferingVO.java 909d7fe
> engine/schema/src/com/cloud/storage/VolumeVO.java 1699afd
> engine/storage/integration-test/test/org/apache/cloudstack/storage/allocator/StorageAllocatorTest.java 9444fa5
> engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java e976980
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 5f8daf4
> engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java ea31be3
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4680fde
> 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 283181f
> server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java e27e2d9
> server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 6d3cdcb
> server/src/com/cloud/api/query/vo/VolumeJoinVO.java 6ef8c91
> server/src/com/cloud/configuration/ConfigurationManager.java 8db037b
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 8baf1fb
> server/src/com/cloud/server/ConfigurationServerImpl.java bc52e9a
> server/src/com/cloud/storage/VolumeManager.java d198e5d
> server/src/com/cloud/storage/VolumeManagerImpl.java 4b654eb
> server/src/com/cloud/test/DatabaseConfig.java 70c8178
> server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 21b3590
> setup/db/db/schema-410to420.sql cf4e98d
> tools/marvin/marvin/cloudstackConnection.py b092ef0
> ui/dictionary.jsp ded9ea0
> ui/scripts/configuration.js 211d7b7
> ui/scripts/docs.js 7c1aaf8
> ui/scripts/storage.js e816334
> ui/scripts/system.js 8b9a81f
>
> 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
>
>
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/
-----------------------------------------------------------
(Updated May 31, 2013, 9:18 p.m.)
Review request for cloudstack, edison su and John Burwell.
Changes
-------
Hi Wei,
I would tend to agree with you about vendor-specific changes to a table. However, the intent is that this is not technically vendor specific. There are other products on the market that utilize IOPS constraints. I believe at the moment rate limiting is the main alternative (which would utilize the Max field). It is possible other products could leverage the Min and Burst later, as well.
Back when I was brainstorming with the e-mail list, this was recommended as the approach I take (and that is why these fields are also visible for all to use on the Add Disk Offering and Add Volume dialogs).
What do you think?
Thanks
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 dd77c70
api/src/com/cloud/storage/Storage.java c130fe2
api/src/com/cloud/storage/Volume.java 4903594
api/src/org/apache/cloudstack/api/ApiConstants.java 1e9435f
api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java aa11599
api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java 86a494b
api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java 377e66e
api/src/org/apache/cloudstack/api/response/VolumeResponse.java 21d7d1a
client/WEB-INF/classes/resources/messages.properties 1638be1
client/pom.xml 0c38ecb
client/tomcatconf/applicationContext.xml.in edf83a9
core/src/com/cloud/agent/api/AttachVolumeCommand.java 302b8f8
core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java 251a6cb
core/test/org/apache/cloudstack/api/agent/test/AttachVolumeCommandTest.java 1ec416a
engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java b2b787c
engine/schema/src/com/cloud/storage/DiskOfferingVO.java 909d7fe
engine/schema/src/com/cloud/storage/VolumeVO.java 1699afd
engine/storage/integration-test/test/org/apache/cloudstack/storage/allocator/StorageAllocatorTest.java 9444fa5
engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java e976980
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 5f8daf4
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java ea31be3
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4680fde
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 283181f
server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java e27e2d9
server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 6d3cdcb
server/src/com/cloud/api/query/vo/VolumeJoinVO.java 6ef8c91
server/src/com/cloud/configuration/ConfigurationManager.java 8db037b
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 8baf1fb
server/src/com/cloud/server/ConfigurationServerImpl.java bc52e9a
server/src/com/cloud/storage/VolumeManager.java d198e5d
server/src/com/cloud/storage/VolumeManagerImpl.java 4b654eb
server/src/com/cloud/test/DatabaseConfig.java 70c8178
server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 21b3590
setup/db/db/schema-410to420.sql cf4e98d
tools/marvin/marvin/cloudstackConnection.py b092ef0
ui/dictionary.jsp ded9ea0
ui/scripts/configuration.js 211d7b7
ui/scripts/docs.js 7c1aaf8
ui/scripts/storage.js e816334
ui/scripts/system.js 8b9a81f
Diff: https://reviews.apache.org/r/11479/diff/
Testing (updated)
-------
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
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by John Burwell <jb...@basho.com>.
> On May 31, 2013, 8:32 p.m., Wei Zhou wrote:
> > Mike,
> >
> > I think it is better to create a table for SolidFire instead of changing disk_offering table.
> > It is not a good idea to change disk_offering for a specific vendor.
> > You can have a look at what nicira did in the past, maybe it helps you.
> >
> >
> > -Wei
+1 to this notion. Could we achieve the same goal by adding a column to the disk_offering table that contained a JSON string of extended, vendor specific attributes? On the Java interface side, the JSON document would represented as a Map<String, Object> from the appropriate interface with driver implementations responsible for validating their contents. This approach would reduce the situations where a driver would require schema modifications.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review21265
-----------------------------------------------------------
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
>
>
Re: Review Request 11479: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Mike Tutkowski <mi...@solidfire.com>.
> On May 31, 2013, 2:32 p.m., Wei Zhou wrote:
> > Mike,
> >
> > I think it is better to create a table for SolidFire instead of changing disk_offering table.
> > It is not a good idea to change disk_offering for a specific vendor.
> > You can have a look at what nicira did in the past, maybe it helps you.
> >
> >
> > -Wei
>
> John Burwell wrote:
> +1 to this notion. Could we achieve the same goal by adding a column to the disk_offering table that contained a JSON string of extended, vendor specific attributes? On the Java interface side, the JSON document would represented as a Map<String, Object> from the appropriate interface with driver implementations responsible for validating their contents. This approach would reduce the situations where a driver would require schema modifications.
I'm not sure how the Storage Quality of Service fields are any different (in principal) from the Hypervisor Quality of Service fields. The Hypervisor QoS fields are storaged in the disk_offering table right next to the Storage QoS ones. Not all hypervisors will support the Hypervisor QoS fields just like not all storage will support the storage QoS fields.
If we change the storage QoS fields to reside elsewhere, we should do the same for the hypervisor QoS fields.
We should make a decision on this, though, quickly because we are SUPER late in the game here and changes like this will invalidate all of the testing I've performed over the past couple weeks (as well as whatever testing Wei has performed).
Thanks!
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review21265
-----------------------------------------------------------
On June 21, 2013, 10:35 a.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated June 21, 2013, 10:35 a.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
>
>
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Wei Zhou <w....@leaseweb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/#review21265
-----------------------------------------------------------
Mike,
I think it is better to create a table for SolidFire instead of changing disk_offering table.
It is not a good idea to change disk_offering for a specific vendor.
You can have a look at what nicira did in the past, maybe it helps you.
-Wei
- Wei Zhou
On May 31, 2013, 12:08 a.m., Mike Tutkowski wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11479/
> -----------------------------------------------------------
>
> (Updated May 31, 2013, 12:08 a.m.)
>
>
> Review request for cloudstack, edison su and John Burwell.
>
>
> 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 dd77c70
> api/src/com/cloud/storage/Storage.java c130fe2
> api/src/com/cloud/storage/Volume.java 4903594
> api/src/org/apache/cloudstack/api/ApiConstants.java 1e9435f
> api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java aa11599
> api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java 86a494b
> api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java 377e66e
> api/src/org/apache/cloudstack/api/response/VolumeResponse.java 21d7d1a
> client/WEB-INF/classes/resources/messages.properties 1638be1
> client/pom.xml 0c38ecb
> client/tomcatconf/applicationContext.xml.in edf83a9
> core/src/com/cloud/agent/api/AttachVolumeCommand.java 302b8f8
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java 251a6cb
> core/test/org/apache/cloudstack/api/agent/test/AttachVolumeCommandTest.java 1ec416a
> engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java b2b787c
> engine/schema/src/com/cloud/storage/DiskOfferingVO.java 909d7fe
> engine/schema/src/com/cloud/storage/VolumeVO.java 1699afd
> engine/storage/integration-test/test/org/apache/cloudstack/storage/allocator/StorageAllocatorTest.java 9444fa5
> engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java e976980
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 5f8daf4
> engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java ea31be3
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4680fde
> 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 283181f
> server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java e27e2d9
> server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 6d3cdcb
> server/src/com/cloud/api/query/vo/VolumeJoinVO.java 6ef8c91
> server/src/com/cloud/configuration/ConfigurationManager.java 8db037b
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 8baf1fb
> server/src/com/cloud/server/ConfigurationServerImpl.java bc52e9a
> server/src/com/cloud/storage/VolumeManager.java d198e5d
> server/src/com/cloud/storage/VolumeManagerImpl.java 4b654eb
> server/src/com/cloud/test/DatabaseConfig.java 70c8178
> server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 21b3590
> setup/db/db/schema-410to420.sql cf4e98d
> tools/marvin/marvin/cloudstackConnection.py b092ef0
> ui/dictionary.jsp ded9ea0
> ui/scripts/configuration.js 211d7b7
> ui/scripts/docs.js 7c1aaf8
> ui/scripts/storage.js e816334
> ui/scripts/system.js 8b9a81f
>
> 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 reattach 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
>
>
Re: Review Request: SolidFire storage plug-in and enhancements to the
storage framework and GUI
Posted by Animesh Chaturvedi <An...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11479/
-----------------------------------------------------------
(Updated May 31, 2013, 12:08 a.m.)
Review request for cloudstack, edison su and John Burwell.
Changes
-------
Adding John as reviewer
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 dd77c70
api/src/com/cloud/storage/Storage.java c130fe2
api/src/com/cloud/storage/Volume.java 4903594
api/src/org/apache/cloudstack/api/ApiConstants.java 1e9435f
api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java aa11599
api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java 86a494b
api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java 377e66e
api/src/org/apache/cloudstack/api/response/VolumeResponse.java 21d7d1a
client/WEB-INF/classes/resources/messages.properties 1638be1
client/pom.xml 0c38ecb
client/tomcatconf/applicationContext.xml.in edf83a9
core/src/com/cloud/agent/api/AttachVolumeCommand.java 302b8f8
core/test/org/apache/cloudstack/api/agent/test/AttachVolumeAnswerTest.java 251a6cb
core/test/org/apache/cloudstack/api/agent/test/AttachVolumeCommandTest.java 1ec416a
engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreParameters.java b2b787c
engine/schema/src/com/cloud/storage/DiskOfferingVO.java 909d7fe
engine/schema/src/com/cloud/storage/VolumeVO.java 1699afd
engine/storage/integration-test/test/org/apache/cloudstack/storage/allocator/StorageAllocatorTest.java 9444fa5
engine/storage/src/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java e976980
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 5f8daf4
engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java ea31be3
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java 4680fde
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 283181f
server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java e27e2d9
server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 6d3cdcb
server/src/com/cloud/api/query/vo/VolumeJoinVO.java 6ef8c91
server/src/com/cloud/configuration/ConfigurationManager.java 8db037b
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 8baf1fb
server/src/com/cloud/server/ConfigurationServerImpl.java bc52e9a
server/src/com/cloud/storage/VolumeManager.java d198e5d
server/src/com/cloud/storage/VolumeManagerImpl.java 4b654eb
server/src/com/cloud/test/DatabaseConfig.java 70c8178
server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 21b3590
setup/db/db/schema-410to420.sql cf4e98d
tools/marvin/marvin/cloudstackConnection.py b092ef0
ui/dictionary.jsp ded9ea0
ui/scripts/configuration.js 211d7b7
ui/scripts/docs.js 7c1aaf8
ui/scripts/storage.js e816334
ui/scripts/system.js 8b9a81f
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 reattach 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