Reviewed-by: Royce Lv<lvroyce@linux.vnet.ibm.com> On 01/22/2015 09:52 AM, Crístian Viana wrote:

When updating a VM, Kimchi always fetches the current VM state regardless of whether it will be needed.

Move the VM state fetching to block where it's needed. That way, Kimchi will save a libvirt call when that result isn't important for the operation.

Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index bae27c1..42cf3e9 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -663,7 +663,6 @@ class VMModel(object): return xml

def _static_vm_update(self, dom, params): - state = DOM_STATE_MAP[dom.info()[0]] old_xml = new_xml = dom.XMLDesc(0)

for key, val in params.items(): @@ -683,6 +682,7 @@ class VMModel(object): conn = self.conn.get() try: if 'name' in params: + state = DOM_STATE_MAP[dom.info()[0]] if state == 'running': msg_args = {'name': dom.name(), 'new_name': params['name']} raise InvalidParameter("KCHVM0003E", msg_args)

On 22/01/2015 12:52, Crístian Viana wrote:

Some functions assume that a VM can only be in two states: shutoff and running. However, that's not always the case (e.g. not being running doesn't mean it's shutoff).

Update some functions so the VM state conditions are more accurate.

Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 42cf3e9..5128d1e 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -683,7 +683,7 @@ class VMModel(object): try: if 'name' in params: state = DOM_STATE_MAP[dom.info()[0]] - if state == 'running': + if state != u'shutoff':

DOM_STATE_MAP returns a string instead of unicode.

msg_args = {'name': dom.name(), 'new_name': params['name']} raise InvalidParameter("KCHVM0003E", msg_args)

@@ -788,7 +788,7 @@ class VMModel(object): paths = self._vm_get_disk_paths(dom) info = self.lookup(name)

- if info['state'] == 'running': + if info['state'] != u'shutoff': self.poweroff(name)

# delete existing snapshots before deleting VM

In real model, vm lookup need to be updated because vm has no statistics to present, and snapshots can be created when vm is "paused"(we just allow it to be taken when vm is "shutoff" previously), etc. Please update all previous state judgement clause and check if it works for "pause" "running" "shutoff". On 01/22/2015 09:52 AM, Crístian Viana wrote:

The user is now able to suspend the VM and resume its execution in a later moment with the following commands:

POST /vms/<vm-name>/suspend POST /vms/<vm-name>/resume

Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/control/vms.py | 2 ++ src/kimchi/i18n.py | 4 ++++ src/kimchi/model/vms.py | 41 +++++++++++++++++++++++++++++++++++++++++ tests/test_model.py | 20 ++++++++++++++++++++ tests/test_rest.py | 28 ++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+)

diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 1a84b6c..4f1f578 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -45,6 +45,8 @@ class VM(Resource): self.reset = self.generate_action_handler('reset') self.connect = self.generate_action_handler('connect') self.clone = self.generate_action_handler_task('clone') + self.suspend = self.generate_action_handler('suspend') + self.resume = self.generate_action_handler('resume')

