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

[Royce Lv]

On 2014年01月14日 12:30, Sheldon wrote:
> Just a minor comment below
>
> On 01/08/2014 11:50 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.
>>
>> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
>> ---
>>   src/kimchi/mockmodel.py | 28 ++++++++++++++++++++++++++++
>>   src/kimchi/model.py     | 26 ++++++++++++++++++++++++++
>>   2 files changed, 54 insertions(+)
>>
>> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
>> index 9488078..0116c1b 100644
>> --- a/src/kimchi/mockmodel.py
>> +++ b/src/kimchi/mockmodel.py
>> @@ -419,6 +419,34 @@ 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
>> +        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:
>> + server_list.append(pool_info['source']['addr'])
>> +            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(target_type=pool_info['type'], 
>> addr=server)
>> +            except NotFoundError:
>> +            # Avoid inconsistent pool result because of lease 
>> between list and lookup
>> +                pass
>> +        raise NotFoundError
>> +
>>       def dummy_interfaces(self):
>>           interfaces = {}
>>           ifaces = {"eth1": "nic", "bond0": "bonding",
>> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
>> index 7115583..f005121 100644
>> --- a/src/kimchi/model.py
>> +++ b/src/kimchi/model.py
>> @@ -1238,6 +1238,32 @@ 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:
>> + server_list.append(pool_info['source']['addr'])
> if we want to to get "netfs",
>
> All servers such as:
> [
> {"server": "192.168.1.2", "target_type ": "SAN"},
> {"server": "192.168.1.2", "target_type ": "netfs"},
> ]
>
> storageserver_lookup may return a SAN server.
> so we will not add it in server_list.
True, we need to filter it out. Thanks for pointing out.
>> +            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:
> if we want to to get "netfs",
> Here, we may return:
> {"server": "192.168.1.2", "target_type ": "SAN"}
> for it is the first element of pool list.
>
> The SAN server and netfs server are same server.
>
> so when will find {"server": "192.168.1.2", "target_type ": "netfs"} ?
>
> so you will report NotFoundError ?
Good catch! I haven't considered this scenario, thanks, Sheldon!
>> +                    return dict(target_type=pool_info['type'], 
>> addr=server)
>> +            except NotFoundError:
>> +            # Avoid inconsistent pool result because of lease 
>> between list and lookup
> Does the inconsistent means what I say above?
No, this is a suggest from Ming that when we get_list and then lookup 
there is a lease that a pool appears in the get_list result, but deleted 
after. so lookup may not find it.
>
>> +                pass
>> +        raise NotFoundError
>> +
>>       def _get_screenshot(self, vm_uuid):
>>           with self.objstore as session:
>>               try:
>
>