Sounds good - thanks!
On Wed, Jun 18, 2014 at 10:46 AM, Yoshikazu Nojima <m...@ynojima.net> wrote: > Mike, > > Thank you for your review. > This is a patch I would like to push. > I don't have anything to add regarding this issue. > I will push this patch in the next few hours. > > Regards, > Noji > > > > > 2014-06-18 9:15 GMT-06:00 Mike Tutkowski <mike.tutkow...@solidfire.com>: > > Sorry...I didn't see until later in my e-mail that you had submitted > code. > > > > It looks good to me. > > > > Is this something you'd like checked in right away or are you planning to > > add to this patch? > > > > Thanks! > > > > > > On Wed, Jun 18, 2014 at 9:06 AM, Mike Tutkowski < > > mike.tutkow...@solidfire.com> wrote: > > > >> Yeah, storagePoolHasEnoughIops returns true in the traditional case > (i.e. > >> non-managed storage) so we won't skip a storage pool when IOPS are not > >> really being taken into consideration. > >> > >> Feel free to make enhancements and I can review your patch. > >> > >> Thanks! > >> > >> > >> On Wed, Jun 18, 2014 at 12:14 AM, Yoshikazu Nojima <m...@ynojima.net> > >> wrote: > >> > >>> Prachi, Mike, > >>> I appreciate your comments. > >>> > >>> I think it is a bug that there is a difference between filter logic of > >>> ClusterScopeStoragePoolAllocator and that of > >>> ZoneWideStoragePoolAllocator > >>> (ex. allocator skip iSCSI type storage pool or not when a volume is a > >>> RootDisk), > >>> I will unify by implementing it in AbstractStoragePoolAllocator#filter > >>> method. > >>> > >>> It is new information for me that managed storages only support zone > >>> level. Thank you for let me know. > >>> But when iopscapacity parameter is not set to a primary storage (i.e. > >>> unmanaged storage), storageMgr.storagePoolHasEnoughIops always returns > >>> true. > >>> I suppose calling storageMgr.storagePoolHasEnoughIops from > >>> AbstractStoragePoolAllocator will not cause a problem, and it will > >>> simplify the code. > >>> > >>> I created a patch to explain my thought. Could you review it? > >>> https://reviews.apache.org/r/22717/ > >>> > >>> Regards, > >>> Noji > >>> > >>> > >>> 2014-06-17 19:19 GMT-06:00 Mike Tutkowski < > mike.tutkow...@solidfire.com>: > >>> > I can comment on the IOPS part. > >>> > > >>> > The IOPS are currently only relevant when using managed storage > (which > >>> is > >>> > currently only applicable at the zone level). That being the case, no > >>> > cluster (or local) storage takes advantage of counting IOPS that have > >>> been > >>> > handed out. > >>> > > >>> > Of course this logic might be expanded in the future, but that's how > it > >>> > works today. > >>> > > >>> > > >>> > On Tue, Jun 17, 2014 at 2:47 PM, Prachi Damle < > prachi.da...@citrix.com> > >>> > wrote: > >>> > > >>> >> >Why ZoneWideStoragePoolAllocator implements "filter" method (and > >>> doesn't > >>> >> call that of base class) rather than just using that of base class? > >>> >> >ZoneWideStoragePoolAllocator#filter method seems doesn't care > "avoid" > >>> >> parameter, doesn't skip iSCSI type storage pool even if a volume is > a > >>> >> RootDisk. > >>> >> > >>> >> The first question seems like a bug to me. > >>> >> > >>> >> I am not sure about the IOPS > >>> >> > >>> >> Prachi > >>> >> > >>> >> -----Original Message----- > >>> >> From: ynoj...@ynojima.net [mailto:ynoj...@ynojima.net] On Behalf Of > >>> >> Yoshikazu Nojima > >>> >> Sent: Tuesday, June 17, 2014 1:07 PM > >>> >> To: dev@cloudstack.apache.org > >>> >> Subject: ZoneWideStoragePoolAllocator#filter seems weird for me > >>> >> > >>> >> Hi, > >>> >> > >>> >> ZoneWideStoragePoolAllocator#filter seems weird for me. > >>> >> > >>> >> ZoneWideStoragePoolAllocator and ClusterScopeStoragePoolAllocator > are > >>> >> allocator classes to select storage pool. > >>> >> They extends AbstractStoragePoolAllocator class, which provides > >>> "filter" > >>> >> method to exclude unavailable storage pools. > >>> >> > >>> >> Why ZoneWideStoragePoolAllocator implements "filter" method (and > >>> doesn't > >>> >> call that of base class) rather than just using that of base class? > >>> >> > >>> >> ZoneWideStoragePoolAllocator#filter method seems doesn't care > "avoid" > >>> >> parameter, doesn't skip iSCSI type storage pool even if a volume is > a > >>> >> RootDisk. > >>> >> (These functions are implemented in > >>> >> AbstractStoragePoolAllocator#filter method, which used by > >>> >> ClusterScopeStoragePoolAllocator.) > >>> >> On the other hand, AbstractStoragePoolAllocator#filter doesn't call > >>> >> storageMgr.storagePoolHasEnoughIops, so a cluster wide primary > storage > >>> >> would be allocated more volumes than its designated IOPS capacity. > >>> >> > >>> >> Is there any difference between a zone wide primary storage and a > >>> cluster > >>> >> wide primary storage except its scope? > >>> >> If it is a bug, I'll fix it. > >>> >> > >>> >> Regards, > >>> >> Noji > >>> >> > >>> > > >>> > > >>> > > >>> > -- > >>> > *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>*™*
