[Kimchi-devel] [PATCH] [Kimchi 3/3] fix 2 test_vm_lifecycle testcase

[Aline Manera]

On 15/12/2015 10:11, pvital at linux.vnet.ibm.com wrote:
> From: Paulo Vital <pvital at 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 at 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)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20151216/bf7fde3a/attachment.html>