[Kimchi-devel] [PATCH] Re: Solve the snapshot revert problem, relate to issue #526

[Aline Manera]

On 25/01/2015 23:31, gouzongmei at 163.com wrote:
> From: Zongmei Gou <gouzongmei at ourfuture.cn>
>
> Hi Aline,
>
> This is the right patch I sent to Kimchi ML.
>
> In fact I added you to CC list every time, but I have no idea why you can't
> receive my patch sometimes. So I use this email this time.

Now, I got it. =)


After applying this patch, I got the following error while running "make 
check"

.....
======================================================================
ERROR: test_vm_lifecycle (test_model.ModelTests)
----------------------------------------------------------------------
Traceback (most recent call last):
   File "test_model.py", line 180, in test_vm_lifecycle
     inst.vm_update('kimchi-vm', {'name': u'kimchi-vm-new'})
   File "/home/alinefm/kimchi/src/kimchi/model/vms.py", line 281, in update
     dom = self._static_vm_update(dom, params)
   File "/home/alinefm/kimchi/src/kimchi/model/vms.py", line 701, in 
_static_vm_update
     'err': e.get_error_message()})
OperationFailed: KCHVM0008E: Unable to update virtual machine kimchi-vm. 
Details: Requested operation is not valid: cannot delete inactive domain 
with 2 snapshots


> ---
>   src/kimchi/control/base.py      | 14 +++++++++++---
>   src/kimchi/model/vmsnapshots.py |  6 ++++++
>   tests/test_model.py             | 12 ++++++++++--
>   3 files changed, 27 insertions(+), 5 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..0c5b601 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):
> @@ -156,6 +157,11 @@ class VMSnapshotModel(object):
>               vir_dom = VMModel.get_vm(vm_name, self.conn)
>               vir_snap = self.get_vmsnapshot(vm_name, name)
>               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:
>               raise OperationFailed('KCHSNAP0009E', {'name': name,
>                                                      'vm': vm_name,
> diff --git a/tests/test_model.py b/tests/test_model.py
> index c956007..b9d71cf 100644
> --- a/tests/test_model.py
> +++ b/tests/test_model.py
> @@ -176,8 +176,16 @@ class ModelTests(unittest.TestCase):
>               current_snap = inst.currentvmsnapshot_lookup(u'kimchi-vm')
>               self.assertEquals(snap, current_snap)
>
> -            snap = inst.vmsnapshot_lookup(u'kimchi-vm', params['name'])
> -            inst.vmsnapshot_revert(u'kimchi-vm', params['name'])
> +            # update vm name
> +            inst.vm_update('kimchi-vm', {'name': u'kimchi-vm-new'})
> +
> +            # Look up the first created snapshot from the renamed vm
> +            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']])
> +
>               vm = inst.vm_lookup(u'kimchi-vm')
>               self.assertEquals(vm['state'], snap['state'])
>