[PATCH 0/2] Issues #682/#684/#685: Change some code errors when start, shutdown or poweroff vms

Changes: v3: Added tests v2: Fixed pep8 issues Ramon Medeiros (2): Issues #682/#684/#685: Change some code errors when start, shutdown or poweroff vms Add unity tests for start/shutdown/poweroff response commands src/kimchi/i18n.py | 4 ++++ src/kimchi/model/vms.py | 20 ++++++++++++++++++++ tests/test_rest.py | 12 ++++++++++++ 3 files changed, 36 insertions(+) -- 2.1.0

When trying to start a running vm, shutoff or poweroff a shutdown vm, the retrieved error was 500 (Internal Server Error). A better error would be 400 (Invalid Request). Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 4 ++++ src/kimchi/model/vms.py | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 862df7f..2b4ac38 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -119,6 +119,10 @@ messages = { "KCHVM0045E": _("There are not enough free slots of 1024 Mib in the guest."), "KCHVM0046E": _("Host's libvirt version does not support memory devices. Libvirt must be >= 1.2.14"), "KCHVM0047E": _("Error attaching memory device. Details: %(error)s"), + "KCHVM0048E": _("Cannot start %(name)s. Virtual machine is already running."), + "KCHVM0049E": _("Cannot power off %(name)s. Virtual machine is shuted off."), + "KCHVM0050E": _("Cannot shutdown %(name)s. Virtual machine is shuted off."), + "KCHVM0051E": _("Cannot reset %(name)s. Virtual machine is already shuted off."), "KCHVMHDEV0001E": _("VM %(vmid)s does not contain directly assigned host device %(dev_name)s."), "KCHVMHDEV0002E": _("The host device %(dev_name)s is not allowed to directly assign to VM."), diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 106e9bc..13c8511 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -1102,6 +1102,11 @@ class VMModel(object): run_setfacl_set_attr(iso, user=user) dom = self.get_vm(name, self.conn) + + # vm already running: return error 403 + if DOM_STATE_MAP[dom.info()[0]] == "running": + raise InvalidOperation("KCHVM0048E", {'name': name}) + try: dom.create() except libvirt.libvirtError as e: @@ -1110,6 +1115,11 @@ class VMModel(object): def poweroff(self, name): dom = self.get_vm(name, self.conn) + + # vm already powered off: return error 403 + if DOM_STATE_MAP[dom.info()[0]] == "shutoff": + raise InvalidOperation("KCHVM0049E", {'name': name}) + try: dom.destroy() except libvirt.libvirtError as e: @@ -1118,6 +1128,11 @@ class VMModel(object): def shutdown(self, name): dom = self.get_vm(name, self.conn) + + # vm already powered off: return error 403 + if DOM_STATE_MAP[dom.info()[0]] == "shutoff": + raise InvalidOperation("KCHVM0050E", {'name': name}) + try: dom.shutdown() except libvirt.libvirtError as e: @@ -1126,6 +1141,11 @@ class VMModel(object): def reset(self, name): dom = self.get_vm(name, self.conn) + + # vm already powered off: return error 403 + if DOM_STATE_MAP[dom.info()[0]] == "shutoff": + raise InvalidOperation("KCHVM0051E", {'name': name}) + try: dom.reset(flags=0) except libvirt.libvirtError as e: -- 2.1.0

On Mon, Aug 24, 2015 at 3:43 PM, Ramon Medeiros <ramonn@linux.vnet.ibm.com> wrote:
+ "KCHVM0048E": _("Cannot start %(name)s. Virtual machine is already running."), + "KCHVM0049E": _("Cannot power off %(name)s. Virtual machine is shuted off."), + "KCHVM0050E": _("Cannot shutdown %(name)s. Virtual machine is shuted off."), + "KCHVM0051E": _("Cannot reset %(name)s. Virtual machine is already shuted off."),
The correct past form <https://en.wiktionary.org/wiki/shut#Verb> of "shut" is "shut", not "shuted". -- www.google.com/+CrístianDeives <http://www.google.com/+Cr%C3%ADstianDeives>

This tests cover issues #682/#684/#685 Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- tests/test_rest.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_rest.py b/tests/test_rest.py index d4715a0..48beae0 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -275,11 +275,23 @@ class RestTests(unittest.TestCase): self.assertEquals(1 << 30, vol['capacity']) self.assertEquals(['test-vm'], vol['used_by']) + # verify if poweroff command returns correct status + resp = self.request('/vms/test-vm/poweroff', '{}', 'POST') + self.assertEquals(400, resp.status) + + # verify if shutdown command returns correct status + resp = self.request('/vms/test-vm/shutdown', '{}', 'POST') + self.assertEquals(400, resp.status) + # Start the VM resp = self.request('/vms/test-vm/start', '{}', 'POST') vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('running', vm['state']) + # verify if poweroff command returns correct status + resp = self.request('/vms/test-vm/start', '{}', 'POST') + self.assertEquals(400, resp.status) + # Test screenshot resp = self.request(vm['screenshot'], method='HEAD') self.assertEquals(200, resp.status) -- 2.1.0

On Mon, Aug 24, 2015 at 3:43 PM, Ramon Medeiros <ramonn@linux.vnet.ibm.com> wrote:
+ # verify if poweroff command returns correct status + resp = self.request('/vms/test-vm/start', '{}', 'POST') + self.assertEquals(400, resp.status)
The comment line doesn't match what the test does. -- www.google.com/+CrístianDeives <http://www.google.com/+Cr%C3%ADstianDeives>
participants (3)
-
Aline Manera
-
Crístian Viana
-
Ramon Medeiros