[Kimchi-devel] [PATCH 2/3] storagevolume: Check storage pool before adding a volume

[Crístian Viana]

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