[Libvirt-cim] [PATCH] [TEST] 2# Update RAFP.01 for LXC support, also add DiskPool and NetworkPool verification

[Kaitlin Rupert]

> +def setup_env(server, virt):
> +    destroy_and_undefine_all(server)
> +    vsxml = None
> +    if virt == "Xen":
> +        test_disk = "xvda"
> +    else:
> +        test_disk = "hda"

hda doesn't apply for LXC - maybe set the value to None in that case?

> 
>      try:
>          key_list = { 'InstanceID' : "MemoryPool/0" }
> @@ -57,6 +104,27 @@
>      except Exception:
>          logger.error(Globals.CIM_ERROR_GETINSTANCE % "ProcessorPool")  
>          return FAIL

I know this is existing code, but you'll want to be sure you call 
cleanup_restore() and undefine your guest whenever you fail. The same 
applied for all the instances where you do a return FAIL below.

> +
> +    try:
> +        key_list = { 'InstanceID' : diskid}
> +        diskpool = enumclass.getInstance(options.ip,
> +                                         "DiskPool",
> +                                         key_list,
> +                                         options.virt)
> +    except Exception:
> +        logger.error(Globals.CIM_ERROR_GETINSTANCE % "DiskPool")
> +        return FAIL
> +
> +    try:
> +        key_list = { 'InstanceID' : "NetworkPool/%s" % test_network }
> +        netpool = enumclass.getInstance(options.ip,
> +                                        "NetworkPool",
> +                                        key_list,
> +                                        options.virt)
> +    except Exception:
> +        logger.error(Globals.CIM_ERROR_GETINSTANCE % "NetworkPool")
> +        return FAIL
> +

LXC doesn't support disk, network, and proc yet.  So we shouldn't check 
these because we don't have any resources we're allocated from a pool.

Also, this test is a bit difficult to read.  I'd suggest creating a 
function that handles everything in the try block.  You can then call 
this function to get each of the instances you need.

>       
>      try:
>          memdata = assoc.AssociatorNames(options.ip, "ResourceAllocationFromPool",
> @@ -86,6 +154,35 @@
>  	    logger.error("ERROR: Association result error")
>              status = FAIL
> 
> +    try:
> +        diskdata = assoc.AssociatorNames(options.ip, "ResourceAllocationFromPool",
> +                                         "DiskPool",
> +                                         options.virt,
> +                                         InstanceID = diskpool.InstanceID)
> +    except Exception:
> +        logger.error(Globals.CIM_ERROR_ASSOCIATORNAMES % diskpool.InstanceID)
> +        status = FAIL

Something similar could be said here - the association calls for each 
device will be similar, so you could make a function out of this.

> +
> +    for i in range(len(diskdata)):
> +        if diskdata[i].classname != get_typed_class(options.virt, "DiskResourceAllocationSettingData"):
> +            logger.error("ERROR: Association result error")
> +            status = FAIL

You'll also want to verify you got the expected number of instances back 
from the association.

If the association returns no instances, this for loop is skipped and 
the test returns a false positive.

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