[Libvirt-cim] [PATCH] [TEST] Update RAFP.01 for LXC support

[Kaitlin Rupert]

yunguol at cn.ibm.com wrote:
> # HG changeset patch
> # User Guolian Yun <yunguol at cn.ibm.com>
> # Date 1214371183 -28800
> # Node ID c8c27374e304a66cd71d1f0b5d0fc462e230a898
> # Parent  727d97c09d77d73f3542ba49a9dd19ba119a67eb
> [TEST] Update RAFP.01 for LXC support
> 

Can you keep the revision number in the subject next time around? 
Having the revision number makes it easier to keep track of which patch 
is the current version.

Also, please include a log of what has changed for each iteration of the 
patch.

Thanks!

> +def verify_rasd(server, assoc_cn, cn, virt, list, rasd):
> +    try:
> +        data = assoc.AssociatorNames(server,
> +                                     assoc_cn,
> +                                     cn,
> +                                     virt,
> +                                     InstanceID=list)
> +    except Exception:
> +        logger.error(Globals.CIM_ERROR_ASSOCIATORNAMES % cn)
> +        return FAIL
> +
> +    if len(data) < 1:
> +        logger.error("Return NULL, expect at least one instance")
> +        return FAIL
> +    
> +    for i in range(0, len(data)):
> +        if data[i].classname == "LXC_MemResourceAllocationSettingData" \
> +                   and data[i]['InstanceID'] == rasd["MemoryPool"]:
> +            logger.info("MemoryPool InstanceID match")
> +            return PASS
> +        elif i == len(data) and data[i]['InstanceID'] != rasd["MemoryPool"]:
> +            logger.error("InstanceID Mismatch, expect %s not %s" % \
> +                         (rasd['MemoryPool'], data[i]['InstanceID']))
> +            return FAIL
> +        if data[i].classname == "LXC_ProcResourceAllocationSettingData" \
> +                   and data[i]['InstanceID'] == rasd["ProcessorPool"]:
> +            logger.info("ProcessorPool InstanceID match")
> +            return PASS
> +        elif i == len(data) and data[i]['InstanceID'] != rasd["ProcessorPool"]:
> +            logger.error("InstanceID Mismatch, expect %s not %s" % \
> +                         (rasd['ProcessorPool'], data[i]['InstanceID']))
> +            return FAIL
> +        if data[i].classname == "LXC_DiskResourceAllocationSettingData" \
> +                   and data[i]['InstanceID'] == rasd["DiskPool"]:
> +            logger.info("DiskPool InstanceID match")
> +            return PASS
> +        elif i == len(data) and data[i]['InstanceID'] != rasd["DiskPool"]:
> +            logger.error("InstanceID Mismatch, expect %s" % \
> +                         (rasd['DiskPool'], data[i]['InstanceID']))
> +            return FAIL
> +        if data[i].classname == "LXC_NetResourceAllocationSettingData" \
> +                   and data[i]['InstanceID'] == rasd["NetworkPool"]:
> +            logger.info("NetworkPool InstanceID match")
> +            return PASS
> +        elif i == len(data) and data[i]['InstanceID'] != rasd["NetworkPool"]:
> +            logger.error("InstanceID Mismatch, expect %s" % \
> +                         (rasd['NetworkPool'], data[i]['InstanceID']))
> +            return FAIL
> +               

This is a lot of duplicated code and is difficult to read.  Also, code 
like this can be difficult to maintain.  This can be collapsed into 
something much simpler:

     for item in data:
             if item['InstanceID'] == rasd[cn]:
                 logger.info("%s InstanceID match - expected %s, got %s" 
% (cn,
                             rasd[cn], item['InstanceID']))
                 return PASS

     logger.error("RASD instance with InstanceID %s not found" % rasd[cn])

     return FAIL


> 
> +    for k, v in pool.iteritems():
> +        status, inst = get_instance(options.ip, k, v, options.virt) 
> +        if status != PASS:
> +            cleanup_restore(options.ip, options.virt)
> +            vsxml.undefine(options.ip)
> +            return status

Instead of duplicating the cleanup code here, you can use break instead. 
  That will drop you out of the for loop, and the rest of the code will 
execute normally.

> + 
> +        status = verify_rasd(options.ip, "ResourceAllocationFromPool", 
> +                             k, options.virt, inst.InstanceID,
> +                             rasd)
> +        if status != PASS:
> +            cleanup_restore(options.ip, options.virt)
> +            vsxml.undefine(options.ip)
> +            return status

Same here - use break instead.

> +
> +
> +    cleanup_restore(options.ip, options.virt)
> +    vsxml.undefine(options.ip)
>      return status 
>          

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