From: Paulo Vital <pvital(a)linux.vnet.ibm.com&gt; Libvirt of the Fedora 23 is not able to delete the storage volume in the testcase test_image_based_template when testing ModelTests. This happens because Libvirt is not able to unlink/rmdir returning an error (-1) and setting errno to ENOENT, causing the error: "cannot unlink file '/var/lib/libvirt/images/base-vol.img': Success" This patch fixes the testcase by manually remove the volume file from the storage pool directory if the wok.plugins.kimchi.model.storagevolume_delete() was not capable to delete the volume. Signed-off-by: Paulo Vital <pvital(a)linux.vnet.ibm.com&gt; --- tests/test_model.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/test_model.py b/tests/test_model.py index 762f0f8..4aa19bd 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -96,6 +96,16 @@ class ModelTests(unittest.TestCase): os.unlink(self.tmp_store) + def temporary_storagevolume_delete(inst, pool, vol): + # Quick fix to passby the Libvirt error: + # "error: cannot unlink file '%(vol)': Success" + # in Fedora <= 23. The Libvirt fix will be present only on Fedora 24 + try: + inst.storagevolume_delete(pool, vol) + except OperationFailed as e: + path = inst.storagevolume_lookup(pool, vol)['path'] + os.remove(path) + def test_vm_info(self): inst = model.Model('test:///default', self.tmp_store) vms = inst.vms_get_list() @@ -250,19 +260,22 @@ class ModelTests(unittest.TestCase): @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_image_based_template(self): + inst = model.Model(objstore_loc=self.tmp_store) with RollbackContext() as rollback: + pool = 'default' vol = 'base-vol.img' params = {'name': vol, 'capacity': 1073741824, # 1 GiB 'allocation': 1048576, # 1 MiB 'format': 'qcow2'} - task_id = inst.storagevolumes_create('default', params)['id'] - rollback.prependDefer(inst.storagevolume_delete, 'default', vol) + task_id = inst.storagevolumes_create(pool, params)['id'] + rollback.prependDefer(temporary_storagevolume_delete, inst, + pool, vol) inst.task_wait(task_id) self.assertEquals('finished', inst.task_lookup(task_id)['status']) - vol_path = inst.storagevolume_lookup('default', vol)['path'] + vol_path = inst.storagevolume_lookup(pool, vol)['path'] # Create template based on IMG file tmpl_name = "img-tmpl" -- 2.5.0

From: Paulo Vital <pvital(a)linux.vnet.ibm.com&gt; 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(a)linux.vnet.ibm.com&gt; --- 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']]) - 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/. + # In addition, if libvirtd is restarted, the name displayed in the + # 'virsh list --all' cmd is the updated name. + 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) -- 2.5.0

On 15/12/2015 10:11, pvital(a)linux.vnet.ibm.com wrote: Does that mean the user will not be able to delete a volume through Kimchi API? DELETE /plugins/kimchi/storagepools/default/<vol> If so, I suggest to move the manual removal to the API. So even though libvirt fails, Kimchi will be able to remove the volume.