From: Zongmei Gou <gouzongmei@ourfuture.cn> This patch is partial for issue #526, in fact for another problem as Cristian mentioned: > There's still a known bug which happens when the user reverts a VM to a snapshot > when the VM's name has been changed since the snapshot was taken.In other words, > the following behavior will still cause a bug: > > - Create a VM named "foo". > - Create a snapshot of VM "foo" named "snap1". > - Rename VM "foo" to "bar". > - Revert VM "bar" to snapshot "snap1". > > The last step works correctly under the hood (i.e. the VM is reverted to that > snapshot) but Kimchi cannot return from its REST request because the VM > changed its name during the revert action. The added test case(test_vm_snapshots) will get passed once Cristian's patch for issue #526 is applied. Please ignore the patch sent by me last time, which contains Cristian's content. --- src/kimchi/control/base.py | 14 +++++++++++--- src/kimchi/model/vmsnapshots.py | 7 ++++++- tests/test_model.py | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 97e789f..484e9b9 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -57,10 +57,18 @@ class Resource(object): self.role_key = None self.admin_methods = [] - def _redirect(self, ident, code=303): - if ident is not None and ident != self.ident: + def _redirect(self, action_result, code=303): + if isinstance(action_result, list): + uri_params = [] + for arg in action_result: + if arg is None: + arg = '' + uri_params.append(urllib2.quote(arg.encode('utf-8'), safe="")) + raise cherrypy.HTTPRedirect(self.uri_fmt % tuple(uri_params), code) + elif action_result is not None and action_result != self.ident: uri_params = list(self.model_args[:-1]) - uri_params += [urllib2.quote(ident.encode('utf-8'), safe="")] + uri_params += [urllib2.quote(action_result.encode('utf-8'), + safe="")] raise cherrypy.HTTPRedirect(self.uri_fmt % tuple(uri_params), code) def generate_action_handler(self, action_name, action_args=None): diff --git a/src/kimchi/model/vmsnapshots.py b/src/kimchi/model/vmsnapshots.py index 725770d..2c9345f 100644 --- a/src/kimchi/model/vmsnapshots.py +++ b/src/kimchi/model/vmsnapshots.py @@ -29,6 +29,7 @@ from kimchi.model.tasks import TaskModel from kimchi.model.vms import DOM_STATE_MAP, VMModel from kimchi.model.vmstorages import VMStorageModel, VMStoragesModel from kimchi.utils import add_task +from kimchi.xmlutils.utils import xpath_get_text class VMSnapshotsModel(object): @@ -155,7 +156,11 @@ class VMSnapshotModel(object): try: vir_dom = VMModel.get_vm(vm_name, self.conn) vir_snap = self.get_vmsnapshot(vm_name, name) - vir_dom.revertToSnapshot(vir_snap, 0) + result = vir_dom.revertToSnapshot(vir_snap, 0) + if result == 0: + xmlObj = vir_snap.getXMLDesc(0) + vm_new_name = xpath_get_text(xmlObj, 'domain/name')[0] + return [vm_new_name, name] except libvirt.libvirtError, e: raise OperationFailed('KCHSNAP0009E', {'name': name, 'vm': vm_name, diff --git a/tests/test_model.py b/tests/test_model.py index c956007..d528b45 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -1074,6 +1074,44 @@ class ModelTests(unittest.TestCase): self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['users']) self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) + def test_vm_snapshots(self): + config.set("authentication", "method", "pam") + inst = model.Model(None, + objstore_loc=self.tmp_store) + + orig_params = {'name': 'test', 'memory': '1024', 'cpus': '1', + 'cdrom': self.kimchi_iso} + inst.templates_create(orig_params) + + with RollbackContext() as rollback: + params = {'name': 'kimchi-vm', 'template': '/templates/test'} + inst.vms_create(params) + rollback.prependDefer(utils.rollback_wrapper, inst.vm_delete, + 'kimchi-vm') + + # create vm snapshot + params_snap = {'name': u'mysnap'} + task = inst.vmsnapshots_create(u'kimchi-vm', params_snap) + rollback.prependDefer(inst.vmsnapshot_delete, + u'kimchi-vm', params_snap['name']) + inst.task_wait(task['id']) + task = inst.task_lookup(task['id']) + self.assertEquals('finished', task['status']) + + # update vm + params = {'name': u'kimchi-vm-new', 'cpus': 2, 'memory': 2048} + inst.vm_update('kimchi-vm', params) + + info = inst.vm_lookup('kimchi-vm-new') + self.assertEquals(2048, info['memory']) + self.assertEquals(2, info['cpus']) + + # snapshot revert + snap = inst.vmsnapshot_lookup(u'kimchi-vm-new', + params_snap['name']) + result = inst.vmsnapshot_revert(u'kimchi-vm-new', snap['name']) + self.assertEquals(result, [u'kimchi-vm', params_snap['name']]) + def test_multithreaded_connection(self): def worker(): for i in xrange(100): -- 1.8.3.1