> 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? > > > > Harikrishna Patnala wrote: > 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. > > > > Prasanna Santhanam wrote: > This is what it looks like before my fix in 7ea2c950. The log as you've > said shows that DMC is disabled when restrict_dmc is true. But the VM doesn't > scale within static-min and static-max. It scales between dynamic-min and > dynamic-max in Xen. The dynamic ranges are set earlier in the method. > static-min is used only during boot time to define the memory overhead > required for the OS to boot. > > > Please see: http://wiki.xen.org/wiki/XCP_FAQ_Dynamic_Memory_Control > > if (hostParams.get("restrict_dmc").equalsIgnoreCase("false")) { > record.memoryStaticMax = 8589934592L; //8GB > record.memoryStaticMin = 134217728L; //128MB > } else { > s_logger.warn("Host "+ _host.uuid + " does not support > Dynamic Memory Control, so we cannot scale up the vm"); > record.memoryStaticMax = vmSpec.getMaxRam(); > record.memoryStaticMin = vmSpec.getMinRam(); > } > > > Prasanna Santhanam wrote: > To ensure that this works, can you also please look at the integration > test? > test/integration/smoke/test_scale_vm.py > > It's been failing consistently: > > http://jenkins.buildacloud.org/job/test-smoke-matrix/lastCompletedBuild/suite=test_scale_vm/testReport/ > > > > Harikrishna Patnala wrote: > Yes, in order to scale the vm's memory we need to change the dynamic min > and dynamic max. These two values should be in between static max and min. > But if host restricts DMC (i.e., restrict_dmc = true) that means we > cannot scale the VM. In this case memory constraints are different on the > HOST. > static_min <= dynamic_min = dynamic_max = static_max. As per your fix > this constraint fails. > if (hostParams.get("restrict_dmc").equalsIgnoreCase("false")) { > record.memoryStaticMax = vmSpec.getMaxRam(); > record.memoryStaticMin = vmSpec.getMinRam(); > } else { > s_logger.warn("Host "+ _host.uuid + " does not support > Dynamic Memory Control, so we cannot scale up the vm"); > record.memoryStaticMax = 8589934592L; //8GB > record.memoryStaticMin = 134217728L; //128MB > } > The vm deployment fails with memory constraint violation. > > As per your fix we cannot scale vm on host(DMC enabled host) since static > max and min will be what is there in the service offering. > and vm deployment itself fails on host(DMC disabled host) because of > memory constraint violation. > >
Ok - I understand what you're trying to do here: You want to adjust the static-{min, max} limits so as to allow scaling between dynamic-{min,max}. The way it looked it seemed like having DMC enabled was changing the dynamic range between static-min and max. I've readjusted the code so it looks as expected and to suit XCP as well which allows scaling by default. I tested the change on xenserver with both dmc=true and dmc=false and it works fine. I didn't try the scaling operations itself because it seems like more fixes need to go in to determine the optimal range for the guest memory to scale. I've left a TODO which I presume you will be sending a patch for. - Prasanna ----------------------------------------------------------- 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 > >