OK, if that's all we're checking, then I'm good with checking it in.

Punith - Can you just close out those last issues in the review?

Thanks!


On Thu, Mar 13, 2014 at 5:02 PM, Edison Su <edison...@citrix.com> wrote:

>  Hi Mike,
>
>    Seems some of your comments are related to the scope of their
> implementation, such as do you support this, do you support that, can you
> do this, can you do that etc. From my point of view, it's not important, as
> the capability of driver is totally controlled by the vendors, vendors can
> implement whatever functionalities they want. My criteria to check in the
> code is very simple: the plugin should not modify common code.
>
>
>
> *From:* Mike Tutkowski [mailto:mike.tutkow...@solidfire.com]
> *Sent:* Thursday, March 13, 2014 3:25 PM
> *To:* Edison Su; Mike Tutkowski
> *Cc:* punith s; cloudstack
> *Subject:* Re: Review Request 19021: Cloudbyte Elastistor storage plug-in
>
>
>
> Hi Punith,
>
>
>
> Once you close out the open issues, we can work to get your code in.
>
>
>
> Feature freeze is Friday, March 14th, so we should do it sometime tomorrow.
>
>
>
> Thanks!
>
>
>
> On Thu, Mar 13, 2014 at 4:24 PM, Mike Tutkowski <
> mike.tutkow...@solidfire.com> wrote:
>
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19021/
>
>
>
> *plugins/storage/volume/cloudbyte/src/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java
> <https://reviews.apache.org/r/19021/diff/5/?file=518383#file518383line99> 
> *(Diff
> revision 5)
>
> *99*
>
>         if(details.get("esaccountid") != null)
>
>   Does this plug-in support multiple CloudByte SANs at the same time?
>
>
>
> It looks like whatever values you set most recently for ElastistorUtil will 
> be utilized if you don't provide them in the details. Is this OK if you don't 
> provide all of the details for your second or more SAN?
>
>
>
> - Mike Tutkowski
>
>
>
> On March 13th, 2014, 9:16 a.m. UTC, punith s wrote:
>
> Review request for cloudstack, edison su and Mike Tutkowski.
>
> By punith s.
>
> *Updated March 13, 2014, 9:16 a.m.*
>
> *Repository: *cloudstack-git
>  Description
>
> This patch implements a basic storage plug-in for cloudbyte elastistor 
> v1.3.0, The plug-in is a new feature for cloudstack 4.4 and above.
>
>
>
> this does not implement managed storage yet, it is been integrated only with 
> CreateStoragePool and DeleteStoragePool api's.
>
>
>
> the desired behavior of the plugin are:
>
>
>
> * Allow an Admin to create a primary storage at cluster level, hence creates 
> a volume in elastistor and gets attached to the host with the given 
> capacityiops and capacitybytes through CreateStoragePool api with provider 
> being elastistor.
>
>
>
> *Allow an admin to delete a primary storage at cluster level, hence it 
> deletes the volume from host in cloudstack and deletes the respective volume 
> in elastistor.
>
>
>
> * volume and datadisks fuctions performs the default storage fuctions, ie. 
> the driver extends the CloudStackPrimaryDataStoreDriverImpl.
>
>
>
> * support for both nfs and icsci primary storage.
>
>   Testing
>
> Build test using,
>
>
>
> mvn -P developer,systemvm clean install, which is successful.
>
>
>
> Manual testing has been performed using cloudmonkey.
>
>
>
> * Creating a primary storage based on cloudbyte storage plugin.
>
>
>
> cloudmonkey# create storagepool scope=cluster 
> zoneid=dac7223c-6d09-4dcb-82fb-bdecf7c657f5 
> podid=20a613c4-eccf-4fdc-b8ca-c51df483326f 
> clusterid=9a89bc12-bf00-496b-b1d8-8e92cdf1795f name=cloudbytevolume 
> provider=elastistor url=nfs://10.10.171.137/cloudbytetest capacityiops=500 
> capacitybytes=214748364800 tags=cloudbytetest
>
> storagepool:
>
> name = cloudbytevolume
>
> id = 57f70aa4-659b-3b53-b8ab-2f712474f107
>
> capacityiops = 500
>
> clusterid = 9a89bc12-bf00-496b-b1d8-8e92cdf1795f
>
> clustername = test000
>
> created = 2014-03-11T12:42:38+0530
>
> disksizeallocated = 0
>
> disksizetotal = 214748364800
>
> hypervisor = Any
>
> ipaddress = 10.10.171.137
>
> path = /cloudbytetest
>
> podid = 20a613c4-eccf-4fdc-b8ca-c51df483326f
>
> podname = test00
>
> scope = CLUSTER
>
> state = Up
>
> tags = cloudbytetest
>
> type = NetworkFilesystem
>
> zoneid = dac7223c-6d09-4dcb-82fb-bdecf7c657f5
>
> zonename = DevCloud0
>
>
>
> * Deleting the primary storage based on cloudbyte storage plugin.
>
>
>
> cloudmonkey# delete storagepool id=57f70aa4-659b-3b53-b8ab-2f712474f107
>
> success = true
>
>
>
> * creation of primary storage with negative capacityiops throws an exception.
>
>
>
> * creation of primary storage with already available name and ip throws an 
> exception.
>
>
>
> * if the elastistor params which are required for plugin configuration are 
> not injected through spring-storage-volume-cloudbyte-context.xml, it can be 
> set from details map.
>
>
>
> cloudmonkey# create storagepool scope=cluster 
> zoneid=afacc706-3f4d-4f50-82e6-bf0f82959ba8 
> podid=821ad540-6c98-43f3-935d-72a47a319b20 
> clusterid=e0ced156-532e-4941-99c0-f34ff1727544 name=nfsvol 
> provider=elastistor url=nfs://10.10.171.143/volnfs 
> details[0].esaccountid=9e9f67d5-e06f-4d63-a0b8-e7255cba84b8 
> details[1].espoolid=d2d15d11-0f06-3426-a097-3e6e8b36f85c 
> details[2].esdefaultgateway=10.10.1.1 details[3].essubnet=8  
> details[4].estntinterface=em0 details[5].esmanagementip=10.10.171.180
>
> details[6].esapikey=PubSInZaCji8hrRfOsCxgbug2I2k_sRJ0i2a9qmAzZIiCTcFPmZelzx6uNK9TYgqkdohCmq1L2J9eYmUe9YO6A
>  capacityiops=100 capacitybytes=214748364800
>
>
>
> storagepool:
>
> name = nfsvol
>
> id = 7ea08bf6-777a-3553-8f1e-c3a9f9b626cb
>
> capacityiops = 100
>
> clusterid = e0ced156-532e-4941-99c0-f34ff1727544
>
> clustername = test000
>
> created = 2014-03-12T17:45:10+0530
>
> disksizeallocated = 0
>
> disksizetotal = 214748364800
>
> hypervisor = Any
>
> ipaddress = 10.10.171.143
>
> path = /volnfs
>
> podid = 821ad540-6c98-43f3-935d-72a47a319b20
>
> podname = test00
>
> scope = CLUSTER
>
> state = Up
>
> type = NetworkFilesystem
>
> zoneid = afacc706-3f4d-4f50-82e6-bf0f82959ba8
>
> zonename = DevCloud0
>
>
>
> * creation of volume on created storage pool.
>
> cloudmonkey# create volume zoneid=afacc706-3f4d-4f50-82e6-bf0f82959ba8 
> diskofferingid=f20e3b76-82e3-43d1-91e3-3ff337d7181d name=testvolume
>
>
>
> accountid = 819b1dfe-a9d3-11e3-a500-f46d04ee0527
>
> cmd = org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd
>
> created = 2014-03-13T11:51:27+0530
>
> jobid = 596294b8-2b49-4b49-8526-9117cf56c95d
>
> jobprocstatus = 0
>
> jobresult:
>
> volume:
>
> name = testvolume
>
> id = e079281e-d822-4b90-9e85-9648350a0369
>
> account = admin
>
> created = 2014-03-13T11:51:27+0530
>
> destroyed = False
>
> diskofferingdisplaytext = Small Disk, 5 GB
>
> diskofferingid = f20e3b76-82e3-43d1-91e3-3ff337d7181d
>
> diskofferingname = Small
>
> displayvolume = True
>
> domain = ROOT
>
> domainid = 819ae3b6-a9d3-11e3-a500-f46d04ee0527
>
> isextractable = True
>
> jobid = 596294b8-2b49-4b49-8526-9117cf56c95d
>
> jobstatus = 0
>
> size = 5368709120
>
> state = Allocated
>
> storagetype = shared
>
> tags:
>
> type = DATADISK
>
> zoneid = afacc706-3f4d-4f50-82e6-bf0f82959ba8
>
> zonename = DevCloud0
>
> jobresultcode = 0
>
> jobresulttype = object
>
> jobstatus = 1
>
> userid = 819b856e-a9d3-11e3-a500-f46d04ee0527
>
>
>
> * attaching a specific volume.
>
> cloudmonkey# attach volume id=e079281e-d822-4b90-9e85-9648350a0369 
> virtualmachineid=3e6eeab1-e624-461e-94e0-230215a8dbc3
>
>
>
> accountid = 819b1dfe-a9d3-11e3-a500-f46d04ee0527
>
> cmd = org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd
>
> created = 2014-03-13T11:55:09+0530
>
> jobid = 364d7636-72ad-410c-891c-c38ac34b6d42
>
> jobprocstatus = 0
>
> jobresult:
>
> volume:
>
> id = e079281e-d822-4b90-9e85-9648350a0369
>
> name = testvolume
>
> account = admin
>
> attached = 2014-03-13T11:55:11+0530
>
> created = 2014-03-13T11:51:27+0530
>
> destroyed = False
>
> deviceid = 2
>
> diskofferingdisplaytext = Small Disk, 5 GB
>
> diskofferingid = f20e3b76-82e3-43d1-91e3-3ff337d7181d
>
> diskofferingname = Small
>
> displayvolume = True
>
> domain = ROOT
>
> domainid = 819ae3b6-a9d3-11e3-a500-f46d04ee0527
>
> hypervisor = XenServer
>
> isextractable = True
>
> jobid = 364d7636-72ad-410c-891c-c38ac34b6d42
>
> jobstatus = 0
>
> size = 5368709120
>
> state = Ready
>
> storage = okay
>
> storagetype = shared
>
> tags:
>
> type = DATADISK
>
> virtualmachineid = 3e6eeab1-e624-461e-94e0-230215a8dbc3
>
> vmdisplayname = tiny
>
> vmname = tiny
>
> vmstate = Running
>
> zoneid = afacc706-3f4d-4f50-82e6-bf0f82959ba8
>
> zonename = DevCloud0
>
> jobresultcode = 0
>
> jobresulttype = object
>
> jobstatus = 1
>
> userid = 819b856e-a9d3-11e3-a500-f46d04ee0527
>
>
>
> *detaching a specific volume.
>
> cloudmonkey# detach volume id=e079281e-d822-4b90-9e85-9648350a0369
>
>
>
> accountid = 819b1dfe-a9d3-11e3-a500-f46d04ee0527
>
> cmd = org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd
>
> created = 2014-03-13T11:57:40+0530
>
> jobid = 38fb6463-7d95-4d4c-8a50-1ec8a1c9ba98
>
> jobprocstatus = 0
>
> jobresult:
>
> volume:
>
> id = e079281e-d822-4b90-9e85-9648350a0369
>
> name = testvolume
>
> account = admin
>
> created = 2014-03-13T11:51:27+0530
>
> destroyed = False
>
> diskofferingdisplaytext = Small Disk, 5 GB
>
> diskofferingid = f20e3b76-82e3-43d1-91e3-3ff337d7181d
>
> diskofferingname = Small
>
> displayvolume = True
>
> domain = ROOT
>
> domainid = 819ae3b6-a9d3-11e3-a500-f46d04ee0527
>
> hypervisor = XenServer
>
> isextractable = True
>
> jobid = 38fb6463-7d95-4d4c-8a50-1ec8a1c9ba98
>
> jobstatus = 0
>
> size = 5368709120
>
> state = Ready
>
> storage = okay
>
> storagetype = shared
>
> tags:
>
> type = DATADISK
>
> zoneid = afacc706-3f4d-4f50-82e6-bf0f82959ba8
>
> zonename = DevCloud0
>
> jobresultcode = 0
>
> jobresulttype = object
>
> jobstatus = 1
>
> userid = 819b856e-a9d3-11e3-a500-f46d04ee0527
>
>
>
> *deleting a volume
>
> cloudmonkey# delete volume id=e079281e-d822-4b90-9e85-9648350a0369
>
> success = true
>
>   Diffs
>
>    - client/pom.xml (af724b1)
>    - plugins/pom.xml (097f224)
>    - plugins/storage/volume/cloudbyte/pom.xml (PRE-CREATION)
>    - 
> plugins/storage/volume/cloudbyte/resources/META-INF/cloudstack/storage-volume-cloudbyte/module.properties
>    (PRE-CREATION)
>    - 
> plugins/storage/volume/cloudbyte/resources/META-INF/cloudstack/storage-volume-cloudbyte/spring-storage-volume-cloudbyte-context.xml
>    (PRE-CREATION)
>    - 
> plugins/storage/volume/cloudbyte/src/org/apache/cloudstack/storage/datastore/driver/ElastistorPrimaryDataStoreDriver.java
>    (PRE-CREATION)
>    - 
> plugins/storage/volume/cloudbyte/src/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java
>    (PRE-CREATION)
>    - 
> plugins/storage/volume/cloudbyte/src/org/apache/cloudstack/storage/datastore/provider/ElastistorHostListener.java
>    (PRE-CREATION)
>    - 
> plugins/storage/volume/cloudbyte/src/org/apache/cloudstack/storage/datastore/provider/ElastistorPrimaryDataStoreProvider.java
>    (PRE-CREATION)
>    - 
> plugins/storage/volume/cloudbyte/src/org/apache/cloudstack/storage/datastore/util/ElastistorUtil.java
>    (PRE-CREATION)
>
> View Diff <https://reviews.apache.org/r/19021/diff/>
>
>
>
>
>
> --
> *Mike Tutkowski*
>
> *Senior CloudStack Developer, SolidFire Inc.*
>
> e: mike.tutkow...@solidfire.com
>
> o: 303.746.7302
>
> Advancing the way the world uses the 
> cloud<http://solidfire.com/solution/overview/?video=play>
> *(tm)*
>



-- 
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkow...@solidfire.com
o: 303.746.7302
Advancing the way the world uses the
cloud<http://solidfire.com/solution/overview/?video=play>
*(tm)*

Reply via email to