cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Harikrishna Patnala" <harikrishna.patn...@citrix.com>
Subject Re: Review Request: Fix for CLOUDSTACK-2085 VM weight on xen remain same as before vmscaleup
Date Mon, 20 May 2013 02:40:05 GMT


> On May 19, 2013, 1:05 p.m., Prasanna Santhanam wrote:
> > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServer56FP1Resource.java,
line 144
> > <https://reviews.apache.org/r/10903/diff/2/?file=288093#file288093line144>
> >
> >     The if-else logic should be the other way round here. When DynamicMemoryControl
(dmc) is allowed on your Xenserver (restrict_dmc=false) you'd expect to set the dynamicMin
and dynamicMax of your guest. You've done it the right way in CitrixResourceBase but not here.
I've fixed this in master for now. Please take a look.
> >     
> >     In effect, you restrict scaling when scaling is in fact allowed.
> >     
> >     Also in the else block - you set the staticMax and staticMin to hardcoded values.
While I agree with the staticMin, the staticMax is determined by the memory available on the
host. You should ideally set that to what is given by the ServiceOffering, provided all the
capacity managers allowed it to get to the ServerResource.
> >     
> >     Lastly, XCP supports DMC since 0.5, can you consider moving all this isDmcRestricted()
logic into CitrixResourceBase so XCP can take advantage of it?
> >

If else logic is correct. During the deployment of VM if(restrict_dmc = false) which implies
if DMC is not restricted then we can scale the VM so we set static memory limit to the VM.
In CitrixResourceBase while scaling the VM, we check if(restrict_dmc = true) which implies
if host restricts DMC then we throw an excpetion saying host doesnt not support memory scaling.

Inorder to scale the vm we have to set static max and min to a limit, so that after wards
we can scale the vm memory within that limit.(we cannot change static max and min dynamically).
But hardcoding the values is also not the right way. I have submitted another patch that removes
the hardcoding these values and setting the values according the memory constraints provided
by the xenserver.


- Harikrishna


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10903/#review20743
-----------------------------------------------------------


On May 15, 2013, 10:06 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10903/
> -----------------------------------------------------------
> 
> (Updated May 15, 2013, 10:06 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek and Nitin Mehta.
> 
> 
> Description
> -------
> 
> CLOUDSTACK-2085: VM weight on xen remain same as before vmscaleup ;because "Add-To-VCPUs-Params-Live.sh"
is not getting copied on xs host 
> Fixed by updating the patch files that has
>  entries to copy scipts on xenserver. Here we added
>  Add-To-VCPUs-Params-Live.sh
> 
> Added a check on Host params whether host restricts Dynamic memory control(DMC) to able
to allow scale up VM.
> If DMC is not enabled then static max and min are set to SO.
> 
> 
> This addresses bug CLOUDSTACK-2085.
> 
> 
> Diffs
> -----
> 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
46ae35a 
>   plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServer56FP1Resource.java
96a90a6 
>   plugins/hypervisors/xen/test/com/cloud/hypervisor/xen/resource/CitrixResourceBaseTest.java
7392cb1 
>   scripts/vm/hypervisor/xenserver/Add-To-VCPUs-Params-Live.sh 0fadcd8 
>   scripts/vm/hypervisor/xenserver/add_to_vcpus_params_live.sh PRE-CREATION 
>   scripts/vm/hypervisor/xenserver/vmops 30b5300 
>   scripts/vm/hypervisor/xenserver/xcpserver/patch b7961bb 
>   scripts/vm/hypervisor/xenserver/xenserver56/patch 36dba3d 
>   scripts/vm/hypervisor/xenserver/xenserver56fp1/patch d20e60f 
>   scripts/vm/hypervisor/xenserver/xenserver60/patch c9125f4 
> 
> Diff: https://reviews.apache.org/r/10903/diff/
> 
> 
> Testing
> -------
> 
> Tested: tried scaling up a vm and checked on xenserver for the new values. 
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message