----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10903/#review20743 -----------------------------------------------------------
Reopening this for review as the fix is incomplete and partly invalid. Reviewers please note. plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServer56FP1Resource.java <https://reviews.apache.org/r/10903/#comment42829> 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? scripts/vm/hypervisor/xenserver/add_to_vcpus_params_live.sh <https://reviews.apache.org/r/10903/#comment42830> disable -x by default scripts/vm/hypervisor/xenserver/add_to_vcpus_params_live.sh <https://reviews.apache.org/r/10903/#comment42831> Use the $(expr) syntax instead of backticks. See http://wiki.bash-hackers.org/syntax/expansion/cmdsubst - Prasanna Santhanam 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 > >
