Mike, I just realized that I forgot to publish my review. I am offline ATM, but I will publish it in the next couple of hours.
Do you plan to update your the patch in Review Board? Sorry for the oversight, -John On Jun 12, 2013, at 2:26 AM, Mike Tutkowski <mike.tutkow...@solidfire.com> wrote: > Hi Edison, John, and Wei (and whoever else is reading this :) ), > > Just an FYI that I believe I have implemented all the areas we wanted > addressed. > > I plan to review the code again tomorrow morning or afternoon, then send in > another patch. > > Thanks for all the work on this everyone! > > > On Tue, Jun 11, 2013 at 12:29 PM, Mike Tutkowski < > mike.tutkow...@solidfire.com> wrote: > >> Sure, that sounds good. >> >> >> On Tue, Jun 11, 2013 at 12:11 PM, Wei ZHOU <ustcweiz...@gmail.com> wrote: >> >>> Hi Mike, >>> >>> It looks the two feature do not have many conflicts in Java code, except >>> the cloudstack UI. >>> If you do not mind, I will merge disk_io_throttling branch into master >>> this >>> week, so that you can develop based on it. >>> >>> -Wei >>> >>> >>> 2013/6/11 Mike Tutkowski <mike.tutkow...@solidfire.com> >>> >>>> Hey John, >>>> >>>> The SolidFire patch does not depend on the object_store branch, but - as >>>> Edison mentioned - it might be easier if we merge the SolidFire branch >>> into >>>> the object_store branch before object_store goes into master. >>>> >>>> I'm not sure how the disk_io_throttling fits into this merge strategy. >>>> Perhaps Wei can chime in on that. >>>> >>>> >>>> On Tue, Jun 11, 2013 at 11:07 AM, John Burwell <jburw...@basho.com> >>> wrote: >>>> >>>>> Mike, >>>>> >>>>> We have a delicate merge dance to perform. The disk_io_throttling, >>>>> solidfire, and object_store appear to have a number of overlapping >>>>> elements. I understand the dependencies between the patches to be as >>>>> follows: >>>>> >>>>> object_store <- solidfire -> disk_io_throttling >>>>> >>>>> Am I correct that the device management aspects of SolidFire are >>> additive >>>>> to the object_store branch or there are circular dependency between >>> the >>>>> branches? Once we understand the dependency graph, we can determine >>> the >>>>> best approach to land the changes in master. >>>>> >>>>> Thanks, >>>>> -John >>>>> >>>>> >>>>> On Jun 10, 2013, at 11:10 PM, Mike Tutkowski < >>>> mike.tutkow...@solidfire.com> >>>>> wrote: >>>>> >>>>>> Also, if we are good with Edison merging my code into his branch >>> before >>>>>> going into master, I am good with that. >>>>>> >>>>>> We can remove the StoragePoolType.Dynamic code after his merge and >>> we >>>> can >>>>>> deal with Burst IOPS then, as well. >>>>>> >>>>>> >>>>>> On Mon, Jun 10, 2013 at 9:08 PM, Mike Tutkowski < >>>>>> mike.tutkow...@solidfire.com> wrote: >>>>>> >>>>>>> Let me make sure I follow where we're going here: >>>>>>> >>>>>>> 1) There should be NO references to hypervisor code in the storage >>>>>>> plug-ins code (this includes the default storage plug-in, which >>>>> currently >>>>>>> sends several commands to the hypervisor in use (although it does >>> not >>>>> know >>>>>>> which hypervisor (XenServer, ESX(i), etc.) is actually in use)) >>>>>>> >>>>>>> 2) managed=true or managed=false can be placed in the url field (if >>>> not >>>>>>> present, we default to false). This info is stored in the >>>>>>> storage_pool_details table. >>>>>>> >>>>>>> 3) When the "attach" command is sent to the hypervisor in >>> question, we >>>>>>> pass the managed property along (this takes the place of the >>>>>>> StoragePoolType.Dynamic check). >>>>>>> >>>>>>> 4) execute(AttachVolumeCommand) in the hypervisor checks for the >>>> managed >>>>>>> property. If true for an attach, the necessary hypervisor data >>>>> structure is >>>>>>> created and the rest of the attach command executes to attach the >>>>> volume. >>>>>>> >>>>>>> 5) When execute(AttachVolumeCommand) is invoked to detach a volume, >>>> the >>>>>>> same check is made. If managed, the hypervisor data structure is >>>>> removed. >>>>>>> >>>>>>> 6) I do not see an clear way to support Burst IOPS in 4.2 unless >>> it is >>>>>>> stored in the volumes and disk_offerings table. If we have some >>> idea, >>>>>>> that'd be cool. >>>>>>> >>>>>>> Thanks! >>>>>>> >>>>>>> >>>>>>> On Mon, Jun 10, 2013 at 8:58 PM, Mike Tutkowski < >>>>>>> mike.tutkow...@solidfire.com> wrote: >>>>>>> >>>>>>>> "+1 -- Burst IOPS can be implemented while avoiding implementation >>>>>>>> attributes. I always wondered about the details field. I think >>> we >>>>> should >>>>>>>> beef up the description in the documentation regarding the >>> expected >>>>> format >>>>>>>> of the field. In 4.1, I noticed that the details are not >>> returned on >>>>> the >>>>>>>> createStoratePool updateStoragePool, or listStoragePool response. >>>> Why >>>>>>>> don't we return it? It seems like it would be useful for clients >>> to >>>> be >>>>>>>> able to inspect the contents of the details field." >>>>>>>> >>>>>>>> Not sure how this would work storing Burst IOPS here. >>>>>>>> >>>>>>>> Burst IOPS need to be variable on a Disk Offering-by-Disk Offering >>>>>>>> basis. For each Disk Offering created, you have to be able to >>>> associate >>>>>>>> unique Burst IOPS. There is a disk_offering_details table. Maybe >>> it >>>>> could >>>>>>>> go there? >>>>>>>> >>>>>>>> I'm also not sure how you would accept the Burst IOPS in the GUI >>> if >>>>> it's >>>>>>>> not stored like the Min and Max fields are in the DB. >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> *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> >>>>>>> *™* >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> *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> >>>>>> *™* >>>> >>>> >>>> -- >>>> *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> >>>> *™* >> >> >> >> -- >> *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> >> *™* > > > > -- > *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> > *™*
