If the user tries to clone a VM while it's already being cloned, the name of the VM being cloned may be reused because Kimchi only avoids name conflicts by looking at the existing VMs, not the ones being created at the moment. When the second clone operation finishes, an error is raised because that name is already defined. Look at the names of the VMs being cloned, as well as the existing VMs', in order to avoid name conflicts when cloning a VM. Signed-off-by: Crístian Viana <vianac(a)linux.vnet.ibm.com&gt; --- src/kimchi/model/vms.py | 19 ++++++++++++++++--- tests/test_model.py | 22 +++++++++++++++------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 3aa1145..9472b1c 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -314,9 +314,22 @@ class VMModel(object): if info['state'] != u'shutoff': raise InvalidParameter('KCHVM0033E', {'name': name}) - # this name will be used as the Task's 'target_uri' so it needs to be - # defined now. - new_name = get_next_clone_name(self.vms.get_list(), name) + # the new VM's name will be used as the Task's 'target_uri' so it needs + # to be defined now. + + vms_being_created = [] + + # lookup names of VMs being created right now + with self.objstore as session: + task_names = session.get_list('task') + for tn in task_names: + t = session.get('task', tn) + if t['target_uri'].startswith('/vms/'): + uri_name = t['target_uri'][5:] # 5 = len('/vms/') + vms_being_created.append(uri_name) + + current_vm_names = self.vms.get_list() + vms_being_created + new_name = get_next_clone_name(current_vm_names, name) # create a task with the actual clone function taskid = add_task(u'/vms/%s' % new_name, self._clone_task, diff --git a/tests/test_model.py b/tests/test_model.py index f1ad017..c8aefd7 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -1374,16 +1374,24 @@ class ModelTests(unittest.TestCase): inst.vm_poweroff(name) rollback.prependDefer(inst.vm_start, name) - task = inst.vm_clone(name) - clone_name = task['target_uri'].split('/')[-1] - rollback.prependDefer(inst.vm_delete, clone_name) - inst.task_wait(task['id']) - task = inst.task_lookup(task['id']) - self.assertEquals('finished', task['status']) + # create two simultaneous clones of the same VM + # and make sure both of them complete successfully + task1 = inst.vm_clone(name) + task2 = inst.vm_clone(name) + clone1_name = task1['target_uri'].split('/')[-1] + rollback.prependDefer(inst.vm_delete, clone1_name) + clone2_name = task2['target_uri'].split('/')[-1] + rollback.prependDefer(inst.vm_delete, clone2_name) + inst.task_wait(task1['id']) + task1 = inst.task_lookup(task1['id']) + self.assertEquals('finished', task1['status']) + inst.task_wait(task2['id']) + task2 = inst.task_lookup(task2['id']) + self.assertEquals('finished', task2['status']) # update the original VM info because its state has changed original_vm = inst.vm_lookup(name) - clone_vm = inst.vm_lookup(clone_name) + clone_vm = inst.vm_lookup(clone1_name) self.assertNotEqual(original_vm['name'], clone_vm['name']) self.assertTrue(re.match(u'%s-clone-\d+' % original_vm['name'], -- 2.1.0