[Libvirt-cim] [PATCH] [TEST] Resubmit RAFP.01 for LXC support, also pass for Xen, KVM and XenFV

[Kaitlin Rupert]

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

It's possible for the AssociatorNames call to return no instances 
without throwing an error.  You'll want to verify that data has at least 
one element.

> +
> +    if data[0]['InstanceID'] not in rasd:

By using data[0], you're relaying on the CIMOM to return the instance 
corresponding to test_dom first in the list.  This might not always be 
the case.  Instead, you should step through the items in data and select 
the instance or instances that correspond to test_dom.  If you don't 
find an instance, you should return an error.

Also, you're checking the value of InstanceID against all the possible 
rasd InstanceIDs for test_dom.  If the provider returns the MemRASD that 
corresponds to test_dom when it should have returned the ProcRASD, this 
check will return a false positive.

To fix this, you could change rasd from an array to a list.  Something 
like the following.  You already have the pool classname in the 
verify_rasd() call, so you can use it as a key value to get the 
corresponding RASD InstanceID.

pool_to_ rasd = { {'MemoryPool' : "%s/mem" % test_dom},
                   {'ProcessorPool' : "%s/proc" % test_dom},
                   {'DiskPool' : "%s/%s" %(test_dom, test_disk)},
                   {'NetworkPool' : "%s/%s" % (test_dom, test_mac)} }


> +        logger.info ('ID %s' % data[0]['InstanceID'])
> +        logger.error("InstanceID Mismatch")
> +        logger.error("Returned %s error" % data[0]['InstanceID'])

Then here, you can print the ID value AssociatorNames returned versus 
the expected value.

> + 
> +    if options.virt == 'LXC':
> +        pool = { "MemoryPool" : {'InstanceID' : "MemoryPool/0"} }
> +        rasd = [ "%s/mem" % test_dom ]
> +    else:
> +        pool = { "MemoryPool"    : {'InstanceID' : "MemoryPool/0"},
> +                 "ProcessorPool" : {'InstanceID' : "ProcessorPool/0"},
> +                 "DiskPool"      : {'InstanceID' : diskid},
> +                 "NetworkPool"   : {'InstanceID' : "NetworkPool/%s" % test_network }}

Can you fix this so that the line is 80 characters in length.

> +        rasd = [ "%s/mem" % test_dom, 
> +                 "%s/proc" % test_dom, 
> +                 "%s/%s" %(test_dom, test_disk), 
> +                 "%s/%s" % (test_dom, test_mac) ]
> 
> +    for k, v in pool.iteritems():
> +        status, inst = get_instance(options.ip, k, v, options.virt) 

If get_instance() returns an error, you should break out of the loop 
here.  The test is only returning the status of the last iteration 
through the loop.  If a previous iteration fails, but the last iteration 
succeeds, the test returns a false positive.

> + 
> +        status = verify_rasd(options.ip, "ResourceAllocationFromPool", 
> +                             k, options.virt, inst.InstanceID,
> +                             rasd)

Same here - you'll need to trap the status and then break from the loop 
in case of error.

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