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

[Sheldon]

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.
> +            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 ?
> +                    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?

> +                pass
> +        raise NotFoundError
> +
>       def _get_screenshot(self, vm_uuid):
>           with self.objstore as session:
>               try:


-- 
Thanks and best regards!

Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center