[Kimchi-devel] [PATCH v3 2/2] issue #543: Generate unique names when creating volumes without name

[Crístian Viana]

If the user creates a volume without setting its name, Kimchi generates a
default value. However, another volume with that generated name may
already exist and the volume creation may fail.

The following commands demonstrate the issue:

$ kimchi_rest /storagepools/default/storagevolumes | grep name
    "name":"60f46fdc-8ecc-4601-90d6-b3b44d35c337-0.img",
    "name":"bdd22e7c-7794-4bdf-8d2b-0089242c8b16-0.img",
    "name":"c16a7e02-6dcf-48c5-83de-1bb2d8a110a8-0.img",

$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}'
{
  "status":"running",
  "message":"OK",
  "id":"2",
  "target_uri":"/storagepools/default/storagevolumes/index.html"
}

$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}'
{
  "reason":"KCHVOL0001E: Storage volume index.html already exists",
  "code":"400 Bad Request",
  "call_stack":"Traceback (most recent call last):\n  File \"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\", line 656, in respond\n    response.body = self.handler()\n  File \"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\", line 188, in __call__\n    self.body = self.oldhandler(*args, **kwargs)\n  File \"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\", line 34, in __call__\n    return self.callable(*self.args, **self.kwargs)\n  File \"/home/vianac/LTC/kimchi/src/kimchi/control/base.py\", line 331, in index\n    raise cherrypy.HTTPError(400, e.message)\nHTTPError: (400, u'KCHVOL0001E: Storage volume index.html already exists')\n"

Generate a unique name when creating a storage volume without a name.

The following commands demonstrate the bug fix:

$ kimchi_rest /storagepools/default/storagevolumes | grep name
    "name":"60f46fdc-8ecc-4601-90d6-b3b44d35c337-0.img",
    "name":"bdd22e7c-7794-4bdf-8d2b-0089242c8b16-0.img",
    "name":"c16a7e02-6dcf-48c5-83de-1bb2d8a110a8-0.img",

$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}'
{
  "status":"running",
  "message":"OK",
  "id":"1",
  "target_uri":"/storagepools/default/storagevolumes/index.html"
}

$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}'
{
  "status":"running",
  "message":"OK",
  "id":"2",
  "target_uri":"/storagepools/default/storagevolumes/index.html (1)"
}

$ kimchi_rest -m POST /storagepools/default/storagevolumes '{"url": "http://www.google.com/index.html"}'
{
  "status":"running",
  "message":"OK",
  "id":"3",
  "target_uri":"/storagepools/default/storagevolumes/index.html (2)"
}

Fix issue #543 ("Download a volume with same basename will result
error").

Signed-off-by: Crístian Viana <vianac at linux.vnet.ibm.com>
---
 src/kimchi/model/storagevolumes.py |  9 +++++++--
 src/kimchi/utils.py                | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
index 406b38b..48f715e 100644
--- a/src/kimchi/model/storagevolumes.py
+++ b/src/kimchi/model/storagevolumes.py
@@ -33,7 +33,8 @@ from kimchi.isoinfo import IsoImage
 from kimchi.model.diskutils import get_disk_ref_cnt
 from kimchi.model.storagepools import StoragePoolModel
 from kimchi.model.tasks import TaskModel
-from kimchi.utils import add_task, get_next_clone_name, kimchi_log
+from kimchi.utils import add_task, get_next_clone_name, get_unique_file_name
+from kimchi.utils import kimchi_log
 from kimchi.xmlutils.utils import xpath_get_text
 
 
@@ -76,6 +77,8 @@ class StorageVolumesModel(object):
             except:
                 raise InvalidParameter('KCHVOL0022E', {'url': url})
 
+        all_vol_names = self.get_list(pool_name)
+
         if name is None:
             # the methods listed in 'REQUIRE_NAME_PARAMS' cannot have
             # 'name' == None
@@ -91,6 +94,8 @@ class StorageVolumesModel(object):
                 name = os.path.basename(params['url'])
             else:
                 name = 'upload-%s' % int(time.time())
+
+            name = get_unique_file_name(all_vol_names, name)
             params['name'] = name
 
         try:
@@ -106,7 +111,7 @@ class StorageVolumesModel(object):
         if pool_info['state'] == 'inactive':
             raise InvalidParameter('KCHVOL0003E', {'pool': pool_name,
                                                    'volume': name})
-        if name in self.get_list(pool_name):
+        if name in all_vol_names:
             raise InvalidParameter('KCHVOL0001E', {'name': name})
 
         params['pool'] = pool_name
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
index 3af701c..6acfa42 100644
--- a/src/kimchi/utils.py
+++ b/src/kimchi/utils.py
@@ -351,3 +351,38 @@ def get_next_clone_name(all_names, basename, name_suffix=''):
         new_name = new_name + name_suffix
 
     return new_name
+
+
+def get_unique_file_name(all_names, name):
+    """Find the next available, unique name for a file.
+
+    If a file named "<name>" isn't found in "<all_names>", use that same
+    "<name>".  There's no need to generate a new name in that case.
+
+    If any file named "<name> (<number>)" is found in "all_names", use the
+    maximum "number" + 1; else, use 1.
+
+    Arguments:
+    all_names -- All existing file names. This list will be used to make sure
+        the new name won't conflict with existing names.
+    name -- The name of the original file.
+
+    Return:
+    A string in the format "<name> (<number>)", or "<name>".
+    """
+    if name not in all_names:
+        return name
+
+    re_group_num = 'num'
+
+    re_expr = u'%s \((?P<%s>\d+)\)' % (name, re_group_num)
+
+    max_num = 0
+    re_compiled = re.compile(re_expr)
+
+    for n in all_names:
+        match = re_compiled.match(n)
+        if match is not None:
+            max_num = max(max_num, int(match.group(re_group_num)))
+
+    return u'%s (%d)' % (name, max_num + 1)
-- 
1.9.3