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> --- mockmodel.py | 7 +++++++ tests/test_model.py | 41 ++++++++++++++++++++--------------------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/mockmodel.py b/mockmodel.py index 9f9de52..2bcb6f0 100644 --- a/mockmodel.py +++ b/mockmodel.py @@ -386,6 +386,13 @@ class MockModel(Model): MockModel._mock_snapshots[new_name] = snapshots return self._model_vm_clone(name) + def _mock_vm_update(self, name, params): + self._model_vm_update(name, params) + if 'name' in params: + snaps = copy.deepcopy(MockModel._mock_snapshots[name]) + MockModel._mock_snapshots[params['name']] = snaps + del MockModel._mock_snapshots[name] + def _mock_vmsnapshots_create(self, vm_name, params): name = params.get('name', unicode(int(time.time()))) params = {'vm_name': vm_name, 'name': name} 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