[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
Aline Manera
alinefm at linux.vnet.ibm.com
Fri Jun 3 21:24:47 UTC 2016
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.
> + 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>
More information about the Kimchi-devel
mailing list