[Kimchi-devel] [PATCH] <Fix problems of edit a guest contains snapshots>

[Aline Manera]

The patch seems good for me.
I will just wait for a reply from Cristian as he was working on it.

On 20/12/2014 04:26, gouzongmei at ourfuture.cn wrote:
> From: Zongmei Gou <gouzongmei at ourfuture.cn>
>
> ---
>   src/kimchi/control/base.py      |   13 ++++++++++---
>   src/kimchi/model/vms.py         |   24 ++++++++++++++++++------
>   src/kimchi/model/vmsnapshots.py |    8 ++++++--
>   src/kimchi/xmlutils/utils.py    |    5 +++++
>   4 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
> index 60db1df..ff31d47 100644
> --- a/src/kimchi/control/base.py
> +++ b/src/kimchi/control/base.py
> @@ -58,10 +58,17 @@ 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/vms.py b/src/kimchi/model/vms.py
> index 3aa1145..0caa1fd 100644
> --- a/src/kimchi/model/vms.py
> +++ b/src/kimchi/model/vms.py
> @@ -668,22 +668,34 @@ class VMModel(object):
>               new_xml = self._update_graphics(dom, new_xml, params)
>
>           conn = self.conn.get()
> +        snapshot_xmls = []
>           try:
>               if 'name' in params:
> -                if state == 'running':
> -                    msg_args = {'name': dom.name(), 'new_name': params['name']}
> -                    raise InvalidParameter("KCHVM0003E", msg_args)
> -
> -                # Undefine old vm, only if name is going to change
> -                dom.undefine()
> +                new_name = params['name']
> +                name = dom.name()
> +                if new_name != name:
> +                    if state == 'running':
> +                        raise InvalidParameter("KCHVM0003E", {'name': name, 'new_name': new_name})
> +
> +                    # get snapshots before vm undefine, original snapshots will be created in the new named vm
> +                    snapshot_names = dom.snapshotListNames(0)
> +                    for snapshot_name in snapshot_names:
> +                        vir_snap = self.vmsnapshot.get_vmsnapshot(name, snapshot_name)
> +                        snapshot_xmls.append(vir_snap.getXMLDesc(0))
> +                    # Undefine old vm, only if name is going to change
> +                    dom.undefineFlags(libvirt.VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA)
>
>               root = ET.fromstring(new_xml)
>               currentMem = root.find('.currentMemory')
>               if currentMem is not None:
>                   root.remove(currentMem)
>               dom = conn.defineXML(ET.tostring(root, encoding="utf-8"))
> +            for snapshot_xml in snapshot_xmls:
> +                dom.snapshotCreateXML(snapshot_xml, 1)
>           except libvirt.libvirtError as e:
>               dom = conn.defineXML(old_xml)
> +            for snapshot_xml in snapshot_xmls:
> +                dom.snapshotCreateXML(snapshot_xml, 1)
>               raise OperationFailed("KCHVM0008E", {'name': dom.name(),
>                                                    'err': e.get_error_message()})
>           return dom
> diff --git a/src/kimchi/model/vmsnapshots.py b/src/kimchi/model/vmsnapshots.py
> index 725770d..569c7ad 100644
> --- a/src/kimchi/model/vmsnapshots.py
> +++ b/src/kimchi/model/vmsnapshots.py
> @@ -29,7 +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 xml_item_get
>
>   class VMSnapshotsModel(object):
>       def __init__(self, **kargs):
> @@ -155,7 +155,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)
> +            ret = vir_dom.revertToSnapshot(vir_snap, 0)
> +            if ret == 0:
> +                xmlObj = vir_snap.getXMLDesc(0)
> +                new_vmname = xml_item_get(xmlObj, 'domain/name')
> +                return [new_vmname, name]
>           except libvirt.libvirtError, e:
>               raise OperationFailed('KCHSNAP0009E', {'name': name,
>                                                      'vm': vm_name,
> diff --git a/src/kimchi/xmlutils/utils.py b/src/kimchi/xmlutils/utils.py
> index be08a14..843a113 100644
> --- a/src/kimchi/xmlutils/utils.py
> +++ b/src/kimchi/xmlutils/utils.py
> @@ -65,3 +65,8 @@ def _dictize(e):
>           else:
>               d[child.tag] = _dictize(child)
>       return d
> +
> +def xml_item_get(xml, xpath):
> +    root = ET.fromstring(xml)
> +    item = root.find(xpath)
> +    return item.text