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