On 15/12/2015 10:11, pvital@linux.vnet.ibm.com wrote:
From: Paulo Vital <pvital@linux.vnet.ibm.com>

The vmsnapshot_revert() updates the name in the VM XML to the previous VM name
(kimchi-vm), but the domain name used by libvirt to lookup this VM/domain, is
the updated name (kimchi-vm-new) as proved here:
https://paste.fedoraproject.org/296617/49064913/. In addition, if libvirtd is
restarted, the name displayed in the  'virsh list --all' command is the updated
name.

This patch changed the name of the VM used to test the lifecycle.

Signed-off-by: Paulo Vital <pvital@linux.vnet.ibm.com>
---
 tests/test_model.py | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/tests/test_model.py b/tests/test_model.py
index ff63182..aebd787 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -157,7 +157,7 @@ class ModelTests(unittest.TestCase):
             params = {'name': 'kimchi-vm',
                       'template': '/plugins/kimchi/templates/test'}
             task = inst.vms_create(params)
-            rollback.prependDefer(inst.vm_delete, 'kimchi-vm')
+            rollback.prependDefer(inst.vm_delete, 'kimchi-vm-new')
             inst.task_wait(task['id'], 10)
             task = inst.task_lookup(task['id'])
             self.assertEquals('finished', task['status'])
@@ -204,7 +204,7 @@ class ModelTests(unittest.TestCase):
             task = inst.vmsnapshots_create(u'kimchi-vm')
             snap_name = task['target_uri'].split('/')[-1]
             rollback.prependDefer(inst.vmsnapshot_delete,
-                                  u'kimchi-vm', snap_name)
+                                  u'kimchi-vm-new', snap_name)
             inst.task_wait(task['id'])
             task = inst.task_lookup(task['id'])
             self.assertEquals('finished', task['status'])
@@ -227,34 +227,42 @@ class ModelTests(unittest.TestCase):

             result = inst.vmsnapshot_revert(u'kimchi-vm-new', params['name'])
             self.assertEquals(result, [u'kimchi-vm', snap['name']])

Usually we don't know when reverting a snapshot will lead on a VM name update.

For that, the revert() function returns a list which contains the new VM name:

    def revert(self, vm_name, name):                                           
        try:                                                                   
            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,              
                                                   'err': e.message})


I strongly recommend to get the 'result' value and use the expected name in the operations below.

That way we make sure the flow is also working when doing it via UI.

-            vm = inst.vm_lookup(u'kimchi-vm')
+            # The vmsnapshot_revert() updates the name in the VM XML to the
+            # previous VM name (kimchi-vm), but the domain name used by libvirt
+            # to lookup this VM/domain, is the updated name (kimchi-vm-new) as
+            # proved here: https://paste.fedoraproject.org/296617/49064913/.

What do we need to do that to have libvirt working in the real VM name after reverting the snapshot?

+            # In addition, if libvirtd is restarted, the name displayed in the
+            # 'virsh list --all' cmd is the updated name.

So the conclusion is "Reverting a snapshot WILL NEVER update the vm name". Is that correct?

What is the actual Kimchi behavior while performing those action via UI?

+            vm = inst.vm_lookup(u'kimchi-vm-new')
             self.assertEquals(vm['state'], snap['state'])

-            current_snap = inst.currentvmsnapshot_lookup(u'kimchi-vm')
+            current_snap = inst.currentvmsnapshot_lookup(u'kimchi-vm-new')
             self.assertEquals(params['name'], current_snap['name'])

             self.assertRaises(NotFoundError, inst.vmsnapshot_delete,
-                              u'kimchi-vm', u'foobar')
+                              u'kimchi-vm-new', u'foobar')

             # suspend and resume the VM
-            info = inst.vm_lookup(u'kimchi-vm')
+            info = inst.vm_lookup(u'kimchi-vm-new')
             self.assertEquals(info['state'], 'shutoff')
-            self.assertRaises(InvalidOperation, inst.vm_suspend, u'kimchi-vm')
-            inst.vm_start(u'kimchi-vm')
-            info = inst.vm_lookup(u'kimchi-vm')
+            self.assertRaises(InvalidOperation, inst.vm_suspend,
+                              u'kimchi-vm-new')
+            inst.vm_start(u'kimchi-vm-new')
+            info = inst.vm_lookup(u'kimchi-vm-new')
             self.assertEquals(info['state'], 'running')
-            inst.vm_suspend(u'kimchi-vm')
-            info = inst.vm_lookup(u'kimchi-vm')
+            inst.vm_suspend(u'kimchi-vm-new')
+            info = inst.vm_lookup(u'kimchi-vm-new')
             self.assertEquals(info['state'], 'paused')
-            self.assertRaises(InvalidParameter, inst.vm_update, u'kimchi-vm',
-                              {'name': 'foo'})
-            inst.vm_resume(u'kimchi-vm')
-            info = inst.vm_lookup(u'kimchi-vm')
+            self.assertRaises(InvalidParameter, inst.vm_update,
+                              u'kimchi-vm-new', {'name': 'foo'})
+            inst.vm_resume(u'kimchi-vm-new')
+            info = inst.vm_lookup(u'kimchi-vm-new')
             self.assertEquals(info['state'], 'running')
-            self.assertRaises(InvalidOperation, inst.vm_resume, u'kimchi-vm')
+            self.assertRaises(InvalidOperation, inst.vm_resume,
+                              u'kimchi-vm-new')
             # leave the VM suspended to make sure a paused VM can be
             # deleted correctly
-            inst.vm_suspend(u'kimchi-vm')
+            inst.vm_suspend(u'kimchi-vm-new')

         vms = inst.vms_get_list()
         self.assertFalse('kimchi-vm' in vms)