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

Changes: v4: Fixed typo 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 | 16 ++++++++++++++++ 3 files changed, 40 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..7b89751 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 shut off."), + "KCHVM0050E": _("Cannot shutdown %(name)s. Virtual machine is shut off."), + "KCHVM0051E": _("Cannot reset %(name)s. Virtual machine is already shut 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..4fbbb1f 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 400 + 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 400 + 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 400 + 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 400 + 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

This tests cover issues #682/#684/#685 Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- tests/test_rest.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_rest.py b/tests/test_rest.py index d4715a0..7e726b0 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -275,11 +275,27 @@ 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) + + # verify if reset command returns correct status + resp = self.request('/vms/test-vm/reset', '{}', '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 start 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

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 25/08/2015 10:19, Ramon Medeiros wrote:
Changes:
v4: Fixed typo
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 | 16 ++++++++++++++++ 3 files changed, 40 insertions(+)

Applied. Thanks. Regards, Aline Manera
participants (2)
-
Aline Manera
-
Ramon Medeiros