> 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
> 
>

Reply via email to