[Kimchi-devel] [PATCHv7 5/8] storage server: Update model and mockmodel

[Royce Lv]

On 2014年01月18日 02:24, Aline Manera wrote:
> On 01/14/2014 01:48 PM, lvroyce0210 at gmail.com wrote:
>> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>>
>> Query all storage pool to retrieve storage server we used.
>> If no query param is given, all supported type will be listed.
>> With param given, only specified type of server is listed.
>>
>> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
>> ---
>> src/kimchi/mockmodel.py | 30 ++++++++++++++++++++++++++++++
>> src/kimchi/model.py | 30 ++++++++++++++++++++++++++++++
>> 2 files changed, 60 insertions(+)
>>
>> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
>> index 1f5178f..04a55b3 100644
>> --- a/src/kimchi/mockmodel.py
>> +++ b/src/kimchi/mockmodel.py
>> @@ -403,6 +403,36 @@ class MockModel(object):
>> iso_volumes.append(res)
>> return iso_volumes
>>
>> + def storageservers_get_list(self, target_type=None):
>> + # FIXME: When added new storage server support, this needs to be 
>> updated
>
> STORAGE_SOURCES only contains 'netfs' but it should also contain 'iscsi'
True, but can we add iscsi in a independant patch after nfs is merged? I 
would like this framework support merged first, based on a agreed 
framework, the other type can be easily accomplished and more focus on 
its functionality.
>
>> + target_type = kimchi.model.STORAGE_SOURCES.keys() \
>> + if not target_type else [target_type]
>> + pools = self.storagepools_get_list()
>> + server_list = []
>> + for pool in pools:
>> + try:
>> + pool_info = self.storagepool_lookup(pool)
>> + if (pool_info['type'] in target_type and
>> + pool_info['source']['addr'] not in server_list):
>> + server_list.append(pool_info['source']['addr'])
>
>
> source has no paremter named 'addr'
> It should be 'host'
No, it is not libvirt response, it is kimchi defined data, see commit 
acbcf04b8892b1c8876344 which I added to parse storage source information.
>
>
>> + except NotFoundError:
>> + pass
>> +
>> + return server_list
>> +
>> + def storageserver_lookup(self, server):
>> + pools = self.storagepools_get_list()
>> + for pool in pools:
>> + try:
>> + pool_info = self.storagepool_lookup(pool)
>> + if pool_info['source'] and pool_info['source']['addr'] == server:
>> + return dict(addr=server)
>> + except NotFoundError:
>> + # Avoid inconsistent pool result because of lease between list and 
>> lookup
>> + pass
>> +
>> + raise NotFoundError
>
> Add a message while raising the exception above
ACK
>
>> +
>> def dummy_interfaces(self):
>> interfaces = {}
>> ifaces = {"eth1": "nic", "bond0": "bonding",
>> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
>> index 671af02..3207e72 100644
>> --- a/src/kimchi/model.py
>> +++ b/src/kimchi/model.py
>> @@ -1292,6 +1292,36 @@ class Model(object):
>> else:
>> raise
>>
>> + def storageservers_get_list(self, target_type=None):
>> + target_type = STORAGE_SOURCES.keys() if not target_type else 
>> [target_type]
>> + pools = self.storagepools_get_list()
>> + server_list = []
>> + for pool in pools:
>> + try:
>> + pool_info = self.storagepool_lookup(pool)
>> + if (pool_info['type'] in target_type and
>> + pool_info['source']['addr'] not in server_list):
>> + # Avoid to add same server for multiple times
>> + # if it hosts more than one storage type
>> + server_list.append(pool_info['source']['addr'])
>
> source has no paremter named 'addr'
> It should be 'host'
Same as above.
>
>> + except NotFoundError:
>> + pass
>> +
>> + return server_list
>> +
>> + def storageserver_lookup(self, server):
>> + pools = self.storagepools_get_list()
>> + for pool in pools:
>> + try:
>> + pool_info = self.storagepool_lookup(pool)
>> + if pool_info['source'] and pool_info['source']['addr'] == server:
>
>
> source has no paremter named 'addr'
> It should be 'host'
Same as above.
>
>> + return dict(addr=server)
>
> You should also return the target_type
>
> [
> {
> host: ...,
> target_type: [...]
> },
> ...
> ]
Scenario is : GET /storageservers?target_type=netfs
return:
{
{'host': 'localhost', 'type': 'netfs, iscsi'}
}
Is that necessary? Users only care about servers.
>
>> + except NotFoundError:
>> + # Avoid inconsistent pool result because of lease between list and 
>> lookup
>> + pass
>> +
>> + raise NotFoundError
>> +
>> def _get_screenshot(self, vm_uuid):
>> with self.objstore as session:
>> try:
>