[Kimchi-devel] [PATCH] [Kimchi] Pass only those fields that have been modified in Edit Guest; Match maxmem to mem when guest is not running or paused
Socorro Stoppler
socorro at linux.vnet.ibm.com
Mon Jun 6 14:17:18 UTC 2016
On 06/03/2016 02:24 PM, Aline Manera wrote:
>
> Hi Socorro,
>
> Only a minor comment below and a question.
>
> On 06/02/2016 07:23 PM, Socorro Stoppler wrote:
>> v2:
>> Made adjustments on how things were being done based on feedback.
>> Also included the fix for github issue#: 941 - max memory should
>> increase when mem is increased.
>>
>> v1:
>> This patch fixes bugzilla#: 141686 -after memory hot plug seeing
>> error 'KCHCPUHP0005E'
>> This patch addresses the issue of what is being passed down to the
>> backend
>> when guest is modified. The issue was passing parameters that were not
>> modified caused the backend to throw an error in the mem hotplug
>> scenario.
>> It now only passes down those parameters that have been modified
>> regarding
>> memory and cpu fields.
>>
>> Signed-off-by: Socorro Stoppler <socorro at linux.vnet.ibm.com>
>> ---
>> ui/js/src/kimchi.guest_edit_main.js | 159
>> ++++++++++++++++++++++++------------
>> ui/pages/guest-edit.html.tmpl | 2 +-
>> 2 files changed, 108 insertions(+), 53 deletions(-)
>>
>> diff --git a/ui/js/src/kimchi.guest_edit_main.js
>> b/ui/js/src/kimchi.guest_edit_main.js
>> index 4cee8fe..fc67d82 100644
>> --- a/ui/js/src/kimchi.guest_edit_main.js
>> +++ b/ui/js/src/kimchi.guest_edit_main.js
>> @@ -668,10 +668,8 @@ kimchi.guest_edit_main = function() {
>> guest['max-processor'] = guest.cpu_info['maxvcpus'];
>> guest['icon'] = guest['icon'] ||
>> 'plugins/kimchi/images/icon-vm.png';
>> $('#form-guest-edit-general').fillWithObject(guest);
>> -
>> $('#guest-edit-memory-textbox').val(parseInt(guest.memory.current));
>> $('#guest-edit-max-memory-textbox').val(parseInt(guest.memory.maxmemory));
>> -
>> kimchi.thisVMState = guest['state'];
>> refreshCDROMs();
>> $('#guest-edit-attach-cdrom-button').on('click',
>> function(event) {
>> @@ -706,6 +704,12 @@ kimchi.guest_edit_main = function() {
>> $('#guest-show-max-processor
>> i.fa').toggleClass('fa-plus-circle fa-minus-circle');
>> });
>>
>> + if ((kimchi.thisVMState !== "running") &&
>> (kimchi.thisVMState !== "paused")) {
>> + $("#guest-edit-memory-textbox").bind('keyup blur',
>> function(e) {
>> + $('#guest-edit-max-memory-textbox').val($(this).val());
>> + });
>> + }
>> +
>> var onAttached = function(params) {
>> refreshCDROMs();
>> };
>> @@ -736,59 +740,110 @@ kimchi.guest_edit_main = function() {
>> kimchi.retrieveVM(kimchi.selectedGuest, initContent);
>>
>> var generalSubmit = function(event) {
>> - $(saveButton).prop('disabled', true);
>> - var data = $('#form-guest-edit-general').serializeObject();
>> - if (data['memory'] !== undefined) {
>> - data['memory'] = Number(data['memory']);
>> - }
>> - if (data['max-memory'] !== undefined) {
>> - data['max-memory'] = Number(data['max-memory']);
>> - }
>> -
>> - // Test memory values before submit. Avoid requests we know
>> are going to fail
>> - if (parseInt($('#guest-edit-memory-textbox').val()) >
>> parseInt($('#guest-edit-max-memory-textbox').val())) {
>> - wok.message.error(i18n['KCHVM0002E'],
>> '#alert-modal-container');
>> - $(saveButton).prop('disabled', false);
>> - return;
>> - }
>> -
>> - // Test CPU values before submit. Avoid requests we know are
>> going to fail
>> - if (parseInt($('#guest-edit-cores-textbox').val()) >
>> parseInt($('#guest-edit-max-processor-textbox').val())) {
>> - wok.message.error(i18n['KCHVM0003E'],
>> '#alert-modal-container');
>> - $(saveButton).prop('disabled', false);
>> - return;
>> - }
>> -
>> - if (data['vcpus'] !== undefined) {
>> - var cpu = Number(data['vcpus']);
>> - var maxCpu = Number(data['max-processor']);
>> - if (data['max-processor'] !== undefined &&
>> data['max-processor'] !== "") {
>> - if (maxCpu >= cpu || maxCpu < cpu ) { // if maxCpu
>> < cpu, this will throw an error from backend
>> - data['cpu_info'] = {
>> - vcpus: cpu,
>> - maxvcpus: maxCpu
>> - };
>> + kimchi.retrieveVM(kimchi.selectedGuest, function(org) {
>> + $(saveButton).prop('disabled', true);
>> + var data = $('#form-guest-edit-general').serializeObject();
>> + data['memory'] = {current: Number(data['memory-ui']),
>> maxmemory: Number(data['max-memory'])};
>> + data['cpu_info'] = {maxvcpus:
>> Number(data['max-processor']), vcpus: Number(data['vcpus']),
>> topology: org['cpu_info']['topology']};
>> + var changedFields = {};
>> + for (var key in data) {
>> + valueFromUI = data[key];
>> + if (valueFromUI instanceof Object) {
>> + // Compare if Objects of original and data are
>> identical
>> + // Handle special case when key is memory and
>> guest is running as valueFromUI will return a null for max mem
>> + // since it is disabled; for cpu_info, when
>> guest is running, just skip it since no processing is required
>> + if (kimchi.thisVMState === 'running' ||
>> kimchi.thisVMState === 'paused') {
>
>> + if (key === 'cpu_info') {
>> + continue;
>> + }
>
> The above 'if' is not needed as there is nothing to do.
This above is needed as otherwise, cpu_info will be added to
changedFields due to topology having a value of {} (i.e. the Objects are
no longer the same).
>
>> + if (key === 'memory') {
>> + // Replace valueFromUI of max mem with
>> one from original as otherwise the value is undefined
>> + data['memory']['maxmemory'] =
>> org.memory.maxmemory;
>> + }
>> + }
>> + // NOTE: Be aware of using this comparison as
>> the order of the properties matter
>> + if (JSON.stringify(valueFromUI) !==
>> JSON.stringify(org[key])) {
>> + changedFields[key] = valueFromUI;
>> + }
>> + } else {
>> + if (org[key] !== undefined) {
>> + if (data[key] != org[key]) {
>> + changedFields[key] = valueFromUI;
>> + }
>> + }
>> }
>> - } else {
>> - data['cpu_info'] = {
>> - vcpus: cpu,
>> - maxvcpus: cpu
>> - };
>> }
>> - delete data['vcpus'];
>> - delete data['max-processor'];
>> - }
>> -
>> - memory = {'current': data['memory'], 'maxmemory':
>> data['max-memory']};
>> + var origMem = Number(org.memory.current);
>> + var origMaxMem = Number(org.memory.maxmemory);
>> + var origCpu = Number(org.cpu_info.vcpus);
>> + var origMaxCpu = Number(org.cpu_info.maxvcpus);
>> + var currentMem = data['memory-ui'];
>> + var currentMaxMem = data['max-memory'];
>> + var currentCpu = data['vcpus'];
>> + var currentMaxCpu = data['max-processor'];
>> +
>> + if ('memory' in changedFields) {
>> + if (currentMaxMem !== undefined) {
>> + currentMaxMem = Number(currentMaxMem);
>> + if (currentMaxMem === origMaxMem) {
>> + delete changedFields.memory.maxmemory;
>> + }
>> + } else {
>> + delete changedFields.memory.maxmemory;
>> + }
>> + if (currentMem !== undefined) {
>> + currentMem = Number(currentMem);
>> + if (kimchi.thisVMState === 'running' ||
>> kimchi.thisVMState === 'paused') {
>> + // Compare to original max mem since current
>> max mem is undefined in UI due to being disabled
>> + if (currentMem > origMaxMem) {
>> + wok.message.error(i18n['KCHVM0002E'], '#alert-modal-container');
>> + $(saveButton).prop('disabled', false);
>> + return;
>> + }
>> + }
>> + if (currentMem === origMem) {
>> + delete changedFields.memory.current;
>> + }
>> + } else {
>> + delete changedFields.memory.current;
>> + }
>> + }
>> + if ('cpu_info' in changedFields) {
>> + if (currentMaxCpu !== undefined) {
>> + currentMaxCpu = Number(currentMaxCpu);
>> + if (currentMaxCpu === origMaxCpu) {
>> + delete changedFields.cpu_info.maxvcpus;
>> + }
>> + } else {
>> + delete changedFields.cpu_info.maxvcpus;
>> + }
>> + if (currentCpu !== undefined) {
>> + currentCpu = Number(currentCpu);
>> + if (currentMaxCpu !== undefined && currentCpu >
>> currentMaxCpu) {
>> + wok.message.error(i18n['KCHVM0003E'],
>> '#alert-modal-container');
>> + $(saveButton).prop('disabled', false);
>> + return;
>> + }
>> + if (currentCpu === origCpu) {
>> + delete changedFields.cpu_info.vcpus;
>> + }
>> + if (currentMaxCpu === origMaxCpu) {
>> + delete changedFields.cpu_info.maxvcpus;
>> + }
>> + } else {
>> + delete changedFields.cpu_info.vcpus;
>> + }
>> + // Delete this as it is not applicable regardless
>> + delete changedFields.cpu_info.topology;
>> + }
>> + kimchi.updateVM(kimchi.selectedGuest, changedFields,
>> function() {
>> + kimchi.listVmsAuto();
>> + wok.window.close();
>> + }, function(err) {
>> + wok.message.error(err.responseJSON.reason,
>> '#alert-modal-container');
>> + $(saveButton).prop('disabled', false);
>> + });
>>
>> - data.memory = memory;
>> - delete data['max-memory'];
>> - kimchi.updateVM(kimchi.selectedGuest, data, function() {
>> - kimchi.listVmsAuto();
>> - wok.window.close();
>> - }, function(err) {
>> - wok.message.error(err.responseJSON.reason,
>> '#alert-modal-container');
>> - $(saveButton).prop('disabled', false);
>> });
>> };
>>
>> diff --git a/ui/pages/guest-edit.html.tmpl
>> b/ui/pages/guest-edit.html.tmpl
>> index 5b7155a..f18d42c 100644
>> --- a/ui/pages/guest-edit.html.tmpl
>> +++ b/ui/pages/guest-edit.html.tmpl
>> @@ -58,7 +58,7 @@
>> <div class="form-group">
>> <label
>> for="guest-edit-memory-textbox">$_("Memory (MB)")</label>
>> <div id="guest-memory">
>
>> - <input id="guest-edit-memory-textbox"
>> class="form-control" name="memory" type="number" min="1024"
>> step="1024" />
>> + <input id="guest-edit-memory-textbox"
>> class="form-control" name="memory-ui" type="number" min="1024"
>> step="1024" />
>
> Any special reason for that change?
I believe it was due to having a conflict w/data['memory'] that we
introduced in generalSubmit.
>
>> <button id="guest-show-max-memory"
>> class="btn btn-primary" type="button"><i class="fa
>> fa-plus-circle"></i> <span class="text">$_("More")</span></button>
>> </div>
>> </div>
>
More information about the Kimchi-devel
mailing list