Re: [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

6 Jun 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@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>


    

Socorro Stoppler

tags

participants (1)