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@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