[Libvirt-cim] [PATCH] [TEST] #2 Rewrite get_pool_details() to return less valeus in SettingsDefineCapabilities.01

[Kaitlin Rupert]

yunguol at cn.ibm.com wrote:
> # HG changeset patch
> # User Guolian Yun <yunguol at cn.ibm.com>
> # Date 1217477534 25200
> # Node ID 780f9b713dcae42c3faf0d73580c51c4c6d4a1e1
> # Parent  fc92abb3ae7cab32dec6718412be1c0d88874a4a
> [TEST] #2 Rewrite get_pool_details() to return less valeus in SettingsDefineCapabilities.01
> 
> Signed-off-by: Guolian Yun <yunguol at cn.ibm.com>
> 
> diff -r fc92abb3ae7c -r 780f9b713dca suites/libvirt-cim/cimtest/SettingsDefineCapabilities/01_forward.py
> --- a/suites/libvirt-cim/cimtest/SettingsDefineCapabilities/01_forward.py	Wed Jul 30 06:55:31 2008 -0700
> +++ b/suites/libvirt-cim/cimtest/SettingsDefineCapabilities/01_forward.py	Wed Jul 30 21:12:14 2008 -0700
> @@ -63,7 +63,7 @@ CIM_ERROR_GETINSTANCE, CIM_ERROR_ASSOCIA
>  CIM_ERROR_GETINSTANCE, CIM_ERROR_ASSOCIATORS
>  from XenKvmLib.classes import get_typed_class
>  from XenKvmLib.common_util import cleanup_restore, create_diskpool_conf, \
> -create_netpool_conf
> +create_netpool_conf, destroy_netpool
>  from XenKvmLib.common_util import print_field_error
> 
>  platform_sup = ['Xen', 'KVM', 'XenFV', 'LXC']
> @@ -130,27 +130,28 @@ def get_pool_info(virt, server, devid, p
> 
>  def get_pool_details(virt, server):  
>      dpool = npool  = mpool  = ppool = None
> +    pool_set = [dpool, npool, mpool, ppool]

This will cause the length of pool_set to be 4.  You'll want to 
something like:

pool_set = []

>      try :
>          status, diskid = create_diskpool_conf(server, virt)
>          if status != PASS:
> -            return status,  dpool, npool, mpool, ppool
> +            return status, pool_set, None
> 
>          dpool = get_pool_info(virt, server, diskid, poolname="DiskPool")
>          mpool = get_pool_info(virt, server, memid, poolname= "MemoryPool")
>          ppool = get_pool_info(virt, server, procid, poolname= "ProcessorPool")
> -
> +        pool_set = [dpool, npool, mpool, ppool]

You do this assignment twice in this function.  I'd remove this one.

>          status, test_network = create_netpool_conf(server, virt)
>          if status != PASS:
> -            return status,  dpool, npool, mpool, ppool
> +            return status, pool_set, None
> 
>          netid = "%s/%s" % ("NetworkPool", test_network)
>          npool = get_pool_info(virt, server, netid, poolname= "NetworkPool")
> -    
> +        pool_set = [dpool, npool, mpool, ppool]  
>      except Exception, detail:
>          logger.error("Exception: %s", detail)
> -        return FAIL, dpool, npool, mpool, ppool
> +        return FAIL, pool_set, test_network
> 
> -    return PASS, dpool, npool, mpool, ppool
> +    return PASS, pool_set, test_network
> 
>  def verify_rasd_fields(loop, assoc_info, cllist, rtype, rangelist):
>      for inst in assoc_info:
> @@ -200,14 +201,17 @@ def main():
>      server = options.ip
>      virt = options.virt
> 
> -    status, dpool, npool, mpool, ppool = get_pool_details(virt, server)
> -    if status != PASS or dpool.InstanceID == None or mpool.InstanceID == None \
> -       or npool.InstanceID == None or ppool.InstanceID == None:
> -        cleanup_restore(server, virt)
> -        return FAIL
> +    status, pool_set, test_network = get_pool_details(virt, server)
> +    for i in range(0, len(pool_set)):
> +        if status != PASS or pool_set[i].InstanceID == None:

Since you return the status variable from get_pool_details(), you should 
check the status between looping through the array values.

I'm not sure you need this loop here.  Instead, in get_pool_details(), 
you should check to make sure the instance returned from get_pool_info() 
isn't None.

> +            cleanup_restore(server, virt)
> +            destroy_netpool(server, virt, test_network)
> +            return FAIL
> 
> -    status = verify_sdc_with_ac(virt, server, dpool, npool, mpool, ppool)
> +    status = verify_sdc_with_ac(virt, server, pool_set[0], pool_set[1], 
> +                                pool_set[2], pool_set[3])

verify_sdc_with_ac() can now just take pool_set and virt as arguments, 
no need to pass each value of the array as a separate param.

>      cleanup_restore(server, virt)


-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com