[Libvirt-cim] [PATCH] [TEST] Fixing the 01_verify_rasd_fields.py tc of RASD

[Kaitlin Rupert]

 >  import sys
> -from CimTest import Globals
>  from XenKvmLib.const import do_main
> -from XenKvmLib.test_doms import destroy_and_undefine_all
>  from XenKvmLib import assoc
>  from XenKvmLib import vxml 

Change this to XenKvmLib.vxml import get_class


> +    for rasd in assoc_info:
> +        guest, dev, status = parse_instance_id(rasd['InstanceID'])
> +        if status != PASS:
> +            logger.error("Unable to parse InstanceID: %s", rasd['InstanceID'])
> +            return status
> +
> +        if guest != test_dom:
> +            continue

Instead of a continue here, this should throw an error.

VirtualSystemSettingDataComponent should only return RASDs that 
correspond to a give VSSD.  And a VSSD instance can only represent one 
guest.

This is a mistake I made in SettingsDefine/02_reverse.py, which is the 
test I told you to reference.  Sorry for the error here!  I'll send a 
patch to fix this bug.

Really, we don't need to do this check at all.  When I was re-writing 
SettingsDefine/02_reverse.py, I was thinking that it is a good idea to 
verify that the RASDs match the guest we're testing.

But really, if VirtualSystemSettingDataComponent returns the wrong 
instances, that's something the VirtualSystemSettingDataComponent 
related tests should find.  This test should be concerned with verifying 
the RASD properties, it shouldn't be concerned with also verifying the 
output of VirtualSystemSettingDataComponent.

But it's still a valid check to have.  So you can leave it in or remove 
it.  Either way is fine by me.  =)

> +
> +        logger.info("Verifying: %s", rasd.classname)
> +        exp_rasd = rasds[rasd.classname]
> +        for item in rasd.items():
> +            key = item[0]
> +            exp_val = eval('exp_rasd.' + key)
> +            ret_rasd_val = rasd[key]
> +            if ret_rasd_val == exp_val:
> +                status = PASS

Will compare_all_prop() work for this?  When we spoke, I said that 
compare_all_prop() should be phased out once the association related 
calls return the same object type as the enumeration related calls.

However, since the the return types of the enumeration calls and the 
association calls don't match yet, I'd rather compare_all_prop() be used 
instead of duplicating the code here.  When it comes time to remove 
compare_all_prop(), it's easier to replace the function name than it is 
to replace an entire code block.

Sorry for the confusion there.

>              else:
> -                status = 1
> -        if status != 0 or proc_status != 0 or net_status != 0 or \
> -           disk_status != 0 or mem_status != 0 :
> -            logger.error("Mistmatching association values" )
> -            status = 1 
> -    except  Exception, detail :
> -        logger.error("Exception in assoc_values function: %s" % detail)
> -        status = 1
> -    
> -    return status
> -   
> +                logger.info("Got %s instead of %s", ret_rasd_val, exp_val)
> +                status = FAIL
> +                break
> +
> +    if status != PASS:
> +        logger.error("RASD with id %s not returned", exp_rasd.InstanceID)

This message is misleading.. because you verify all the properties.  So 
even if the InstanceID matches, one of the other properties could be 
different.  I would use a more generic message to indicate that a 
matching RASD wasn't found.

The providers shouldn't return more than one RASD with the same 
InstanceID.  But it's possible it could happen in some kind of odd error 
condition.

> 
>      virt_xml = vxml.get_class(virt)
>      if virt == 'LXC':
>          cxml = virt_xml(test_dom)
>      else:
>          cxml = virt_xml(test_dom, mem=test_mem, vcpus = test_vcpus,
> -                        mac = test_mac, disk = test_disk)
> -    ret = cxml.create(options.ip)
> +                        mac = test_mac)
> +                        
> +    ret = cxml.create(server)

Can you change this to define() and cim_start().

>      if not ret:
>          logger.error('Unable to create domain %s' % test_dom)
>          return FAIL 
> -    if status == 1: 
> -        destroy_and_undefine_all(options.ip)
> -        return FAIL

>          status = FAIL 
> -    
> -    try:
> -        cxml.destroy(options.ip)
> -        cxml.undefine(options.ip)
> -    except Exception:
> -        logger.error("Destroy or undefine domain failed")
> +
> +    cxml.destroy(server)

Add an undefine() here.

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