Done applied to 4.3

> -----Original Message-----
> From: Harikrishna Patnala
> Sent: Friday, February 07, 2014 1:31 PM
> To: Nitin Mehta; dev@cloudstack.apache.org
> Cc: Anthony Xu; Animesh Chaturvedi
> Subject: RE: Review Request 17747: CLOUDSTACK-6023:Non windows
> instances are created on XenServer with a vcpu-max above supported
> xenserver limits
> 
> Here is the commit id on 4.3-forward
> 
> commit 95e41fdf0da50c165a9317847058fce4efeddbcf
> 
> -Harikrishna
> ________________________________________
> From: Nitin Mehta
> Sent: Saturday, February 08, 2014 2:46 AM
> To: dev@cloudstack.apache.org
> Cc: Harikrishna Patnala; Anthony Xu
> Subject: Re: Review Request 17747: CLOUDSTACK-6023:Non windows
> instances are created on XenServer with a vcpu-max above supported
> xenserver limits
> 
> Animesh - done. Please cherry pick to 4.3
> 
> Daan - for pushing this to master I will have to revert your fix in master.
> 
> On 07/02/14 11:48 AM, "Animesh Chaturvedi"
> <animesh.chaturv...@citrix.com>
> wrote:
> 
> >Can I have the patch applied in 4.3-forward?
> >
> >> -----Original Message-----
> >> From: Nitin Mehta [mailto:nitin.me...@citrix.com]
> >> Sent: Friday, February 07, 2014 11:33 AM
> >> To: dev@cloudstack.apache.org
> >> Cc: Harikrishna Patnala; Anthony Xu
> >> Subject: Re: Review Request 17747: CLOUDSTACK-6023:Non windows
> >> instances are created on XenServer with a vcpu-max above supported
> >> xenserver limits
> >>
> >> Hey Daan,
> >> I guess you are talking about your fix submitted in master [1].
> >> You might want to see Hari's latest patch [2] as well. It solves the
> >>following  two things which I guess your fix misses.
> >>
> >> 1. It sets the vcpu max to a hardcoded value (say 16) only when
> >>dynamic  scaling is enabled. Do note that by default dynamic scaling is
> disabled.
> >> This is an important fix bcz only those who need dynamic scaling get
> >>impacted.
> >> 2. It has made the vcpu max configurable at cluster level which makes
> >>it  flexible for the admin to customize it depending on the load in
> >>his/her  cluster.(I guess even you guys mentioned keeping it
> >>configurable) In case  there are issues with keeping it at 16, there
> >>is a flexibility to change it to a  lower value during runtime.
> >>
> >> Let me know if you have any concerns.
> >>
> >>
> >> [1]
> >>
> >>https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=blobdiff;f=
> >>plu
> >>gi
> >>
> >>ns/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourc
> >>eB
> >> ase
> >> .java;h=cf5c6d6c7623e682e6c5bd66d829351b2bf6ad49;hp=200a72ff219d5
> 214
> >> ba3ebfc
> >>
> 2c198517e849e03a9;hb=0839fbc;hpb=b3829e54d6b7af426f797ffb9fa54b4cd2
> >> abffc0
> >>
> >> [2] https://reviews.apache.org/r/17747/
> >>
> >> Thanks,
> >> -Nitin
> >>
> >> On 06/02/14 10:10 PM, "Daan Hoogland" <daan.hoogl...@gmail.com>
> >> wrote:
> >>
> >> >Animesh,
> >> >
> >> >I put in a patch that makes it double the number of assigned vcpu or
> >> >16 whichever is smaller. it is on 4.3-forward
> >> >
> >> >On Fri, Feb 7, 2014 at 6:28 AM, Animesh Chaturvedi
> >> ><animesh.chaturv...@citrix.com> wrote:
> >> >>
> >> >>
> >> >>> -----Original Message-----
> >> >>> From: Daan Hoogland [mailto:daan.hoogl...@gmail.com]
> >> >>> Sent: Thursday, February 06, 2014 6:33 AM
> >> >>> To: Harikrishna Patnala
> >> >>> Cc: Nitin Mehta; cloudstack
> >> >>> Subject: Re: Review Request 17747: CLOUDSTACK-6023:Non windows
> >> >>> instances are created on XenServer with a vcpu-max above
> >> >>> supported xenserver limits
> >> >>>
> >> >>> we have hosts with 80 vms. 80*16 > 160 , which is spedcified in
> >> >>>the xenserver  docs Joris came up with. That last part is not
> >> >>>important to me but I am still  worried about the size of the
> >> >>>statistics post by the members to the
> >> >>>pool-
> >> >>> master. If we can make sure we don't cross this boundary I am
> >> >>>fine with not  making it optional. So to stress my point: even
> >> >>>with a documented limit of
> >> >>> 16 per vm there is also a limit of 160 per host. And the real
> >> >>>limit is neither as  we can instantiate vms with 32 vcpu (even on
> >> >>> 6.0.2 i think Joris?) the actual problem is in the internal xapi
> >> >>>traffic.
> >> >>>
> >> >> [Animesh] xapi traffic issues need to be addressed by xen and
> >> >>outside of cloudstack. From xen doc the vCPU  per host is 4000 not
> >> >>160 (which is logical processor/host). For 4.3 does it make sense
> >> >>to keep the value lower to like 8 to reduce chances of overloading
> >> >>xapi. For 4.4 this can be reworked to a configurable or computed
> >> >>value bases on different limits
> >> >>>
> >> >>>
> >> >>> On Thu, Feb 6, 2014 at 1:47 PM, Harikrishna Patnala <
> >> >>> harikrishna.patn...@citrix.com> wrote:
> >> >>>
> >> >>> >    This is an automatically generated e-mail. To reply, visit:
> >> >>> > https://reviews.apache.org/r/17747/
> >> >>> >
> >> >>> > On February 6th, 2014, 8:41 a.m. UTC, *daan Hoogland* wrote:
> >> >>> >
> >> >>> > Harikrishna, I would like to see the intermediate option of
> >> >>> > scale up
> >> >>>to
> >> >>> double the amount as well. Did you revert it? Is there a problem
> >> >>>with this  approach?
> >> >>> >
> >> >>> > Otherwise your submission is fine, of course.
> >> >>> >
> >> >>> >  Hi Daan,
> >> >>> > I did not revert any changes. We can put an option to scale
> >> >>> > upto
> >> >>>double
> >> >>> but why it is required if vm can go till 16 (if at all 16 is the
> >> >>>correct limit).
> >> >>> >
> >> >>> >
> >> >>> >
> >> >>> > - Harikrishna
> >> >>> >
> >> >>> > On February 5th, 2014, 5:19 p.m. UTC, Harikrishna Patnala wrote:
> >> >>> >   Review request for cloudstack and Nitin Mehta.
> >> >>> > By Harikrishna Patnala.
> >> >>> >
> >> >>> > *Updated Feb. 5, 2014, 5:19 p.m.*
> >> >>> >  *Bugs: *
> >> >>> > CLOUDSTACK-
> >> 6023<https://issues.apache.org/jira/browse/CLOUDSTACK-
> >> >>> 6023>
> >> >>> >  *Repository: * cloudstack-git
> >> >>> > Description
> >> >>> >
> >> >>> > CLOUDSTACK-6023:Non windows instances are created on
> XenServer
> >> >>> > with a vcpu-max above supported xenserver limits
> >> >>> >
> >> >>> > VCPUs-max value is changed to 16 and only when dynamic scaling
> >> >>> > is
> >> >>> enabled.
> >> >>> >
> >> >>> >   Diffs
> >> >>> >
> >> >>> >    -
> >> >>>
> >> >>>plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/Citr
> >> >>>ixR
> >> >>>eso
> >> >>> urceBase.java
> >> >>> >    (bf9b068)
> >> >>> >
> >> >>> > View Diff <https://reviews.apache.org/r/17747/diff/>
> >> >>> >
> >> >>>
> >> >>>
> >> >>>
> >> >>> --
> >> >>> Daan
> >> >
> >> >
> >> >
> >> >--
> >> >Daan
> >

Reply via email to