In order to add a volume to a storage pool, it must not be one of a read-only type, it must be active and the volume name must not already exist. That verification should happen regardless of the storage volume creation method (i.e. download, upload). Add the verifications mentioned above when creating any type of volume by adding them to the common "create" function. Also, fix a test case which was broken (commit ff1bb81) because a storage volume with an existing name was being added. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 21 +++++++++++++-------- src/kimchi/model/storagevolumes.py | 17 +++++++++++++---- tests/test_rest.py | 2 +- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 9fab036..a772f50 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -43,7 +43,7 @@ except ImportError: from kimchi import config from kimchi.asynctask import AsyncTask -from kimchi.config import config as kconfig +from kimchi.config import READONLY_POOL_TYPE, config as kconfig from kimchi.distroloader import DistroLoader from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -480,6 +480,8 @@ class MockModel(object): def storagevolumes_create(self, pool_name, params): vol_source = ['file', 'url', 'capacity'] + name = params['name'] + index_list = list(i for i in range(len(vol_source)) if vol_source[i] in params) if len(index_list) != 1: @@ -492,6 +494,16 @@ class MockModel(object): except AttributeError: raise InvalidParameter("KCHVOL0019E", {'param': vol_source[index_list[0]]}) + + pool = self._get_storagepool(pool_name) + if pool.info['type'] in READONLY_POOL_TYPE: + raise InvalidParameter("KCHVOL0012E", {'type': pool.info['type']}) + if pool.info['state'] == 'inactive': + raise InvalidParameter('KCHVOL0003E', {'pool': pool_name, + 'volume': name}) + if name in pool._volumes: + raise InvalidOperation("KCHVOL0001E", {'name': name}) + params['pool'] = pool_name taskid = self.add_task('', create_func, params) return self.task_lookup(taskid) @@ -499,10 +511,6 @@ class MockModel(object): def _create_volume_with_capacity(self, cb, params): pool_name = params.pop('pool') pool = self._get_storagepool(pool_name) - if pool.info['state'] == 'inactive': - raise InvalidOperation("KCHVOL0003E", - {'pool': pool_name, - 'volume': params['name']}) try: name = params['name'] @@ -518,9 +526,6 @@ class MockModel(object): raise MissingParameter("KCHVOL0004E", {'item': str(item), 'volume': name}) - if name in pool._volumes: - raise InvalidOperation("KCHVOL0001E", {'name': name}) - pool._volumes[name] = volume cb('OK', True) diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py index 3062e78..fac34d5 100644 --- a/src/kimchi/model/storagevolumes.py +++ b/src/kimchi/model/storagevolumes.py @@ -53,6 +53,8 @@ class StorageVolumesModel(object): def create(self, pool_name, params): vol_source = ['file', 'url', 'capacity'] + name = params['name'] + index_list = list(i for i in range(len(vol_source)) if vol_source[i] in params) if len(index_list) != 1: @@ -65,6 +67,17 @@ class StorageVolumesModel(object): except AttributeError: raise InvalidParameter("KCHVOL0019E", {'param': vol_source[index_list[0]]}) + + pool_info = StoragePoolModel(conn=self.conn, + objstore=self.objstore).lookup(pool_name) + if pool_info['type'] in READONLY_POOL_TYPE: + raise InvalidParameter("KCHVOL0012E", {'type': pool_info['type']}) + if pool_info['state'] == 'inactive': + raise InvalidParameter('KCHVOL0003E', {'pool': pool_name, + 'volume': name}) + if name in self.get_list(pool_name): + raise InvalidParameter('KCHVOL0001E', {'name': name}) + params['pool'] = pool_name taskid = add_task('', create_func, self.objstore, params) return self.task.lookup(taskid) @@ -95,10 +108,6 @@ class StorageVolumesModel(object): raise MissingParameter("KCHVOL0004E", {'item': str(item), 'volume': name}) - pool_info = StoragePoolModel(conn=self.conn, - objstore=self.objstore).lookup(pool_name) - if pool_info['type'] in READONLY_POOL_TYPE: - raise InvalidParameter("KCHVOL0012E", {'type': pool_info['type']}) try: pool.createXML(xml, 0) except libvirt.libvirtError as e: diff --git a/tests/test_rest.py b/tests/test_rest.py index b325201..227e5ac 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1052,7 +1052,7 @@ class RestTests(unittest.TestCase): 'format': 'raw'}) resp = self.request('/storagepools/pool-2/storagevolumes/', req, 'POST') - self.assertEquals(202, resp.status) + self.assertEquals(400, resp.status) resp = self.request('/storagepools/pool-2/activate', '{}', 'POST') self.assertEquals(200, resp.status) resp = self.request('/storagepools/pool-2/storagevolumes/', -- 1.9.3