[PATCH V4] [Kimchi 0/3] Fix VM name conflicts with snapshot reverts
Changes in V4: * Fixed test_rest Lucio Correia (3): Always update snapshot XML with new name and UUID Use ASCII name in XML Update tests to reflect new behavior model/vms.py | 14 ++++++++++++-- tests/test_model.py | 41 ++++++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 23 deletions(-) -- 1.9.1
When a guest with snapshots is renamed, and afterwards guest is reverted to a snapshot created before the name change, the guest is also renamed back. This patch fixes this issue by updating snapshot XML when guest is renamed. Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- model/vms.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/model/vms.py b/model/vms.py index b0b4eb2..f68e262 100644 --- a/model/vms.py +++ b/model/vms.py @@ -97,6 +97,8 @@ XPATH_DOMAIN_DEV_CPU_ID = '/domain/devices/spapr-cpu-socket/@id' XPATH_CPU = './cpu' XPATH_NAME = './name' XPATH_NUMA_CELL = './cpu/numa/cell' +XPATH_SNAP_VM_NAME = './domain/name' +XPATH_SNAP_VM_UUID = './domain/uuid' XPATH_TOPOLOGY = './cpu/topology' XPATH_VCPU = './vcpu' XPATH_MAX_MEMORY = './maxMemory' @@ -728,7 +730,15 @@ class VMModel(object): if info['current']: flags |= libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT - dom.snapshotCreateXML(info['xml'], flags) + # Snapshot XML contains the VM xml from the time it was created. + # Thus VM name and uuid must be updated to current ones. Otherwise, + # when reverted, the vm name will be inconsistent. + name = dom.name().decode('utf-8') + uuid = dom.UUIDString().decode('utf-8') + xml = xml_item_update(info['xml'], XPATH_SNAP_VM_NAME, name, None) + xml = xml_item_update(xml, XPATH_SNAP_VM_UUID, uuid, None) + + dom.snapshotCreateXML(xml, flags) def _update_metadata_name(self, dom, nonascii_name): if nonascii_name is not None: -- 1.9.1
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- model/vms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/vms.py b/model/vms.py index f68e262..30e49df 100644 --- a/model/vms.py +++ b/model/vms.py @@ -755,7 +755,7 @@ class VMModel(object): nonascii_name = None if name is not None: - params['name'], nonascii_name = get_ascii_nonascii_name(name) + name, nonascii_name = get_ascii_nonascii_name(name) new_xml = xml_item_update(new_xml, XPATH_NAME, name, None) # Update CPU info -- 1.9.1
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- tests/test_model.py | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/tests/test_model.py b/tests/test_model.py index f4a145f..9a6a1aa 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -159,7 +159,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']) @@ -206,7 +206,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,39 +227,38 @@ class ModelTests(unittest.TestCase): # snapshot revert to the first created vm result = inst.vmsnapshot_revert(u'kimchi-vm-new', params['name']) - self.assertEquals(result, [u'kimchi-vm', snap['name']]) + self.assertEquals(result, ['kimchi-vm-new', snap['name']]) - vm = inst.vm_lookup(u'kimchi-vm') + 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') - # 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('kimchi-vm-new') vms = inst.vms_get_list() - self.assertFalse('kimchi-vm' in vms) + self.assertFalse('kimchi-vm-new' in vms) @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_image_based_template(self): -- 1.9.1
On 06/14/2016 05:40 PM, Lucio Correia wrote:
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- tests/test_model.py | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py index f4a145f..9a6a1aa 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -159,7 +159,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']) @@ -206,7 +206,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,39 +227,38 @@ class ModelTests(unittest.TestCase):
# snapshot revert to the first created vm result = inst.vmsnapshot_revert(u'kimchi-vm-new', params['name']) - self.assertEquals(result, [u'kimchi-vm', snap['name']]) + self.assertEquals(result, ['kimchi-vm-new', snap['name']])
- vm = inst.vm_lookup(u'kimchi-vm') + 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') -
Why did you remove the test above?
# 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('kimchi-vm-new')
vms = inst.vms_get_list() - self.assertFalse('kimchi-vm' in vms) + self.assertFalse('kimchi-vm-new' in vms)
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_image_based_template(self):
Reviewed-by: Daniel Barboza <dhbarboza82@gmail.com> On 06/14/2016 05:40 PM, Lucio Correia wrote:
Changes in V4: * Fixed test_rest
Lucio Correia (3): Always update snapshot XML with new name and UUID Use ASCII name in XML Update tests to reflect new behavior
model/vms.py | 14 ++++++++++++-- tests/test_model.py | 41 ++++++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 23 deletions(-)
Hi Lucio, There is still one test failing: ***** Running unit test: test_model... FAILED ====================================================================== ERROR: test_vm_memory_hotplug (test_model.ModelTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "test_model.py", line 816, in test_vm_memory_hotplug inst.task_wait(task1['id']) File "/home/alinefm/wok/src/wok/model/tasks.py", line 66, in wait 'task': task['target_uri']}) TimeoutExpired: WOKASYNC0003E: WOKASYNC0003E ---------------------------------------------------------------------- Ran 25 tests in 169.177s FAILED (errors=1, skipped=1) On 06/14/2016 05:40 PM, Lucio Correia wrote:
Changes in V4: * Fixed test_rest
Lucio Correia (3): Always update snapshot XML with new name and UUID Use ASCII name in XML Update tests to reflect new behavior
model/vms.py | 14 ++++++++++++-- tests/test_model.py | 41 ++++++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 23 deletions(-)
Nevermind! I ran it again and all tests passed \o/ There was some leftovers in my system which as causing conflicts with the tests. On 06/20/2016 11:31 AM, Aline Manera wrote:
Hi Lucio,
There is still one test failing:
***** Running unit test: test_model... FAILED ====================================================================== ERROR: test_vm_memory_hotplug (test_model.ModelTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "test_model.py", line 816, in test_vm_memory_hotplug inst.task_wait(task1['id']) File "/home/alinefm/wok/src/wok/model/tasks.py", line 66, in wait 'task': task['target_uri']}) TimeoutExpired: WOKASYNC0003E: WOKASYNC0003E
---------------------------------------------------------------------- Ran 25 tests in 169.177s
FAILED (errors=1, skipped=1)
On 06/14/2016 05:40 PM, Lucio Correia wrote:
Changes in V4: * Fixed test_rest
Lucio Correia (3): Always update snapshot XML with new name and UUID Use ASCII name in XML Update tests to reflect new behavior
model/vms.py | 14 ++++++++++++-- tests/test_model.py | 41 ++++++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 23 deletions(-)
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
participants (3)
-
Aline Manera -
Daniel Henrique Barboza -
Lucio Correia