Re: [Kimchi-devel] [PATCH 1/3] change the stop to power off for VM in backend

From: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt; "Stop" is obscure for user. Currently, stopping guests in Kimchi is not graceful in the sense that the guest becomes 'shut down' immediately. This could be be problematic because guest file systems could become corrupted. Perhaps a warning should be added before a guest is immediately stopped or reset. So "power off" is more exact than "stop" in literal meaning. The aciton power off a VM forcefully is same that we cut the physical machine power. This action may produce undesirable results, for example unflushed disk cache in the guest. update API.md, backend source code and test case code. Signed-off-by: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com&gt; --- docs/API.md | 3 ++- src/kimchi/control/vms.py | 2 +- src/kimchi/i18n.py | 2 +- src/kimchi/mockmodel.py | 2 +- src/kimchi/model/vms.py | 4 ++-- tests/test_model.py | 12 ++++++------ tests/test_rest.py | 12 ++++++------ 7 files changed, 19 insertions(+), 18 deletions(-) diff --git a/docs/API.md b/docs/API.md index 630015e..143c70c 100644 --- a/docs/API.md +++ b/docs/API.md @@ -102,7 +102,8 @@ the following general conventions: **Actions (POST):** * start: Power on a VM -* stop: Power off forcefully +* poweroff: Power off a VM forcefully. Note this action may produce undesirable + results, for example unflushed disk cache in the guest. * connect: Prepare the connection for spice or vnc ### Sub-resource: Virtual Machine Screenshot diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 81ad3a6..e75b27f 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -38,7 +38,7 @@ class VM(Resource): for ident, node in sub_nodes.items(): setattr(self, ident, node(model, self.ident)) self.start = self.generate_action_handler('start') - self.stop = self.generate_action_handler('stop') + self.poweroff = self.generate_action_handler('poweroff') self.connect = self.generate_action_handler('connect') @property diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 0eb721a..7cf0970 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -76,7 +76,7 @@ messages = { "KCHVM0015E": _("Graphics address to listen on must be IPv4 or IPv6"), "KCHVM0016E": _("Specify a template to create a virtual machine from"), "KCHVM0019E": _("Unable to start virtual machine %(name)s. Details: %(err)s"), - "KCHVM0020E": _("Unable to stop virtual machine %(name)s. Details: %(err)s"), + "KCHVM0020E": _("Unable to poweroff 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"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index dbdd57e..98fb080 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -129,7 +129,7 @@ class MockModel(object): def vm_start(self, name): self._get_vm(name).info['state'] = 'running' - def vm_stop(self, name): + def vm_poweroff(self, name): self._get_vm(name).info['state'] = 'shutoff' def vm_connect(self, name): diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index c8dfc90..8334d9e 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -335,7 +335,7 @@ class VMModel(object): info = self.lookup(name) if info['state'] == 'running': - self.stop(name) + self.poweroff(name) try: dom.undefine() @@ -372,7 +372,7 @@ class VMModel(object): raise OperationFailed("KCHVM0019E", {'name': name, 'err': e.get_error_message()}) - def stop(self, name): + def poweroff(self, name): dom = self.get_vm(name, self.conn) try: dom.destroy() diff --git a/tests/test_model.py b/tests/test_model.py index b8e6d47..3041196 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -91,7 +91,7 @@ class ModelTests(unittest.TestCase): self.assertTrue('kimchi-vm' in vms) inst.vm_start('kimchi-vm') - rollback.prependDefer(inst.vm_stop, 'kimchi-vm') + rollback.prependDefer(inst.vm_poweroff, 'kimchi-vm') info = inst.vm_lookup('kimchi-vm') self.assertEquals('running', info['state']) @@ -222,7 +222,7 @@ class ModelTests(unittest.TestCase): inst.vmstorage_update(vm_name, cdrom_dev, {'path': iso_path}) cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev) self.assertEquals(iso_path, cdrom_info['path']) - inst.vm_stop(vm_name) + inst.vm_poweroff(vm_name) # removing non existent cdrom self.assertRaises(NotFoundError, inst.vmstorage_delete, vm_name, @@ -587,7 +587,7 @@ class ModelTests(unittest.TestCase): self.assertTrue('kimchi-vm1' in vms) inst.vm_start('kimchi-vm1') - rollback.prependDefer(self._rollback_wrapper, inst.vm_stop, + rollback.prependDefer(self._rollback_wrapper, inst.vm_poweroff, 'kimchi-vm1') info = inst.vm_lookup('kimchi-vm1') @@ -597,7 +597,7 @@ class ModelTests(unittest.TestCase): self.assertRaises(InvalidParameter, inst.vm_update, 'kimchi-vm1', params) - inst.vm_stop('kimchi-vm1') + inst.vm_poweroff('kimchi-vm1') params = {'name': u'пeω-∨м'} self.assertRaises(OperationFailed, inst.vm_update, 'kimchi-vm1', {'name': 'kimchi-vm2'}) @@ -795,7 +795,7 @@ class ModelTests(unittest.TestCase): self.assertEquals('failed', inst.task_lookup(taskid)['status']) # This wrapper function is needed due to the new backend messaging in - # vm model. vm_stop and vm_delete raise exception if vm is not found. + # vm model. vm_poweroff and vm_delete raise exception if vm is not found. # These functions are called after vm has been deleted if test finishes # correctly, then NofFoundError exception is raised and rollback breaks def _rollback_wrapper(self, func, vmname): @@ -820,7 +820,7 @@ class ModelTests(unittest.TestCase): u'kīмсhī-∨м') inst.vm_start(u'kīмсhī-∨м') - rollback.prependDefer(self._rollback_wrapper, inst.vm_stop, + rollback.prependDefer(self._rollback_wrapper, inst.vm_poweroff, u'kīмсhī-∨м') inst.vm_delete(u'kīмсhī-∨м') diff --git a/tests/test_rest.py b/tests/test_rest.py index 8f4573e..cd40d55 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -201,7 +201,7 @@ class RestTests(unittest.TestCase): resp = self.request('/vms/vm-1', req, 'PUT') self.assertEquals(400, resp.status) - resp = self.request('/vms/vm-1/stop', '{}', 'POST') + resp = self.request('/vms/vm-1/poweroff', '{}', 'POST') self.assertEquals(200, resp.status) req = json.dumps({'name': 12}) @@ -257,8 +257,8 @@ class RestTests(unittest.TestCase): self.assertEquals(200, resp.status) self.assertTrue(resp.getheader('Content-type').startswith('image')) - # Force stop the VM - resp = self.request('/vms/test-vm/stop', '{}', 'POST') + # Force poweroff the VM + resp = self.request('/vms/test-vm/poweroff', '{}', 'POST') vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('shutoff', vm['state']) @@ -572,8 +572,8 @@ class RestTests(unittest.TestCase): vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('running', vm['state']) - # Force stop the VM - resp = self.request('/vms/test-vm/stop', '{}', 'POST') + # Force poweroff the VM + resp = self.request('/vms/test-vm/poweroff', '{}', 'POST') vm = json.loads(self.request('/vms/test-vm').read()) self.assertEquals('shutoff', vm['state']) @@ -1222,7 +1222,7 @@ class RestTests(unittest.TestCase): self.assertEquals(405, resp.status) # No screenshot after stopped the VM - self.request('/vms/test-vm/stop', '{}', 'POST') + self.request('/vms/test-vm/poweroff', '{}', 'POST') resp = self.request('/vms/test-vm/screenshot') self.assertEquals(404, resp.status)