Not checked in yet, but here's the way that interface looks in my sandbox
now:
public interface PrimaryDataStoreLifeCycle extends DataStoreLifeCycle {
public static final String CAPACITY_BYTES = "capacityBytes";
public static final String CAPACITY_IOPS = "capacityIops";
void updateStoragePool(StoragePool storagePool, Map<String, String>
details);
}
On Fri, Jun 20, 2014 at 7:04 AM, Mike Tutkowski <
mike.tutkow...@solidfire.com> wrote:
> Yes, looks good. Thanks for fixing this!
>
>
> On Friday, June 20, 2014, Anshul Gangwar <anshul.gang...@citrix.com>
> wrote:
>
>> I have submitted the patch for this https://reviews.apache.org/r/22807/.
>> Can somebody have a look into the patch?
>>
>> -----Original Message-----
>> From: Anshul Gangwar [mailto:anshul.gang...@citrix.com]
>> Sent: Friday, June 20, 2014 3:32 PM
>> To: dev@cloudstack.apache.org
>> Subject: RE: ZoneWideStoragePoolAllocator#filter seems weird for me
>>
>> 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>*™*
>>
>
>
> --
> *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>*™*
