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=plugi
ns/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase
.java;h=cf5c6d6c7623e682e6c5bd66d829351b2bf6ad49;hp=200a72ff219d5214ba3ebfc
2c198517e849e03a9;hb=0839fbc;hpb=b3829e54d6b7af426f797ffb9fa54b4cd2abffc0

[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/CitrixReso
>>> urceBase.java
>>> >    (bf9b068)
>>> >
>>> > View Diff <https://reviews.apache.org/r/17747/diff/>
>>> >
>>>
>>>
>>>
>>> --
>>> Daan
>
>
>
>-- 
>Daan

Reply via email to