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(a)linux.vnet.ibm.com&gt; --- 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]

On 04/10/2014 01:14 PM, Christy Perez wrote: > 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(a)linux.vnet.ibm.com&gt; > --- > 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' ? The STORAGE_SOURCES pools are those that need a source input: server IP, adapter name, ect The dir storage pool does not need those kind of info so it should not be listed there. In docs/API.md you can see which pools have source element Otherwise, patch looks good: Reviewed-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; If you agree I can update/remove the above comment before applying

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