[Kimchi-devel] [PATCH v2] Do not reuse names when cloning the same VM more than once at the same time

[Crístian Viana]

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 at linux.vnet.ibm.com>
---
 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