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