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