@property def data(self): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 4eccc3e..79c0b23 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -106,6 +106,10 @@ messages = { "KCHVM0033E": _("Virtual machine '%(name)s' must be stopped before cloning it."), "KCHVM0034E": _("Insufficient disk space to clone virtual machine '%(name)s'"), "KCHVM0035E": _("Unable to clone VM '%(name)s'. Details: %(err)s"), + "KCHVM0036E": _("Cannot suspend VM '%(name)s' because it is not running."), + "KCHVM0037E": _("Unable to suspend VM '%(name)s'. Details: %(err)s"), + "KCHVM0038E": _("Cannot resume VM '%(name)s' because it is not paused."), + "KCHVM0039E": _("Unable to resume VM '%(name)s'. Details: %(err)s"),

"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 5128d1e..55a80e6 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -939,6 +939,47 @@ class VMModel(object): kimchi_log.error('Error trying to delete vm screenshot from ' 'database due error: %s', e.message)

+ def suspend(self, name): + """Suspend the virtual machine's execution and puts it in the + state 'paused'. Use the function "resume" to restore its state. + If the VM is not running, an exception will be raised. + + Parameters: + name -- the name of the VM to be suspended. + """ + vm = self.lookup(name) + if vm['state'] != 'running': + raise InvalidOperation('KCHVM0036E', {'name': name}) + + vir_dom = self.get_vm(name, self.conn) + + try: + vir_dom.suspend() + except libvirt.libvirtError, e: + raise OperationFailed('KCHVM0037E', {'name': name, + 'err': e.message}) + + def resume(self, name): + """Resume the virtual machine's execution and puts it in the + state 'running'. The VM should have been suspended previously by the + function "suspend" and be in the state 'paused', otherwise an exception + will be raised. + + Parameters: + name -- the name of the VM to be resumed. + """ + vm = self.lookup(name) + if vm['state'] != 'paused': + raise InvalidOperation('KCHVM0038E', {'name': name}) + + vir_dom = self.get_vm(name, self.conn) + + try: + vir_dom.resume() + except libvirt.libvirtError, e: + raise OperationFailed('KCHVM0039E', {'name': name, + 'err': e.message}) +

class VMScreenshotModel(object): def __init__(self, **kargs): diff --git a/tests/test_model.py b/tests/test_model.py index c956007..2013f3c 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -187,6 +187,26 @@ class ModelTests(unittest.TestCase): self.assertRaises(NotFoundError, inst.vmsnapshot_delete, u'kimchi-vm', u'foobar')

+ # suspend and resume the VM + info = inst.vm_lookup(u'kimchi-vm') + self.assertEquals(info['state'], 'shutoff') + self.assertRaises(InvalidOperation, inst.vm_suspend, u'kimchi-vm') + inst.vm_start(u'kimchi-vm') + info = inst.vm_lookup(u'kimchi-vm') + self.assertEquals(info['state'], 'running') + inst.vm_suspend(u'kimchi-vm') + info = inst.vm_lookup(u'kimchi-vm') + self.assertEquals(info['state'], 'paused') + self.assertRaises(InvalidParameter, inst.vm_update, u'kimchi-vm', + {'name': 'foo'}) + inst.vm_resume(u'kimchi-vm') + info = inst.vm_lookup(u'kimchi-vm') + self.assertEquals(info['state'], 'running') + self.assertRaises(InvalidOperation, inst.vm_resume, u'kimchi-vm') + # leave the VM suspended to make sure a paused VM can be + # deleted correctly + inst.vm_suspend(u'kimchi-vm') + vms = inst.vms_get_list() self.assertFalse('kimchi-vm' in vms)

diff --git a/tests/test_rest.py b/tests/test_rest.py index 7416463..6ff7cf8 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -460,6 +460,34 @@ class RestTests(unittest.TestCase): '{}', 'DELETE') self.assertEquals(204, resp.status)

+ # Suspend the VM + resp = self.request('/vms/test-vm', '{}', 'GET') + self.assertEquals(200, resp.status) + vm = json.loads(resp.read()) + self.assertEquals(vm['state'], 'shutoff') + resp = self.request('/vms/test-vm/suspend', '{}', 'POST') + self.assertEquals(400, resp.status) + resp = self.request('/vms/test-vm/start', '{}', 'POST') + self.assertEquals(200, resp.status) + resp = self.request('/vms/test-vm', '{}', 'GET') + self.assertEquals(200, resp.status) + vm = json.loads(resp.read()) + self.assertEquals(vm['state'], 'running') + resp = self.request('/vms/test-vm/suspend', '{}', 'POST') + self.assertEquals(200, resp.status) + resp = self.request('/vms/test-vm', '{}', 'GET') + self.assertEquals(200, resp.status) + vm = json.loads(resp.read()) + self.assertEquals(vm['state'], 'paused') + + # Resume the VM + resp = self.request('/vms/test-vm/resume', '{}', 'POST') + self.assertEquals(200, resp.status) + resp = self.request('/vms/test-vm', '{}', 'GET') + self.assertEquals(200, resp.status) + vm = json.loads(resp.read()) + self.assertEquals(vm['state'], 'running') + # Delete the VM resp = self.request('/vms/test-vm', '{}', 'DELETE') self.assertEquals(204, resp.status)