[Kimchi-devel] [PATCH 2/3] snapshot: Change the action 'revert' to a Task

Crístian Deives cristiandeives at gmail.com
Mon Apr 20 18:58:03 UTC 2015


The action 'revert' is currently synchronous; that means that there's no
way for the user to run it in background and check its status
periodically. In some cases, reverting a snapshot can take a a few
seconds/minutes to finish, so the best option is to change it to an
asynchronous call.

Change the implementation of the action 'revert' so it returns an
asynchronous Task.

Signed-off-by: Crístian Deives <cristiandeives at gmail.com>
---
 src/kimchi/control/vm/snapshots.py |  2 +-
 src/kimchi/mockmodel.py            | 11 +++++++++++
 src/kimchi/model/vmsnapshots.py    | 28 ++++++++++++++++++++--------
 tests/test_model.py                |  6 ++++--
 tests/test_rest.py                 |  9 +++++++++
 5 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/src/kimchi/control/vm/snapshots.py b/src/kimchi/control/vm/snapshots.py
index bbebc9a..4847d59 100644
--- a/src/kimchi/control/vm/snapshots.py
+++ b/src/kimchi/control/vm/snapshots.py
@@ -39,7 +39,7 @@ class VMSnapshot(Resource):
         self.ident = ident
         self.model_args = [self.vm, self.ident]
         self.uri_fmt = '/vms/%s/snapshots/%s'
-        self.revert = self.generate_action_handler('revert')
+        self.revert = self.generate_action_handler_task('revert')
 
     @property
     def data(self):
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index dcb8bc1..286f903 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -431,6 +431,15 @@ class MockModel(Model):
         MockModel._mock_snapshots[vm_name] = snapshots
 
     def _mock_vmsnapshot_revert(self, vm_name, name):
+        params = {'vm_name': vm_name, 'name': name}
+        taskid = add_task(u'/vms/%s/snapshots/%s/revert' % (vm_name, name),
+                          self._vmsnapshot_revert_task, self.objstore, params)
+        return self.task_lookup(taskid)
+
+    def _vmsnapshot_revert_task(self, cb, params):
+        vm_name = params['vm_name']
+        name = params['name']
+
         snapshots = MockModel._mock_snapshots.get(vm_name, [])
         for sn in snapshots:
             if sn.current:
@@ -440,6 +449,8 @@ class MockModel(Model):
             if sn.name == name:
                 sn.current = True
 
+        cb('OK', True)
+
 
 class MockStorageVolumes(object):
     def __init__(self):
diff --git a/src/kimchi/model/vmsnapshots.py b/src/kimchi/model/vmsnapshots.py
index 3a92cdc..331f532 100644
--- a/src/kimchi/model/vmsnapshots.py
+++ b/src/kimchi/model/vmsnapshots.py
@@ -29,7 +29,6 @@ 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):
@@ -120,6 +119,8 @@ class VMSnapshotsModel(object):
 class VMSnapshotModel(object):
     def __init__(self, **kargs):
         self.conn = kargs['conn']
+        self.objstore = kargs['objstore']
+        self.task = TaskModel(**kargs)
 
     def lookup(self, vm_name, name):
         vir_snap = self.get_vmsnapshot(vm_name, name)
@@ -153,19 +154,30 @@ class VMSnapshotModel(object):
                                                    'err': e.message})
 
     def revert(self, vm_name, name):
+        vir_dom = VMModel.get_vm(vm_name, self.conn)
+        vir_snap = self.get_vmsnapshot(vm_name, name)
+
+        task_params = {'name': name, 'vm_name': vm_name,
+                       'dom': vir_dom, 'snap': vir_snap}
+        taskid = add_task(u'/vms/%s/snapshots/%s/revert' % (vm_name, name),
+                          self._revert_task, self.objstore, task_params)
+        return self.task.lookup(taskid)
+
+    def _revert_task(self, cb, params):
+        vir_dom = params['dom']
+        vir_snap = params['snap']
+
         try:
-            vir_dom = VMModel.get_vm(vm_name, self.conn)
-            vir_snap = self.get_vmsnapshot(vm_name, name)
+            cb('reverting to snapshot')
             vir_dom.revertToSnapshot(vir_snap, 0)
-
-            # get vm name recorded in the snapshot and return new uri params
-            vm_new_name = xpath_get_text(vir_snap.getXMLDesc(0),
-                                         'domain/name')[0]
-            return [vm_new_name, name]
         except libvirt.libvirtError, e:
+            name = params['name']
+            vm_name = params['vm_name']
+
             raise OperationFailed('KCHSNAP0009E', {'name': name,
                                                    'vm': vm_name,
                                                    'err': e.message})
+        cb('OK', True)
 
     def get_vmsnapshot(self, vm_name, name):
         vir_dom = VMModel.get_vm(vm_name, self.conn)
diff --git a/tests/test_model.py b/tests/test_model.py
index c25929f..b53822a 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -198,8 +198,10 @@ class ModelTests(unittest.TestCase):
             snap = inst.vmsnapshot_lookup(u'kimchi-vm-new', params['name'])
 
             # snapshot revert to the first created vm
-            result = inst.vmsnapshot_revert(u'kimchi-vm-new', params['name'])
-            self.assertEquals(result, [u'kimchi-vm', snap['name']])
+            task = inst.vmsnapshot_revert(u'kimchi-vm-new', params['name'])
+            inst.task_wait(task['id'])
+            task = inst.task_lookup(task['id'])
+            self.assertEquals('finished', task['status'])
 
             vm = inst.vm_lookup(u'kimchi-vm')
             self.assertEquals(vm['state'], snap['state'])
diff --git a/tests/test_rest.py b/tests/test_rest.py
index c1b81e8..3b592d6 100644
--- a/tests/test_rest.py
+++ b/tests/test_rest.py
@@ -382,6 +382,15 @@ class RestTests(unittest.TestCase):
         # Revert to snapshot
         resp = self.request('/vms/test-vm/snapshots/%s/revert' %
                             params['name'], '{}', 'POST')
+        self.assertEquals(202, resp.status)
+        task = json.loads(resp.read())
+        wait_task(self._task_lookup, task['id'])
+        resp = self.request('/tasks/%s' % task['id'], '{}', 'GET')
+        task = json.loads(resp.read())
+        self.assertEquals('finished', task['status'])
+
+        resp = self.request('/vms/test-vm/snapshots/%s' % params['name'],
+                            '{}', 'GET')
         self.assertEquals(200, resp.status)
         snap = json.loads(resp.read())
         resp = self.request('/vms/test-vm', '{}', 'GET')
-- 
2.1.0




More information about the Kimchi-devel mailing list