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