[Kimchi-devel] [PATCH] issue #526: Support updating name for VMs with snapshots

[Crístian Viana]

In order to update the name, the corresponding VM must be removed and
then added again. And a VM cannot be removed if it has snapshots. So, a
VM with snapshots fails to have its name changed.

Before removing a VM, a backup of the existing snapshots will be taken
so they can be recreated when the VM is added again.

Fix issue #526 ("Cannot edit VM with snapshots").
---
 src/kimchi/model/vms.py | 56 +++++++++++++++++++++++++++++++++++++++++++++++--
 tests/test_model.py     |  3 +++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
index 3aa1145..463c350 100644
--- a/src/kimchi/model/vms.py
+++ b/src/kimchi/model/vms.py
@@ -649,6 +649,39 @@ class VMModel(object):
                               libvirt.VIR_DOMAIN_AFFECT_LIVE)
         return xml
 
+    def _backup_snapshots(self, snap, all_info):
+        """ Append "snap" and the children of "snap" to the list "all_info".
+
+        The list *must* always contain the parent snapshots before their
+        children so the function "_redefine_snapshots" can work correctly.
+
+        Arguments:
+        snap -- a native domain snapshot.
+        all_info -- a list of dict keys:
+                "{'xml': <snap XML>, 'current': <is snap current?>'}"
+        """
+        all_info.append({'xml': snap.getXMLDesc(0),
+                         'current': snap.isCurrent(0)})
+
+        for child in snap.listAllChildren(0):
+            self._backup_snapshots(child, all_info)
+
+    def _redefine_snapshots(self, dom, all_info):
+        """ Restore the snapshots stored in "all_info" to the domain "dom".
+
+        Arguments:
+        dom -- the domain which will have its snapshots restored.
+        all_info -- a list of dict keys, as described in "_backup_snapshots",
+            containing the original snapshot information.
+        """
+        for info in all_info:
+            flags = libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE
+
+            if info['current']:
+                flags |= libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT
+
+            dom.snapshotCreateXML(info['xml'], flags)
+
     def _static_vm_update(self, dom, params):
         state = DOM_STATE_MAP[dom.info()[0]]
         old_xml = new_xml = dom.XMLDesc(0)
@@ -667,13 +700,26 @@ class VMModel(object):
         if 'graphics' in params:
             new_xml = self._update_graphics(dom, new_xml, params)
 
+        snapshots_info = []
+        vm_name = dom.name()
         conn = self.conn.get()
         try:
             if 'name' in params:
                 if state == 'running':
-                    msg_args = {'name': dom.name(), 'new_name': params['name']}
+                    msg_args = {'name': vm_name, 'new_name': params['name']}
                     raise InvalidParameter("KCHVM0003E", msg_args)
 
+                lflags = libvirt.VIR_DOMAIN_SNAPSHOT_LIST_ROOTS
+                dflags = (libvirt.VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
+                          libvirt.VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY)
+
+                for virt_snap in dom.listAllSnapshots(lflags):
+                    snapshots_info.append({'xml': virt_snap.getXMLDesc(0),
+                                           'current': virt_snap.isCurrent(0)})
+                    self._backup_snapshots(virt_snap, snapshots_info)
+
+                    virt_snap.delete(dflags)
+
                 # Undefine old vm, only if name is going to change
                 dom.undefine()
 
@@ -681,10 +727,16 @@ class VMModel(object):
             currentMem = root.find('.currentMemory')
             if currentMem is not None:
                 root.remove(currentMem)
+
             dom = conn.defineXML(ET.tostring(root, encoding="utf-8"))
+            if 'name' in params:
+                self._redefine_snapshots(dom, snapshots_info)
         except libvirt.libvirtError as e:
             dom = conn.defineXML(old_xml)
-            raise OperationFailed("KCHVM0008E", {'name': dom.name(),
+            if 'name' in params:
+                self._redefine_snapshots(dom, snapshots_info)
+
+            raise OperationFailed("KCHVM0008E", {'name': vm_name,
                                                  'err': e.get_error_message()})
         return dom
 
diff --git a/tests/test_model.py b/tests/test_model.py
index 0820386..b77545c 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -983,6 +983,9 @@ class ModelTests(unittest.TestCase):
             vms = inst.vms_get_list()
             self.assertTrue('kimchi-vm1' in vms)
 
+            # make sure "vm_update" works when the domain has a snapshot
+            inst.vmsnapshots_create(u'kimchi-vm1')
+
             # update vm graphics when vm is not running
             inst.vm_update(u'kimchi-vm1',
                            {"graphics": {"passwd": "123456"}})
-- 
2.1.0