[Kimchi-devel] [PATCH] Fix get_storageservers API and UI retrieval of storage servers.

[Christy Perez]

This issue was discovered when it was realized that the list of NFS
servers currently on the system was not being displayed in a drop-down
for the server IP/name while adding a new NFS server.

The UI was calling the getstorageservers API and passing a filter of netfs, but
the StorageServers class was referencing a non-existent object. This threw an
exception that was not passed up as an error, and an empty list was returned
every time.

Additionally, a change to the STORAGE_SOURCES dictionary was causing errors
when the StorageServersModel::lookup() tried to access a non-existent key in
in the new scsi value. This is seen if a scsi pool exists on a system.

To allow the API to work, and sucessfully retrieve only *server* objects, this
patch makes the following changes:

1. Add a STORAGE_SERVERS list to storageservers.py containing only valid
   server types.
   The API doc states that currently only netfs is supported.
2. Since storagetargets correlates to an individual server (see the
   API doc for storagetargets) use the new STORAGE_SERVERS list instead
   of the sources ones.
3. Move the STORAGE_SOURCES dictionary into storagepools.py, since it should
   cover all pool types. In the future, it should be expanded to include
   iscsi, netfs, dir, scsi fc_host, etc.
4. Updates mockmodel to incorporate the new STORAGE_SERVERS list.

To test, create different types of pools, one nfs, and use:

$ curl -k -u user:pass -H 'Content-type: applicatiojson' -H 'Accept: application/json' https://localhost:8001/storageservers?_target_type=netfs

and

$ curl -k -u user:pass -H 'Content-type: applicatiojson' -H 'Accept: application/json' https://localhost:8001/storageservers

Or create at least one NFS pool, then when creating a second NFS pool, the
  names/IPs of the others will appear in the UI's server field called
  "NFS Server IP"

Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
---
 src/kimchi/mockmodel.py            |  5 +++--
 src/kimchi/model/storagepools.py   |  3 ++-
 src/kimchi/model/storageservers.py | 16 +++++++++-------
 src/kimchi/model/storagetargets.py |  4 ++--
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index dbdd57e..fe02abe 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -46,7 +46,8 @@ from kimchi.config import config as kconfig
 from kimchi.distroloader import DistroLoader
 from kimchi.exception import InvalidOperation, InvalidParameter
 from kimchi.exception import MissingParameter, NotFoundError, OperationFailed
-from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
+from kimchi.model.storagepools import ISO_POOL_NAME
+from kimchi.model.storageservers import STORAGE_SERVERS
 from kimchi.model.utils import get_vm_name
 from kimchi.model.vms import VM_STATIC_UPDATE_PARAMS
 from kimchi.objectstore import ObjectStore
@@ -503,7 +504,7 @@ class MockModel(object):
 
     def storageservers_get_list(self, _target_type=None):
         # FIXME: This needs to be updted when adding new storage server support
-        target_type = STORAGE_SOURCES.keys() \
+        target_type = STORAGE_SERVERS \
             if not _target_type else [_target_type]
         pools = self.storagepools_get_list()
         server_list = []
diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py
index 5af33b7..0f673bb 100644
--- a/src/kimchi/model/storagepools.py
+++ b/src/kimchi/model/storagepools.py
@@ -36,6 +36,8 @@ POOL_STATE_MAP = {0: 'inactive',
                   3: 'degraded',
                   4: 'inaccessible'}
 
+# Types of pools supported
+# FIXME: Add 'dir', 'logical' 'iscsi', 'fc_host' ?
 STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name',
                              'path': '/pool/source/dir/@path'},
                    'scsi': {'adapter_type': '/pool/source/adapter/@type',
@@ -207,7 +209,6 @@ class StoragePoolModel(object):
                 source[key] = ""
             else:
                 source[key] = res
-
         return source
 
     def _nfs_status_online(self, pool, poolArgs=None):
diff --git a/src/kimchi/model/storageservers.py b/src/kimchi/model/storageservers.py
index 167fedd..c9dfb25 100644
--- a/src/kimchi/model/storageservers.py
+++ b/src/kimchi/model/storageservers.py
@@ -18,24 +18,26 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
 
 from kimchi.exception import NotFoundError
-from kimchi.model.storagepools import StoragePoolModel, STORAGE_SOURCES
+from kimchi.model.storagepools import StoragePoolModel, StoragePoolsModel
+
+# Types of remote storage servers supported
+# FIXME: Add iscsi?
+STORAGE_SERVERS = ['netfs']
 
 
 class StorageServersModel(object):
     def __init__(self, **kargs):
         self.conn = kargs['conn']
         self.pool = StoragePoolModel(**kargs)
+        self.pools = StoragePoolsModel(**kargs)
 
     def get_list(self, _target_type=None):
         if not _target_type:
-            target_type = STORAGE_SOURCES.keys()
+            target_type = STORAGE_SERVERS
         else:
             target_type = [_target_type]
-        pools = self.pools.get_list()
 
-        conn = self.conn.get()
-        pools = conn.listStoragePools()
-        pools += conn.listDefinedStoragePools()
+        pools = self.pools.get_list()
 
         server_list = []
         for pool in pools:
@@ -64,7 +66,7 @@ class StorageServerModel(object):
         for pool in pools:
             try:
                 pool_info = self.pool.lookup(pool)
-                if (pool_info['source'] and
+                if (pool_info['type'] in STORAGE_SERVERS and
                         pool_info['source']['addr'] == server):
                     return dict(host=server)
             except NotFoundError:
diff --git a/src/kimchi/model/storagetargets.py b/src/kimchi/model/storagetargets.py
index e090d84..caa8dbe 100644
--- a/src/kimchi/model/storagetargets.py
+++ b/src/kimchi/model/storagetargets.py
@@ -23,7 +23,7 @@ from lxml import objectify
 from lxml.builder import E
 
 from kimchi.model.config import CapabilitiesModel
-from kimchi.model.storagepools import STORAGE_SOURCES
+from kimchi.model.storageservers import STORAGE_SERVERS
 from kimchi.utils import kimchi_log, patch_find_nfs_target
 
 
@@ -36,7 +36,7 @@ class StorageTargetsModel(object):
         target_list = list()
 
         if not _target_type:
-            target_types = STORAGE_SOURCES.keys()
+            target_types = STORAGE_SERVERS
         else:
             target_types = [_target_type]
 
-- 
1.9.0