This patch has introduced the NullPointerException in code. Since ZoneWideStoragePoolAllocator's filter method has been removed it is using AbstractStoragePoolAllocator's filter method. In the filter method there is check for cluster's hypervisor type and and disk profile hypervisor type. But cluster is null for ZoneWideStoragePool and Hence the exception.
Created the issue for same https://issues.apache.org/jira/browse/CLOUDSTACK-6965. Regards, Anshul -----Original Message----- From: Mike Tutkowski [mailto:mike.tutkow...@solidfire.com] Sent: Wednesday, June 18, 2014 10:19 PM To: dev@cloudstack.apache.org Subject: Re: ZoneWideStoragePoolAllocator#filter seems weird for me 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>*™*
