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

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

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