[Kimchi-devel] [PATCH] Fix get_storageservers API and UI retrieval of storage servers.
Christy Perez
christy at linux.vnet.ibm.com
Fri Apr 11 15:01:31 UTC 2014
On Fri, 2014-04-11 at 00:15 -0300, Aline Manera wrote:
> 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 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' ?
>
> 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 at linux.vnet.ibm.com>
>
> If you agree I can update/remove the above comment before applying
Thanks Aline! I'm fine with that.
>
>
> > 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]
> >
>
Regards,
- Christy
More information about the Kimchi-devel
mailing list