[PATCH 0/5] Fix VM error messages handling
The Guest tab was not showing backend error messages properly. So user was not able to know what happened if a VM failed to start, stop, reset or delete. In order to fix this, both backend and frontend had to be changed. Rodrigo Trujillo (5): Fix vm start UI error return message Fix VM stop error messages handling (backend/UI) Fix VM reset (UI) error messages hnadling Fix VM delete error message handling (UI/Backend) Delete unsed _vm_exists funtion src/kimchi/i18n.py | 2 ++ src/kimchi/model/vms.py | 51 ++++++++++++++++++++---------------------- ui/js/src/kimchi.guest_main.js | 26 ++++++++++----------- ui/pages/i18n.html.tmpl | 1 + 4 files changed, 40 insertions(+), 40 deletions(-) -- 1.8.5.3
This patch fixed the UI error return message and also fix the button name used in the JS function. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_main.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index b5e7af3..56fe065 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -29,9 +29,9 @@ kimchi.vmstart = function(event) { kimchi.startVM(vm_id, function(result) { button.removeClass('loading'); kimchi.listVmsAuto(); - }, function() { - startButton.removeClass('loading'); - kimchi.message.error(i18n['msg.fail.start']); + }, function(err) { + button.removeClass('loading'); + kimchi.message.error(err.responseJSON.reason); } ); } else { -- 1.8.5.3
The UI was not able to catch the error messages sent from backend when a user tried to stop a VM. This patch fixes the processes and also change the 'stop' backend function, then exceptions from "get_vm" are send to frontend. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/vms.py | 7 +++++-- ui/js/src/kimchi.guest_main.js | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index fea0184..ade845e 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -77,6 +77,7 @@ messages = { "KCHVM0017E": _("Volume list (LUNs names) not given."), "KCHVM0018E": _("Virtual machine volumes must be a list of strings with distinct LUNs names."), "KCHVM0019E": _("Unable to start virtual machine %(name)s. Details: %(err)s"), + "KCHVM0020E": _("Unable to stop virtual machine %(name)s. Details: %(err)s"), "KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"), "KCHVMIF0002E": _("Network %(network)s specified for virtual machine %(name)s does not exist"), diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index b6a42e6..7f29f1a 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -383,9 +383,12 @@ class VMModel(object): {'name': name, 'err': e.get_error_message()}) def stop(self, name): - if self._vm_exists(name): - dom = self.get_vm(name, self.conn) + dom = self.get_vm(name, self.conn) + try: dom.destroy() + except libvirt.libvirtError as e: + raise OperationFailed("KCHVM0020E", + {'name': name, 'err': e.get_error_message()}) def _vm_get_graphics(self, name): dom = self.get_vm(name, self.conn) diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index 56fe065..4070c01 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -50,8 +50,8 @@ kimchi.vmstop = function(event) { kimchi.stopVM(vm_id, function(result) { button.removeClass('loading'); kimchi.listVmsAuto(); - }, function() { - kimchi.message.error(i18n['msg.fail.stop']); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); }); } else { event.preventDefault(); -- 1.8.5.3
This patch fixes the UI in order to show properly the messages sent by backend when usert tries to reset a running VM. Error messages are basically sent from vm 'stop' and 'start' functions. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- ui/js/src/kimchi.guest_main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index 4070c01..2976cac 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -64,8 +64,8 @@ kimchi.vmreset = function(event){ var vm_id=vm.attr("id"); kimchi.resetVM(vm_id, function(result) { kimchi.listVmsAuto(); - }, function() { - kimchi.message.error(i18n['msg.fail.reset']); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); } ); }; -- 1.8.5.3
This patch fixes delete confirmation window, changes backend in order to raise proper exception and fixes the ui to catch the right message from backend. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/vms.py | 31 +++++++++++++++++-------------- ui/js/src/kimchi.guest_main.js | 12 ++++++------ ui/pages/i18n.html.tmpl | 1 + 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index ade845e..f1631f4 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -78,6 +78,7 @@ messages = { "KCHVM0018E": _("Virtual machine volumes must be a list of strings with distinct LUNs names."), "KCHVM0019E": _("Unable to start virtual machine %(name)s. Details: %(err)s"), "KCHVM0020E": _("Unable to stop virtual machine %(name)s. Details: %(err)s"), + "KCHVM0021E": _("Unable to delete virtual machine %(name)s. Details: %(err)s"), "KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"), "KCHVMIF0002E": _("Network %(network)s specified for virtual machine %(name)s does not exist"), diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 7f29f1a..f999ed0 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -345,26 +345,29 @@ class VMModel(object): raise def delete(self, name): - if self._vm_exists(name): - conn = self.conn.get() - dom = self.get_vm(name, self.conn) - self._vmscreenshot_delete(dom.UUIDString()) - paths = self._vm_get_disk_paths(dom) - info = self.lookup(name) + conn = self.conn.get() + dom = self.get_vm(name, self.conn) + self._vmscreenshot_delete(dom.UUIDString()) + paths = self._vm_get_disk_paths(dom) + info = self.lookup(name) - if info['state'] == 'running': - self.stop(name) + if info['state'] == 'running': + self.stop(name) + try: dom.undefine() + except libvirt.libvirtError as e: + raise OperationFailed("KCHVM0021E", + {'name': name, 'err': e.get_error_message()}) - for path in paths: - vol = conn.storageVolLookupByPath(path) - vol.delete(0) + for path in paths: + vol = conn.storageVolLookupByPath(path) + vol.delete(0) - with self.objstore as session: - session.delete('vm', dom.UUIDString(), ignore_missing=True) + with self.objstore as session: + session.delete('vm', dom.UUIDString(), ignore_missing=True) - vnc.remove_proxy_token(name) + vnc.remove_proxy_token(name) def start(self, name): # make sure the ISO file has read permission diff --git a/ui/js/src/kimchi.guest_main.js b/ui/js/src/kimchi.guest_main.js index 2976cac..d31a01d 100644 --- a/ui/js/src/kimchi.guest_main.js +++ b/ui/js/src/kimchi.guest_main.js @@ -74,16 +74,16 @@ kimchi.vmdelete = function(event) { var vm = $(this).closest('li[name=guest]'); var vm_id=vm.attr("id"); var settings = { - title : i18n['msg.confirm.delete.title'], - content : i18n['msg.vm.confirm.delete'], - confirm : i18n['msg.confirm.delete.confirm'], - cancel : i18n['msg.confirm.delete.cancel'] + title : i18n['KCHVM6002M'], + content : i18n['KCHVM6001M'], + confirm : i18n['KCHAPI6002M'], + cancel : i18n['KCHAPI6003M'] }; kimchi.confirm(settings, function() { kimchi.deleteVM(vm_id, function(result) { kimchi.listVmsAuto(); - }, function() { - kimchi.message.error(i18n['msg.fail.delete']); + }, function(err) { + kimchi.message.error(err.responseJSON.reason); }); }, function() { }); diff --git a/ui/pages/i18n.html.tmpl b/ui/pages/i18n.html.tmpl index f8af68e..b3eeab8 100644 --- a/ui/pages/i18n.html.tmpl +++ b/ui/pages/i18n.html.tmpl @@ -95,6 +95,7 @@ var i18n = { 'KCHDR6011M': "$_("Report name should contain only letters, digits and/or hyphen ('-').")", 'KCHVM6001M': "$_("This will delete the virtual machine and its virtual disks. This operation cannot be undone. Would you like to continue?")", + 'KCHVM6002M': "$_("Delete Confirmation")", 'KCHNET6001E': "$_("The VLAN id must be between 1 and 4094.")", -- 1.8.5.3
Due to the new error messaging handling the function _vm_exists is not necessary anymore and moved OperationFailed exception to get_vm function, where makes more sence. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index f999ed0..14867e1 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -322,16 +322,6 @@ class VMModel(object): xpath = "/domain/devices/disk[@device='disk']/source/@file" return xmlutils.xpath_get_text(xml, xpath) - def _vm_exists(self, name): - try: - self.get_vm(name, self.conn) - return True - except NotFoundError: - return False - except Exception, e: - raise OperationFailed("KCHVM0009E", {'name': name, - 'err': e.message}) - @staticmethod def get_vm(name, conn): conn = conn.get() @@ -342,7 +332,8 @@ class VMModel(object): if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: raise NotFoundError("KCHVM0002E", {'name': name}) else: - raise + raise OperationFailed("KCHVM0009E", {'name': name, + 'err': e.message}) def delete(self, name): conn = self.conn.get() -- 1.8.5.3
The tests are failing with this patch set. Seems it is because you remove the _vm_exists verification while deleting the vm ====================================================================== ERROR: test_delete_running_vm (test_model.ModelTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/alinefm/kimchi/tests/test_model.py", line 780, in test_delete_running_vm self.assertFalse(u'kīмсhī-∨м' in vms) File "/home/alinefm/kimchi/src/kimchi/rollbackcontext.py", line 58, in __exit__ undo(*args, **kwargs) File "/home/alinefm/kimchi/src/kimchi/model/vms.py", line 380, in stop dom = self.get_vm(name, self.conn) File "/home/alinefm/kimchi/src/kimchi/model/vms.py", line 333, in get_vm raise NotFoundError("KCHVM0002E", {'name': name}) NotFoundError: KCHVM0002E: KCHVM0002E ====================================================================== ERROR: test_vm_edit (test_model.ModelTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/alinefm/kimchi/tests/test_model.py", line 572, in test_vm_edit rollback.prependDefer(inst.vm_delete, u'пeω-∨м') File "/home/alinefm/kimchi/src/kimchi/rollbackcontext.py", line 58, in __exit__ undo(*args, **kwargs) File "/home/alinefm/kimchi/src/kimchi/model/vms.py", line 380, in stop dom = self.get_vm(name, self.conn) File "/home/alinefm/kimchi/src/kimchi/model/vms.py", line 333, in get_vm raise NotFoundError("KCHVM0002E", {'name': name}) NotFoundError: KCHVM0002E: KCHVM0002E ---------------------------------------------------------------------- Ran 157 tests in 130.604s FAILED (errors=2) [25/Feb/2014:10:21:53] ENGINE Waiting for child threads to terminate... make[3]: *** [check-local] Error 1 make[3]: Leaving directory `/home/alinefm/kimchi/tests' make[2]: *** [check-am] Error 2 make[2]: Leaving directory `/home/alinefm/kimchi/tests' make[1]: *** [check] Error 2 make[1]: Leaving directory `/home/alinefm/kimchi/tests' make: *** [check-recursive] Error 1 On 02/24/2014 05:27 PM, Rodrigo Trujillo wrote:
The Guest tab was not showing backend error messages properly. So user was not able to know what happened if a VM failed to start, stop, reset or delete. In order to fix this, both backend and frontend had to be changed.
Rodrigo Trujillo (5): Fix vm start UI error return message Fix VM stop error messages handling (backend/UI) Fix VM reset (UI) error messages hnadling Fix VM delete error message handling (UI/Backend) Delete unsed _vm_exists funtion
src/kimchi/i18n.py | 2 ++ src/kimchi/model/vms.py | 51 ++++++++++++++++++++---------------------- ui/js/src/kimchi.guest_main.js | 26 ++++++++++----------- ui/pages/i18n.html.tmpl | 1 + 4 files changed, 40 insertions(+), 40 deletions(-)
participants (3)
-
Aline Manera -
Crístian Viana -
Rodrigo Trujillo