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