8:23 a.m.
On 2014年01月14日 12:30, Sheldon wrote:
Just a minor comment below
On 01/08/2014 11:50 PM, lvroyce0210(a)gmail.com wrote:
> From: Royce Lv <lvroyce(a)linux.vnet.ibm.com>
>
> Query all storage pool to retrieve storage server we used.
>
> Signed-off-by: Royce Lv <lvroyce(a)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: